Skip to content

Decouple W3C lastParentId from shared propagation-tag mutation - #11702

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 3 commits into
masterfrom
dougqh/ptags-decouple-lastparentid
Jul 20, 2026
Merged

Decouple W3C lastParentId from shared propagation-tag mutation#11702
gh-worker-dd-mergequeue-cf854d[bot] merged 3 commits into
masterfrom
dougqh/ptags-decouple-lastparentid

Conversation

@dougqh

@dougqhdougqh commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

What Does This Do

Stop mutating the (trace-level, shared) PropagationTags with the injecting span's id at W3C inject time; supply it as a parameter instead.

Motivation

Correctness fix - but originally found while trying to optimize pTags allocation (see #11701)

Additional Notes

From Claude...

W3CHttpCodec.injectTraceState did:

propagationTags.updateLastParentId(DDSpanId.toHexStringPadded(context.getSpanId()));
Stringtracestate = propagationTags.headerValue(W3C);

But getPropagationTags() routes to the root (getRootSpanContextOrThis()), so the field is effectively shared across the segment. Concurrent sibling injects race on it — span A writes A, span B writes B, A reads back B and emits the wrong p: (W3C last-parent-id). It also thrashed the W3C header cache (invalidated on every differing id). Pre-existing; not introduced by the per-span sharing work (#11701).

How

  • New PropagationTags.headerValue(HeaderType, CharSequence lastParentIdOverride). W3CHttpCodec passes context.getSpanId(); W3CPTagsCodec uses the override for p:, falling back to the stored inbound last-parent-id when there's no override (span-link traceState, extraction reads).
  • No inject-time mutation of the shared tags for the last-parent-id → no race, no cross-talk between sibling injects, and the inbound last-parent-id is preserved for its ctor/extraction reads.
  • updateLastParentId is now unused and removed (it was the only inject-time mutator whose value varies per-span — hence the only one that raced; the OPM stamp is inject-time too but idempotent. The inbound value is set at construction).

Deliberately out of scope

OPM stamping is left as-is. It writes a process-constant local OPM, so concurrent injects are idempotent — no real race — and OPM is new; let it settle. Same-shape decouple is a clean future follow-up (or it rides the TraceSegment migration).

Test / validation

  • PropagationTagsLastParentIdTest — override supplies p:; sibling injects don't cross-talk; inbound p: preserved and unmutated by override use.
  • Full dd-trace-core propagation + span-build suite green (1947 tests, 0 failures), incl. W3CHttpInjectorTest.updateLastParentIdOnChildSpan, the 611 W3CPropagationTags cases, and span-link traceState.

Relationship

Step 2 of the PTags-per-segment work; pairs with #11701 (share parent's PTags for local children — the allocation win). Both are early steps toward a first-class TraceSegment.

🤖 Generated with Claude Code

Injecting a W3C tracestate wrote the injecting span's id into the
(trace-level, shared via getRootSpanContextOrThis) PropagationTags via
updateLastParentId, then read it back to build the `p:` member.
Concurrent sibling injects race on that shared field -- A writes A, B
writes B, A can emit B as its last-parent.
Supply the injecting span's id as a parameter instead:
PropagationTags.headerValue(HeaderType, lastParentIdOverride).
W3CHttpCodec passes context.getSpanId(); the W3C codec uses the override
for `p:`, falling back to the stored inbound last-parent-id for
non-inject callers (span-link traceState). No inject-time mutation of
the shared tags -> no race, no cross-talk between sibling injects, and
the inbound last-parent-id is preserved for its ctor/extraction reads.
updateLastParentId is now unused and removed (it was the only
inject-time mutator; the inbound value is set at construction).
OPM stamping is left as-is on purpose: it writes a process-constant
local OPM, so concurrent injects are idempotent (no race).
Full dd-trace-core propagation + span-build suite passes (1947 tests).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dd-octo-sts

dd-octo-stsBot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

SuiteStatus
Startup🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
ScenarioCandidatemasterΔ (95% CI of mean)
startup:insecure-bank:iast:Agent14.01 s13.97 s[-0.4%; +1.0%] (no difference)
startup:insecure-bank:tracing:Agent12.93 s12.97 s[-0.9%; +0.3%] (no difference)
startup:petclinic:appsec:Agent16.95 s16.74 s[+0.2%; +2.3%] (maybe worse)
startup:petclinic:iast:Agent16.98 s16.95 s[-0.7%; +1.0%] (no difference)
startup:petclinic:profiling:Agent16.67 s16.40 s[-2.9%; +6.1%] (no difference)
startup:petclinic:sca:Agent16.91 s16.83 s[-0.5%; +1.5%] (no difference)
startup:petclinic:tracing:Agent16.02 s16.11 s[-1.6%; +0.5%] (no difference)

Commit:734f641c · CI Pipeline · Benchmarking Platform UI


Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🎯 Code Coverage (details)
Patch Coverage: 66.67%
Overall Coverage: 57.07% (-0.21%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 734f641 | Docs | Datadog PR Page | Give us feedback!

@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

View session in Datadog

Bits Code status: ✅ Done

Comment @DataDog to request changes

@dougqh
dougqh marked this pull request as ready for review July 20, 2026 18:11
@dougqh
dougqh requested review from a team as code ownersJuly 20, 2026 18:11
@dougqh
dougqh requested review from mcculls and removed request for a teamJuly 20, 2026 18:11
@dd-octo-stsdd-octo-stsBot added the tag: ai generated Largely based on code generated by an AI or LLM label Jul 20, 2026
@dd-octo-sts

Copy link
Copy Markdown
Contributor

Hi! 👋 Thanks for your pull request! 🎉

To help us review it, please make sure to:

  • Add at least one type, and one component or instrumentation label to the pull request

If you need help, please check our contributing guidelines.

@dougqhdougqh added comp: context propagation Trace context propagation type: bug fix Bug fix labels Jul 20, 2026
@dougqh
dougqh added this pull request to the merge queueJul 20, 2026
@dd-octo-sts

Copy link
Copy Markdown
Contributor

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351Bot commented Jul 20, 2026

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-07-20 19:20:48 UTC ℹ️ Start processing command /merge


2026-07-20 19:20:52 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in master is approximately 2h (p90).


2026-07-20 20:28:19 UTC ℹ️ MergeQueue: This merge request was merged

@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to failed status checks Jul 20, 2026
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854dBot merged commit 057a33f into masterJul 20, 2026
590 of 596 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854dBot deleted the dougqh/ptags-decouple-lastparentid branch July 20, 2026 20:28
@github-actionsgithub-actionsBot added this to the 1.65.0 milestone Jul 20, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: context propagationTrace context propagationtag: ai generatedLargely based on code generated by an AI or LLMtype: bug fixBug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@dougqh@mhlidd