refactor(propagation): Parse OpenTelemetry tracestate member - #12405
refactor(propagation): Parse OpenTelemetry tracestate member#12405MilanGarnier wants to merge 2 commits into
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: 397a1ff | Docs | View more details | Give us feedback! |
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 397a1ff69a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| public void updateW3CTracestate(String tracestate) { | ||
| clearCachedHeader(W3C); | ||
| this.tracestate = tracestate; | ||
| setOtelTraceState(W3CPTagsCodec.extractOtelTraceState(tracestate)); |
There was a problem hiding this comment.
Reuse the parsed OTel state during reconciliation
perf: With the default DATADOG,TRACECONTEXT extraction order, matching Datadog and W3C headers cause fromHeaderValue to parse the tracestate before HttpCodec.applyTraceContextToFirstContext passes the same string here. Calling extractOtelTraceState then validates and scans it again—and allocates another substring/state when ot is present—on the inbound request path. Transfer the already-parsed state with the propagation tags instead of reparsing the raw header.
AGENTS.md reference: AGENTS.md:L76-L79
Useful? React with 👍 / 👎.
| OtelTraceState otelTraceState = pTags.getOtelTraceState(); | ||
| if (otelTraceState != null) { | ||
| size += OTEL_MEMBER_KEY.length() + otelTraceState.length() + 1; |
There was a problem hiding this comment.
Avoid counting the OTel member twice in the buffer estimate
perf: When the original tracestate length is already included above—either for a parsed W3CPTags containing dd or for a Datadog PTags updated during multi-style reconciliation—it already contains the ot= member and its delimiter. Adding its size again here over-allocates the StringBuilder backing storage by up to 260 characters on each fresh W3C encoding, including per-span encodings that use a last-parent override. Subtract the managed member from the original-size contribution or omit this second addition in those cases.
AGENTS.md reference: AGENTS.md:L76-L79
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
More details
The change keeps the raw ot member and its inherited position when it rebuilds tracestate. The static review finds no reportable regression in the changed parsing and serialization paths.
🤖 Datadog Autotest · Commit 397a1ff · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
mcculls
left a comment
There was a problem hiding this comment.
The double-parsing issue will need to be fixed before this can be merged.
Claude suggests giving PTags a way to accept an already-built OtelTraceState directly (e.g. setOtelTraceState), and have applyTraceContextToFirstContext pass the parsed object from traceContext.getPropagationTags() instead of round-tripping through the string.
The double-counting issue is easily solved by removing the second addition.
What Does This Do
Refactors W3C tracestate handling so the OpenTelemetry
otmember isrepresented separately while its value remains opaque.
otmember.rvorthor connect the member to sampling.Motivation
Isolate the tracestate parsing foundation from the consistent-sampling
behavior added by the follow-up PR.
Additional Notes
This is the first PR in a two-PR stack. The sampling behavior follows in
#12397.
Validation:
:dd-trace-core:compileJava:dd-trace-core:spotlessCheckgit diff --checkContributor Checklist
type:andcomp:labels.Jira ticket: APMAPI-2171