Collect Datadog security-testing headers on HTTP server entry spans#11418
Collect Datadog security-testing headers on HTTP server entry spans#11418christophe-papazian wants to merge 3 commits into
Conversation
Tags x-datadog-endpoint-scan and x-datadog-security-test as http.request.headers.<name> on every HTTP server entry span, unconditionally (independent of DD_TRACE_HEADER_TAGS and AppSec). When an inferred proxy span is the local root, the markers are forwarded from the service-entry span at finish time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
| if (carrier == null || getter == null) { | ||
| return; | ||
| } | ||
| getter.forEachKey(carrier, new SecurityTestingHeaderTagClassifier(span)); |
There was a problem hiding this comment.
This seems a bit too heavy if we just want to test for the presence of two headers, have you though about using protected String getRequestHeader(REQUEST request, String key)?
I know that not all frameworks are implemented, but we could ask APM to extend it.
There was a problem hiding this comment.
Good point — I went back and forth on this. I chose the iteration approach because getRequestHeader defaults to null on HttpServerDecorator and is currently only overridden where DSM needs it (a handful of integrations). With ~120 instrumentations in tree, switching to direct lookups means we'd ship the feature only for the few decorators that already implement it, until APM extends the rest.
The iteration cost is also mitigated:
- the classifier short-circuits via
return falseafter both markers are found, so on requests without the markers it scans the carrier once (same cost as the existingResponseHeaderTagClassifierdoes forDD_TRACE_HEADER_TAGS) and on requests with them it stops as soon as both are seen; - it follows the same pattern as
ResponseHeaderTagClassifier, so the perf trade-off is one we've already taken on the response path.
Happy to switch to getRequestHeader in a follow-up once APM is on board with extending it across server decorators — that'd be the cleaner long-term shape. Let me know if you'd rather hold this PR for that or land it now and refactor later.
Address PR review nit: replace two near-identical anonymous HttpServerDecorator subclasses with one helper that takes the ContextVisitor as a parameter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
christophe-papazian
requested review from
claponcet,
manuel-alvarez-alvarez,
mtoffl01 and
vandonr
and removed request for
a team
dd-octo-sts
Bot
added
the
tag: ai generated
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
christophe-papazian
added
comp: asm waf
2 participants
APPSEC-64531
Tags
x-datadog-endpoint-scanandx-datadog-security-testHTTP request headerson every HTTP server entry span as
http.request.headers.<name>, unconditionally— independent of
DD_TRACE_HEADER_TAGSand AppSec enablement.These markers let the API endpoint reducer distinguish Datadog scan/test traffic
from real user traffic and keep it out of the API inventory. They are not
propagated downstream: Java's propagator injects only
DDSpanContext, notarbitrary span tags.
When an inferred proxy span is the local root, the markers are forwarded from
the service-entry span at finish time (matching the Node.js implementation).
Test plan
HttpServerDecoratorSecurityTestingHeadersTest(new, JUnit 5)InferredProxySpanTestsfor inferred-proxy forwardingHttpServerDecoratorTest(Groovy) passes — no regression🤖 Generated with Claude Code