RUM-9899: post-review fixes

satween · satween · commit 0f11ceac1aa9 · 2025-08-18T15:49:57.000+01:00
diff --git a/ci/pipelines/default-pipeline.yml b/ci/pipelines/default-pipeline.yml
@@ -554,7 +554,7 @@ publish:release-trace-internal:
   timeout: 30m
   script:
     - !reference [ .snippets, set-publishing-credentials ]
-    - ./gradlew :features:dd-sdk-android-trace-internal:publishToSonatype --stacktrace --no-daemon
+    - ./gradlew :features:dd-sdk-android-trace-internal:publishToSonatype closeSonatypeStagingRepository --stacktrace --no-daemon
   artifacts:
     when: on_success
     expire_in: 7 days
diff --git a/features/dd-sdk-android-trace-api/api/dd-sdk-android-trace-api.api b/features/dd-sdk-android-trace-api/api/dd-sdk-android-trace-api.api
@@ -84,7 +84,6 @@ public abstract interface class com/datadog/android/trace/api/scope/DatadogScope
 
 public abstract interface class com/datadog/android/trace/api/span/DatadogSpan {
 	public abstract fun addThrowable (Ljava/lang/Throwable;)V
-	public abstract fun addThrowable (Ljava/lang/Throwable;B)V
 	public abstract fun context ()Lcom/datadog/android/trace/api/span/DatadogSpanContext;
 	public abstract fun drop ()V
 	public abstract fun finish ()V
diff --git a/features/dd-sdk-android-trace-api/src/main/kotlin/com/datadog/android/trace/api/propagation/DatadogPropagation.kt b/features/dd-sdk-android-trace-api/src/main/kotlin/com/datadog/android/trace/api/propagation/DatadogPropagation.kt
@@ -9,7 +9,7 @@ import com.datadog.android.trace.api.span.DatadogSpanContext
 
 /**
  * Provides an interface for injecting and extracting span context to/from specified carriers.
- * Used for propagating context to http headers
+ * Used for propagating context to http headers.
  */
 interface DatadogPropagation {
 
diff --git a/features/dd-sdk-android-trace-api/src/main/kotlin/com/datadog/android/trace/api/span/DatadogSpan.kt b/features/dd-sdk-android-trace-api/src/main/kotlin/com/datadog/android/trace/api/span/DatadogSpan.kt
@@ -116,15 +116,6 @@ interface DatadogSpan {
      */
     fun addThrowable(throwable: Throwable)
 
-    /**
-     * Associates a throwable with the current span, marking it as an error
-     * and capturing the provided throwable for additional context with a specified error priority.
-     *
-     * @param throwable The throwable to associate with the current span.
-     * @param errorPriority The priority level of the error, represented as a byte.
-     */
-    fun addThrowable(throwable: Throwable, errorPriority: Byte)
-
     /**
      * Associates a tag with the specified value for the current span.
      *
diff --git a/features/dd-sdk-android-trace-otel/src/main/java/com/datadog/opentelemetry/trace/OtelSpan.java b/features/dd-sdk-android-trace-otel/src/main/java/com/datadog/opentelemetry/trace/OtelSpan.java
@@ -21,6 +21,7 @@
 import com.datadog.android.trace.api.span.DatadogSpan;
 import com.datadog.android.trace.api.span.DatadogSpanContext;
 import com.datadog.android.trace.api.tracer.DatadogTracer;
+import com.datadog.android.trace.internal.DatadogTracingToolkit;
 
 import java.util.List;
 import java.util.concurrent.TimeUnit;
@@ -114,7 +115,7 @@ public StatusCode getStatusCode() {
   public Span recordException(Throwable exception, Attributes additionalAttributes) {
     if (this.recording) {
       // Store exception as span tags as span events are not supported yet
-      this.delegate.addThrowable(exception, DatadogTracingConstants.ErrorPriorities.UNSET);
+      DatadogTracingToolkit.addThrowable(delegate, exception, DatadogTracingConstants.ErrorPriorities.UNSET);
     }
     return this;
   }
diff --git a/features/dd-sdk-android-trace-otel/src/test/kotlin/com/datadog/opentelemetry/trace/OtelSpanTest.kt b/features/dd-sdk-android-trace-otel/src/test/kotlin/com/datadog/opentelemetry/trace/OtelSpanTest.kt
@@ -10,6 +10,7 @@ import com.datadog.android.trace.api.DatadogTracingConstants.DEFAULT_ASYNC_PROPA
 import com.datadog.android.trace.api.DatadogTracingConstants.ErrorPriorities
 import com.datadog.android.trace.api.span.DatadogSpan
 import com.datadog.android.trace.api.tracer.DatadogTracer
+import com.datadog.android.trace.internal.DatadogTracingToolkit
 import com.datadog.android.trace.opentelemetry.utils.forge.Configurator
 import fr.xgouchet.elmyr.annotation.Forgery
 import fr.xgouchet.elmyr.annotation.StringForgery
@@ -28,14 +29,14 @@ import org.junit.jupiter.params.provider.Arguments
 import org.junit.jupiter.params.provider.EnumSource
 import org.junit.jupiter.params.provider.MethodSource
 import org.mockito.Mock
+import org.mockito.Mockito
 import org.mockito.junit.jupiter.MockitoExtension
 import org.mockito.junit.jupiter.MockitoSettings
 import org.mockito.kotlin.any
 import org.mockito.kotlin.anyOrNull
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.never
 import org.mockito.kotlin.verify
-import org.mockito.kotlin.verifyNoInteractions
 import org.mockito.kotlin.verifyNoMoreInteractions
 import org.mockito.quality.Strictness
 
@@ -212,27 +213,33 @@ internal class OtelSpanTest {
     fun `M delegate to AgentSpan W recordException`(@Forgery fakeThrowable: Throwable) {
         // Given
         val mockAttributes: Attributes = mock()
-
-        // When
-        testedSpan.recordException(fakeThrowable, mockAttributes)
-
-        // Then
-        verify(mockAgentSpan).addThrowable(fakeThrowable, ErrorPriorities.UNSET)
-        verifyNoInteractions(mockAttributes)
+        Mockito.mockStatic(DatadogTracingToolkit::class.java).use { mockedStatic ->
+            // When
+            testedSpan.recordException(fakeThrowable, mockAttributes)
+            // Then
+            mockedStatic.verify {
+                DatadogTracingToolkit.addThrowable(
+                    mockAgentSpan,
+                    fakeThrowable,
+                    ErrorPriorities.UNSET
+                )
+            }
+            mockedStatic.verifyNoMoreInteractions()
+        }
     }
 
     @Test
     fun `M do nothing W recordException { isRecording is false }`(@Forgery fakeThrowable: Throwable) {
         // Given
         testedSpan.end()
         val mockAttributes: Attributes = mock()
+        Mockito.mockStatic(DatadogTracingToolkit::class.java).use { mockedStatic ->
+            // When
+            testedSpan.recordException(fakeThrowable, mockAttributes)
 
-        // When
-        testedSpan.recordException(fakeThrowable, mockAttributes)
-
-        // Then
-        verify(mockAgentSpan, never()).addThrowable(fakeThrowable, ErrorPriorities.UNSET)
-        verifyNoInteractions(mockAttributes)
+            // Then
+            mockedStatic.verifyNoMoreInteractions()
+        }
     }
 
     // endregion
diff --git a/features/dd-sdk-android-trace/api/apiSurface b/features/dd-sdk-android-trace/api/apiSurface
@@ -31,6 +31,7 @@ object com.datadog.android.trace.internal.DatadogTracingToolkit
   fun setTracingSamplingPriorityIfNecessary(com.datadog.android.trace.api.span.DatadogSpanContext)
   fun setTraceId128BitGenerationEnabled(com.datadog.android.trace.api.tracer.DatadogTracerBuilder): com.datadog.android.trace.api.tracer.DatadogTracerBuilder
   fun setSdkV2Compatible(com.datadog.android.trace.api.tracer.DatadogTracerBuilder): com.datadog.android.trace.api.tracer.DatadogTracerBuilder
+  fun addThrowable(com.datadog.android.trace.api.span.DatadogSpan, Throwable, Byte)
 object com.datadog.android.trace.internal.SpanAttributes
   const val DATADOG_INITIAL_CONTEXT: String
 fun <T> android.database.sqlite.SQLiteDatabase.transactionTraced(String, Boolean = true, com.datadog.android.trace.api.span.DatadogSpan.(android.database.sqlite.SQLiteDatabase) -> T): T
diff --git a/features/dd-sdk-android-trace/api/dd-sdk-android-trace.api b/features/dd-sdk-android-trace/api/dd-sdk-android-trace.api
@@ -65,6 +65,7 @@ public final class com/datadog/android/trace/internal/DatadogTraceExtKt {
 public final class com/datadog/android/trace/internal/DatadogTracingToolkit {
 	public static final field INSTANCE Lcom/datadog/android/trace/internal/DatadogTracingToolkit;
 	public static final field spanIdConverter Lcom/datadog/android/trace/internal/DatadogSpanIdConverter;
+	public static final fun addThrowable (Lcom/datadog/android/trace/api/span/DatadogSpan;Ljava/lang/Throwable;B)V
 	public final fun getPropagationHelper ()Lcom/datadog/android/trace/internal/DatadogPropagationHelper;
 	public final fun setSdkV2Compatible (Lcom/datadog/android/trace/api/tracer/DatadogTracerBuilder;)Lcom/datadog/android/trace/api/tracer/DatadogTracerBuilder;
 	public final fun setTraceId128BitGenerationEnabled (Lcom/datadog/android/trace/api/tracer/DatadogTracerBuilder;)Lcom/datadog/android/trace/api/tracer/DatadogTracerBuilder;
diff --git a/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/DatadogSpanAdapter.kt b/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/DatadogSpanAdapter.kt
@@ -94,10 +94,6 @@ internal class DatadogSpanAdapter(
         delegate.addThrowable(throwable)
     }
 
-    override fun addThrowable(throwable: Throwable, errorPriority: Byte) {
-        delegate.addThrowable(throwable, errorPriority)
-    }
-
     override fun logThrowable(throwable: Throwable) {
         spanLogger.log(throwable, this)
     }
@@ -113,4 +109,8 @@ internal class DatadogSpanAdapter(
     override fun logAttributes(attributes: Map<String, Any>) {
         spanLogger.log(attributes, this)
     }
+
+    internal fun addThrowable(throwable: Throwable, errorPriority: Byte) {
+        delegate.addThrowable(throwable, errorPriority)
+    }
 }
diff --git a/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/DatadogTracingToolkit.kt b/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/DatadogTracingToolkit.kt
@@ -6,6 +6,7 @@
 package com.datadog.android.trace.internal
 
 import com.datadog.android.lint.InternalApi
+import com.datadog.android.trace.api.span.DatadogSpan
 import com.datadog.android.trace.api.span.DatadogSpanContext
 import com.datadog.android.trace.api.tracer.DatadogTracerBuilder
 
@@ -57,4 +58,17 @@ object DatadogTracingToolkit {
         (builder as? DatadogTracerBuilderAdapter)?.setSdkV2Compatible()
         return builder
     }
+
+    /**
+     * Associates a throwable with the current span, marking it as an error.
+     * Note that error flag will be set only if priority is higher than the current one.
+     *
+     * @param span The span to associate the throwable with.
+     * @param throwable The throwable to associate with the current span.
+     * @param errorPriority The priority level of the error, represented as a byte.
+     */
+    @JvmStatic // this method is called from OTel code, written in java
+    fun addThrowable(span: DatadogSpan, throwable: Throwable, errorPriority: Byte) {
+        (span as? DatadogSpanAdapter)?.addThrowable(throwable, errorPriority)
+    }
 }
diff --git a/integrations/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/trace/TracingInterceptor.kt b/integrations/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/trace/TracingInterceptor.kt
@@ -206,7 +206,7 @@ internal constructor(
         request: Request,
         tracer: DatadogTracer
     ): Response {
-        val span: DatadogSpan = buildSpan(tracer, request)
+        val span = buildSpan(tracer, request)
         val isSampled = span.sample(request)
 
         if (span.isRootSpan) {
@@ -485,14 +485,14 @@ internal constructor(
             val spanId = span.context().spanId.toString()
             requestBuilder.addHeader(
                 W3C_TRACEPARENT_KEY,
-                @Suppress("UnsafeThirdPartyFunctionCall", "InvalidStringFormat") // Format string is static
+                @Suppress("UnsafeThirdPartyFunctionCall") // Format string is static
                 W3C_TRACEPARENT_DROP_SAMPLING_DECISION.format(
                     traceId.padStart(length = W3C_TRACE_ID_LENGTH, padChar = '0'),
                     spanId.padStart(length = W3C_PARENT_ID_LENGTH, padChar = '0')
                 )
             )
             // TODO RUM-2121 3rd party vendor information will be erased
-            @Suppress("UnsafeThirdPartyFunctionCall", "InvalidStringFormat") // Format string is static
+            @Suppress("UnsafeThirdPartyFunctionCall") // Format string is static
             var traceStateHeader = W3C_TRACESTATE_DROP_SAMPLING_DECISION
                 .format(spanId.padStart(length = W3C_PARENT_ID_LENGTH, padChar = '0'))
             if (traceOrigin != null) {
diff --git a/reliability/single-fit/okhttp/src/test/kotlin/com/datadog/android/okhttp/HeadBasedSamplingTest.kt b/reliability/single-fit/okhttp/src/test/kotlin/com/datadog/android/okhttp/HeadBasedSamplingTest.kt
@@ -138,7 +138,7 @@ class HeadBasedSamplingTest {
                     leastSignificantTraceId.toLong().toHexString().padStart(16, '0')
                 )
                 hasMostSignificant64BitsTraceId(mostSignificantTraceId)
-                hasSpanId(spanId.cleanDecimalFormat())
+                hasSpanId(spanId.toHexPaddedFromDecimalString())
                 hasVersion(stubSdkCore.getDatadogContext().version)
                 hasSource(stubSdkCore.getDatadogContext().source)
                 hasTracerVersion(stubSdkCore.getDatadogContext().sdkVersion)
@@ -367,8 +367,8 @@ class HeadBasedSamplingTest {
                     leastSignificantTraceId.toLong().toHexString().padStart(16, '0')
                 )
                 hasMostSignificant64BitsTraceId(mostSignificantTraceId)
-                hasSpanId(spanId.cleanDecimalFormat())
-                hasParentId(localSpanId.cleanHexFormat())
+                hasSpanId(spanId.toHexPaddedFromDecimalString())
+                hasParentId(localSpanId.toHexPaddedFromHexString())
                 hasVersion(stubSdkCore.getDatadogContext().version)
                 hasSource(stubSdkCore.getDatadogContext().source)
                 hasTracerVersion(stubSdkCore.getDatadogContext().sdkVersion)
@@ -464,8 +464,8 @@ class HeadBasedSamplingTest {
                     leastSignificantTraceId.toLong().toHexString().padStart(16, '0')
                 )
                 hasMostSignificant64BitsTraceId(mostSignificantTraceId)
-                hasSpanId(spanId.cleanDecimalFormat())
-                hasParentId(localSpanId.cleanHexFormat())
+                hasSpanId(spanId.toHexPaddedFromDecimalString())
+                hasParentId(localSpanId.toHexPaddedFromHexString())
                 hasVersion(stubSdkCore.getDatadogContext().version)
                 hasSource(stubSdkCore.getDatadogContext().source)
                 hasTracerVersion(stubSdkCore.getDatadogContext().sdkVersion)
@@ -612,8 +612,8 @@ class HeadBasedSamplingTest {
                     leastSignificantTraceId.toLong().toHexString().padStart(16, '0')
                 )
                 hasMostSignificant64BitsTraceId(mostSignificantTraceId)
-                hasSpanId(spanId.cleanDecimalFormat())
-                hasParentId(localSpanId.cleanHexFormat())
+                hasSpanId(spanId.toHexPaddedFromDecimalString())
+                hasParentId(localSpanId.toHexPaddedFromHexString())
                 hasVersion(stubSdkCore.getDatadogContext().version)
                 hasSource(stubSdkCore.getDatadogContext().source)
                 hasTracerVersion(stubSdkCore.getDatadogContext().sdkVersion)
@@ -657,12 +657,12 @@ class HeadBasedSamplingTest {
     private fun String.toTags(): Map<String, String> = split(",")
         .associate { it.split("=").let { it[0] to it[1] } }
 
-    private fun String.cleanHexFormat(): String = with(DatadogTracingToolkit.spanIdConverter) {
-        toHexStringPadded(fromHex(this@cleanHexFormat))
+    private fun String.toHexPaddedFromHexString(): String = with(DatadogTracingToolkit.spanIdConverter) {
+        toHexStringPadded(fromHex(this@toHexPaddedFromHexString))
     }
 
-    private fun String.cleanDecimalFormat(): String = with(DatadogTracingToolkit.spanIdConverter) {
-        toHexStringPadded(this@cleanDecimalFormat.toLong())
+    private fun String.toHexPaddedFromDecimalString(): String = with(DatadogTracingToolkit.spanIdConverter) {
+        toHexStringPadded(this@toHexPaddedFromDecimalString.toLong())
     }
 
     companion object {

Original file line number	Diff line number	Diff line change
`@@ -94,10 +94,6 @@ internal class DatadogSpanAdapter(`
`94`	`94`	`delegate.addThrowable(throwable)`
`95`	`95`	`}`
`96`	`96`
`97`		`- override fun addThrowable(throwable: Throwable, errorPriority: Byte) {`
`98`		`- delegate.addThrowable(throwable, errorPriority)`
`99`		`- }`
`100`		`-`
`101`	`97`	`override fun logThrowable(throwable: Throwable) {`
`102`	`98`	`spanLogger.log(throwable, this)`
`103`	`99`	`}`
`@@ -113,4 +109,8 @@ internal class DatadogSpanAdapter(`
`113`	`109`	`override fun logAttributes(attributes: Map<String, Any>) {`
`114`	`110`	`spanLogger.log(attributes, this)`
`115`	`111`	`}`
	`112`	`+`
	`113`	`+ internal fun addThrowable(throwable: Throwable, errorPriority: Byte) {`
	`114`	`+ delegate.addThrowable(throwable, errorPriority)`
	`115`	`+ }`
`116`	`116`	`}`