Share parent's PropagationTags for local child spans#11701
Draft
dougqh wants to merge 2 commits into
Draft
Conversation
CoreTracer span-build allocates a fresh propagationTagsFactory.empty() per local child span; only the distributed ExtractedContext path reuses the parent's. Before optimizing that per-span allocation away, this test pins the load-bearing behavior: that a non-root (local child) span still carries the inbound distributed _dd.p.* tags when it injects. Verdict (both tests pass): it does. DDSpanContext.getPropagationTags() routes to the root (getRootSpanContextOrThis), so a non-root child's own propagationTags field is never read for injection. The per-span empty() is pure allocation waste, not a latent correctness bug. This test is the gate + safety net for the planned "share the parent's PropagationTags for local children" allocation fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CoreTracer span-build allocated a fresh propagationTagsFactory.empty() for every local child span, reused from the parent only on the distributed ExtractedContext path — N+1 PropagationTags per N-span trace, N of them needless empties. A non-root child's own PropagationTags is never read for injection: DDSpanContext.getPropagationTags() routes to the root (getRootSpanContextOrThis), confirmed by PropagationTagsChildSpanTest (inbound _dd.p.* survive child injection). So local children can share the parent's (trace-level) instance instead of allocating their own, collapsing N+1 -> 1 per trace. Safe: the ctor's updateTraceIdHighOrderBits stamp is guard-idempotent and a no-op on the already-stamped shared root instance (same trace => same high-order bits). Full dd-trace-core propagation + span-build suite passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Contributor
🟢 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. |
dougqh
commented
Jun 23, 2026
| ciVisibilityContextData = null; | ||
| } | ||
| propagationTags = tracer.propagationTagsFactory.empty(); | ||
| // Local child: share the parent's (trace-level) PropagationTags instead of allocating a |
Contributor
Author
There was a problem hiding this comment.
To Claude - can we simplify this a bit?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What This Does
Local child spans now share the parent's (trace-level)
PropagationTagsinstead of each allocating a freshempty(). Collapses N+1 → 1PropagationTagsper N-span trace.Motivation
CoreTracerspan-build allocatedpropagationTagsFactory.empty()for every local child span (the common in-process case); only the distributedExtractedContextpath reused the parent's. A non-root child's ownPropagationTags, however, is never read —DDSpanContext.getPropagationTags()routes to the root (getRootSpanContextOrThis().propagationTags). So the per-spanempty()was pure allocation waste.Additional Notes
Correctness — gated by a characterization test
First commit adds
PropagationTagsChildSpanTest, which pins the load-bearing behavior: a non-root (local child) span still carries the inbound distributed_dd.p.*tags on injection. Both tests pass, confirming this is pure allocation waste, not a latent correctness bug. The test is the gate + safety net for the change.Safety
updateTraceIdHighOrderBitsstamp is guard-idempotent (if (traceIdHighOrderBits != highOrderBits)) and a no-op on the already-stamped shared root instance (same trace ⇒ same high-order bits).dd-trace-corepropagation + span-build suite passes (W3C/Datadog extractors+injectors,W3CPropagationTags, OPM round-trip, OrgGuard,CoreSpanBuilderTest,DDSpanContextTest) — 0 failures.Misc
The
lastParentId/OPM inject-time mutation race (a transient per-injection value parked in the shared object) is pre-existing — injection already routes to the root's shared instance viagetPropagationTags()— so it is a separate concurrency concern, not introduced or worsened by this change. Tracked for a follow-up.🤖 Generated with Claude Code