Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -18,16 +14,12 @@ public class EndHandlerWrapper implements Handler<Void> {

@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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package server;

import static org.junit.jupiter.api.Assertions.assertEquals;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DataDog fix this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can only run on private repositories.


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.
*
* <p>{@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);
}
}
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -18,16 +14,12 @@ public class EndHandlerWrapper implements Handler<Void> {

@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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand All @@ -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) {
Expand Down