feat(otel): otel sampling support in distributed tracing - #10117
feat(otel): otel sampling support in distributed tracing#10117vpellan wants to merge 19 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Overall package sizeSelf size: 8.84 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.4.0 | 127.33 kB | 447.04 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49eb6259f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
BenchmarksBenchmark execution time: 2026-09-07 12:21:41 Comparing candidate commit c0614ce in PR branch Found 0 performance improvements and 6 performance regressions! Performance is the same for 2294 metrics, 13 unstable metrics.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42005a60a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
# Conflicts: # .github/CODEOWNERS # packages/dd-trace/src/opentracing/propagation/text_map.js # packages/dd-trace/test/opentracing/propagation/text_map.spec.js
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 660cb44f39
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10117 +/- ##
========================================
Coverage 98.65% 98.65%
========================================
Files 1005 1007 +2
Lines 157435 157824 +389
Branches 74 74
========================================
+ Hits 155312 155701 +389
Misses 2123 2123
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
vpellan
added
the
AI Generated
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf02963117
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const selectedPriority = firstSpanContext._sampling.priority | ||
| if (selectedPriority !== undefined && | ||
| (selectedPriority >= AUTO_KEEP) !== (w3cSpanContext._sampling.priority >= AUTO_KEEP)) { |
There was a problem hiding this comment.
When extraction prefers B3 or Datadog and that header omits its sampling decision, selectedPriority is undefined, so this guard does nothing while line 717 still copies the matching W3C ot member. A child span then makes a local probability decision, but buildOtelMember() preserves the already-valid inbound rv/th; for example, an unsampled W3C rv:00000000000000;th:8 can be reinjected with a locally sampled traceparent. The fresh case not covered by the prior conflict fix is this missing, rather than disagreeing, selected priority; either inherit the W3C decision or discard/rebuild its sampling fields before local sampling.
Useful? React with 👍 / 👎.
| if (isSamplingPriorityTag(key)) { | ||
| this._prioritySampler.setPriorityFromTag(this, key, value) |
There was a problem hiding this comment.
After sampling has already been materialized—for example by injection or reading OTel traceFlags—calling setTag('sampling.priority', 1n) or passing a Symbol now reaches Math.trunc() through this unconditional override path and throws a TypeError. Previously the defined priority made this path a no-op, so the same late tag could not crash the application; validate the value's type before attempting the override.
AGENTS.md reference: AGENTS.md:L109-L109
Useful? React with 👍 / 👎.
| samplingTags = tags | ||
| mayChangeSamplingPriority = true |
There was a problem hiding this comment.
With DD_MAJOR < 6, every string or array passed to addTags() now marks sampling as changed and passes the entire live tag map to setPriorityFromTags(), even when the new input contains no sampling tag. For example, after addTags('manual.keep:true') and a later setTag('manual.drop', true) establish a drop, an unrelated addTags('foo:bar') rescans both stored tags, gives manual.keep precedence, and silently flips the trace back to keep. Detect sampling keys in the newly parsed input, or avoid reapplying old tags for unrelated additions.
AGENTS.md reference: AGENTS.md:L119-L122
Useful? React with 👍 / 👎.
# Conflicts: # packages/dd-trace/src/priority_sampler.js
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0614ce61b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const context = this._getContext(span) | ||
| const priority = this._getPriorityFromTag(key, value, context) | ||
| if (this.validate(priority)) this.setPriority(span, priority) |
There was a problem hiding this comment.
When AppSec or AI Guard has already called keepTrace(..., product), a subsequent span.setTag('manual.drop', true) now reaches this unconditional setPriority call, replaces the product's USER_KEEP with USER_REJECT, and clears its decision-maker tag. Previously, sampling tags were ignored once any priority was established, so security or evaluation traces could not be dropped merely because the tag was applied after the product decision; the final code fixes refreshing automatic decisions but should restrict that override to automatic decisions rather than product force-keeps (and apply the same restriction to the batch-tag helper).
Useful? React with 👍 / 👎.
| } else if ((selectedPriority >= AUTO_KEEP) !== (w3cSpanContext._sampling.priority >= AUTO_KEEP)) { | ||
| // The W3C threshold describes its sampled bit, not the conflicting decision selected from another style. | ||
| firstSpanContext._sampling.isProbabilityDecision = false |
There was a problem hiding this comment.
When B3 is the selected extraction style and carries its debug flag (d or X-B3-Flags: 1), getB3Priority() produces the non-probabilistic USER_KEEP, but a matching W3C header with its sampled bit set also satisfies this equality check. The newly copied W3C tracestate therefore retains ot.th, so reinjection represents the B3 force-keep as a probability decision and downstream metrics can apply an incorrect sampling weight. The final condition handles sampled-bit disagreement, but this same-bit non-probability case is fresh evidence that debug contexts also need isProbabilityDecision = false before the tracestate is merged.
Useful? React with 👍 / 👎.
1 participant
What does this PR do?
Add support for OTel sampling in distributed tracing, by reading and emitting
ot.thandot.rvtags. These tags will be forwarded to services using OTel SDKs, that should forward them to the backend.Motivation
Distributed tracing will work between DD-instrumented services and OTel-instrumented services, downstream OTel services will be able to forward the
thfield it to the collector and backend, which will calculate metrics correctlyAdditional Notes
Locally passes DataDog/system-tests#7518