Skip to content

Commit e50d3bf

Browse files
committed
RUM-15138: Fix _dd.agent_psr to report the effective sample rate matching the actual sampling decision
applyPriority() and RequestTracingState.sampleRate now use a span-aware effectiveSampleRate() lookup instead of the argument-less getSampleRate(). For spans linked to an active RUM session the rebased rate is reported; for spans without a session ID the raw trace rate is reported, eliminating the mismatch where the backend received a rebased PSR for non-session spans sampled at the raw trace rate. DeterministicTraceSampler gains an internal getSampleRate(DatadogSpan) method that resolves the effective rate per span, with a compatibility fallback to the session_sample_rate span tag for integrations that do not use ApmNetworkInstrumentationConfiguration. RumContextKeys centralises the session_sample_rate string constant across the trace module.
1 parent b6a78e8 commit e50d3bf

17 files changed

Lines changed: 339 additions & 107 deletions

File tree

features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/domain/RumContextTest.kt

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
package com.datadog.android.rum.internal.domain
88

99
import com.datadog.android.rum.utils.forge.Configurator
10+
import fr.xgouchet.elmyr.Forge
1011
import fr.xgouchet.elmyr.annotation.Forgery
1112
import fr.xgouchet.elmyr.junit5.ForgeConfiguration
1213
import fr.xgouchet.elmyr.junit5.ForgeExtension
@@ -31,16 +32,17 @@ internal class RumContextTest {
3132
}
3233

3334
@Test
34-
fun `M parse session sample rate W fromFeatureContext() {value is Double}`() {
35-
// Given
35+
fun `M parse session sample rate W fromFeatureContext() {value is Double}`(forge: Forge) {
36+
// Given: use an explicit Double to verify that as? Number handles Double (as? Float would return null)
37+
val fakeDouble: Double = forge.aDouble(min = 0.0, max = 100.0)
3638
val featureContext = mapOf<String, Any?>(
37-
RumContext.SESSION_SAMPLE_RATE to 42.5
39+
RumContext.SESSION_SAMPLE_RATE to fakeDouble
3840
)
3941

4042
// When
4143
val rumContext = RumContext.fromFeatureContext(featureContext)
4244

4345
// Then
44-
assertThat(rumContext.sessionSampleRate).isEqualTo(42.5f)
46+
assertThat(rumContext.sessionSampleRate).isEqualTo(fakeDouble.toFloat())
4547
}
4648
}

features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/domain/scope/RumSessionScopeTest.kt

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -493,19 +493,23 @@ internal class RumSessionScopeTest {
493493
forge: Forge
494494
) {
495495
// Given
496-
whenever(mockSessionSampler.getSampleRate()).thenReturn(100f, 20f)
496+
// The scope field initializer consumes the first getSampleRate() call (irrelevant to the
497+
// snapshot). The second call, inside renewSession(), is the one that gets snapshotted.
498+
val fakeSessionRate = forge.aFloat(min = 0f, max = 100f)
499+
val fakeLaterRate = forge.aFloat(min = 0f, max = 100f)
500+
whenever(mockSessionSampler.getSampleRate()).thenReturn(RumContext.FULL_SESSION_SAMPLE_RATE, fakeSessionRate)
497501
whenever(mockSessionSampler.sample(any())).thenReturn(true)
498502
initializeTestedScope()
499503

500504
// When
501505
testedScope.handleEvent(forge.startViewEvent(), fakeDatadogContext, mockEventWriteScope, mockWriter)
502506
val firstContext = testedScope.getRumContext()
503-
whenever(mockSessionSampler.getSampleRate()).thenReturn(100f)
507+
whenever(mockSessionSampler.getSampleRate()).thenReturn(fakeLaterRate)
504508
val secondContext = testedScope.getRumContext()
505509

506-
// Then
507-
assertThat(firstContext.sessionSampleRate).isEqualTo(20f)
508-
assertThat(secondContext.sessionSampleRate).isEqualTo(20f)
510+
// Then — both contexts must report the rate snapshotted at session creation, not the changed rate
511+
assertThat(firstContext.sessionSampleRate).isEqualTo(fakeSessionRate)
512+
assertThat(secondContext.sessionSampleRate).isEqualTo(fakeSessionRate)
509513
}
510514

511515
@Test

features/dd-sdk-android-trace/api/apiSurface

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ open class com.datadog.android.trace.DeterministicTraceSampler : com.datadog.and
2222
constructor(() -> Float)
2323
constructor(Float)
2424
constructor(Double)
25+
override fun getSampleRate(): Float
2526
override fun sample(com.datadog.android.trace.api.span.DatadogSpan): Boolean
2627
annotation com.datadog.android.trace.ExperimentalTraceApi
2728
object com.datadog.android.trace.GlobalDatadogTracer

features/dd-sdk-android-trace/api/dd-sdk-android-trace.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public class com/datadog/android/trace/DeterministicTraceSampler : com/datadog/a
3232
public fun <init> (D)V
3333
public fun <init> (F)V
3434
public fun <init> (Lkotlin/jvm/functions/Function0;)V
35+
public fun getSampleRate ()Ljava/lang/Float;
3536
public fun sample (Lcom/datadog/android/trace/api/span/DatadogSpan;)Z
3637
public synthetic fun sample (Ljava/lang/Object;)Z
3738
}

features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/ApmNetworkInstrumentationConfiguration.kt

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,16 @@ package com.datadog.android.trace
77

88
import androidx.annotation.FloatRange
99
import com.datadog.android.api.SdkCore
10+
import com.datadog.android.api.feature.Feature
11+
import com.datadog.android.core.InternalSdkCore
12+
import com.datadog.android.core.SdkReference
1013
import com.datadog.android.core.configuration.HostsSanitizer
1114
import com.datadog.android.core.internal.net.DefaultFirstPartyHostHeaderTypeResolver
1215
import com.datadog.android.core.sampling.Sampler
1316
import com.datadog.android.trace.api.span.DatadogSpan
1417
import com.datadog.android.trace.api.tracer.DatadogTracer
1518
import com.datadog.android.trace.internal.ApmNetworkInstrumentation
19+
import com.datadog.android.trace.internal.RumContextKeys
1620
import com.datadog.android.trace.internal.net.TracerProvider
1721

1822
/**
@@ -46,7 +50,8 @@ class ApmNetworkInstrumentationConfiguration internal constructor(
4650
internal var localTracerFactory: (SdkCore, Set<TracingHeaderType>) -> DatadogTracer = DEFAULT_LOCAL_TRACER_FACTORY,
4751
internal var traceContextInjection: TraceContextInjection = TraceContextInjection.SAMPLED,
4852
internal var tracedRequestListener: NetworkTracedRequestListener = NoOpNetworkTracedRequestListener(),
49-
internal var traceSampler: Sampler<DatadogSpan> = DeterministicTraceSampler(DEFAULT_TRACE_SAMPLE_RATE),
53+
internal var traceSampleRate: Float = DEFAULT_TRACE_SAMPLE_RATE,
54+
internal var customTraceSampler: Sampler<DatadogSpan>? = null,
5055
internal var globalTracerProvider: () -> DatadogTracer? = { GlobalDatadogTracer.getOrNull() },
5156
internal var networkTracingScope: ApmNetworkTracingScope = ApmNetworkTracingScope.EXCLUDE_INTERNAL_REDIRECTS,
5257
internal var headerPropagationOnly: Boolean = false
@@ -116,7 +121,7 @@ class ApmNetworkInstrumentationConfiguration internal constructor(
116121
* @param sampleRate the sample rate to use (percentage between 0f and 100f, default is 100f).
117122
*/
118123
fun setTraceSampleRate(@FloatRange(from = 0.0, to = 100.0) sampleRate: Float) = apply {
119-
this.traceSampler = DeterministicTraceSampler(sampleRate)
124+
this.traceSampleRate = sampleRate
120125
}
121126

122127
/**
@@ -127,7 +132,7 @@ class ApmNetworkInstrumentationConfiguration internal constructor(
127132
* By default it is a sampler accepting 100% of the traces.
128133
*/
129134
fun setTraceSampler(traceSampler: Sampler<DatadogSpan>) = apply {
130-
this.traceSampler = traceSampler
135+
this.customTraceSampler = traceSampler
131136
}
132137

133138
/**
@@ -213,7 +218,8 @@ class ApmNetworkInstrumentationConfiguration internal constructor(
213218
localTracerFactory = localTracerFactory,
214219
traceContextInjection = traceContextInjection,
215220
tracedRequestListener = tracedRequestListener,
216-
traceSampler = traceSampler,
221+
traceSampleRate = traceSampleRate,
222+
customTraceSampler = customTraceSampler,
217223
globalTracerProvider = globalTracerProvider,
218224
networkTracingScope = networkTracingScope,
219225
headerPropagationOnly = headerPropagationOnly
@@ -241,10 +247,21 @@ class ApmNetworkInstrumentationConfiguration internal constructor(
241247

242248
val tracerProvider = TracerProvider(localTracerFactory, globalTracerProvider)
243249

250+
val sdkRef = SdkReference(sdkInstanceName)
251+
val sessionSampleRateProvider: () -> Float = {
252+
resolveSessionSampleRate(
253+
(sdkRef.get() as? InternalSdkCore)
254+
?.getFeatureContext(Feature.RUM_FEATURE_NAME, useContextThread = false)
255+
?.get(RumContextKeys.SESSION_SAMPLE_RATE)
256+
)
257+
}
258+
val effectiveSampler = customTraceSampler
259+
?: DeterministicTraceSampler(traceSampleRate, sessionSampleRateProvider)
260+
244261
return ApmNetworkInstrumentation(
245262
canSendSpan = !headerPropagationOnly,
246263
traceOrigin = traceOrigin,
247-
traceSampler = traceSampler,
264+
traceSampler = effectiveSampler,
248265
tracerProvider = tracerProvider,
249266
sdkInstanceName = sdkInstanceName,
250267
injectionType = traceContextInjection,
@@ -270,6 +287,10 @@ class ApmNetworkInstrumentationConfiguration internal constructor(
270287

271288
private fun Map<String, Set<TracingHeaderType>>.deepCopy() = mapValues { (_, v) -> v.toSet() }
272289

290+
private fun resolveSessionSampleRate(rawSessionSampleRate: Any?): Float {
291+
return (rawSessionSampleRate as? Number)?.toFloat() ?: DEFAULT_TRACE_SAMPLE_RATE
292+
}
293+
273294
private val DEFAULT_LOCAL_TRACER_FACTORY: (SdkCore, Set<TracingHeaderType>) -> DatadogTracer =
274295
{ sdkCore, tracingHeaderTypes: Set<TracingHeaderType> ->
275296
DatadogTracing.newTracerBuilder(sdkCore)

features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/DeterministicTraceSampler.kt

Lines changed: 73 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,35 +8,47 @@ package com.datadog.android.trace
88

99
import androidx.annotation.FloatRange
1010
import com.datadog.android.core.sampling.DeterministicSampler
11-
import com.datadog.android.log.LogAttributes
1211
import com.datadog.android.trace.api.span.DatadogSpan
13-
import com.datadog.android.trace.internal.RumContextPropagator
12+
import com.datadog.android.trace.internal.RumContextKeys
1413
import com.datadog.android.trace.internal.net.SpanSamplingIdProvider
1514

1615
/**
1716
* A [com.datadog.android.core.sampling.DeterministicSampler] using the TraceID of a Span to compute the sampling decision.
1817
*
19-
* When a span is linked to an active RUM session (i.e. the span carries a `rum.session.id` tag),
20-
* the sampling threshold is automatically rebased against the RUM session sample rate so that
21-
* the combined effective sampling probability reflects the configured trace sample rate applied
22-
* only to the sessions already tracked by RUM:
18+
* The sampling threshold is automatically rebased against the RUM session sample rate when it is
19+
* available so that the combined effective sampling probability reflects:
2320
*
2421
* ```
2522
* rebasedTraceSampleRate = sessionSampleRate * traceSampleRate / 100
2623
* ```
2724
*
28-
* @param sampleRateProvider Provider for the sample rate value which will be called each time
29-
* the sampling decision needs to be made. All the values should be in the range [0;100].
25+
* Note: overriding [getSampleRate] is not a supported extension point for per-span sampling
26+
* behavior.
27+
*
28+
* @param sampleRateProvider Provider for the trace sample rate. Called each time the sampling
29+
* decision needs to be made. Values must be in the range [0;100].
30+
* @param sessionSampleRateProvider Provider for the RUM session sample rate used to rebase the
31+
* trace sample rate. Defaults to 100 (no rebasing).
3032
*/
31-
open class DeterministicTraceSampler(
32-
sampleRateProvider: () -> Float
33+
open class DeterministicTraceSampler private constructor(
34+
private val sampleRateProvider: () -> Float,
35+
private val sessionSampleRateProvider: () -> Float
3336
) : DeterministicSampler<DatadogSpan>(
3437
SpanSamplingIdProvider::provideId,
3538
sampleRateProvider
3639
) {
3740

3841
/**
39-
* Creates a new instance of [DeterministicSampler] with the given sample rate.
42+
* Creates a new instance of [DeterministicTraceSampler] with the given sample rate provider.
43+
*
44+
* @param sampleRateProvider Provider for the sample rate value.
45+
*/
46+
constructor(
47+
sampleRateProvider: () -> Float
48+
) : this(sampleRateProvider, { DeterministicSampler.SAMPLE_ALL_RATE })
49+
50+
/**
51+
* Creates a new instance of [DeterministicTraceSampler] with the given sample rate.
4052
*
4153
* @param sampleRate Sample rate to use.
4254
*/
@@ -45,17 +57,48 @@ open class DeterministicTraceSampler(
4557
) : this({ sampleRate })
4658

4759
/**
48-
* Creates a new instance of [DeterministicSampler] with the given sample rate.
60+
* Creates a new instance of [DeterministicTraceSampler] with the given sample rate.
4961
*
5062
* @param sampleRate Sample rate to use.
5163
*/
5264
constructor(
5365
@FloatRange(from = 0.0, to = 100.0) sampleRate: Double
5466
) : this(sampleRate.toFloat())
5567

68+
/**
69+
* Internal constructor used by [com.datadog.android.trace.ApmNetworkInstrumentationConfiguration]
70+
* to wire the RUM session sample rate provider at instrumentation setup time.
71+
*/
72+
internal constructor(
73+
@FloatRange(from = 0.0, to = 100.0) sampleRate: Float,
74+
sessionSampleRateProvider: () -> Float
75+
) : this({ sampleRate }, sessionSampleRateProvider)
76+
77+
/**
78+
* Returns the effective sample rate, rebased against the RUM session sample rate when it is
79+
* available:
80+
*
81+
* ```
82+
* effectiveRate = traceSampleRate * sessionSampleRate / 100
83+
* ```
84+
*
85+
* Falls back to the raw trace sample rate when no session sample rate is available
86+
* (i.e. [sessionSampleRateProvider] returns 100).
87+
*/
88+
override fun getSampleRate(): Float {
89+
val traceSampleRate = super.getSampleRate()
90+
val sessionSampleRate = sessionSampleRateProvider()
91+
return if (sessionSampleRate >= 0f && sessionSampleRate < DeterministicSampler.SAMPLE_ALL_RATE) {
92+
val rebased = traceSampleRate * sessionSampleRate / DeterministicSampler.SAMPLE_ALL_RATE
93+
rebased.coerceAtMost(DeterministicSampler.SAMPLE_ALL_RATE)
94+
} else {
95+
traceSampleRate
96+
}
97+
}
98+
5699
/** @inheritDoc */
57100
override fun sample(item: DatadogSpan): Boolean {
58-
val sampleRate = rebasedSampleRate(item)
101+
val sampleRate = getSampleRate(item)
59102
return when {
60103
sampleRate >= DeterministicSampler.SAMPLE_ALL_RATE -> true
61104
sampleRate <= 0f -> false
@@ -69,16 +112,25 @@ open class DeterministicTraceSampler(
69112
}
70113
}
71114

72-
private fun rebasedSampleRate(item: DatadogSpan): Float {
73-
val traceSampleRate = getSampleRate()
74-
val sessionId = item.context().tags[LogAttributes.RUM_SESSION_ID] as? String
75-
val sessionSampleRate = item.context().tags[RumContextPropagator.SESSION_SAMPLE_RATE_KEY] as? Number
115+
internal fun getSampleRate(item: DatadogSpan): Float {
116+
// Compatibility path for integrations creating this sampler via public constructors
117+
// (no sessionSampleRateProvider). In that case the session sample rate is propagated on
118+
// the span context as `session_sample_rate` and must be used for rebasing.
119+
val providerRate = sessionSampleRateProvider()
120+
val providerRateIsValid = providerRate >= 0f && providerRate < DeterministicSampler.SAMPLE_ALL_RATE
121+
val sessionSampleRate = when {
122+
providerRateIsValid -> providerRate
123+
else -> (item.context().tags[RumContextKeys.SESSION_SAMPLE_RATE] as? Number)?.toFloat()
124+
}
76125

77-
return if (sessionId != null && sessionSampleRate != null) {
78-
(traceSampleRate * sessionSampleRate.toFloat() / DeterministicSampler.SAMPLE_ALL_RATE)
79-
.coerceAtMost(DeterministicSampler.SAMPLE_ALL_RATE)
126+
return if (sessionSampleRate != null &&
127+
sessionSampleRate >= 0f &&
128+
sessionSampleRate < DeterministicSampler.SAMPLE_ALL_RATE
129+
) {
130+
val rebased = super.getSampleRate() * sessionSampleRate / DeterministicSampler.SAMPLE_ALL_RATE
131+
if (rebased > DeterministicSampler.SAMPLE_ALL_RATE) DeterministicSampler.SAMPLE_ALL_RATE else rebased
80132
} else {
81-
traceSampleRate
133+
super.getSampleRate()
82134
}
83135
}
84136
}

features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/ApmNetworkInstrumentation.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import com.datadog.android.trace.internal.net.RequestTracingState
3131
import com.datadog.android.trace.internal.net.TracerProvider
3232
import com.datadog.android.trace.internal.net.applyPriority
3333
import com.datadog.android.trace.internal.net.buildSpan
34+
import com.datadog.android.trace.internal.net.effectiveSampleRate
3435
import com.datadog.android.trace.internal.net.finishRumAware
3536
import com.datadog.android.trace.internal.net.sample
3637
import java.net.HttpURLConnection
@@ -143,7 +144,7 @@ class ApmNetworkInstrumentation internal constructor(
143144
return RequestTracingState(
144145
span = span,
145146
isSampled = isSampled,
146-
sampleRate = traceSampler.getSampleRate(),
147+
sampleRate = traceSampler.effectiveSampleRate(span),
147148
tracedRequestInfoBuilder = tracedRequestInfoBuilder
148149
)
149150
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
3+
* This product includes software developed at Datadog (https://www.datadoghq.com/).
4+
* Copyright 2016-Present Datadog, Inc.
5+
*/
6+
7+
package com.datadog.android.trace.internal
8+
9+
internal object RumContextKeys {
10+
// Must match RumContext.SESSION_SAMPLE_RATE in dd-sdk-android-rum.
11+
internal const val SESSION_SAMPLE_RATE = "session_sample_rate"
12+
}

features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/RumContextPropagator.kt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,10 @@ class RumContextPropagator(private val sdkCoreProvider: () -> FeatureSdkCore?) {
6262
instance.setTag(LogAttributes.RUM_SESSION_ID, rumContext["session_id"])
6363
instance.setTag(LogAttributes.RUM_VIEW_ID, rumContext["view_id"])
6464
instance.setTag(LogAttributes.RUM_ACTION_ID, rumContext["action_id"])
65-
instance.setTag(SESSION_SAMPLE_RATE_KEY, rumContext[SESSION_SAMPLE_RATE_KEY])
65+
instance.setTag(
66+
RumContextKeys.SESSION_SAMPLE_RATE,
67+
rumContext[RumContextKeys.SESSION_SAMPLE_RATE]
68+
)
6669
instance.setTag(HttpCodec.RUM_KEY_USER_ID, datadogContext.userInfo.id)
6770
instance.setTag(HttpCodec.RUM_KEY_ACCOUNT_ID, datadogContext.accountInfo?.id)
6871
}
@@ -110,7 +113,6 @@ class RumContextPropagator(private val sdkCoreProvider: () -> FeatureSdkCore?) {
110113

111114
companion object {
112115
internal const val DATADOG_INITIAL_CONTEXT: String = "_dd.datadog_initial_context"
113-
internal const val SESSION_SAMPLE_RATE_KEY: String = "session_sample_rate"
114116

115117
internal const val INITIAL_DATADOG_CONTEXT_NOT_AVAILABLE_ERROR = "Initial span creation Datadog context" +
116118
" is not available at the write time."

features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/net/ApmNetworkInstrumentationExt.kt

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import com.datadog.android.api.feature.Feature
99
import com.datadog.android.api.feature.FeatureSdkCore
1010
import com.datadog.android.api.instrumentation.network.HttpRequestInfo
1111
import com.datadog.android.core.sampling.Sampler
12+
import com.datadog.android.trace.DeterministicTraceSampler
1213
import com.datadog.android.trace.api.DatadogTracingConstants.PrioritySampling
1314
import com.datadog.android.trace.api.DatadogTracingConstants.Tags
1415
import com.datadog.android.trace.api.span.DatadogSpan
@@ -35,11 +36,18 @@ internal fun DatadogSpan.applyPriority(isSampled: Boolean, traceSampler: Sampler
3536
if (spanContext.setSamplingPriority(samplingPriority)) {
3637
spanContext.setMetric(
3738
AGENT_PSR_ATTRIBUTE,
38-
(traceSampler.getSampleRate() ?: ZERO_SAMPLE_RATE) / ALL_IN_SAMPLE_RATE
39+
(traceSampler.effectiveSampleRate(this) ?: ZERO_SAMPLE_RATE) / ALL_IN_SAMPLE_RATE
3940
)
4041
}
4142
}
4243

44+
internal fun Sampler<DatadogSpan>.effectiveSampleRate(span: DatadogSpan): Float? {
45+
return when (this) {
46+
is DeterministicTraceSampler -> getSampleRate(span)
47+
else -> getSampleRate()
48+
}
49+
}
50+
4351
internal fun DatadogSpan.sample(request: HttpRequestInfo, traceSampler: Sampler<DatadogSpan>): Boolean {
4452
val samplingPriority = samplingPriority
4553
return if (samplingPriority != null) {

0 commit comments

Comments
 (0)