[RUM-9899]: TracingInterceptor migration #2708

satween · 2025-06-06T09:27:28Z

What does this PR do?

This PR contains major change for tracer migration.
I tried to reduce amount of changes with some tricks with kotlin's typealias but it still huge. Maybe some of you will find it easier to review by commits.

Main changes:

Migration from AndroidTracer to CoreTracer in TracingInterceptor and it descendants.
Refactor and removing duplication logic in unit tests.

Please pay attention to:

Despite the fact that the APIs of CoreTracer and AndroidTracer are similar, some methods were either not supported or supported at a different level of abstraction. In such cases, I implemented the closest and simplest solution and added comments so you could spot them.

Known issues

In some tests StringForgery with regex inside generated 33 length string instead of 32 which was leading to expeption in DDTraceId parsing. Temporary replaced regex "[a-f0-9]{31}". Going to dig deeper in separate ticket.
There is no replacement for GlobalTracer, but TracingInterceptor relies on some globally stored CoreTracer instance. Going to discuss this with the team and provide solution with separate PR.

Review checklist (to be filled by reviewers)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
Make sure you discussed the feature or bugfix with the maintaining team in an Issue
Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

...-sdk-android-trace-internal/src/main/java/com/datadog/trace/api/interceptor/MutableSpan.java

satween · 2025-06-06T09:31:03Z

features/dd-sdk-android-trace-internal/src/main/java/com/datadog/trace/core/PendingTrace.java

@@ -223,6 +223,14 @@ void registerSpan(final DDSpan span) {
        }
    }

+    public void unregisterSpan(final DDSpan span){


Important
Another point that I want to pay attention to: Dropping span wasn't supported in CoreTracer and I am not sure that removing span from ROOT_SPAN is the right thing here. Please let me know if I missing something

Hmm...not sure it's ok here, I am thinking about the case where you have a parent span already and you only want to create the Okhttp span for the ids. In this case dropping the OkHttp span will completely drop all the trace ?

...tions/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/DatadogInterceptor.kt

...k-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/internal/otel/TraceContextExt.kt

...dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/trace/TracingInterceptor.kt

...oid-okhttp/src/test/kotlin/com/datadog/android/okhttp/DatadogInterceptorWithoutTracesTest.kt

codecov-commenter · 2025-06-06T13:54:53Z

Codecov Report

❌ Patch coverage is 65.73146% with 171 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.60%. Comparing base (5957478) to head (d05eaa2).
⚠️ Report is 15 commits behind head on feature/v3.

Files with missing lines	Patch %	Lines
.../com/datadog/opentelemetry/trace/OtelSpanLink.java	0.00%	28 Missing ⚠️
...tadog/android/trace/internal/DatadogSpanAdapter.kt	58.33%	12 Missing and 3 partials ⚠️
...dog/android/trace/internal/DatadogTracerAdapter.kt	64.52%	4 Missing and 7 partials ⚠️
...adog/opentelemetry/trace/OtelExtractedContext.java	0.00%	10 Missing ⚠️
...android/trace/internal/DatadogPropagationHelper.kt	9.09%	10 Missing ⚠️
...atadog/android/trace/internal/DatadogSpanLogger.kt	79.59%	4 Missing and 6 partials ⚠️
...og/trace/core/propagation/TagContextExtractor.java	12.50%	7 Missing ⚠️
...m/datadog/opentelemetry/trace/OtelConventions.java	25.00%	6 Missing ⚠️
...ndroid/trace/opentelemetry/DatadogOpenTelemetry.kt	0.00%	6 Missing ⚠️
...kotlin/com/datadog/android/trace/DatadogTracing.kt	81.25%	2 Missing and 4 partials ⚠️
... and 31 more

Additional details and impacted files

@@              Coverage Diff               @@
##           feature/v3    #2708      +/-   ##
==============================================
+ Coverage       67.63%   70.60%   +2.97%     
==============================================
  Files             825      797      -28     
  Lines           31026    29124    -1902     
  Branches         5217     4875     -342     
==============================================
- Hits            20983    20561     -422     
+ Misses           8636     7209    -1427     
+ Partials         1407     1354      -53

Files with missing lines	Coverage Δ
...og/android/core/internal/DatadogContextProvider.kt	`98.11% <100.00%> (+0.24%)`	⬆️
...android/log/internal/domain/DatadogLogGenerator.kt	`97.91% <100.00%> (+0.03%)`	⬆️
...ndroid/telemetry/internal/TelemetryEventHandler.kt	`87.82% <100.00%> (+1.45%)`	⬆️
...al/src/main/java/com/datadog/trace/api/Config.java	`62.39% <100.00%> (-0.18%)`	⬇️
...ain/java/com/datadog/trace/api/ConfigDefaults.java	`100.00% <100.00%> (ø)`
...com/datadog/trace/api/interceptor/MutableSpan.java	`0.00% <ø> (ø)`
...trace/bootstrap/instrumentation/api/AgentSpan.java	`0.00% <ø> (ø)`
...race/bootstrap/instrumentation/api/TagContext.java	`53.85% <ø> (ø)`
...og/trace/common/sampling/ForcePrioritySampler.java	`60.00% <ø> (+60.00%)`	⬆️
...va/com/datadog/trace/common/writer/NoOpWriter.java	`66.67% <100.00%> (ø)`
... and 71 more

... and 37 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

...dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/trace/TracingInterceptor.kt

...tions/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/DatadogInterceptor.kt

features/dd-sdk-android-trace-internal/src/main/java/com/datadog/trace/core/CoreTracer.java

...dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/trace/TracingInterceptor.kt

...k-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/internal/otel/TraceContextExt.kt

...-sdk-android-trace-internal/src/main/java/com/datadog/trace/api/interceptor/MutableSpan.java

features/dd-sdk-android-trace-internal/src/main/java/com/datadog/trace/core/DDSpan.java

mariusc83 · 2025-06-10T12:12:52Z

features/dd-sdk-android-trace-internal/src/main/java/com/datadog/trace/core/PendingTrace.java

@@ -223,6 +223,14 @@ void registerSpan(final DDSpan span) {
        }
    }

+    public void unregisterSpan(final DDSpan span){


Hmm...not sure it's ok here, I am thinking about the case where you have a parent span already and you only want to create the Okhttp span for the ids. In this case dropping the OkHttp span will completely drop all the trace ?

mariusc83

LGTM in general. We're missing the part where we are able to inject our own tracer into the DatadogInterceptor#builder, see the Slack thread but I guess this can still be added into this PR.

...ions/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/trace/OtelMigration.kt

dd-sdk-android-internal/src/main/java/com/datadog/android/internal/utils/CastExt.kt

@nikita

the moment of this commit this new API was never used, so by composite decision with @nikita and @marius we decided to use EXTERNAL_OVERRIDE as sampling mechanism here.

… resolution during one of last commit squashing procedure

…xtension implementations

…eTracer and related classes for customers. The only publicity available ways for tracing feature usage: - okhttp instrumentation - manual instrumentation with Otel so it's okay that TracerContext is the only way to set the parent span (otherwise AgentSpan will be exposed)

…ods flow

…/RUM-9899_2 # Conflicts: # features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/domain/event/CoreTracerSpanToSpanEventMapper.kt # features/dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/domain/event/DdSpanToSpanEventMapper.kt

…/RUM-9899_2

aleksandr-gringauz · 2025-08-08T12:47:20Z

lgtm

but better wait also for @mariusc83 approval

mariusc83 · 2025-08-11T12:56:17Z

features/dd-sdk-android-trace/build.gradle.kts

    implementation(project(":dd-sdk-android-internal"))
-    // TODO RUM-9902 This is temporary for compilation. Gonna change to implementation after removing opentracing code
-    api(project(":features:dd-sdk-android-trace-internal"))
+    implementation(project(":features:dd-sdk-android-trace-internal"))


any reason why this is exposed through implementation dependency now ?

This module contains adapters mapping api interfaces into dd-trace-java implementation.
:features:dd-sdk-android-trace-internal added as implementation dependency in order to make it visible to current module, but hide from others.

Not sure that I've got the question right, please correct me if you meant something else

ok I see it now, you need these APIs later in module like dd-sdk-android-trace-otel to delegate to ?

Let me put bit more context here:

:features:dd-sdk-android-trace-internal - is basically dd-trace-java implementation

:features:dd-sdk-android-trace-api - module that's being used everywhere ( in other modules like -okhttp, -otel and so on) to interact with tracing feature. But it only contain interfaces, without implementation.
*:features:dd-sdk-android-trace - bounds -api to -internal.

So -trace module is aware about -api and -internal but don't expose -internal for descendants that's why it is included as implementation

features/dd-sdk-android-trace-api/build.gradle.kts

...trace-internal/src/main/java/com/datadog/trace/bootstrap/instrumentation/api/TagContext.java

mariusc83 · 2025-08-11T13:27:18Z

...s/dd-sdk-android-trace-internal/src/main/java/com/datadog/trace/api/config/TracerConfig.java

@@ -44,6 +50,7 @@ public final class TracerConfig {
  public static final String TRACE_SAMPLING_OPERATION_RULES = "trace.sampling.operation.rules";
  // JSON rules
  public static final String TRACE_SAMPLING_RULES = "trace.sampling.rules";
+  public static final String SDK_V2_COMPATIBILITY_FLAG = "v2.compatibility.enabled";


what is this flag needed for ? I see that for now it is not set from anywhere in the sdk.

Context: AndroidTracer user SAMPLER_KEEP and SAMPLER_DROP constant for sampling priority. CoreTracer uses USER_KEEP and USER_DROP for sampling priority.

If we change all SAMPLER_KEEP/SAMPLER_DROP to USER_KEEP/USER_DROP everywhere - it could affect manual traces.

If we change all USER_KEEP/USER_DROP to SAMPLER_KEEP/SAMPLER_DROP it could affect manual traces made with otel (because otel implementation already in public and implemented with CoreTracer).

To keep Sample object JSON looks same as before migration, but keep Otel implementation working same way as it was before v3 we need to add this flag, which set to true if tracer created from otel provider.

mariusc83 · 2025-08-11T14:08:17Z

sample/kotlin/src/main/kotlin/com/datadog/android/sample/SampleApplication.kt

@@ -221,6 +195,16 @@ class SampleApplication : Application() {
            }
        }.build()
        Trace.enable(tracesConfig)
+
+        GlobalDatadogTracer.registerIfAbsent(


Are we keeping this only for the OkHttp instrumentation ?

if this is the case this will look very weird. IMO the tracing feature in the new state should do 2 things:

initialise with a configuration

provide otel implementation by using the dd-sdk-android-otel module on top

provide the DatadogInterceptor from the dd-sdk-android-okhttp mdule and in case this requires the DatadogTracer to create/open spans it should register this under the hood.

Are we keeping this only for the OkHttp instrumentation ?

Nope, we have bunch of places where global instance is being used:

SqliteDatabaseExt, SpanExt, CoroutineExt and so on.

if this is the case this will look very weird. IMO the tracing feature in the new state should do 2 things:

initialise with a configuration

provide otel implementation by using the dd-sdk-android-otel module on top

provide the DatadogInterceptor from the dd-sdk-android-okhttp mdule and in case this requires the DatadogTracer to create/open spans it should register this under the hood.

That would be our best scenario, but we have few limitation here:

If we force customers using Otel version - they would have to add desugaring support. To make it possible avoid otel dependencies we need DatadogTracer itself.

If we remove GlobalDatadogTracer we would have to migrate our integration modules (like okhttp, sqldelight, etc) to use Otel instead and they gonna require desugaring as well. Otherwise all of those integrations will have to create it's own instance locally and could miss some specific parameters that customer wants (and will only provide on configuration side).

TL;DR We need to wait until minSdk = 26 for the SDK to get rid from `GlobalDatadogTracer

I am not saying to have these module depending on Otel. I am suggesting that you keep the GlobalDatadogTracer but you register yourself this instance under the hood and you do not expose this API to the users. Correct me if I am wrong but the intent for this instance is to be used internally by all our instrumentations that require spans/traces right now as as OkHttp. ?

The problem with that approach is that we need some place where DatadogTracer is being configured by the customer before we could put it into GlobalDatadogTracer. We cannot create it by ourselves because customer should provide some specific parameters (like tracingHeadersTypes for example).

Technically we could put registering logic inside build() method of a DatadogTracerBuilder implementation, but it could lead to issues, because it's not expected from developer side that creating builder makes them globally accessible, and I personally don't like such side effects of a functions. But I am open to discuss any other possible solution if you know some

Let's discuss this also tomorrow when @0xnm will be in the office, I want to hear also his opinion on this matter before going forward.

… to meta, reverting _dd object back to log