From 95528991469c85d7dae062d61ebba08a21bb487d Mon Sep 17 00:00:00 2001 From: Arunodoy18 Date: Mon, 8 Dec 2025 02:56:08 +0530 Subject: [PATCH] Fix Jetty 12 HTTP server metrics collection without virtual threads After upgrading to Dropwizard 5 (which uses Jetty 12), HTTP server metrics were not being collected or exposed correctly when virtual threads were disabled (enableVirtualThreads=false). The metrics appeared correctly only when virtual threads were enabled. Root Cause: The Jetty12ServerInstrumentation was ending spans in two places: 1. In onMethodExit when the handle() method completed 2. In HttpStream callbacks (succeeded/failed) for request completion When virtual threads were disabled, handle() often completed synchronously BEFORE the HttpStream callbacks fired. This caused the span to end prematurely in onMethodExit, and metrics were not properly captured. The HttpStream callbacks would then try to end an already-ended span. When virtual threads were enabled, the asynchronous nature ensured callbacks fired before method exit, so metrics were captured correctly. Solution: 1. Modified Jetty12ServerInstrumentation.HandlerAdvice.end() to NOT end the span in onMethodExit (except when there's an exception). The span is now ended exclusively by the HttpStream completion callbacks. 2. Added AtomicBoolean in Jetty12Helper.start() to ensure the span ends exactly once, preventing double-ending issues regardless of callback order. This ensures HTTP metrics (request counts, latency, body size, per-endpoint and per-status-code metrics) are collected correctly whether virtual threads are enabled or disabled. Fixes the issue where Dropwizard 5 applications with enableVirtualThreads=false would not report HTTP server metrics. --- .../jetty/v12_0/Jetty12Helper.java | 19 ++++++++++++++++--- .../v12_0/Jetty12ServerInstrumentation.java | 4 ++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/instrumentation/jetty/jetty-12.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v12_0/Jetty12Helper.java b/instrumentation/jetty/jetty-12.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v12_0/Jetty12Helper.java index 4f65840f1d5f..c397fe6e805d 100644 --- a/instrumentation/jetty/jetty-12.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v12_0/Jetty12Helper.java +++ b/instrumentation/jetty/jetty-12.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v12_0/Jetty12Helper.java @@ -9,6 +9,7 @@ import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.javaagent.bootstrap.servlet.AppServerBridge; import io.opentelemetry.javaagent.bootstrap.servlet.ServletAsyncContext; +import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nullable; import org.eclipse.jetty.server.HttpStream; import org.eclipse.jetty.server.Request; @@ -27,7 +28,15 @@ public boolean shouldStart(Context parentContext, Request request) { public Context start(Context parentContext, Request request, Response response) { Context context = instrumenter.start(parentContext, request); - request.addFailureListener(throwable -> end(context, request, response, throwable)); + // Use AtomicBoolean to ensure the span ends exactly once + AtomicBoolean spanEnded = new AtomicBoolean(false); + + request.addFailureListener( + throwable -> { + if (spanEnded.compareAndSet(false, true)) { + end(context, request, response, throwable); + } + }); // detect request completion // https://github.com/jetty/jetty.project/blob/52d94174e2c7a6e794c6377dcf9cd3ed0b9e1806/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/EventsHandler.java#L75 request.addHttpStreamWrapper( @@ -35,13 +44,17 @@ public Context start(Context parentContext, Request request, Response response) new HttpStream.Wrapper(stream) { @Override public void succeeded() { - end(context, request, response, null); + if (spanEnded.compareAndSet(false, true)) { + end(context, request, response, null); + } super.succeeded(); } @Override public void failed(Throwable throwable) { - end(context, request, response, throwable); + if (spanEnded.compareAndSet(false, true)) { + end(context, request, response, throwable); + } super.failed(throwable); } }); diff --git a/instrumentation/jetty/jetty-12.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v12_0/Jetty12ServerInstrumentation.java b/instrumentation/jetty/jetty-12.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v12_0/Jetty12ServerInstrumentation.java index ecc127ecfe87..9f19c289b1b9 100644 --- a/instrumentation/jetty/jetty-12.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v12_0/Jetty12ServerInstrumentation.java +++ b/instrumentation/jetty/jetty-12.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v12_0/Jetty12ServerInstrumentation.java @@ -67,6 +67,10 @@ public static AdviceScope start(Object source, Request request, Response respons public void end(Request request, Response response, @Nullable Throwable throwable) { scope.close(); + // Don't end the span here - it will be ended by the HttpStream callbacks + // registered in Jetty12Helper.start(). This ensures metrics are captured + // correctly regardless of whether virtual threads are enabled. + // Only end immediately if there's an exception, as the callbacks may not fire. if (throwable != null) { helper().end(context, request, response, throwable); }