Skip to content

[RUM-11854]: Proper support propagation of RUM context for automatic http tracing via baggage header#2904

Merged
satween merged 4 commits into
developfrom
tvaleev/feature/RUM-11854-rum-context-fix
Oct 7, 2025
Merged

[RUM-11854]: Proper support propagation of RUM context for automatic http tracing via baggage header#2904
satween merged 4 commits into
developfrom
tvaleev/feature/RUM-11854-rum-context-fix

Conversation

@satween

@satween satween commented Sep 29, 2025

Copy link
Copy Markdown
Contributor

What does this PR do?

  1. Fixes issue with missing RUM context in tracing feature that uses CoreTracer for automatic tracing.
  2. Ensures that RUM context is being added in one single place and works for both custom tracing and automatic tracing
  3. Ensures that sampling decision based on sampleId/traceId values works properly

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)

@satween
satween requested review from a team as code owners September 29, 2025 16:26
0xnm
0xnm reviewed Sep 30, 2025
View reviewed changes
Comment thread ...sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/DatadogTracerAdapter.kt Outdated
Comment thread ...ty/single-fit/okhttp/src/test/kotlin/com/datadog/android/okhttp/RumContextPropagationTest.kt Outdated
Comment thread ...ty/single-fit/okhttp/src/test/kotlin/com/datadog/android/okhttp/RumContextPropagationTest.kt Outdated
Comment thread ...ty/single-fit/okhttp/src/test/kotlin/com/datadog/android/okhttp/RumContextPropagationTest.kt
@satween
satween force-pushed the tvaleev/feature/RUM-11854-rum-context-fix branch from 7c3f6b8 to 1645104 Compare October 2, 2025 09:53
@codecov-commenter

codecov-commenter commented Oct 2, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 67.77778% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.61%. Comparing base (7e0f8d6) to head (a8bb138).
⚠️ Report is 1394 commits behind head on develop.

Files with missing lines Patch % Lines
...dog/android/trace/internal/RumContextPropagator.kt 74.07% 7 Missing and 7 partials ⚠️
...in/com/datadog/android/core/internal/SdkFeature.kt 0.00% 7 Missing ⚠️
...com/datadog/android/api/feature/FeatureScopeExt.kt 0.00% 4 Missing ⚠️
...dog/android/trace/internal/DatadogTracerAdapter.kt 76.92% 0 Missing and 3 partials ⚠️
...datadog/android/okhttp/trace/TracingInterceptor.kt 50.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #2904      +/-   ##
===========================================
- Coverage    70.94%   70.61%   -0.34%     
===========================================
  Files          815      815              
  Lines        30013    30050      +37     
  Branches      5022     5142     +120     
===========================================
- Hits         21292    21217      -75     
- Misses        7336     7379      +43     
- Partials      1385     1454      +69
Files with missing lines Coverage Δ
...adog/android/core/sampling/DeterministicSampler.kt 85.19% <100.00%> (ø)
...va/com/datadog/trace/api/IdGenerationStrategy.java 86.67% <100.00%> (-3.33%) ⬇️
...m/datadog/opentelemetry/trace/OtelSpanBuilder.java 21.67% <ø> (ø)
...roid/trace/internal/DatadogTracerBuilderAdapter.kt 98.11% <100.00%> (-1.89%) ⬇️
...dog/android/trace/internal/data/CoreTraceWriter.kt 91.18% <100.00%> (+0.79%) ⬆️
...datadog/android/okhttp/trace/TracingInterceptor.kt 82.71% <50.00%> (-0.53%) ⬇️
...dog/android/trace/internal/DatadogTracerAdapter.kt 75.00% <76.92%> (+10.48%) ⬆️
...com/datadog/android/api/feature/FeatureScopeExt.kt 0.00% <0.00%> (ø)
...in/com/datadog/android/core/internal/SdkFeature.kt 89.91% <0.00%> (-2.98%) ⬇️
...dog/android/trace/internal/RumContextPropagator.kt 74.07% <74.07%> (ø)

... and 86 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

0xnm
0xnm reviewed Oct 2, 2025
View reviewed changes
Comment thread dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/sampling/DeterministicSampler.kt Outdated
Comment thread .../dd-sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/RumContextHelper.kt Outdated
@satween
satween requested a review from 0xnm October 3, 2025 17:45
@satween
satween force-pushed the tvaleev/feature/RUM-11854-rum-context-fix branch 2 times, most recently from f52f6f3 to dc89915 Compare October 6, 2025 11:17
0xnm
0xnm reviewed Oct 6, 2025
View reviewed changes
Comment thread ...sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/RumContextPropagator.kt

private fun injectRumContext(builder: DatadogSpanBuilder) {
sdkCore?.getFeature(Feature.RUM_FEATURE_NAME)
?.getContextFuture(withFeatureContexts = setOf(Feature.RUM_FEATURE_NAME))

@0xnm 0xnm Oct 6, 2025

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.

question: is there advantage for creating getContextFuture method instead of using withContext? Seems like we added a method to the core API just for a single use-case, where withContext can be used.

@satween satween Oct 6, 2025

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.

This is what I've highlighted in DM: It's not being added to the public API. It's an internal method that accessible only through extension method.

The advantage of having a Future here is that it allows us to:

  • Avoid reinventing the wheel with our own CompletableFuture, which could lead to potential issues. The Java Future class feels more stable. All required logic is also supported by Future.
  • Properly block the thread when the result is obtained.

@0xnm 0xnm Oct 6, 2025

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.

Yes, but then you need to do type-check for SdkFeature, because it is not a part of interface (which is for good).

The goal is to have a limited ways to work with context, to simplify the DevEx, like a single simple thing which do the stuff, otherwise we may create a problem of choice.

But I'm ok with having it like that, given it doesn't make it to the interface.

Comment thread ...sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/RumContextPropagator.kt
Comment thread ...sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/RumContextPropagator.kt Outdated
Comment thread ...sdk-android-trace/src/main/kotlin/com/datadog/android/trace/internal/RumContextPropagator.kt Outdated
Comment thread ...dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/trace/TracingInterceptor.kt Outdated
0xnm
0xnm previously approved these changes Oct 6, 2025
View reviewed changes
@satween
satween force-pushed the tvaleev/feature/RUM-11854-rum-context-fix branch 2 times, most recently from 3da9feb to 98981e7 Compare October 7, 2025 10:35
@satween
satween force-pushed the tvaleev/feature/RUM-11854-rum-context-fix branch from 98981e7 to a8bb138 Compare October 7, 2025 14:07
@satween
satween merged commit 8612241 into develop Oct 7, 2025
25 checks passed
@satween
satween deleted the tvaleev/feature/RUM-11854-rum-context-fix branch October 7, 2025 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0xnm 0xnm 0xnm approved these changes
@aleksandr-gringauz aleksandr-gringauz aleksandr-gringauz left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@satween @codecov-commenter @0xnm @aleksandr-gringauz