Repository navigation
Emit OpenTelemetry semantic conventions for Akka HTTP, Play, and Spring WebClient - #12901
zacharycmontoya wants to merge 17 commits into
Conversation
9a7511d to
7654f82
Compare
🟢 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. |
b5d0c00 to
c8d62ca
Compare
b1d1812 to
32198d1
Compare
8a835c3 to
fd65cad
Compare
5f15074 to
b2a637e
Compare
Play already decorates its framework span with the matched route, but the underlying HTTP server span also needs that route for OpenTelemetry naming. Apply it to the local root before application code can trigger sampling, using the shared HTTP resource decorator. Keep the existing Play span decoration and API Security route dispatch.
Observe successful path matchers and combine their literal segments and parameter placeholders into a route on the local root HTTP server span. Reuse the shared HTTP resource decorator to set both http.route and the method-and-route resource name without retaining parameter values. Allow the required matcher classes to be transformed and include a forked test that verifies stable routes across different parameter values.
Add a scoped context marker for HTTP client spans owned by higher-level instrumentation. Enable suppression only with OpenTelemetry semantics and only when the marker identifies the active request span. When marked, Netty propagates that context without creating another HTTP span. Response, error and close handlers retain the callback context and leave decoration and completion to the instrumentation that owns the span. Unmarked requests and Datadog semantics keep the existing Netty behavior.
Mark the WebClient span while subscribing so Netty reuses its context instead of creating a duplicate HTTP client span. In OpenTelemetry mode, finish the WebClient span before response or error callbacks, which may wait for that span to complete. Preserve the existing two spans and completion timing in Datadog mode. Update the Spring 5 and Spring 6 client suites to check the mode-dependent span counts and add forked OpenTelemetry coverage for both versions. Skip HEAD response-body consumption only in OpenTelemetry mode in the Spring 5 test helper.
b2a637e to
6b33693
Compare
zacharycmontoya
added
type: feature
zacharycmontoya
changed the title
Wrap the request handler in a single try/finally so the AWS and suppressed-span early returns no longer repeat the parent scope close, and derive the client span context from the already-captured parent.
Keep the Uri.Path on each frame and render it only when the match succeeds, since most path matcher attempts fail and never need it. Nested matchers also reuse the root span checked by the outermost frame.
When the outer client span owns the request, run the callback under it and return, leaving the existing client span handling unchanged instead of threading the suppression flag through each branch.
Mirror the Akka HTTP path matcher instrumentation for Pekko HTTP so successful matchers build a route on the local root HTTP server span, and add a forked test that verifies stable routes across parameters.
Mirror the Play 2.6 change so the underlying HTTP server span also gets the matched route for OpenTelemetry naming.
A path prefix that matched before the request was rejected left its
partial template on the span, so 404s under a valid prefix were reported
as real endpoints such as "GET /users/{param}".
Follow the OpenTelemetry Akka HTTP rule: the route only stands if the
last outermost path matcher tried also matched. Otherwise, when the
response is written, remove http.route and restore the method-only
resource that OpenTelemetry semantics give unrouted requests.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a4acd77a5
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isNestedClientSpanSuppressed(storedContext, span)) { | ||
| // The outer client span owns this request, so run the callback under it. | ||
| try (final ContextScope scope = activateSpan(span)) { | ||
| ctx.fireChannelRead(msg); | ||
| } | ||
| return; |
There was a problem hiding this comment.
When OpenTelemetry semantics suppresses the Netty transport span, this early return leaves CONTEXT_ATTRIBUTE_KEY pointing at the outer WebClient span. WebClient finishes that span as soon as the header-level ClientResponse is emitted, but chunked or streaming responses subsequently deliver HttpContent through this handler, so body processing and eventual channel events run under an already-finished span and can create late or misparented child spans. After forwarding the initial HttpResponse under the owner span, replace the channel attribute with the parent context while preserving the owner for errors that occur before headers.
Useful? React with 👍 / 👎.
| Frame parent = CURRENT.get(); | ||
| if (parent != null) { | ||
| // nested matchers share the root span already checked by the outermost frame | ||
| Frame frame = new Frame(path, parent.span, parent.state, parent); |
There was a problem hiding this comment.
perf: With OpenTelemetry semantics enabled, this advice-reachable path allocates a Frame for every nested PathMatcher.apply before knowing whether it matched, and the outermost branch similarly allocates at line 44. Akka routes commonly search multiple alternatives that fail, so this adds short-lived allocations proportional to route-table search on every request; the mirrored Pekko implementation has the same behavior. Reuse a cleared ThreadLocal frame stack or pool, and verify the reduction with an allocation profile.
AGENTS.md reference: AGENTS.md:L84-L87
Useful? React with 👍 / 👎.
| /** Drops the route of a request rejected after its last path matcher failed. */ | ||
| public static void dropRejectedRoute(Context context, AgentSpan span) { | ||
| AkkaHttpRouteState state = context.get(KEY); | ||
| if (state == null || state.route == null || state.lastMatched) { |
There was a problem hiding this comment.
lastMatched records only the result of the most recent PathMatcher, but an Akka route can successfully match its path and then be rejected by an inner authentication, header, or parameter directive before a catch-all alternative handles the request. In that case lastMatched remains true, so this guard preserves the rejected branch's http.route and resource name on the final server span; the mirrored Pekko implementation has the same problem. Track acceptance or rejection at the route-directive result level instead of treating the last successful path match as proof that its route was selected.
Useful? React with 👍 / 👎.
| try (final ContextScope scope = activateSpan(span); | ||
| final ContextScope suppressScope = suppressNestedClientSpans(span)) { | ||
| TraceWebClientSubscriber tracingSubscriber = | ||
| new TraceWebClientSubscriber(subscriber, span, parent); | ||
| next.exchange(request).doOnCancel(tracingSubscriber::onCancel).subscribe(tracingSubscriber); |
There was a problem hiding this comment.
Because the tracing filter is inserted at index 0, this suppression scope covers every downstream user filter as well as the actual connector. If a custom filter performs an independent Reactor Netty request—for example, to obtain an authentication token or resolve a service—before delegating the original exchange, that request inherits the same active span and suppression key, so HttpClientRequestTracingHandler mistakes it for the WebClient transport, injects the outer context, and omits its own HTTP client span. Apply suppression only around the connector request or associate it with the specific request that is allowed to consume it.
Useful? React with 👍 / 👎.
![datadog-prod-us1-4[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
With OpenTelemetry semantics enabled, WebClient URL-rewriting filters can bypass the Netty SSRF check. Akka/Pekko also lose matched route attributes for encoded paths when raw-resource mode is enabled.
🤖 Bits Code Review · Commit 5a4acd7
| if (ctx.channel().parent() == null) { | ||
| ctx.channel().attr(CLIENT_PARENT_ATTRIBUTE_KEY).set(parent); | ||
| } | ||
| if (isNestedClientSpanSuppressed(parentContext, parent)) { |
There was a problem hiding this comment.
With OpenTelemetry semantics and AppSec RASP enabled, a downstream WebClient filter can rewrite an allowed URL to a blocked destination. The tracing filter checks the original URL, while this early return skips decorate.onRequest and its SSRF callback for the actual transport URL, allowing a previously blocked request to be sent. Preserve transport-level security validation independently of span creation and without overwriting WebClient span metadata.
React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it
What each reaction means
- 👍 Useful: correct, and you're fixing it or will.
- 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
- 😕 Unclear: you can't tell what it means or why it matters.
- 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.
🤖 Bits Code Review · Open Bits AI session
| } | ||
| Route previous = null; | ||
| Object requestPath = frame.span.getTag(KnownTags.URL_PATH_NAME); | ||
| if (requestPath == null || !requestPath.toString().equals(URIUtils.decode(path))) { |
There was a problem hiding this comment.
With OpenTelemetry semantics and DD_HTTP_SERVER_RAW_RESOURCE=true, a matched request such as /items/a%20b has an encoded url.path, but this comparison decodes the matcher path. The mismatch leaves no previous route at the initial match, so the helper returns without recording http.route or the expected GET /items/{param} span name. Normalize encoding consistently in both this helper and PekkoHttpRouteHelper.
React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it
What each reaction means
- 👍 Useful: correct, and you're fixing it or will.
- 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
- 😕 Unclear: you can't tell what it means or why it matters.
- 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.
🤖 Bits Code Review · Open Bits AI session
What Does This Do
This PR completes the framework-specific HTTP semantic convention follow-ups from #12765 for Akka HTTP, Play, and Spring WebClient when
DD_TRACE_OTEL_SEMANTICS_ENABLED=true.Akka HTTP and Play apply matched routes to the root HTTP server span, allowing its name to follow the standard span name template
{method} {http.route}. Spring WebClient backed by Netty produces one HTTP client span per request.Implementation Details
Akka HTTP routes: When
DD_TRACE_OTEL_SEMANTICS_ENABLED=true, we instrument Akka’s path matchers to reconstruct route templates for supported routing patterns, using matched literals and{param}placeholders for extracted values. The resulting template is applied to the existing local root HTTP server span through the shared HTTP resource decorator.Play root-span routes: When
DD_TRACE_OTEL_SEMANTICS_ENABLED=true, the matched Play route is applied to both the Play framework span and its local root HTTP server span. Notably, the root span receives the route before sampling occurs and the route continues to be dispatched to AppSec API security.Spring WebClient and Netty: When
DD_TRACE_OTEL_SEMANTICS_ENABLED=true, WebClient marks its span in the current context so Netty can track when the active scope is "suppressed" and it should not create an additional HTTP client span. The WebClient span context is still used for context propagation and for recording response/error information and span completion.System Tests
This is being tested by this PR: DataDog/system-tests#8324. It enables the test cases in test_otel_http_semantics.py that were previously skipped for the
akka-httpandplayweblog applications.Motivation
When
DD_TRACE_OTEL_SEMANTICS_ENABLED=true, the Java tracer should produce HTTP spans that align with the OpenTelemetry semantic conventions instead of emitting a mixture of Datadog and OpenTelemetry naming and attributes. The shared HTTP decorator changes in #12765 left several framework-specific gaps:http.routespan attributeAddressing these gaps makes the affected frameworks consistent with OpenTelemetry HTTP span naming and span-count expectations when OpenTelemetry semantics are enabled.
Additional Notes
These changes are opt-in only, and span information is unchanged for Datadog semantics.
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(s): APMAPI-2038, [APMAPI-2039]