Honour request header tags mapped onto headers - #12695
gh-worker-dd-mergequeue-cf854d[bot] merged 3 commits into
Conversation
![datadog-prod-us1-4[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
This comment has been minimized.
This comment has been minimized.
🟢 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.
One thing to note over #12662 is that if there are any mappings defined then we now check all headers against the mapping, including our own/W3C trace headers which arguably would never appear in a mapping because it doesn't make sense to map those to tags/baggage.
Benchmarks show that this would have a measurable impact, increasing time to extract headers by 40 ns/op (~15%)
In absolute terms it's only a small bump - and only when you define a mapping.
Good point - I've reworked the fix so this is no longer an issue |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
3 participants
What Does This Do
Makes a
DD_TRACE_REQUEST_HEADER_TAGSmapping on a header that the tracer also consumes for another purpose actually produce its tag.Every context interpreter recognises certain headers -
User-Agent, theForwarded/X-Forwarded-*family, and the client-IP headers (X-Real-IP,True-Client-IP,CF-Connecting-IP, the configurableDD_TRACE_CLIENT_IP_HEADER, …) - captures them via thehandled*helpers, and then returns fromaccept()immediately. That early return means the header never reacheshandleTags(), so a header tag mapped onto it is silently dropped.This is the same class of bug that #12662 fixed for
X-Amzn-Trace-Id, generalised to the remaining consumed headers. Instead of removing each early return one by one, this PR applies the header→tag / header→baggage mappings once up front inaccept(), so no later early return can bypass them. The semantic capture of each header (IP, user-agent, forwarding, trace context) is unchanged.The fix is applied to every interpreter that carries this pattern:
DatadogHttpCodec,XRayHttpCodec,W3CHttpCodec,HaystackHttpCodec,B3HttpCodec(both multi and single) andNoneCodec. Each interpreter keeps its existing tag/baggage semantics; the mappings simply run before the switch rather than after it.Motivation
APMS-20654
Additional Notes
Because the mappings now run before the switch, a mapping onto a header that was previously consumed and returned early - including the trace-context headers themselves - is now honoured, consistent with every other header.
X-Amzn-Trace-Idis covered as a side effect, so this supersedes #12662 if merged first.ConsumedHeaderTagMappingTestexercises every consumed header across all seven interpreters and asserts that the mapped tag is produced, the semantic capture is preserved, and the trace context is still extracted. All new tests were confirmed to fail without the codec changes.Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]