Skip to content
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
1 change: 1 addition & 0 deletions dd-sdk-android-core/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,7 @@ object com.datadog.android.log.LogAttributes
const val RUM_SESSION_ID: String
const val RUM_VIEW_ID: String
const val RUM_ACTION_ID: String
const val RUM_SESSION_SAMPLE_RATE: String
const val SERVICE_NAME: String
const val SOURCE: String
const val STATUS: String
Expand Down
1 change: 1 addition & 0 deletions dd-sdk-android-core/api/dd-sdk-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -1081,6 +1081,7 @@ public final class com/datadog/android/log/LogAttributes {
public static final field RUM_ACTION_ID Ljava/lang/String;
public static final field RUM_APPLICATION_ID Ljava/lang/String;
public static final field RUM_SESSION_ID Ljava/lang/String;
public static final field RUM_SESSION_SAMPLE_RATE Ljava/lang/String;
public static final field RUM_VIEW_ID Ljava/lang/String;
public static final field SERVICE Ljava/lang/String;
public static final field SERVICE_NAME Ljava/lang/String;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package com.datadog.android.core.sampling

import androidx.annotation.FloatRange
import com.datadog.android.api.InternalLogger
import com.datadog.android.internal.sampling.computeSamplingDecision

/**
* [Sampler] with the given sample rate using a deterministic algorithm for a stable
Expand Down Expand Up @@ -46,19 +47,8 @@ open class DeterministicSampler<T : Any>(
) : this(idConverter, sampleRate.toFloat())

/** @inheritDoc */
override fun sample(item: T): Boolean {
val sampleRate = getSampleRate()

return when {
sampleRate >= SAMPLE_ALL_RATE -> true
sampleRate <= 0f -> false
else -> {
val hash = idConverter(item) * SAMPLER_HASHER
val threshold = (MAX_ID.toDouble() * sampleRate / SAMPLE_ALL_RATE).toULong()
hash < threshold
}
}
}
override fun sample(item: T): Boolean =
computeSamplingDecision(getSampleRate(), idConverter(item))

/** @inheritDoc */
override fun getSampleRate(): Float {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,13 @@ object LogAttributes {
*/
const val RUM_ACTION_ID: String = "user_action.id"

/**
* Key for the RUM session sample rate used for cross-product trace sampling rebasing.
* This key is written to spans by [com.datadog.android.trace.internal.RumContextPropagator]
* and read by [com.datadog.android.trace.DeterministicTraceSampler].
*/
const val RUM_SESSION_SAMPLE_RATE: String = "session_sample_rate"

/**
* The name of the application or service generating the log events. (String)
* This value is filled automatically by the [Logger].
Expand Down
1 change: 1 addition & 0 deletions dd-sdk-android-internal/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ sealed class com.datadog.android.internal.profiling.ProfilerStopEvent
constructor(TTIDRumContext? = null)
data class com.datadog.android.internal.profiling.TTIDRumContext
constructor(String, String, String, String?, String?, String?)
fun computeSamplingDecision(Float, ULong): Boolean
interface com.datadog.android.internal.system.BuildSdkVersionProvider
val version: Int
val isAtLeastN: Boolean
Expand Down
4 changes: 4 additions & 0 deletions dd-sdk-android-internal/api/dd-sdk-android-internal.api
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,10 @@ public final class com/datadog/android/internal/profiling/TTIDRumContext {
public fun toString ()Ljava/lang/String;
}

public final class com/datadog/android/internal/sampling/DeterministicSamplingKt {
public static final fun computeSamplingDecision-2TYgG_w (FJ)Z
}

public abstract interface class com/datadog/android/internal/system/BuildSdkVersionProvider {
public static final field Companion Lcom/datadog/android/internal/system/BuildSdkVersionProvider$Companion;
public abstract fun getVersion ()I
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
* This product includes software developed at Datadog (https://www.datadoghq.com/).
* Copyright 2016-Present Datadog, Inc.
*/

package com.datadog.android.internal.sampling

private const val SAMPLE_ALL_RATE: Float = 100f
private const val SAMPLER_HASHER: ULong = 1111111111111111111u
private const val MAX_ID: ULong = 0xFFFFFFFFFFFFFFFFUL

/**
* Computes a deterministic sampling decision based on the given sample rate and identifier.
*
* @param sampleRate the sample rate in the range [0, 100].
* @param id a stable numerical identifier derived from the item being sampled.
* @return true if the item should be sampled, false otherwise.
*/
fun computeSamplingDecision(sampleRate: Float, id: ULong): Boolean {

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.

why did you moved this code out of the DeterministicSampler ? It's seems like only descendants of it are using it...

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.

You can see the discussion: #3342 (comment)

return when {
sampleRate >= SAMPLE_ALL_RATE -> true
sampleRate <= 0f -> false
else -> {
val hash = id * SAMPLER_HASHER
val threshold = (MAX_ID.toDouble() * sampleRate / SAMPLE_ALL_RATE).toULong()
hash < threshold
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
* This product includes software developed at Datadog (https://www.datadoghq.com/).
* Copyright 2016-Present Datadog, Inc.
*/

package com.datadog.android.internal.sampling

import fr.xgouchet.elmyr.annotation.FloatForgery
import fr.xgouchet.elmyr.annotation.LongForgery
import fr.xgouchet.elmyr.junit5.ForgeExtension
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.RepeatedTest
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.jupiter.api.extension.Extensions
import org.mockito.junit.jupiter.MockitoExtension
import org.mockito.junit.jupiter.MockitoSettings
import org.mockito.quality.Strictness

@Extensions(
ExtendWith(MockitoExtension::class),
ExtendWith(ForgeExtension::class)
)
@MockitoSettings(strictness = Strictness.LENIENT)
internal class DeterministicSamplingTest {

@RepeatedTest(32)
fun `M always return true W computeSamplingDecision() {sampleRate is 100}`(
@LongForgery fakeId: Long
) {
assertThat(computeSamplingDecision(100f, fakeId.toULong())).isTrue()
}

@RepeatedTest(32)
fun `M always return true W computeSamplingDecision() {sampleRate above 100}`(
@FloatForgery(min = 100.01f, max = 200f) fakeSampleRate: Float,
@LongForgery fakeId: Long
) {
assertThat(computeSamplingDecision(fakeSampleRate, fakeId.toULong())).isTrue()
}

@RepeatedTest(32)
fun `M always return false W computeSamplingDecision() {sampleRate is 0}`(
@LongForgery fakeId: Long
) {
assertThat(computeSamplingDecision(0f, fakeId.toULong())).isFalse()
}

@RepeatedTest(32)
fun `M always return false W computeSamplingDecision() {sampleRate below 0}`(
@FloatForgery(min = -100f, max = -0.01f) fakeSampleRate: Float,
@LongForgery fakeId: Long
) {
assertThat(computeSamplingDecision(fakeSampleRate, fakeId.toULong())).isFalse()
}

@Test
fun `M return deterministic result W computeSamplingDecision() {same id same rate}`(
@FloatForgery(min = 1f, max = 99f) fakeSampleRate: Float,
@LongForgery fakeId: Long
) {
val first = computeSamplingDecision(fakeSampleRate, fakeId.toULong())
val second = computeSamplingDecision(fakeSampleRate, fakeId.toULong())
assertThat(first).isEqualTo(second)
}
}
1 change: 1 addition & 0 deletions detekt_custom_safe_calls.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1132,6 +1132,7 @@ datadog:
- "kotlin.Double.toInt()"
- "kotlin.Double.toLong()"
- "kotlin.Double.toULong()"
- "kotlin.Float.coerceAtMost(kotlin.Float)"
- "kotlin.Float.fromBits(kotlin.Int)"
- "kotlin.Float.percent()"
- "kotlin.Float.roundToInt()"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ internal data class RumContext(
val syntheticsResultId: String? = null,
val viewTimestamp: Long = 0L,
val viewTimestampOffset: Long = 0L,
val hasReplay: Boolean = false
val hasReplay: Boolean = false,
val sessionSampleRate: Float = FULL_SESSION_SAMPLE_RATE
) {

fun toMap(): Map<String, Any?> {
Expand All @@ -44,7 +45,8 @@ internal data class RumContext(
SYNTHETICS_RESULT_ID to syntheticsResultId,
VIEW_TIMESTAMP to viewTimestamp,
HAS_REPLAY to hasReplay,
VIEW_TIMESTAMP_OFFSET to viewTimestampOffset
VIEW_TIMESTAMP_OFFSET to viewTimestampOffset,
SESSION_SAMPLE_RATE to sessionSampleRate
)
}

Expand All @@ -68,6 +70,8 @@ internal data class RumContext(
const val HAS_REPLAY = "view_has_replay"
const val VIEW_TIMESTAMP = "view_timestamp"
const val VIEW_TIMESTAMP_OFFSET = "view_timestamp_offset"
const val SESSION_SAMPLE_RATE = "session_sample_rate"
const val FULL_SESSION_SAMPLE_RATE: Float = 100f

fun fromFeatureContext(featureContext: Map<String, Any?>): RumContext {
val applicationId = featureContext[APPLICATION_ID] as? String
Expand All @@ -89,6 +93,8 @@ internal data class RumContext(
val hasReplay = featureContext[HAS_REPLAY] as? Boolean ?: false
val viewTimestamp = featureContext[VIEW_TIMESTAMP] as? Long ?: 0L
val viewTimestampOffset = featureContext[VIEW_TIMESTAMP_OFFSET] as? Long ?: 0L
val sessionSampleRate = (featureContext[SESSION_SAMPLE_RATE] as? Number)
?.toFloat() ?: FULL_SESSION_SAMPLE_RATE

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just curious: is there a motivation behind using 100% as a fallback, and not 0%?

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.

The main reason was that if we use 0% as a fallback, the rebased sampling would drop everything, so it seemed safer to me. However, now that you’ve mentioned it, I’m not sure if this is what we expect.


return RumContext(
applicationId = applicationId ?: NULL_UUID,
Expand All @@ -105,7 +111,8 @@ internal data class RumContext(
syntheticsResultId = syntheticsResultId,
viewTimestamp = viewTimestamp,
viewTimestampOffset = viewTimestampOffset,
hasReplay = hasReplay
hasReplay = hasReplay,
sessionSampleRate = sessionSampleRate
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ internal class RumSessionScope(

internal var sessionId = RumContext.NULL_UUID
internal var sessionState: State = State.NOT_TRACKED
internal var sessionSampleRate: Float = sessionSampler.getSampleRate() ?: RumContext.FULL_SESSION_SAMPLE_RATE
private var startReason: StartReason = StartReason.USER_APP_LAUNCH
internal var isActive: Boolean = true
private val sessionStartNs = AtomicLong(sdkCore.timeProvider.getDeviceElapsedTimeNanos())
Expand Down Expand Up @@ -211,7 +212,8 @@ internal class RumSessionScope(
sessionId = sessionId,
sessionState = sessionState,
sessionStartReason = startReason,
isSessionActive = isActive
isSessionActive = isActive,
sessionSampleRate = sessionSampleRate
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
package com.datadog.android.rum.internal.domain

import com.datadog.android.rum.utils.forge.Configurator
import fr.xgouchet.elmyr.Forge
import fr.xgouchet.elmyr.annotation.Forgery
import fr.xgouchet.elmyr.junit5.ForgeConfiguration
import fr.xgouchet.elmyr.junit5.ForgeExtension
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.RepeatedTest
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith

@ExtendWith(ForgeExtension::class)
Expand All @@ -28,4 +30,19 @@ internal class RumContextTest {
// Then
assertThat(anotherRumContext).isEqualTo(fakeRumContext)
}

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

// When
val rumContext = RumContext.fromFeatureContext(featureContext)

// Then
assertThat(rumContext.sessionSampleRate).isEqualTo(fakeDouble.toFloat())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,16 @@ internal class RumSessionScopeTest {
assertThat(childScope?.sampleRate).isCloseTo(fakeSampleRate, offset(0.001f))
}

@Test
fun `M use full session sample rate W init() { getSampleRate returns null }`() {
// Given
whenever(mockSessionSampler.getSampleRate()).thenReturn(null)
initializeTestedScope(withMockChildScope = false)

// Then
assertThat(testedScope.sessionSampleRate).isEqualTo(RumContext.FULL_SESSION_SAMPLE_RATE)
}

@Test
fun `M delegate events to child scope W handleViewEvent() {TRACKED}`(
forge: Forge
Expand Down Expand Up @@ -478,6 +488,23 @@ internal class RumSessionScopeTest {
assertThat(context.viewId).isEqualTo(fakeParentContext.viewId)
}

@Test
fun `M use full session sample rate W getRumContext() {getSampleRate returns null}`(
forge: Forge
) {
// Given
whenever(mockSessionSampler.getSampleRate()).thenReturn(null)
whenever(mockSessionSampler.sample(any())).thenReturn(true)
initializeTestedScope()

// When
testedScope.handleEvent(forge.startViewEvent(), fakeDatadogContext, mockEventWriteScope, mockWriter)
val context = testedScope.getRumContext()

// Then
assertThat(context.sessionSampleRate).isEqualTo(RumContext.FULL_SESSION_SAMPLE_RATE)
}

@Test
fun `M set TRACKED W renewSession() {sampler returns true}`(forge: Forge) {
// Given
Expand Down
2 changes: 2 additions & 0 deletions features/dd-sdk-android-trace/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ open class com.datadog.android.trace.DeterministicTraceSampler : com.datadog.and
constructor(() -> Float)
constructor(Float)
constructor(Double)
override fun sample(com.datadog.android.trace.api.span.DatadogSpan): Boolean
DEPRECATED fun getSampleRate(com.datadog.android.trace.api.span.DatadogSpan): Float
annotation com.datadog.android.trace.ExperimentalTraceApi
object com.datadog.android.trace.GlobalDatadogTracer
fun registerIfAbsent(com.datadog.android.trace.api.tracer.DatadogTracer): Boolean
Expand Down
3 changes: 3 additions & 0 deletions features/dd-sdk-android-trace/api/dd-sdk-android-trace.api
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ public class com/datadog/android/trace/DeterministicTraceSampler : com/datadog/a
public fun <init> (D)V
public fun <init> (F)V
public fun <init> (Lkotlin/jvm/functions/Function0;)V
public final fun getSampleRate (Lcom/datadog/android/trace/api/span/DatadogSpan;)F
public fun sample (Lcom/datadog/android/trace/api/span/DatadogSpan;)Z
public synthetic fun sample (Ljava/lang/Object;)Z
}

public abstract interface annotation class com/datadog/android/trace/ExperimentalTraceApi : java/lang/annotation/Annotation {
Expand Down
Loading
Loading