fix(vertx-web): finish vertx.route-handler via RoutingContext.addEndHandler fallback#11312
fix(vertx-web): finish vertx.route-handler via RoutingContext.addEndHandler fallback#11312zarirhamza wants to merge 6 commits into
Conversation
…andler fallback Vert.x's `Http1xServerResponse.end(Buffer, PromiseInternal)` invokes the registered `endHandler` only when `closed == false` at the moment the response body has been written. In synthetic transports such as quarkus-amazon-lambda-rest's `VirtualClientConnection` (in-memory Netty channel) the writes and the connection close happen synchronously inside `responseComplete()`, so by the time the `!closed` guard runs `closed` is already `true` and `endHandler` is silently skipped. Symptom: `RouteHandlerWrapper` starts a `vertx.route-handler` span for every route in the chain (e.g. Quarkus's AuthenticationHandler) but `EndHandlerWrapper.handle` is never called, so the span is never finished. The span dies in PendingTrace and is not enqueued on the writer. All children parented to that span (`jakarta-rs.request`, `netty.client.request`, downstream `aws.http`/`aws.apigateway` inferred spans) end up orphaned in the trace UI. Fix: also register a finish via `RoutingContext.addEndHandler`, which fires on routing-context completion regardless of underlying connection state and on both success and failure. Both paths funnel through a shared idempotent `finishHandlerSpan` so the second one to fire on real-network transports is a no-op. Verified end-to-end against a Quarkus 3.15.4 / Java 21 Lambda chain (caller -> netty.client.request -> callee) on Datadog Lambda Extension v96. Pre-fix: 5/5 invocations Started, 0/5 Finished. Post-fix: 5/5 Started, 5/5 Finished, single connected trace tree in the UI. Refs: SLES-2837
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
ddef381 to
97f9933
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 490cd63eb7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -0,0 +1,109 @@ | |||
| package server; | |||
|
|
|||
| import static org.junit.jupiter.api.Assertions.assertEquals; | |||
There was a problem hiding this comment.
Add JUnit Jupiter to the test compile classpath
This module's shared Gradle setup only adds JUnit 5 as testRuntimeOnly, while testImplementation is Spock/Groovy plus instrumentation-testing, so a Java test source that imports org.junit.jupiter.api is not on the compileTestJava classpath. As a result :dd-java-agent:instrumentation:vertx:vertx-web:vertx-web-3.4:compileTestJava will fail with package org.junit.jupiter.api does not exist unless the module adds a Jupiter API/testImplementation dependency for this new JUnit test.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I can only run on private repositories.
Summary
In synthetic Vert.x transports — most importantly
quarkus-amazon-lambda-rest'sVirtualClientConnection(the in-memory Netty channel that AWS Lambda HTTP runtimes use) —Http1xServerResponse.end()synchronously closes the underlying connection, so by the time the inlinedif (!closed && endHandler != null)guard runs,closedistrueand the registeredendHandleris silently skipped.RouteHandlerWrapper.handle()relies onresponse().endHandler(...)to finish thevertx.route-handlerspan. When that hook never fires, the span is started but never finished, never enqueued on the writer, and silently dropped. Every child span parented to it (jakarta-rs.request,netty.client.request, downstreamaws.http/aws.apigatewayinferred spans) ends up orphaned in the trace UI.Fix
Also register a finish via
RoutingContext.addEndHandler(...), which fires on routing-context completion regardless of underlying connection state and on both success and failure. Both paths funnel through a shared idempotentRouteHandlerWrapper.finishHandlerSpan(routingContext)so the second one to fire on real-network transports is a no-op.Files touched:
RouteHandlerWrapper.java(+23/-0): register the fallback in the span-starter branch; addfinishHandlerSpanstatic helper.EndHandlerWrapper.java(-9/+1): delegate to the shared helper instead of duplicating the finish logic.Test plan
HttpServerResponsewhoseend()skips the user-registeredendHandler(mimicking the closed-connection path) and asserts the route-handler span is still finished viaaddEndHandler.caller -> netty.client.request -> callee) with Datadog Lambda Extension v96. Pre-fix: 5/5 invocations Started, 0/5 Finished. Post-fix: 5/5 Started, 5/5 Finished, single connected trace tree at the backend.Refs
SLES-2837