From 5410db4a96e3d53298bfa84c42a840f6a815c99b Mon Sep 17 00:00:00 2001 From: Austin Brooks Date: Mon, 29 Jun 2026 15:38:45 -0700 Subject: [PATCH] RUM-16886: Mark sampled-out network spans as droppable while preserving stats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CoreTracer needs to see all spans to calculate APM client-side stats. Since network spans have their own sampling rate, sampled-out spans are now finished with the `_dd.local.force_drop` tag instead of being dropped. This tells downstream systems to discard the span while still letting CoreTracer process it for stats calculations. This only applies when the active tracer is the SDK's own `DatadogTracerAdapter`, which routes through `CoreTraceWriter` — the only implementation that honors `_dd.local.force_drop`. When a custom or global `DatadogTracer` is in use, sampled-out spans are dropped via `drop()` as before, preserving backward compatibility. --- features/dd-sdk-android-trace/api/apiSurface | 4 +- .../api/dd-sdk-android-trace.api | 15 +- .../internal/ApmNetworkInstrumentation.kt | 35 +++-- .../trace/internal/_TraceInternalProxy.kt | 7 + .../trace/internal/data/CoreTraceWriter.kt | 6 +- .../trace/internal/domain/event/MetaKeys.kt | 7 + .../net/ApmNetworkInstrumentationExt.kt | 9 +- .../trace/internal/net/RequestTracingState.kt | 4 +- .../trace/internal/TraceInternalProxyTest.kt | 18 +++ .../internal/data/CoreTraceWriterTest.kt | 19 +++ .../net/ApmNetworkInstrumentationTest.kt | 128 ++++++++++++------ .../internal/net/RequestTracingStateTest.kt | 14 +- .../cronet/internal/CronetRequestCallback.kt | 7 +- .../android/okhttp/DatadogInterceptor.kt | 7 +- .../internal/RequestTracingStateRegistry.kt | 5 +- .../okhttp/trace/TracingInterceptor.kt | 59 ++++---- .../DatadogInterceptorWithoutRumTest.kt | 4 +- .../ApmInstrumentationOkHttpAdapterTest.kt | 27 ++-- .../RequestTracingStateRegistryTest.kt | 18 ++- .../RumInstrumentationOkHttpAdapterTest.kt | 30 ++-- .../RumInstrumentationTimingsCounterTest.kt | 8 +- .../TracingInterceptorNonDdTracerTest.kt | 39 ++++++ .../okhttp/trace/TracingInterceptorTest.kt | 107 ++++++++++++++- 23 files changed, 440 insertions(+), 137 deletions(-) diff --git a/features/dd-sdk-android-trace/api/apiSurface b/features/dd-sdk-android-trace/api/apiSurface index e8def0ceca..4b85fa713a 100644 --- a/features/dd-sdk-android-trace/api/apiSurface +++ b/features/dd-sdk-android-trace/api/apiSurface @@ -90,13 +90,15 @@ object com.datadog.android.trace.internal._TraceInternalProxy 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) fun activateSpan(com.datadog.android.trace.api.tracer.DatadogTracer, com.datadog.android.trace.api.span.DatadogSpan, Boolean): com.datadog.android.trace.api.scope.DatadogScope? + fun isSDKBackedTracer(com.datadog.android.trace.api.tracer.DatadogTracer): Boolean fun mergeBaggage(String?, String): String fun createApmNetworkInstrumentation(String, com.datadog.android.trace.ApmNetworkInstrumentationConfiguration) +const val FORCE_DROP_SPAN: String fun com.datadog.android.core.sampling.Sampler.effectiveSampleRate(com.datadog.android.trace.api.span.DatadogSpan): Float? fun com.datadog.android.trace.api.span.DatadogSpanContext?.isDropped(): Boolean fun Int?.isDroppedPriority(): Boolean data class com.datadog.android.trace.internal.net.RequestTracingState - constructor(com.datadog.android.api.instrumentation.network.HttpRequestInfoBuilder, Boolean = false, com.datadog.android.trace.api.span.DatadogSpan? = null, Float? = null) + constructor(com.datadog.android.api.instrumentation.network.HttpRequestInfoBuilder, Boolean = false, com.datadog.android.trace.api.span.DatadogSpan? = null, Float? = null, Boolean) fun createRequestInfo(): com.datadog.android.api.instrumentation.network.HttpRequestInfo fun RequestTracingState?.toAttributesMap(String, String, String): Map class com.datadog.android.trace.internal.net.SessionRebasedSampler : com.datadog.android.core.sampling.Sampler, SpanAwareSampler 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 index 7d866c4596..fc3412aa56 100644 --- a/features/dd-sdk-android-trace/api/dd-sdk-android-trace.api +++ b/features/dd-sdk-android-trace/api/dd-sdk-android-trace.api @@ -167,12 +167,17 @@ public final class com/datadog/android/trace/internal/_TraceInternalProxy { public static final fun addThrowable (Lcom/datadog/android/trace/api/span/DatadogSpan;Ljava/lang/Throwable;B)V public final fun createApmNetworkInstrumentation (Ljava/lang/String;Lcom/datadog/android/trace/ApmNetworkInstrumentationConfiguration;)Lcom/datadog/android/trace/internal/ApmNetworkInstrumentation; public final fun getPropagationHelper ()Lcom/datadog/android/trace/internal/DatadogPropagationHelper; + public final fun isSDKBackedTracer (Lcom/datadog/android/trace/api/tracer/DatadogTracer;)Z public final fun mergeBaggage (Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String; 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; public final fun setTracingSamplingPriorityIfNecessary (Lcom/datadog/android/trace/api/span/DatadogSpanContext;)V } +public final class com/datadog/android/trace/internal/domain/event/MetaKeysKt { + public static final field FORCE_DROP_SPAN Ljava/lang/String; +} + public final class com/datadog/android/trace/internal/net/ApmNetworkInstrumentationExtKt { public static final fun effectiveSampleRate (Lcom/datadog/android/core/sampling/Sampler;Lcom/datadog/android/trace/api/span/DatadogSpan;)Ljava/lang/Float; public static final fun isDropped (Lcom/datadog/android/trace/api/span/DatadogSpanContext;)Z @@ -180,20 +185,22 @@ public final class com/datadog/android/trace/internal/net/ApmNetworkInstrumentat } public final class com/datadog/android/trace/internal/net/RequestTracingState { - public fun (Lcom/datadog/android/api/instrumentation/network/HttpRequestInfoBuilder;ZLcom/datadog/android/trace/api/span/DatadogSpan;Ljava/lang/Float;)V - public synthetic fun (Lcom/datadog/android/api/instrumentation/network/HttpRequestInfoBuilder;ZLcom/datadog/android/trace/api/span/DatadogSpan;Ljava/lang/Float;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lcom/datadog/android/api/instrumentation/network/HttpRequestInfoBuilder;ZLcom/datadog/android/trace/api/span/DatadogSpan;Ljava/lang/Float;Z)V + public synthetic fun (Lcom/datadog/android/api/instrumentation/network/HttpRequestInfoBuilder;ZLcom/datadog/android/trace/api/span/DatadogSpan;Ljava/lang/Float;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V public final fun component1 ()Lcom/datadog/android/api/instrumentation/network/HttpRequestInfoBuilder; public final fun component2 ()Z public final fun component3 ()Lcom/datadog/android/trace/api/span/DatadogSpan; public final fun component4 ()Ljava/lang/Float; - public final fun copy (Lcom/datadog/android/api/instrumentation/network/HttpRequestInfoBuilder;ZLcom/datadog/android/trace/api/span/DatadogSpan;Ljava/lang/Float;)Lcom/datadog/android/trace/internal/net/RequestTracingState; - public static synthetic fun copy$default (Lcom/datadog/android/trace/internal/net/RequestTracingState;Lcom/datadog/android/api/instrumentation/network/HttpRequestInfoBuilder;ZLcom/datadog/android/trace/api/span/DatadogSpan;Ljava/lang/Float;ILjava/lang/Object;)Lcom/datadog/android/trace/internal/net/RequestTracingState; + public final fun component5 ()Z + public final fun copy (Lcom/datadog/android/api/instrumentation/network/HttpRequestInfoBuilder;ZLcom/datadog/android/trace/api/span/DatadogSpan;Ljava/lang/Float;Z)Lcom/datadog/android/trace/internal/net/RequestTracingState; + public static synthetic fun copy$default (Lcom/datadog/android/trace/internal/net/RequestTracingState;Lcom/datadog/android/api/instrumentation/network/HttpRequestInfoBuilder;ZLcom/datadog/android/trace/api/span/DatadogSpan;Ljava/lang/Float;ZILjava/lang/Object;)Lcom/datadog/android/trace/internal/net/RequestTracingState; public final fun createRequestInfo ()Lcom/datadog/android/api/instrumentation/network/HttpRequestInfo; public fun equals (Ljava/lang/Object;)Z public final fun getRequestInfoBuilder ()Lcom/datadog/android/api/instrumentation/network/HttpRequestInfoBuilder; public final fun getSampleRate ()Ljava/lang/Float; public final fun getSpan ()Lcom/datadog/android/trace/api/span/DatadogSpan; public fun hashCode ()I + public final fun isSDKBacked ()Z public final fun isSampled ()Z public fun toString ()Ljava/lang/String; } diff --git a/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/ApmNetworkInstrumentation.kt b/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/ApmNetworkInstrumentation.kt index e7de8c4893..66ec57f29d 100644 --- a/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/ApmNetworkInstrumentation.kt +++ b/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/ApmNetworkInstrumentation.kt @@ -115,7 +115,7 @@ class ApmNetworkInstrumentation internal constructor( } if (sdkCore == null) { - return RequestTracingState(requestInfoBuilder) + return RequestTracingState(requestInfoBuilder = requestInfoBuilder, isSDKBacked = false) } val tracer = tracerProvider.provideTracer( @@ -125,7 +125,7 @@ class ApmNetworkInstrumentation internal constructor( ) if (tracer == null || !request.isTraceable(sdkCore)) { - return RequestTracingState(requestInfoBuilder) + return RequestTracingState(requestInfoBuilder = requestInfoBuilder, isSDKBacked = false) } val span = tracer.buildSpan( @@ -155,7 +155,8 @@ class ApmNetworkInstrumentation internal constructor( span = span, isSampled = isSampled, sampleRate = traceSampler.effectiveSampleRate(span), - requestInfoBuilder = tracedRequestInfoBuilder + requestInfoBuilder = tracedRequestInfoBuilder, + isSDKBacked = tracer is DatadogTracerAdapter ) } @@ -167,17 +168,19 @@ class ApmNetworkInstrumentation internal constructor( * @param response the HTTP response information. */ fun onResponseSucceeded(requestTracingState: RequestTracingState, response: HttpResponseInfo) { - if (requestTracingState.isSampled) { - requestTracingState.span?.setTag(DatadogTracingConstants.Tags.KEY_HTTP_STATUS, response.statusCode) - if (response.statusCode in HttpURLConnection.HTTP_BAD_REQUEST until HttpURLConnection.HTTP_INTERNAL_ERROR) { - requestTracingState.span?.isError = true - } - if (response.statusCode == HttpURLConnection.HTTP_NOT_FOUND && redacted404ResourceName) { - requestTracingState.span?.resourceName = RESOURCE_NAME_404 - } + requestTracingState.span?.setTag(DatadogTracingConstants.Tags.KEY_HTTP_STATUS, response.statusCode) + if (response.statusCode in HttpURLConnection.HTTP_BAD_REQUEST until HttpURLConnection.HTTP_INTERNAL_ERROR) { + requestTracingState.span?.isError = true + } + if (response.statusCode == HttpURLConnection.HTTP_NOT_FOUND && redacted404ResourceName) { + requestTracingState.span?.resourceName = RESOURCE_NAME_404 } requestTracingState.onRequestIntercepted(response, null) - requestTracingState.span?.finishRumAware(requestTracingState.isSampled, canSendSpan) + requestTracingState.span?.finishRumAware( + requestTracingState.isSampled, + canSendSpan, + requestTracingState.isSDKBacked + ) } /** @@ -188,8 +191,8 @@ class ApmNetworkInstrumentation internal constructor( * @param throwable the exception that caused the failure. */ fun onResponseFailed(requestTracingState: RequestTracingState, throwable: Throwable) { + requestTracingState.span?.isError = true if (requestTracingState.isSampled) { - requestTracingState.span?.isError = true requestTracingState.span?.setTag(DatadogTracingConstants.Tags.KEY_ERROR_MSG, throwable.message) requestTracingState.span?.setTag(DatadogTracingConstants.Tags.KEY_ERROR_TYPE, throwable.javaClass.name) requestTracingState.span?.setTag( @@ -198,7 +201,11 @@ class ApmNetworkInstrumentation internal constructor( ) } requestTracingState.onRequestIntercepted(null, throwable) - requestTracingState.span?.finishRumAware(requestTracingState.isSampled, canSendSpan) + requestTracingState.span?.finishRumAware( + requestTracingState.isSampled, + canSendSpan, + requestTracingState.isSDKBacked + ) } /** diff --git a/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/_TraceInternalProxy.kt b/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/_TraceInternalProxy.kt index 8dbe7c073f..e38d2ee40f 100644 --- a/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/_TraceInternalProxy.kt +++ b/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/_TraceInternalProxy.kt @@ -93,6 +93,13 @@ object _TraceInternalProxy { return (tracer as? DatadogTracerAdapter)?.activateSpan(span, asyncPropagating) } + /** + * Returns true if [tracer] is the SDK's built-in tracer, meaning it routes spans through + * CoreTraceWriter which honors the FORCE_DROP_SPAN tag. Custom tracer implementations + * do not honor that tag, so callers must call drop() instead of setTag+finish() for them. + */ + fun isSDKBackedTracer(tracer: DatadogTracer): Boolean = tracer is DatadogTracerAdapter + /** * Merges two baggage headers into a single string representation. The [oldHeader] represents the * previously existing baggage header, and the [newHeader] represents the new baggage information diff --git a/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/data/CoreTraceWriter.kt b/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/data/CoreTraceWriter.kt index 8c97d456eb..892bfff821 100644 --- a/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/data/CoreTraceWriter.kt +++ b/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/data/CoreTraceWriter.kt @@ -19,6 +19,7 @@ import com.datadog.android.event.NoOpEventMapper import com.datadog.android.trace.internal.RumContextPropagator import com.datadog.android.trace.internal.RumContextPropagator.Companion.extractRumContext import com.datadog.android.trace.internal.domain.event.ContextAwareMapper +import com.datadog.android.trace.internal.domain.event.FORCE_DROP_SPAN import com.datadog.android.trace.internal.storage.ContextAwareSerializer import com.datadog.android.trace.model.SpanEvent import com.datadog.trace.api.sampling.PrioritySampling @@ -45,7 +46,10 @@ internal class CoreTraceWriter( sdkCore.getFeature(Feature.TRACING_FEATURE_NAME) ?.withWriteContext { datadogContext, writeScope -> val writeSpans = trace - .filter { it.getTraceSamplingPriority() !in DROP_SAMPLING_PRIORITIES } + .filter { + it.getTraceSamplingPriority() !in DROP_SAMPLING_PRIORITIES && + it.getTag(FORCE_DROP_SPAN) == null + } .map { it.extractRumContext(rumContextPropagator) } // TODO RUM-4092 Add the capability in the serializer to handle multiple spans in one payload writeScope { diff --git a/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/domain/event/MetaKeys.kt b/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/domain/event/MetaKeys.kt index 2640496b66..1bc34c6c76 100644 --- a/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/domain/event/MetaKeys.kt +++ b/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/domain/event/MetaKeys.kt @@ -6,6 +6,13 @@ package com.datadog.android.trace.internal.domain.event +import com.datadog.android.lint.InternalApi + +/** + * Internal tag which indicates the span should be created locally but never sent to the backend. + */ +@InternalApi +const val FORCE_DROP_SPAN: String = "_dd.local.force_drop" internal const val TRACE_ID_META_KEY = "_dd.p.id" internal const val APPLICATION_VARIANT_KEY = "variant" internal const val COMPUTE_STATS_META_KEY = "_dd.compute_stats" diff --git a/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/net/ApmNetworkInstrumentationExt.kt b/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/net/ApmNetworkInstrumentationExt.kt index 305e5f5d59..859e3b239a 100644 --- a/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/net/ApmNetworkInstrumentationExt.kt +++ b/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/net/ApmNetworkInstrumentationExt.kt @@ -23,6 +23,7 @@ import com.datadog.android.trace.internal.ApmNetworkInstrumentation.Companion.ZE import com.datadog.android.trace.internal.ParentContextSource import com.datadog.android.trace.internal._TraceInternalProxy import com.datadog.android.trace.internal._TraceInternalProxy.propagationHelper +import com.datadog.android.trace.internal.domain.event.FORCE_DROP_SPAN import java.util.Locale internal val FeatureSdkCore?.isRumEnabled: Boolean @@ -71,8 +72,12 @@ internal fun DatadogSpan.sample( } } -internal fun DatadogSpan.finishRumAware(isSampled: Boolean, canSendSpan: Boolean) { - if (canSendSpan && isSampled) { +internal fun DatadogSpan.finishRumAware(isSampled: Boolean, canSendSpan: Boolean, isSDKBacked: Boolean) { + if (canSendSpan && (isSDKBacked || isSampled)) { + if (isSDKBacked && !isSampled) { + // SDK-backed tracer: tag the span so CoreTraceWriter suppresses it instead of calling drop() + setTag(FORCE_DROP_SPAN, true) + } finish() } else { drop() diff --git a/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/net/RequestTracingState.kt b/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/net/RequestTracingState.kt index ee27924ddc..73434ddd1c 100644 --- a/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/net/RequestTracingState.kt +++ b/features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/net/RequestTracingState.kt @@ -23,13 +23,15 @@ import com.datadog.android.trace.api.span.DatadogSpan * @property isSampled whether the request trace was sampled. * @property span the trace span created for this request, or null if not sampled or tracing is disabled. * @property sampleRate the sample rate used for the sampling decision. + * @property isSDKBacked whether the tracer is the SDK's built-in implementation */ @InternalApi data class RequestTracingState( val requestInfoBuilder: HttpRequestInfoBuilder, val isSampled: Boolean = false, val span: DatadogSpan? = null, - val sampleRate: Float? = null + val sampleRate: Float? = null, + val isSDKBacked: Boolean ) { /** diff --git a/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/TraceInternalProxyTest.kt b/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/TraceInternalProxyTest.kt index fa4ab6022f..95d546ab94 100644 --- a/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/TraceInternalProxyTest.kt +++ b/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/TraceInternalProxyTest.kt @@ -8,6 +8,7 @@ package com.datadog.android.trace.internal import com.datadog.android.trace.ApmNetworkInstrumentationConfiguration import com.datadog.android.trace.TracingHeaderType +import com.datadog.android.trace.api.tracer.DatadogTracer import com.datadog.android.utils.forge.Configurator import fr.xgouchet.elmyr.Forge import fr.xgouchet.elmyr.annotation.StringForgery @@ -18,8 +19,10 @@ import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith import org.junit.jupiter.api.extension.Extensions +import org.mockito.Mock import org.mockito.junit.jupiter.MockitoExtension import org.mockito.junit.jupiter.MockitoSettings +import org.mockito.kotlin.mock import org.mockito.quality.Strictness @Extensions( @@ -30,6 +33,9 @@ import org.mockito.quality.Strictness @ForgeConfiguration(Configurator::class) internal class TraceInternalProxyTest { + @Mock + lateinit var mockCustomTracer: DatadogTracer + private lateinit var fakeTracedHosts: Map> @StringForgery @@ -83,4 +89,16 @@ internal class TraceInternalProxyTest { assertThat(result.sdkInstanceName).isEqualTo(fakeSdkInstanceName) assertThat(result.traceOrigin).isEqualTo(fakeTraceOrigin) } + + @Test + fun `M return false W isSDKBackedTracer() {custom tracer}`() { + assertThat(_TraceInternalProxy.isSDKBackedTracer(mockCustomTracer)).isFalse() + } + + @Test + fun `M return true W isSDKBackedTracer() {SDK DatadogTracerAdapter}`() { + val sdkTracer: DatadogTracer = mock() + + assertThat(_TraceInternalProxy.isSDKBackedTracer(sdkTracer)).isTrue() + } } diff --git a/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/data/CoreTraceWriterTest.kt b/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/data/CoreTraceWriterTest.kt index 17cb8bc2c2..488cf8ea17 100644 --- a/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/data/CoreTraceWriterTest.kt +++ b/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/data/CoreTraceWriterTest.kt @@ -19,6 +19,7 @@ import com.datadog.android.event.EventMapper import com.datadog.android.log.LogAttributes import com.datadog.android.trace.internal.RumContextPropagator import com.datadog.android.trace.internal.domain.event.ContextAwareMapper +import com.datadog.android.trace.internal.domain.event.FORCE_DROP_SPAN import com.datadog.android.trace.internal.storage.ContextAwareSerializer import com.datadog.android.trace.model.SpanEvent import com.datadog.android.trace.utils.RumContextTestsUtils.RUM_CONTEXT_ACTION_ID @@ -319,6 +320,24 @@ internal class CoreTraceWriterTest { } } + @Test + fun `M not write spans with force drop tag W write()`(forge: Forge) { + // GIVEN + val ddSpans = createNonEmptyDdSpans( + forge = forge, + includeDropSamplingPriority = false + ) + ddSpans.forEach { whenever(it.getTag(FORCE_DROP_SPAN)) doReturn true } + + // WHEN + testedWriter.write(ddSpans) + + // THEN + verifyNoInteractions(mockEventBatchWriter) + + ddSpans.forEach { it.finish() } + } + @Test fun `M not write non-mapped spans W write()`(forge: Forge) { // GIVEN diff --git a/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/net/ApmNetworkInstrumentationTest.kt b/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/net/ApmNetworkInstrumentationTest.kt index 12ae9d5081..6b65335a98 100644 --- a/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/net/ApmNetworkInstrumentationTest.kt +++ b/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/net/ApmNetworkInstrumentationTest.kt @@ -34,6 +34,7 @@ import com.datadog.android.trace.internal.ApmNetworkInstrumentation import com.datadog.android.trace.internal.DatadogPropagationHelper import com.datadog.android.trace.internal.ParentContextSource import com.datadog.android.trace.internal._TraceInternalProxy +import com.datadog.android.trace.internal.domain.event.FORCE_DROP_SPAN import com.datadog.android.utils.forge.Configurator import com.datadog.android.utils.verifyLog import fr.xgouchet.elmyr.Forge @@ -727,7 +728,12 @@ internal class ApmNetworkInstrumentationTest { testedInstrumentation.onResponseSucceeded(traceState, mockResponseInfo) // Then - if (fakeIsSampled) verify(mockSpan).finish() else verify(mockSpan).drop() + if (fakeIsSampled) { + verify(mockSpan, never()).setTag(FORCE_DROP_SPAN, true) + } else { + verify(mockSpan).setTag(FORCE_DROP_SPAN, true) + } + verify(mockSpan).finish() } } @@ -794,7 +800,7 @@ internal class ApmNetworkInstrumentationTest { } @Test - fun `M not set error tags W onResponseFailed() {not sampled}`( + fun `M set isError but not verbose tags W onResponseFailed() {not sampled}`( @Forgery fakeThrowable: Throwable ) { // Given @@ -804,8 +810,9 @@ internal class ApmNetworkInstrumentationTest { // When testedInstrumentation.onResponseFailed(traceState, fakeThrowable) - // Then - verify(mockSpan, never()).isError = true + // Then — isError must be set so the APM stats counts this as an error, + // but verbose payload tags are omitted for unsampled spans + verify(mockSpan).isError = true verify(mockSpan, never()).setTag(eq(Tags.KEY_ERROR_MSG), any()) verify(mockSpan, never()).setTag(eq(Tags.KEY_ERROR_TYPE), any()) verify(mockSpan, never()).setTag(eq(Tags.KEY_ERROR_STACK), any()) @@ -827,7 +834,63 @@ internal class ApmNetworkInstrumentationTest { testedInstrumentation.onResponseFailed(traceState, fakeThrowable) // Then - if (fakeIsSampled) verify(mockSpan).finish() else verify(mockSpan).drop() + if (fakeIsSampled) { + verify(mockSpan, never()).setTag(FORCE_DROP_SPAN, true) + } else { + verify(mockSpan).setTag(FORCE_DROP_SPAN, true) + } + verify(mockSpan).finish() + } + } + + @ParameterizedTest + @ValueSource(booleans = [true, false]) + fun `M finish or drop span W onResponseSucceeded() {custom tracer}`( + fakeIsSampled: Boolean, + @IntForgery(min = 200, max = 299) fakeStatusCode: Int + ) { + // Given + whenever(mockResponseInfo.statusCode) doReturn fakeStatusCode + val traceState = createTraceState(isSampled = fakeIsSampled, isSDKBacked = false) + + _TraceInternalProxy.withMockPropagationHelper(mockPropagationHelper) { + // When + testedInstrumentation.onResponseSucceeded(traceState, mockResponseInfo) + + // Then — custom tracers must call drop() directly; FORCE_DROP_SPAN is SDK-internal + verify(mockSpan, never()).setTag(FORCE_DROP_SPAN, true) + if (fakeIsSampled) { + verify(mockSpan).finish() + verify(mockSpan, never()).drop() + } else { + verify(mockSpan, never()).finish() + verify(mockSpan).drop() + } + } + } + + @ParameterizedTest + @ValueSource(booleans = [true, false]) + fun `M finish or drop span W onResponseFailed() {custom tracer}`( + fakeIsSampled: Boolean, + @Forgery fakeThrowable: Throwable + ) { + // Given + val traceState = createTraceState(isSampled = fakeIsSampled, isSDKBacked = false) + + _TraceInternalProxy.withMockPropagationHelper(mockPropagationHelper) { + // When + testedInstrumentation.onResponseFailed(traceState, fakeThrowable) + + // Then — custom tracers must call drop() directly; FORCE_DROP_SPAN is SDK-internal + verify(mockSpan, never()).setTag(FORCE_DROP_SPAN, true) + if (fakeIsSampled) { + verify(mockSpan).finish() + verify(mockSpan, never()).drop() + } else { + verify(mockSpan, never()).finish() + verify(mockSpan).drop() + } } } @@ -857,25 +920,6 @@ internal class ApmNetworkInstrumentationTest { } } - @Test - fun `M finish span W onResponseSucceeded() {APP_LEVEL, canSendSpan=true, RUM enabled, sampled}`( - @IntForgery(min = 200, max = 299) fakeStatusCode: Int - ) { - // Given - canSendSpan=true is set in setUp - whenever(mockResponseInfo.statusCode) doReturn fakeStatusCode - - val traceState = createTraceState() - - _TraceInternalProxy.withMockPropagationHelper(mockPropagationHelper) { - // When - testedInstrumentation.onResponseSucceeded(traceState, mockResponseInfo) - - // Then - verify(mockSpan).finish() - verify(mockSpan, never()).drop() - } - } - @Test fun `M drop span W onResponseSucceeded() {APP_LEVEL, canSendSpan=false, RUM disabled, sampled}`( @IntForgery(min = 200, max = 299) fakeStatusCode: Int @@ -918,23 +962,6 @@ internal class ApmNetworkInstrumentationTest { } } - @Test - fun `M finish span W onResponseFailed() {APP_LEVEL, canSendSpan=true, RUM enabled, sampled}`( - @Forgery fakeThrowable: Throwable - ) { - // Given - canSendSpan=true is set in setUp - val traceState = createTraceState() - - _TraceInternalProxy.withMockPropagationHelper(mockPropagationHelper) { - // When - testedInstrumentation.onResponseFailed(traceState, fakeThrowable) - - // Then - verify(mockSpan).finish() - verify(mockSpan, never()).drop() - } - } - @Test fun `M drop span W onResponseFailed() {APP_LEVEL, canSendSpan=false, RUM disabled, sampled}`( @Forgery fakeThrowable: Throwable @@ -991,7 +1018,12 @@ internal class ApmNetworkInstrumentationTest { testedInstrumentation.onResponseSucceeded(traceState, mockResponseInfo) // Then - if (fakeIsSampled) verify(mockSpan).finish() else verify(mockSpan).drop() + if (fakeIsSampled) { + verify(mockSpan, never()).setTag(FORCE_DROP_SPAN, true) + } else { + verify(mockSpan).setTag(FORCE_DROP_SPAN, true) + } + verify(mockSpan).finish() } } @@ -1010,7 +1042,12 @@ internal class ApmNetworkInstrumentationTest { testedInstrumentation.onResponseFailed(traceState, fakeThrowable) // Then - if (fakeIsSampled) verify(mockSpan).finish() else verify(mockSpan).drop() + if (fakeIsSampled) { + verify(mockSpan, never()).setTag(FORCE_DROP_SPAN, true) + } else { + verify(mockSpan).setTag(FORCE_DROP_SPAN, true) + } + verify(mockSpan).finish() } } @@ -1156,11 +1193,12 @@ internal class ApmNetworkInstrumentationTest { // endregion - private fun createTraceState(isSampled: Boolean = true) = RequestTracingState( + private fun createTraceState(isSampled: Boolean = true, isSDKBacked: Boolean = true) = RequestTracingState( requestInfoBuilder = mockRequestBuilder, isSampled = isSampled, span = mockSpan, - sampleRate = fakeSampleRate + sampleRate = fakeSampleRate, + isSDKBacked = isSDKBacked ) private fun createInstrumentation( diff --git a/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/net/RequestTracingStateTest.kt b/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/net/RequestTracingStateTest.kt index 6ee7b3f049..5219ca9fd5 100644 --- a/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/net/RequestTracingStateTest.kt +++ b/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/net/RequestTracingStateTest.kt @@ -62,7 +62,7 @@ internal class RequestTracingStateTest { fun `M return request info W createRequestInfo()`() { // Given whenever(mockRequestBuilder.build()) doReturn mockRequestInfo - val state = RequestTracingState(requestInfoBuilder = mockRequestBuilder) + val state = RequestTracingState(requestInfoBuilder = mockRequestBuilder, isSDKBacked = false) // When / Then assertThat(state.createRequestInfo()).isSameAs(mockRequestInfo) @@ -86,7 +86,8 @@ internal class RequestTracingStateTest { val state = RequestTracingState( requestInfoBuilder = mockRequestBuilder, isSampled = true, - span = null + span = null, + isSDKBacked = false ) // When @@ -102,7 +103,8 @@ internal class RequestTracingStateTest { val state = RequestTracingState( requestInfoBuilder = mockRequestBuilder, isSampled = false, - span = mockSpan + span = mockSpan, + isSDKBacked = false ) // When @@ -129,7 +131,8 @@ internal class RequestTracingStateTest { requestInfoBuilder = mockRequestBuilder, isSampled = true, span = mockSpan, - sampleRate = fakeSampleRate + sampleRate = fakeSampleRate, + isSDKBacked = false ) // When @@ -157,7 +160,8 @@ internal class RequestTracingStateTest { requestInfoBuilder = mockRequestBuilder, isSampled = true, span = mockSpan, - sampleRate = null + sampleRate = null, + isSDKBacked = false ) // When diff --git a/integrations/dd-sdk-android-cronet/src/main/kotlin/com/datadog/android/cronet/internal/CronetRequestCallback.kt b/integrations/dd-sdk-android-cronet/src/main/kotlin/com/datadog/android/cronet/internal/CronetRequestCallback.kt index cd25c39ac5..af3ed6222e 100644 --- a/integrations/dd-sdk-android-cronet/src/main/kotlin/com/datadog/android/cronet/internal/CronetRequestCallback.kt +++ b/integrations/dd-sdk-android-cronet/src/main/kotlin/com/datadog/android/cronet/internal/CronetRequestCallback.kt @@ -41,7 +41,12 @@ internal class CronetRequestCallback( apmNetworkInstrumentation?.onRequest(finalRequestInfo) .also(apmTracingStateHolder::set) - return distributedTracingState ?: RequestTracingState(initialRequestInfo.newBuilder()) + // Fallback when no APM instrumentation is registered — no span is created so isSDKBacked + // is never consulted (finishRumAware is a no-op on a null span). + return distributedTracingState ?: RequestTracingState( + requestInfoBuilder = initialRequestInfo.newBuilder(), + isSDKBacked = false + ) } override fun onRedirectReceived( diff --git a/integrations/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/DatadogInterceptor.kt b/integrations/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/DatadogInterceptor.kt index c9dc44ab6b..08240f1a5a 100644 --- a/integrations/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/DatadogInterceptor.kt +++ b/integrations/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/DatadogInterceptor.kt @@ -39,6 +39,7 @@ import com.datadog.android.trace.TraceContextInjection import com.datadog.android.trace.TracingHeaderType import com.datadog.android.trace.api.span.DatadogSpan import com.datadog.android.trace.api.tracer.DatadogTracer +import com.datadog.android.trace.internal._TraceInternalProxy import com.datadog.android.trace.internal.net.SessionRebasedSampler import com.datadog.android.trace.internal.net.effectiveSampleRate import okhttp3.Interceptor @@ -92,7 +93,8 @@ open class DatadogInterceptor internal constructor( redacted404ResourceName: Boolean, localTracerFactory: (SdkCore, Set) -> DatadogTracer, globalTracerProvider: () -> DatadogTracer?, - internal val resourceHeadersExtractor: ResourceHeadersExtractor? = null + internal val resourceHeadersExtractor: ResourceHeadersExtractor? = null, + sdkBackedCheck: (DatadogTracer) -> Boolean = _TraceInternalProxy::isSDKBackedTracer ) : TracingInterceptor( sdkInstanceName, tracedHosts, @@ -102,7 +104,8 @@ open class DatadogInterceptor internal constructor( traceContextInjection, redacted404ResourceName, localTracerFactory, - globalTracerProvider + globalTracerProvider, + sdkBackedCheck ) { internal val rumResourceAttributesProvider: RumResourceAttributesProvider = rumResourceAttributesProvider as? NoOpRumResourceAttributesProvider diff --git a/integrations/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/internal/RequestTracingStateRegistry.kt b/integrations/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/internal/RequestTracingStateRegistry.kt index def12a0fe0..487d6120d7 100644 --- a/integrations/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/internal/RequestTracingStateRegistry.kt +++ b/integrations/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/internal/RequestTracingStateRegistry.kt @@ -59,10 +59,11 @@ internal class RequestTracingStateRegistry( } requestTracingStateByCall[call] = RequestTracingState( - call.request() + requestInfoBuilder = call.request() .toHttpRequestInfo() .newBuilder() - .addTag(UUID::class.java, UUID.randomUUID()) + .addTag(UUID::class.java, UUID.randomUUID()), + isSDKBacked = false ) } 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 index 7432bcaa01..80ad522a7b 100644 --- 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 @@ -38,6 +38,7 @@ import com.datadog.android.trace.internal.ParentContextSource import com.datadog.android.trace.internal.RumContextPropagator import com.datadog.android.trace.internal.RumContextPropagator.Companion.extractRumContext import com.datadog.android.trace.internal._TraceInternalProxy +import com.datadog.android.trace.internal.domain.event.FORCE_DROP_SPAN import com.datadog.android.trace.internal.net.TraceContext import com.datadog.android.trace.internal.net.effectiveSampleRate import com.datadog.android.trace.internal.net.isDropped @@ -90,7 +91,8 @@ internal constructor( internal val traceContextInjection: TraceContextInjection, internal val redacted404ResourceName: Boolean, internal val localTracerFactory: (SdkCore, Set) -> DatadogTracer, - private val globalTracerProvider: () -> DatadogTracer? + private val globalTracerProvider: () -> DatadogTracer?, + internal val sdkBackedCheck: (DatadogTracer) -> Boolean = _TraceInternalProxy::isSDKBackedTracer ) : Interceptor { private val localTracerReference: AtomicReference = AtomicReference() @@ -250,6 +252,7 @@ internal constructor( request: Request, tracer: DatadogTracer ): Response { + val isSDKBacked = sdkBackedCheck(tracer) val span = buildSpan(tracer, request) val isSampled = span.extractRumContext(rumContextPropagator, block = true) .sample(request, ignoreLocalDroppedParent = !canSendSpan()) @@ -284,10 +287,10 @@ internal constructor( try { val response = chain.proceed(updatedRequest) - handleResponse(sdkCore, request, response, span, isSampled) + handleResponse(sdkCore, request, response, span, isSampled, isSDKBacked) return response } catch (e: Throwable) { - handleThrowable(sdkCore, request, e, span, isSampled) + handleThrowable(sdkCore, request, e, span, isSampled, isSDKBacked) throw e } } @@ -698,22 +701,23 @@ internal constructor( request: Request, response: Response, span: DatadogSpan, - isSampled: Boolean + isSampled: Boolean, + isSDKBacked: Boolean ) { - if (!isSampled) { - onRequestIntercepted(sdkCore, request, null, response, null) - } else { - val statusCode = response.code - span.setTag(Tags.KEY_HTTP_STATUS, statusCode) - if (statusCode in HttpURLConnection.HTTP_BAD_REQUEST until HttpURLConnection.HTTP_INTERNAL_ERROR) { - span.isError = true - } - if (statusCode == HttpURLConnection.HTTP_NOT_FOUND && redacted404ResourceName) { - span.resourceName = RESOURCE_NAME_404 - } + val statusCode = response.code + span.setTag(Tags.KEY_HTTP_STATUS, statusCode) + if (statusCode in HttpURLConnection.HTTP_BAD_REQUEST until HttpURLConnection.HTTP_INTERNAL_ERROR) { + span.isError = true + } + if (statusCode == HttpURLConnection.HTTP_NOT_FOUND && redacted404ResourceName) { + span.resourceName = RESOURCE_NAME_404 + } + if (isSampled) { onRequestIntercepted(sdkCore, request, span, response, null) + } else { + onRequestIntercepted(sdkCore, request, null, response, null) } - span.finishRumAware(isSampled) + span.finishRumAware(isSampled, isSDKBacked) } private fun handleThrowable( @@ -721,23 +725,28 @@ internal constructor( request: Request, throwable: Throwable, span: DatadogSpan, - isSampled: Boolean + isSampled: Boolean, + isSDKBacked: Boolean ) { - if (!isSampled) { - onRequestIntercepted(sdkCore, request, null, null, throwable) - } else { - span.isError = true + span.isError = true + if (isSampled) { span.setTag(Tags.KEY_ERROR_MSG, throwable.message) span.setTag(Tags.KEY_ERROR_TYPE, throwable.javaClass.name) span.setTag(Tags.KEY_ERROR_STACK, throwable.loggableStackTrace()) onRequestIntercepted(sdkCore, request, span, null, throwable) + } else { + onRequestIntercepted(sdkCore, request, null, null, throwable) } - span.finishRumAware(isSampled) + span.finishRumAware(isSampled, isSDKBacked) } - private fun DatadogSpan.finishRumAware(isSampled: Boolean) { - if (canSendSpan()) { - if (isSampled) finish() else drop() + private fun DatadogSpan.finishRumAware(isSampled: Boolean, isSDKBacked: Boolean) { + if (canSendSpan() && (isSDKBacked || isSampled)) { + if (isSDKBacked && !isSampled) { + // SDK-backed tracer: tag the span so CoreTraceWriter suppresses it instead of calling drop() + setTag(FORCE_DROP_SPAN, true) + } + finish() } else { drop() } diff --git a/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/DatadogInterceptorWithoutRumTest.kt b/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/DatadogInterceptorWithoutRumTest.kt index ad84e9ce9d..4a25ad2667 100644 --- a/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/DatadogInterceptorWithoutRumTest.kt +++ b/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/DatadogInterceptorWithoutRumTest.kt @@ -54,6 +54,7 @@ internal class DatadogInterceptorWithoutRumTest : TracingInterceptorTest() { override fun instantiateTestedInterceptor( tracedHosts: Map>, globalTracerProvider: () -> DatadogTracer?, + sdkBackedCheck: (DatadogTracer) -> Boolean, localTracerFactory: (SdkCore, Set) -> DatadogTracer ): TracingInterceptor { return DatadogInterceptor( @@ -65,7 +66,8 @@ internal class DatadogInterceptorWithoutRumTest : TracingInterceptorTest() { traceContextInjection = TraceContextInjection.ALL, redacted404ResourceName = fakeRedacted404Resources, localTracerFactory = localTracerFactory, - globalTracerProvider = globalTracerProvider + globalTracerProvider = globalTracerProvider, + sdkBackedCheck = sdkBackedCheck ) } diff --git a/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/ApmInstrumentationOkHttpAdapterTest.kt b/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/ApmInstrumentationOkHttpAdapterTest.kt index e3db1d2b4b..d1253e2a81 100644 --- a/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/ApmInstrumentationOkHttpAdapterTest.kt +++ b/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/ApmInstrumentationOkHttpAdapterTest.kt @@ -107,7 +107,8 @@ internal class ApmInstrumentationOkHttpAdapterTest { val mockRequestBuilder = mockRequestInfoBuilder(modifiedRequestInfo) val fakeTracingState = RequestTracingState( requestInfoBuilder = mockRequestBuilder, - isSampled = true + isSampled = true, + isSDKBacked = false ) whenever(mockApmNetworkInstrumentation.onRequest(any())) doReturn fakeTracingState @@ -139,7 +140,8 @@ internal class ApmInstrumentationOkHttpAdapterTest { val mockRequestBuilder = mockRequestInfoBuilder(modifiedRequestInfo) val fakeTracingState = RequestTracingState( requestInfoBuilder = mockRequestBuilder, - isSampled = true + isSampled = true, + isSDKBacked = false ) whenever(mockApmNetworkInstrumentation.onRequest(any())) doReturn fakeTracingState @@ -199,7 +201,8 @@ internal class ApmInstrumentationOkHttpAdapterTest { val mockRequestBuilder = mockRequestInfoBuilder(modifiedRequestInfo) val fakeTracingState = RequestTracingState( requestInfoBuilder = mockRequestBuilder, - isSampled = true + isSampled = true, + isSDKBacked = false ) whenever(mockApmNetworkInstrumentation.onRequest(any())) doReturn fakeTracingState @@ -222,7 +225,8 @@ internal class ApmInstrumentationOkHttpAdapterTest { val mockRequestBuilder = mockRequestInfoBuilder(null) val fakeTracingState = RequestTracingState( requestInfoBuilder = mockRequestBuilder, - isSampled = true + isSampled = true, + isSDKBacked = false ) whenever(mockApmNetworkInstrumentation.onRequest(any())) doReturn fakeTracingState @@ -312,7 +316,8 @@ internal class ApmInstrumentationOkHttpAdapterTest { val mockRequestBuilder = mockRequestInfoBuilder(modifiedRequestInfo) val fakeTracingState = RequestTracingState( requestInfoBuilder = mockRequestBuilder, - isSampled = true + isSampled = true, + isSDKBacked = false ) whenever(mockApmNetworkInstrumentation.onRequest(any())) doReturn fakeTracingState whenever(mockChain.proceed(any())) doReturn forgeResponse(statusCode) @@ -342,7 +347,8 @@ internal class ApmInstrumentationOkHttpAdapterTest { val mockRequestBuilder = mockRequestInfoBuilder(modifiedRequestInfo) val fakeTracingState = RequestTracingState( requestInfoBuilder = mockRequestBuilder, - isSampled = true + isSampled = true, + isSDKBacked = false ) whenever(mockApmNetworkInstrumentation.onRequest(any())) doReturn fakeTracingState @@ -380,7 +386,8 @@ internal class ApmInstrumentationOkHttpAdapterTest { val mockRequestBuilder = mockRequestInfoBuilder(modifiedRequestInfo) val fakeTracingState = RequestTracingState( requestInfoBuilder = mockRequestBuilder, - isSampled = true + isSampled = true, + isSDKBacked = false ) whenever(mockApmNetworkInstrumentation.onRequest(any())) doReturn fakeTracingState @@ -411,7 +418,8 @@ internal class ApmInstrumentationOkHttpAdapterTest { val mockRequestBuilder = mockRequestInfoBuilder(tracedRequestInfo) val fakeTracingState = RequestTracingState( requestInfoBuilder = mockRequestBuilder, - isSampled = true + isSampled = true, + isSDKBacked = false ) whenever(mockApmNetworkInstrumentation.onRequest(any())) doReturn fakeTracingState @@ -447,7 +455,8 @@ internal class ApmInstrumentationOkHttpAdapterTest { val mockRequestBuilder = mockRequestInfoBuilder(modifiedRequestInfo) val fakeTracingState = RequestTracingState( requestInfoBuilder = mockRequestBuilder, - isSampled = true + isSampled = true, + isSDKBacked = false ) whenever(mockApmNetworkInstrumentation.onRequest(any())) doReturn fakeTracingState diff --git a/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/RequestTracingStateRegistryTest.kt b/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/RequestTracingStateRegistryTest.kt index b64ed3f85f..d11b46548e 100644 --- a/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/RequestTracingStateRegistryTest.kt +++ b/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/RequestTracingStateRegistryTest.kt @@ -167,7 +167,8 @@ internal class RequestTracingStateRegistryTest { ) val newState = RequestTracingState( requestInfoBuilder = newRequestBuilder, - isSampled = true + isSampled = true, + isSDKBacked = false ) // When @@ -184,7 +185,8 @@ internal class RequestTracingStateRegistryTest { requestInfoBuilder = OkHttpRequestInfoBuilder( Request.Builder().url(fakeUrl) ), - isSampled = true + isSampled = true, + isSDKBacked = false ) // When @@ -206,7 +208,8 @@ internal class RequestTracingStateRegistryTest { ) val newState = RequestTracingState( requestInfoBuilder = newRequestBuilder, - isSampled = true + isSampled = true, + isSDKBacked = false ) // When @@ -226,7 +229,8 @@ internal class RequestTracingStateRegistryTest { ) val newState = RequestTracingState( requestInfoBuilder = newRequestBuilder, - isSampled = true + isSampled = true, + isSDKBacked = false ) // When @@ -320,7 +324,8 @@ internal class RequestTracingStateRegistryTest { requestInfoBuilder = newRequestBuilder, isSampled = true, span = mockSpan, - sampleRate = fakeSampleRate + sampleRate = fakeSampleRate, + isSDKBacked = false ) // When @@ -366,7 +371,8 @@ internal class RequestTracingStateRegistryTest { requestInfoBuilder = OkHttpRequestInfoBuilder( Request.Builder().url(call.request().url.toString()) ), - isSampled = true + isSampled = true, + isSDKBacked = false ) ) testedRegistry.get(call) diff --git a/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/RumInstrumentationOkHttpAdapterTest.kt b/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/RumInstrumentationOkHttpAdapterTest.kt index cb6516c747..3b05eb96b6 100644 --- a/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/RumInstrumentationOkHttpAdapterTest.kt +++ b/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/RumInstrumentationOkHttpAdapterTest.kt @@ -111,7 +111,7 @@ internal class RumInstrumentationOkHttpAdapterTest { // Given val fakeRequestBuilder = OkHttpRequestInfoBuilder(fakeRequest.newBuilder()) .addTag(UUID::class.java, fakeUuid) - val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder) + val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder, isSDKBacked = false) whenever(mockRegistry.get(mockCall)) doReturn fakeState val fakeResponse = forgeResponse(statusCode) whenever(mockChain.proceed(any())) doReturn fakeResponse @@ -134,7 +134,7 @@ internal class RumInstrumentationOkHttpAdapterTest { // Given val fakeRequestBuilder = OkHttpRequestInfoBuilder(fakeRequest.newBuilder()) .addTag(UUID::class.java, fakeUuid) - val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder) + val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder, isSDKBacked = false) whenever(mockRegistry.get(mockCall)) doReturn fakeState val fakeException = IOException("network error") whenever(mockChain.proceed(any())) doThrow fakeException @@ -205,7 +205,7 @@ internal class RumInstrumentationOkHttpAdapterTest { // Given val fakeRequestBuilder = OkHttpRequestInfoBuilder(fakeRequest.newBuilder()) .addTag(UUID::class.java, fakeUuid) - val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder) + val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder, isSDKBacked = false) whenever(mockRegistry.get(mockCall)) doReturn fakeState val fakeResponse = forgeResponse(statusCode) whenever(mockChain.proceed(any())) doReturn fakeResponse @@ -228,7 +228,7 @@ internal class RumInstrumentationOkHttpAdapterTest { // Given val fakeRequestBuilder = OkHttpRequestInfoBuilder(fakeRequest.newBuilder()) .addTag(UUID::class.java, fakeUuid) - val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder) + val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder, isSDKBacked = false) whenever(mockRegistry.get(mockCall)) doReturn fakeState val fakeResponse = forgeResponse(statusCode) whenever(mockChain.proceed(any())) doReturn fakeResponse @@ -255,7 +255,7 @@ internal class RumInstrumentationOkHttpAdapterTest { // Given val fakeRequestBuilder = OkHttpRequestInfoBuilder(fakeRequest.newBuilder()) .addTag(UUID::class.java, fakeUuid) - val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder) + val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder, isSDKBacked = false) var callCount = 0 whenever(mockRegistry.get(mockCall)).thenAnswer { callCount++ @@ -293,7 +293,7 @@ internal class RumInstrumentationOkHttpAdapterTest { val fakeRequestBuilder = OkHttpRequestInfoBuilder(fakeRequest.newBuilder()) .addTag(UUID::class.java, fakeUuid) - val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder) + val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder, isSDKBacked = false) whenever(mockRegistry.get(mockCall)) doReturn fakeState val fakeResponse = forgeResponse(statusCode) whenever(mockChain.proceed(any())) doReturn fakeResponse @@ -322,7 +322,7 @@ internal class RumInstrumentationOkHttpAdapterTest { val fakeRequestBuilder = OkHttpRequestInfoBuilder(fakeRequest.newBuilder()) .addTag(UUID::class.java, fakeUuid) - val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder) + val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder, isSDKBacked = false) whenever(mockRegistry.get(mockCall)) doReturn fakeState val fakeResponse = forgeResponse(statusCode) whenever(mockChain.proceed(any())) doReturn fakeResponse @@ -349,7 +349,7 @@ internal class RumInstrumentationOkHttpAdapterTest { val fakeRequestBuilder = OkHttpRequestInfoBuilder(fakeRequest.newBuilder()) .addTag(UUID::class.java, fakeUuid) - val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder) + val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder, isSDKBacked = false) whenever(mockRegistry.get(mockCall)) doReturn fakeState val fakeResponse = forgeResponse(statusCode) whenever(mockChain.proceed(any())) doReturn fakeResponse @@ -378,7 +378,7 @@ internal class RumInstrumentationOkHttpAdapterTest { val fakeRequestBuilder = OkHttpRequestInfoBuilder(fakeRequest.newBuilder()) .addTag(UUID::class.java, fakeUuid) - val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder) + val fakeState = RequestTracingState(requestInfoBuilder = fakeRequestBuilder, isSDKBacked = false) whenever(mockRegistry.get(mockCall)) doReturn fakeState val fakeResponse = forgeResponse(statusCode) whenever(mockChain.proceed(any())) doReturn fakeResponse @@ -409,7 +409,8 @@ internal class RumInstrumentationOkHttpAdapterTest { ) val fakeDistributedTracingState = RequestTracingState( requestInfoBuilder = OkHttpRequestInfoBuilder(fakeRequest.newBuilder()) - .addTag(UUID::class.java, fakeUuid) + .addTag(UUID::class.java, fakeUuid), + isSDKBacked = false ) whenever(mockDistributedTracingInstrumentation.onRequest(any())) doReturn fakeDistributedTracingState whenever(mockChain.proceed(any())) doReturn forgeResponse(statusCode) @@ -435,7 +436,8 @@ internal class RumInstrumentationOkHttpAdapterTest { val fakeDistributedTracingState = RequestTracingState( requestInfoBuilder = OkHttpRequestInfoBuilder(fakeRequest.newBuilder()) .addTag(UUID::class.java, fakeUuid) - .addHeader("x-datadog-trace-id", fakeTraceId) + .addHeader("x-datadog-trace-id", fakeTraceId), + isSDKBacked = false ) whenever(mockDistributedTracingInstrumentation.onRequest(any())) doReturn fakeDistributedTracingState whenever(mockChain.proceed(any())) doReturn forgeResponse(statusCode) @@ -462,7 +464,8 @@ internal class RumInstrumentationOkHttpAdapterTest { ) val fakeDistributedTracingState = RequestTracingState( requestInfoBuilder = OkHttpRequestInfoBuilder(fakeRequest.newBuilder()) - .addTag(UUID::class.java, fakeUuid) + .addTag(UUID::class.java, fakeUuid), + isSDKBacked = false ) whenever(mockDistributedTracingInstrumentation.onRequest(any())) doReturn fakeDistributedTracingState whenever(mockChain.proceed(any())) doReturn forgeResponse(statusCode) @@ -487,7 +490,8 @@ internal class RumInstrumentationOkHttpAdapterTest { ) val fakeDistributedTracingState = RequestTracingState( requestInfoBuilder = OkHttpRequestInfoBuilder(fakeRequest.newBuilder()) - .addTag(UUID::class.java, fakeUuid) + .addTag(UUID::class.java, fakeUuid), + isSDKBacked = false ) whenever(mockDistributedTracingInstrumentation.onRequest(any())) doReturn fakeDistributedTracingState val fakeException = IOException("network error") diff --git a/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/RumInstrumentationTimingsCounterTest.kt b/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/RumInstrumentationTimingsCounterTest.kt index 19fac92597..bcd060376f 100644 --- a/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/RumInstrumentationTimingsCounterTest.kt +++ b/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/RumInstrumentationTimingsCounterTest.kt @@ -92,7 +92,7 @@ internal class RumInstrumentationTimingsCounterTest { .build() whenever(mockRequestBuilder.build()) doReturn mockRequestInfo - val fakeTracingState = RequestTracingState(requestInfoBuilder = mockRequestBuilder) + val fakeTracingState = RequestTracingState(requestInfoBuilder = mockRequestBuilder, isSDKBacked = false) whenever(mockRequestInfoRegistry.get(mockCall)) doReturn fakeTracingState whenever(mockSdkCore.time).thenReturn( @@ -285,7 +285,7 @@ internal class RumInstrumentationTimingsCounterTest { fun `M send timing W callEnd() { full request lifecycle }`() { // Given whenever(mockRequestInfoRegistry.remove(mockCall)) doReturn - RequestTracingState(requestInfoBuilder = mockRequestBuilder) + RequestTracingState(requestInfoBuilder = mockRequestBuilder, isSDKBacked = false) // When testedListener.callStart(mockCall) @@ -325,7 +325,7 @@ internal class RumInstrumentationTimingsCounterTest { ) { // Given whenever(mockRequestInfoRegistry.remove(mockCall)) doReturn - RequestTracingState(requestInfoBuilder = mockRequestBuilder) + RequestTracingState(requestInfoBuilder = mockRequestBuilder, isSDKBacked = false) // When testedListener.callStart(mockCall) @@ -390,7 +390,7 @@ internal class RumInstrumentationTimingsCounterTest { fun `M send timing with zeroes for missing phases W callEnd() { reused connection }`() { // Given whenever(mockRequestInfoRegistry.remove(mockCall)) doReturn - RequestTracingState(requestInfoBuilder = mockRequestBuilder) + RequestTracingState(requestInfoBuilder = mockRequestBuilder, isSDKBacked = false) // When testedListener.callStart(mockCall) diff --git a/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/trace/TracingInterceptorNonDdTracerTest.kt b/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/trace/TracingInterceptorNonDdTracerTest.kt index 0aacc3c5bf..f73b248443 100644 --- a/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/trace/TracingInterceptorNonDdTracerTest.kt +++ b/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/trace/TracingInterceptorNonDdTracerTest.kt @@ -67,6 +67,7 @@ import org.mockito.kotlin.doAnswer import org.mockito.kotlin.doReturn import org.mockito.kotlin.doThrow import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions @@ -1024,6 +1025,44 @@ internal open class TracingInterceptorNonDdTracerTest { verify(mockSpan).finish() } + @Test + fun `M drop span W intercept() {custom global tracer, not sampled, successful request}`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given - backwards-compat guarantee: custom tracers do not honour FORCE_DROP_SPAN, + // so sampled-out spans must be dropped via drop(), not setTag+finish. + whenever(mockTraceSampler.sample(mockSpan)).thenReturn(false) + whenever(mockResolver.isFirstPartyUrl(fakeUrl.toHttpUrl())).thenReturn(true) + stubChain(mockChain, statusCode) + + // When + testedInterceptor.intercept(mockChain) + + // Then + verify(mockSpan).drop() + verify(mockSpan, never()).finish() + } + + @Test + fun `M drop span W intercept() {custom global tracer, not sampled, throwing request}`( + @Forgery throwable: Throwable + ) { + // Given + whenever(mockTraceSampler.sample(mockSpan)).thenReturn(false) + whenever(mockResolver.isFirstPartyUrl(fakeUrl.toHttpUrl())).thenReturn(true) + whenever(mockChain.request()) doReturn fakeRequest + whenever(mockChain.proceed(any())) doThrow throwable + + // When + assertThrows(throwable.message.orEmpty()) { + testedInterceptor.intercept(mockChain) + } + + // Then + verify(mockSpan).drop() + verify(mockSpan, never()).finish() + } + @Test fun `M warn the user W intercept() no tracer registered and TracingFeature not initialized`( @IntForgery(min = 200, max = 300) statusCode: Int diff --git a/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/trace/TracingInterceptorTest.kt b/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/trace/TracingInterceptorTest.kt index 9c4cc0d873..a9c4fb50aa 100644 --- a/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/trace/TracingInterceptorTest.kt +++ b/integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/trace/TracingInterceptorTest.kt @@ -35,6 +35,7 @@ import com.datadog.android.trace.api.tracer.DatadogTracer import com.datadog.android.trace.api.withMockPropagationHelper import com.datadog.android.trace.internal.DatadogPropagationHelper import com.datadog.android.trace.internal._TraceInternalProxy +import com.datadog.android.trace.internal.domain.event.FORCE_DROP_SPAN import com.datadog.android.trace.internal.fromHex import com.datadog.android.trace.internal.net.TraceContext import com.datadog.android.utils.verifyLog @@ -229,6 +230,7 @@ internal open class TracingInterceptorTest { open fun instantiateTestedInterceptor( tracedHosts: Map> = emptyMap(), globalTracerProvider: () -> DatadogTracer? = { null }, + sdkBackedCheck: (DatadogTracer) -> Boolean = _TraceInternalProxy::isSDKBackedTracer, localTracerFactory: (SdkCore, Set) -> DatadogTracer ): TracingInterceptor { return TracingInterceptor( @@ -240,7 +242,8 @@ internal open class TracingInterceptorTest { localTracerFactory = localTracerFactory, redacted404ResourceName = fakeRedacted404Resources, traceContextInjection = TraceContextInjection.ALL, - globalTracerProvider = globalTracerProvider + globalTracerProvider = globalTracerProvider, + sdkBackedCheck = sdkBackedCheck ) } @@ -1263,6 +1266,108 @@ internal open class TracingInterceptorTest { verify(mockSpan).finish() } + @Test + fun `M force drop span W intercept() {not sampled, successful request, SDK-backed tracer}`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + testedInterceptor = instantiateTestedInterceptor( + fakeLocalHosts, + globalTracerProvider = { mockTracer }, + sdkBackedCheck = { true }, + localTracerFactory = { _, _ -> mockLocalTracer } + ) + whenever(mockResolver.isFirstPartyUrl(fakeUrl.toHttpUrl())).thenReturn(true) + whenever(mockTraceSampler.sample(mockSpan)).thenReturn(false) + stubChain(mockChain, statusCode) + + // When + testedInterceptor.intercept(mockChain) + + // Then + verify(mockSpan).setTag(FORCE_DROP_SPAN, true) + verify(mockSpan).finish() + verify(mockSpan, never()).drop() + } + + @Test + fun `M drop span W intercept() {not sampled, successful request, custom tracer}`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + testedInterceptor = instantiateTestedInterceptor( + fakeLocalHosts, + globalTracerProvider = { mockTracer }, + sdkBackedCheck = { false }, + localTracerFactory = { _, _ -> mockLocalTracer } + ) + whenever(mockResolver.isFirstPartyUrl(fakeUrl.toHttpUrl())).thenReturn(true) + whenever(mockTraceSampler.sample(mockSpan)).thenReturn(false) + stubChain(mockChain, statusCode) + + // When + testedInterceptor.intercept(mockChain) + + // Then + verify(mockSpan, never()).setTag(FORCE_DROP_SPAN, true) + verify(mockSpan, never()).finish() + verify(mockSpan).drop() + } + + @Test + fun `M force drop span W intercept() {not sampled, throwing request, SDK-backed tracer}`( + @Forgery throwable: Throwable + ) { + // Given + testedInterceptor = instantiateTestedInterceptor( + fakeLocalHosts, + globalTracerProvider = { mockTracer }, + sdkBackedCheck = { true }, + localTracerFactory = { _, _ -> mockLocalTracer } + ) + whenever(mockResolver.isFirstPartyUrl(fakeUrl.toHttpUrl())).thenReturn(true) + whenever(mockTraceSampler.sample(mockSpan)).thenReturn(false) + whenever(mockChain.request()) doReturn fakeRequest + whenever(mockChain.proceed(any())) doThrow throwable + + // When + assertThrows(throwable.message.orEmpty()) { + testedInterceptor.intercept(mockChain) + } + + // Then + verify(mockSpan).setTag(FORCE_DROP_SPAN, true) + verify(mockSpan).finish() + verify(mockSpan, never()).drop() + } + + @Test + fun `M drop span W intercept() {not sampled, throwing request, custom tracer}`( + @Forgery throwable: Throwable + ) { + // Given + testedInterceptor = instantiateTestedInterceptor( + fakeLocalHosts, + globalTracerProvider = { mockTracer }, + sdkBackedCheck = { false }, + localTracerFactory = { _, _ -> mockLocalTracer } + ) + whenever(mockResolver.isFirstPartyUrl(fakeUrl.toHttpUrl())).thenReturn(true) + whenever(mockTraceSampler.sample(mockSpan)).thenReturn(false) + whenever(mockChain.request()) doReturn fakeRequest + whenever(mockChain.proceed(any())) doThrow throwable + + // When + assertThrows(throwable.message.orEmpty()) { + testedInterceptor.intercept(mockChain) + } + + // Then + verify(mockSpan, never()).setTag(FORCE_DROP_SPAN, true) + verify(mockSpan, never()).finish() + verify(mockSpan).drop() + } + @Test fun `M warn the user W intercept() no tracer registered and TracingFeature not initialized`( @IntForgery(min = 200, max = 300) statusCode: Int