diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/build.gradle b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/build.gradle index 56311523214..6dd15db800f 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/build.gradle +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/build.gradle @@ -46,6 +46,12 @@ dependencies { testImplementation project(':dd-java-agent:appsec:appsec-test-fixtures') + // Make JUnit Jupiter API explicit for the Java JUnit 5 regression test + // (RouteHandlerSendFileTest). The shared gradle/java_deps.gradle only declares + // JUnit 5 as testRuntimeOnly; otherwise we'd be relying on Spock 2.x's transitive + // junit-jupiter-api on testCompileClasspath, which is implicit and brittle. + testImplementation libs.junit.jupiter + latestDepTestImplementation group: 'io.vertx', name: 'vertx-web', version: '3.+' latestDepTestImplementation group: 'io.vertx', name: 'vertx-web-client', version: '3.+' } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/EndHandlerWrapper.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/EndHandlerWrapper.java index a8cb1ebb079..f4b30bc4d44 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/EndHandlerWrapper.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/EndHandlerWrapper.java @@ -1,9 +1,5 @@ package datadog.trace.instrumentation.vertx_3_4.server; -import static datadog.trace.instrumentation.vertx_3_4.server.RouteHandlerWrapper.HANDLER_SPAN_CONTEXT_KEY; -import static datadog.trace.instrumentation.vertx_3_4.server.VertxDecorator.DECORATE; - -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import io.vertx.core.Handler; import io.vertx.ext.web.RoutingContext; @@ -18,16 +14,12 @@ public class EndHandlerWrapper implements Handler { @Override public void handle(final Void event) { - AgentSpan span = routingContext.get(HANDLER_SPAN_CONTEXT_KEY); try { if (actual != null) { actual.handle(event); } } finally { - if (span != null) { - DECORATE.onResponse(span, routingContext.response()); - span.finish(); - } + RouteHandlerWrapper.finishHandlerSpan(routingContext); } } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RouteHandlerWrapper.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RouteHandlerWrapper.java index 37120c2f0cd..5357a2a7bcf 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RouteHandlerWrapper.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RouteHandlerWrapper.java @@ -48,6 +48,16 @@ public void handle(final RoutingContext routingContext) { routingContext.put(HANDLER_SPAN_CONTEXT_KEY, span); routingContext.response().endHandler(new EndHandlerWrapper(routingContext)); + // Fallback finish path. The response.endHandler we register above can be + // silently skipped on Vert.x 3.x in two situations: + // 1. sendFile() — only bodyEndHandler is invoked on this path. + // 2. Synthetic transports (e.g. an in-memory Netty channel) on 3.9, + // where HttpServerResponseImpl.end gates endHandler behind `!closed` + // and the response is closed synchronously by responseComplete(). + // RoutingContext.addBodyEndHandler is wired to response.bodyEndHandler, + // which HttpServerResponseImpl invokes on every response-end path across + // the 3.x range. RoutingContext.addEndHandler does not exist until 4.0. + routingContext.addBodyEndHandler(v -> finishHandlerSpan(routingContext)); DECORATE.afterStart(span); span.setResourceName(DECORATE.className(actual.getClass())); } @@ -63,6 +73,20 @@ public void handle(final RoutingContext routingContext) { } } + // Idempotently finish the route-handler span. Both EndHandlerWrapper (the + // response.endHandler path) and the routingContext.addBodyEndHandler fallback + // may call this; the first one to win clears HANDLER_SPAN_CONTEXT_KEY so the + // second is a no-op. + static void finishHandlerSpan(final RoutingContext routingContext) { + final AgentSpan span = routingContext.get(HANDLER_SPAN_CONTEXT_KEY); + if (span == null) { + return; + } + routingContext.put(HANDLER_SPAN_CONTEXT_KEY, null); + DECORATE.onResponse(span, routingContext.response()); + span.finish(); + } + private void setRoute(RoutingContext routingContext) { final AgentSpan parentSpan = routingContext.get(PARENT_SPAN_CONTEXT_KEY); if (parentSpan == null) { diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/server/RouteHandlerSendFileTest.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/server/RouteHandlerSendFileTest.java new file mode 100644 index 00000000000..50303793fa2 --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/java/server/RouteHandlerSendFileTest.java @@ -0,0 +1,109 @@ +package server; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import datadog.trace.agent.test.AbstractInstrumentationTest; +import io.vertx.core.Vertx; +import io.vertx.core.http.HttpServer; +import io.vertx.ext.web.Router; +import java.io.BufferedReader; +import java.io.InputStreamReader; +import java.net.HttpURLConnection; +import java.net.ServerSocket; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** + * Regression test for the vertx-web 3.x route-handler span lifecycle on the {@code + * response.sendFile(...)} path. + * + *

{@code HttpServerResponseImpl.doSendFile} (vertx-core 3.x) only invokes {@code bodyEndHandler} + * after the file is written; it never invokes {@code endHandler}. With only the {@code endHandler} + * registration (pre-fix), the {@code vertx.route-handler} span never finishes on this path, the + * trace fails to flush, and {@code waitForTraces} times out. With the fallback {@code + * addBodyEndHandler} registration, the span finishes on every response-end path. + */ +class RouteHandlerSendFileTest extends AbstractInstrumentationTest { + + private static Vertx vertx; + private static HttpServer server; + private static int port; + private static Path payload; + + @BeforeAll + static void startServer() throws Exception { + payload = Files.createTempFile("vertx-sendfile-", ".txt"); + Files.write(payload, "vertx sendFile payload\n".getBytes(StandardCharsets.UTF_8)); + payload.toFile().deleteOnExit(); + + try (ServerSocket socket = new ServerSocket(0)) { + port = socket.getLocalPort(); + } + + vertx = Vertx.vertx(); + Router router = Router.router(vertx); + router + .route("/sendfile") + .handler(ctx -> ctx.response().sendFile(payload.toAbsolutePath().toString())); + + CountDownLatch ready = new CountDownLatch(1); + server = + vertx + .createHttpServer() + .requestHandler(router::accept) + .listen( + port, + result -> { + if (result.failed()) { + throw new RuntimeException("Failed to start Vert.x server", result.cause()); + } + ready.countDown(); + }); + if (!ready.await(10, TimeUnit.SECONDS)) { + throw new IllegalStateException("Vert.x server did not start in time"); + } + } + + @AfterAll + static void stopServer() throws Exception { + if (server != null) { + CountDownLatch closed = new CountDownLatch(1); + server.close(ar -> closed.countDown()); + closed.await(10, TimeUnit.SECONDS); + } + if (vertx != null) { + CountDownLatch closed = new CountDownLatch(1); + vertx.close(ar -> closed.countDown()); + closed.await(10, TimeUnit.SECONDS); + } + if (payload != null) { + Files.deleteIfExists(payload); + } + } + + @Test + void sendFileFinishesRouteHandlerSpan() throws Exception { + HttpURLConnection conn = + (HttpURLConnection) new URL("http://localhost:" + port + "/sendfile").openConnection(); + conn.setRequestMethod("GET"); + conn.setConnectTimeout(5000); + conn.setReadTimeout(5000); + assertEquals(200, conn.getResponseCode()); + try (BufferedReader reader = + new BufferedReader(new InputStreamReader(conn.getInputStream(), StandardCharsets.UTF_8))) { + assertEquals("vertx sendFile payload", reader.readLine()); + } + + // Strict-mode trace writes only publish a trace when every span in it has finished. + // Pre-fix: the route-handler span never finishes on the sendFile path, so the trace + // is never published and this call throws TimeoutException. + writer.waitForTraces(1); + } +} diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/EndHandlerWrapper.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/EndHandlerWrapper.java index a71584b11a7..fd4a042edf8 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/EndHandlerWrapper.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/EndHandlerWrapper.java @@ -1,9 +1,5 @@ package datadog.trace.instrumentation.vertx_4_0.server; -import static datadog.trace.instrumentation.vertx_4_0.server.RouteHandlerWrapper.HANDLER_SPAN_CONTEXT_KEY; -import static datadog.trace.instrumentation.vertx_4_0.server.VertxDecorator.DECORATE; - -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import io.vertx.core.Handler; import io.vertx.ext.web.RoutingContext; @@ -18,16 +14,12 @@ public class EndHandlerWrapper implements Handler { @Override public void handle(final Void event) { - AgentSpan span = routingContext.get(HANDLER_SPAN_CONTEXT_KEY); try { if (actual != null) { actual.handle(event); } } finally { - if (span != null) { - DECORATE.onResponse(span, routingContext.response()); - span.finish(); - } + RouteHandlerWrapper.finishHandlerSpan(routingContext); } } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RouteHandlerWrapper.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RouteHandlerWrapper.java index 8706f816e1c..19b344b75f6 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RouteHandlerWrapper.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RouteHandlerWrapper.java @@ -44,6 +44,16 @@ public void handle(final RoutingContext routingContext) { routingContext.put(HANDLER_SPAN_CONTEXT_KEY, span); routingContext.response().endHandler(new EndHandlerWrapper(routingContext)); + // Fallback finish path: HttpServerResponse.endHandler is silently skipped + // by Vert.x's Http1xServerResponse.end() when the underlying connection + // has already closed (Http1xServerResponse#end gates `endHandler.handle()` + // behind `!closed`). This happens in synthetic transports such as + // quarkus-amazon-lambda-rest's virtual Netty channel, where writes and + // close are synchronous in-memory, leaving the route-handler span unfinished + // and orphaning all jakarta-rs.request / aws.http child spans in the trace. + // RoutingContext#addEndHandler fires on routing-context completion regardless + // of underlying connection state and on both success and failure. + routingContext.addEndHandler(ar -> finishHandlerSpan(routingContext)); DECORATE.afterStart(span); span.setResourceName(DECORATE.className(actual.getClass())); } @@ -60,6 +70,19 @@ public void handle(final RoutingContext routingContext) { } } + // Idempotently finish the route-handler span. Both EndHandlerWrapper (the + // response.endHandler path) and the routingContext.addEndHandler fallback may call + // this; the first one to win clears HANDLER_SPAN_CONTEXT_KEY so the second is a no-op. + static void finishHandlerSpan(final RoutingContext routingContext) { + final AgentSpan span = routingContext.get(HANDLER_SPAN_CONTEXT_KEY); + if (span == null) { + return; + } + routingContext.put(HANDLER_SPAN_CONTEXT_KEY, null); + DECORATE.onResponse(span, routingContext.response()); + span.finish(); + } + private void setRoute(RoutingContext routingContext) { final AgentSpan parentSpan = routingContext.get(PARENT_SPAN_CONTEXT_KEY); if (parentSpan == null) {