Skip to content

RUM-16886: Mark sampled-out network spans as droppable while preserving stats #3596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abrooksv wants to merge 1 commit into
base: feature/client-side-stats
Choose a base branch
from
Open

RUM-16886: Mark sampled-out network spans as droppable while preserving stats #3596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion features/dd-sdk-android-trace/api/apiSurface
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<com.datadog.android.trace.api.span.DatadogSpan>.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<String, Any?>
class com.datadog.android.trace.internal.net.SessionRebasedSampler : com.datadog.android.core.sampling.Sampler<com.datadog.android.trace.api.span.DatadogSpan>, SpanAwareSampler
Expand Down
15 changes: 11 additions & 4 deletions features/dd-sdk-android-trace/api/dd-sdk-android-trace.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,33 +167,40 @@ 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
public static final fun isDroppedPriority (Ljava/lang/Integer;)Z
}

public final class com/datadog/android/trace/internal/net/RequestTracingState {
public fun <init> (Lcom/datadog/android/api/instrumentation/network/HttpRequestInfoBuilder;ZLcom/datadog/android/trace/api/span/DatadogSpan;Ljava/lang/Float;)V
public synthetic fun <init> (Lcom/datadog/android/api/instrumentation/network/HttpRequestInfoBuilder;ZLcom/datadog/android/trace/api/span/DatadogSpan;Ljava/lang/Float;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (Lcom/datadog/android/api/instrumentation/network/HttpRequestInfoBuilder;ZLcom/datadog/android/trace/api/span/DatadogSpan;Ljava/lang/Float;Z)V
public synthetic fun <init> (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;
}
Expand Down
35 changes: 21 additions & 14 deletions ...oid-trace/src/main/kotlin/com/datadog/android/trace/internal/ApmNetworkInstrumentation.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ class ApmNetworkInstrumentation internal constructor(
}

if (sdkCore == null) {
return RequestTracingState(requestInfoBuilder)
return RequestTracingState(requestInfoBuilder = requestInfoBuilder, isSDKBacked = false)
}

val tracer = tracerProvider.provideTracer(
Expand All @@ -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(
Expand Down Expand Up @@ -155,7 +155,8 @@ class ApmNetworkInstrumentation internal constructor(
span = span,
isSampled = isSampled,
sampleRate = traceSampler.effectiveSampleRate(span),
requestInfoBuilder = tracedRequestInfoBuilder
requestInfoBuilder = tracedRequestInfoBuilder,
isSDKBacked = tracer is DatadogTracerAdapter
)
}

Expand All @@ -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
)
}

/**
Expand All @@ -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(
Expand All @@ -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
)
}

/**
Expand Down
7 changes: 7 additions & 0 deletions ...k-android-trace/src/main/kotlin/com/datadog/android/trace/internal/_TraceInternalProxy.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion ...-android-trace/src/main/kotlin/com/datadog/android/trace/internal/data/CoreTraceWriter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions ...android-trace/src/main/kotlin/com/datadog/android/trace/internal/domain/event/MetaKeys.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

@satween satween Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be const

@abrooksv abrooksv Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I follow, it is a const?

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"
9 changes: 7 additions & 2 deletions ...ce/src/main/kotlin/com/datadog/android/trace/internal/net/ApmNetworkInstrumentationExt.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)) {

@satween satween Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's hard to understand that complex boolean combination.

maybe could be simplified?

when {
!canSendSpan -> drop()
isSDKBacked && !sSampled -> {
    setTag(FORCE_DROP_SPAN, true)
    finish()
}
else -> dtop()

}

Not sure if the code does the same, but flattening it a bit will make it more readable."

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()
Expand Down
4 changes: 3 additions & 1 deletion ...droid-trace/src/main/kotlin/com/datadog/android/trace/internal/net/RequestTracingState.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

@satween satween Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe isDefaultTracer bit better name for this var ?

@abrooksv abrooksv Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that is much better :D

) {

/**
Expand Down
18 changes: 18 additions & 0 deletions ...ndroid-trace/src/test/kotlin/com/datadog/android/trace/internal/TraceInternalProxyTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand All @@ -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<String, Set<TracingHeaderType>>

@StringForgery
Expand Down Expand Up @@ -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<DatadogTracerAdapter>()

assertThat(_TraceInternalProxy.isSDKBackedTracer(sdkTracer)).isTrue()
}
}
19 changes: 19 additions & 0 deletions ...roid-trace/src/test/kotlin/com/datadog/android/trace/internal/data/CoreTraceWriterTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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() }

@satween satween Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is wired line:

If this is an assertion and you've expected for example that there is no exception thrown - it's better to wrap into assertNotThrowing or something similar.

If this code tries to free up resources - this is the wrong way to do so as if test fails on the lines above - this line won't be executed

@abrooksv abrooksv Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few of the tests in this file have that and I am not sure why, they don't look to serve a purpose to me given they are Forge generated spans

dd-sdk-android/features/dd-sdk-android-trace/src/test/kotlin/com/datadog/android/trace/internal/data/CoreTraceWriterTest.kt

Lines 304 to 321 in 5410db4

@Test
fun `M not write spans with drop sampling priority W write()`(forge: Forge) {
// GIVEN
val ddSpans = createNonEmptyDdSpans(
forge = forge,
includeDropSamplingPriority = 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
Expand Down
Loading
Loading