Skip to content

Commit

Permalink
RUM-3516 Provide the correct sampling priority for our Span events ba…
Browse files Browse the repository at this point in the history 
…sed on the new APM rules
  • Loading branch information
@mariusc83
mariusc83 committed Mar 15, 2024
1 parent 5506818 commit e546eed
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 41 deletions.
2 changes: 2 additions & 0 deletions features/dd-sdk-android-trace/src/main/java/com/datadog/trace/core/DDSpan.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.NANOSECONDS;

import android.util.Log;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

Expand Down
1 change: 0 additions & 1 deletion features/dd-sdk-android-trace/src/main/java/com/datadog/trace/core/PendingTrace.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,6 @@ public void setSamplingPriorityIfNecessary() {
if (traceConfig.sampler instanceof PrioritySampler
&& rootSpan != null
&& rootSpan.context().getSamplingPriority() == PrioritySampling.UNSET) {

((PrioritySampler) traceConfig.sampler).setSamplingPriority(rootSpan);
}
}
Expand Down
25 changes: 19 additions & 6 deletions ...ures/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/OtelTracerProvider.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class OtelTracerProvider(
) {
private var tracingHeaderTypes: Set<TracingHeaderType> =
setOf(TracingHeaderType.DATADOG, TracingHeaderType.TRACECONTEXT)
private var sampleRate: Double = DEFAULT_SAMPLE_RATE
private var sampleRate: Double? = null
private var serviceName: String = ""
get() {
return field.ifEmpty {
Expand Down Expand Up @@ -201,11 +201,24 @@ class OtelTracerProvider(
TracerConfig.SPAN_TAGS,
globalTags.map { "${it.key}:${it.value}" }.joinToString(",")
)
properties.setProperty(
TracerConfig.TRACE_SAMPLE_RATE,
(sampleRate / DEFAULT_SAMPLE_RATE).toString()
)

// In case the sample rate is not set we should not specify it. The agent code under the hood
// will provide different sampler based on this property and also different sampling priorities used
// in the metrics
// -1 MANUAL_DROP User indicated to drop the trace via configuration (sampling rate).
// 0 AUTO_DROP Sampler indicated to drop the trace using a sampling rate provided by the Agent through
// a remote configuration. The Agent API is not used in Android so this `sampling_priority:0` will never
// be used.
// 1 AUTO_KEEP Sampler indicated to keep the trace using a sampling rate from the default configuration
// which right now is 100.0
// (Default sampling priority value. or in our case no specified sample rate will be considered as 100)
// 2 MANUAL_KEEP User indicated to keep the trace, either manually or via configuration (sampling rate)
sampleRate?.let {
properties.setProperty(
TracerConfig.TRACE_SAMPLE_RATE,
(it / KEEP_ALL_SAMPLE_RATE_PERCENT).toString()
)
}
val propagationStyles = tracingHeaderTypes.joinToString(",")
properties.setProperty(TracerConfig.PROPAGATION_STYLE_EXTRACT, propagationStyles)
properties.setProperty(TracerConfig.PROPAGATION_STYLE_INJECT, propagationStyles)
Expand All @@ -224,7 +237,7 @@ class OtelTracerProvider(
internal const val TRACER_ALREADY_EXISTS_WARNING_MESSAGE =
"Tracer for %s already exists. Returning existing instance."
internal const val DEFAULT_TRACER_NAME = "android"
internal const val DEFAULT_SAMPLE_RATE = 100.0
internal const val KEEP_ALL_SAMPLE_RATE_PERCENT = 100.0

internal const val TRACING_NOT_ENABLED_ERROR_MESSAGE =
"You're trying to create an OtelTracerProvider instance, " +
Expand Down
4 changes: 2 additions & 2 deletions ...ain/kotlin/com/datadog/android/trace/internal/domain/event/OtelDdSpanToSpanEventMapper.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import com.datadog.android.core.internal.utils.toHexString
import com.datadog.android.log.LogAttributes
import com.datadog.android.trace.model.SpanEvent
import com.datadog.trace.core.DDSpan
import com.datadog.trace.core.DDSpanContext

internal class OtelDdSpanToSpanEventMapper(
internal val networkInfoEnabled: Boolean
Expand Down Expand Up @@ -52,8 +53,7 @@ internal class OtelDdSpanToSpanEventMapper(

private fun resolveMetrics(event: DDSpan): SpanEvent.Metrics {
val metrics = resolveMetricsFromSpanContext(event).apply {
this["_dd.agent_psr"] = 1.0f
this["_sampling_priority_v1"] = 1
this[DDSpanContext.PRIORITY_SAMPLING_KEY] = event.samplingPriority()
}
return SpanEvent.Metrics(
topLevel = if (event.parentId == 0L) 1 else null,
Expand Down
161 changes: 131 additions & 30 deletions ...-android-trace/src/test/kotlin/com/datadog/android/trace/OtelTracerBuilderProviderTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import com.datadog.trace.api.Config
import com.datadog.trace.api.config.TracerConfig
import com.datadog.trace.common.writer.Writer
import com.datadog.trace.core.CoreTracer
import com.datadog.trace.core.DDSpan
import com.datadog.trace.core.DDSpanContext
import fr.xgouchet.elmyr.Forge
import fr.xgouchet.elmyr.annotation.DoubleForgery
Expand Down Expand Up @@ -72,6 +73,9 @@ internal class OtelTracerBuilderProviderTest {
@StringForgery
lateinit var fakeInstrumentationName: String

@StringForgery
lateinit var fakeOperationName: String

@Mock
lateinit var mockTraceWriter: Writer

Expand Down Expand Up @@ -343,36 +347,6 @@ internal class OtelTracerBuilderProviderTest {
assertThat(coreTracer.partialFlushMinSpans).isEqualTo(threshold)
}

@Test
fun `M use the default sample rate W creating a tracer`() {
// Given
val expectedNormalizedSampleRate = OtelTracerProvider.DEFAULT_SAMPLE_RATE / 100.0
val tracer = testedOtelTracerProviderBuilder.build()
.tracerBuilder(fakeInstrumentationName).build()

// When
val coreTracer: CoreTracer = tracer.getFieldValue("tracer")

// Then
val config: Config = coreTracer.getFieldValue("initialConfig")
assertThat(config.traceSampleRate).isCloseTo(expectedNormalizedSampleRate, offset(0.005))
}

@Test
fun `M use the sample rate W setSampleRate`(@DoubleForgery(min = 0.0, max = 100.0) sampleRate: Double) {
// Given
val expectedNormalizedSampleRate = sampleRate / 100.0
val tracer = testedOtelTracerProviderBuilder.setSampleRate(sampleRate).build()
.tracerBuilder(fakeInstrumentationName).build()

// When
val coreTracer: CoreTracer = tracer.getFieldValue("tracer")

// Then
val config: Config = coreTracer.getFieldValue("initialConfig")
assertThat(config.traceSampleRate).isCloseTo(expectedNormalizedSampleRate, offset(0.005))
}

@Test
fun `M set correct propagating style W setting tracing header types`(forge: Forge) {
// Given
Expand Down Expand Up @@ -446,4 +420,131 @@ internal class OtelTracerBuilderProviderTest {
}

// endregion

// region Sampling priority

@Test
fun `M not add a sample rate by default W creating a tracer`() {
// Given
val tracer = testedOtelTracerProviderBuilder.build()
.tracerBuilder(fakeInstrumentationName).build()

// When
val coreTracer: CoreTracer = tracer.getFieldValue("tracer")

// Then
val config: Config = coreTracer.getFieldValue("initialConfig")
val traceSampleRate: Double? = config.traceSampleRate
assertThat(traceSampleRate).isNull()
}

@Test
fun `M use the sample rate W setSampleRate`(@DoubleForgery(min = 0.0, max = 100.0) sampleRate: Double) {
// Given
val expectedNormalizedSampleRate = sampleRate / 100.0
val tracer = testedOtelTracerProviderBuilder.setSampleRate(sampleRate).build()
.tracerBuilder(fakeInstrumentationName).build()

// When
val coreTracer: CoreTracer = tracer.getFieldValue("tracer")

// Then
val config: Config = coreTracer.getFieldValue("initialConfig")
assertThat(config.traceSampleRate).isCloseTo(expectedNormalizedSampleRate, offset(0.005))
}

@Test
fun `M use user-keep priority W buildSpan { provided keep sample rate }`() {
// Given
val tracer = testedOtelTracerProviderBuilder
.setPartialFlushThreshold(1)
.setSampleRate(100.0)
.build()
.tracerBuilder(fakeInstrumentationName)
.build()

// When
val span = tracer
.spanBuilder(fakeOperationName)
.startSpan()
val delegateSpan: DDSpan = span.getFieldValue("delegate")
delegateSpan.forceSamplingDecision()
span.end()

// Then
val priority = delegateSpan.samplingPriority
assertThat(priority).isEqualTo(2)
}

@Test
fun `M use user-not-keep priority W buildSpan { provide not keep sample rate }`() {
// Given
val tracer = testedOtelTracerProviderBuilder
.setPartialFlushThreshold(1)
.setSampleRate(0.0)
.build()
.tracerBuilder(fakeInstrumentationName)
.build()

// When
val span = tracer
.spanBuilder(fakeOperationName)
.startSpan()
val delegateSpan: DDSpan = span.getFieldValue("delegate")
delegateSpan.forceSamplingDecision()
span.end()

// Then
val priority = delegateSpan.samplingPriority
assertThat(priority).isEqualTo(-1)
}

@Test
fun `M use user-keep or user-not-keep priority W buildSpan { provided random sample rate }`(
@DoubleForgery(min = 0.0, max = 100.0) sampleRate: Double
) {
// Given
val tracer = testedOtelTracerProviderBuilder
.setPartialFlushThreshold(1)
.setSampleRate(sampleRate)
.build()
.tracerBuilder(fakeInstrumentationName)
.build()

// When
val span = tracer
.spanBuilder(fakeOperationName)
.startSpan()
val delegateSpan: DDSpan = span.getFieldValue("delegate")
delegateSpan.forceSamplingDecision()
span.end()

// Then
val priority = delegateSpan.samplingPriority
assertThat(priority).isIn(-1, 2)
}

@Test
fun `M use keep priority W buildSpan { not provided sample rate }`() {
// Given
val tracer = testedOtelTracerProviderBuilder
.setPartialFlushThreshold(1)
.build()
.tracerBuilder(fakeInstrumentationName)
.build()

// When
val span = tracer
.spanBuilder(fakeOperationName)
.startSpan()
val delegateSpan: DDSpan = span.getFieldValue("delegate")
delegateSpan.forceSamplingDecision()
span.end()

// Then
val priority = delegateSpan.samplingPriority
assertThat(priority).isEqualTo(1)
}

// endregion
}
4 changes: 2 additions & 2 deletions ...kotlin/com/datadog/android/trace/internal/domain/event/OtelDdSpanToSpanEventMapperTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import com.datadog.android.trace.assertj.SpanEventAssert.Companion.assertThat
import com.datadog.android.utils.forge.Configurator
import com.datadog.trace.api.DD128bTraceId
import com.datadog.trace.core.DDSpan
import com.datadog.trace.core.DDSpanContext
import fr.xgouchet.elmyr.Forge
import fr.xgouchet.elmyr.annotation.BoolForgery
import fr.xgouchet.elmyr.annotation.Forgery
Expand Down Expand Up @@ -223,8 +224,7 @@ internal class OtelDdSpanToSpanEventMapperTest {

private fun DDSpan.expectedMetrics(): Map<String, Number> {
return tags.filterValues { it is Number }.mapValues { it.value as Number }.toMutableMap().apply {
this["_dd.agent_psr"] = 1.0f
this["_sampling_priority_v1"] = 1
this[DDSpanContext.PRIORITY_SAMPLING_KEY] = samplingPriority()
}
}

Expand Down
2 changes: 2 additions & 0 deletions ...android-trace/src/test/kotlin/com/datadog/android/utils/forge/CoreDDSpanForgeryFactory.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ internal class CoreDDSpanForgeryFactory : ForgeryFactory<DDSpan> {
val traceId = forge.aLong(min = 1)
val spanId = forge.aLong(min = 1)
val parentId = forge.aLong(min = 1)
val samplingPriority = forge.anInt()
val tagsAndMetrics = tags + metrics
val mockSpanContext: DDSpanContext = mock {
whenever(it.baggageItems).thenReturn(baggageItems)
Expand All @@ -48,6 +49,7 @@ internal class CoreDDSpanForgeryFactory : ForgeryFactory<DDSpan> {
whenever(it.parentId).thenReturn(parentId)
whenever(it.baggage).thenReturn(baggageItems)
whenever(it.tags).thenReturn(tagsAndMetrics)
whenever(it.samplingPriority()).thenReturn(samplingPriority)
}
return mockDDSpan
}
Expand Down

0 comments on commit e546eed

Please sign in to comment.