diff --git a/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11HandlerAdvice.java b/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11HandlerAdvice.java index 6e4d29b99cad..f5221d3806df 100644 --- a/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11HandlerAdvice.java +++ b/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11HandlerAdvice.java @@ -19,7 +19,8 @@ public class Jetty11HandlerAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter( @Advice.This Object source, - @Advice.Argument(value = 2, readOnly = false) HttpServletRequest request, + @Advice.Argument(2) HttpServletRequest request, + @Advice.Argument(3) HttpServletResponse response, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { @@ -31,6 +32,9 @@ public static void onEnter( context = tracer().startServerSpan(request); scope = context.makeCurrent(); + + // Must be set here since Jetty handlers can use startAsync outside of servlet scope. + tracer().setAsyncListenerResponse(request, response); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) diff --git a/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8HandlerAdvice.java b/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8HandlerAdvice.java index 7f8e1a48f4b7..0fe4711b3df8 100644 --- a/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8HandlerAdvice.java +++ b/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8HandlerAdvice.java @@ -19,7 +19,8 @@ public class Jetty8HandlerAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter( @Advice.This Object source, - @Advice.Argument(value = 2, readOnly = false) HttpServletRequest request, + @Advice.Argument(2) HttpServletRequest request, + @Advice.Argument(3) HttpServletResponse response, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { @@ -31,6 +32,9 @@ public static void onEnter( context = tracer().startServerSpan(request); scope = context.makeCurrent(); + + // Must be set here since Jetty handlers can use startAsync outside of servlet scope. + tracer().setAsyncListenerResponse(request, response); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) diff --git a/instrumentation/jetty/jetty-common/javaagent/jetty-common-javaagent.gradle b/instrumentation/jetty/jetty-common/javaagent/jetty-common-javaagent.gradle index f401290f4c33..bfc2052cd7d1 100644 --- a/instrumentation/jetty/jetty-common/javaagent/jetty-common-javaagent.gradle +++ b/instrumentation/jetty/jetty-common/javaagent/jetty-common-javaagent.gradle @@ -2,4 +2,5 @@ apply from: "$rootDir/gradle/instrumentation.gradle" dependencies { api(project(':instrumentation:servlet:servlet-common:library')) + implementation(project(':instrumentation:servlet:servlet-common:javaagent')) } diff --git a/instrumentation/jetty/jetty-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/common/JettyHandlerAdviceHelper.java b/instrumentation/jetty/jetty-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/common/JettyHandlerAdviceHelper.java index 3eed15c96e23..0af191a78854 100644 --- a/instrumentation/jetty/jetty-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/common/JettyHandlerAdviceHelper.java +++ b/instrumentation/jetty/jetty-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/common/JettyHandlerAdviceHelper.java @@ -7,10 +7,8 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; -import io.opentelemetry.instrumentation.servlet.ServletAccessor; import io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer; -import io.opentelemetry.instrumentation.servlet.TagSettingAsyncListener; -import java.util.concurrent.atomic.AtomicBoolean; +import io.opentelemetry.javaagent.instrumentation.servlet.common.service.ServletAndFilterAdviceHelper; public class JettyHandlerAdviceHelper { /** Shared method exit implementation for Jetty handler advices. */ @@ -45,23 +43,7 @@ public static void stopSpan( return; } - AtomicBoolean responseHandled = new AtomicBoolean(false); - ServletAccessor servletAccessor = tracer.getServletAccessor(); - - // In case of async servlets wait for the actual response to be ready - if (servletAccessor.isRequestAsyncStarted(request)) { - try { - servletAccessor.addRequestAsyncListener( - request, new TagSettingAsyncListener<>(tracer, responseHandled, context)); - } catch (IllegalStateException e) { - // org.eclipse.jetty.server.Request may throw an exception here if request became - // finished after check above. We just ignore that exception and move on. - } - } - - // Check again in case the request finished before adding the listener. - if (!servletAccessor.isRequestAsyncStarted(request) - && responseHandled.compareAndSet(false, true)) { + if (ServletAndFilterAdviceHelper.mustEndOnHandlerMethodExit(tracer, request)) { tracer.end(context, response); } } diff --git a/instrumentation/liberty/liberty/javaagent/liberty-javaagent.gradle b/instrumentation/liberty/liberty/javaagent/liberty-javaagent.gradle index dee3addc0bb3..e08c0bbb9dfd 100644 --- a/instrumentation/liberty/liberty/javaagent/liberty-javaagent.gradle +++ b/instrumentation/liberty/liberty/javaagent/liberty-javaagent.gradle @@ -6,5 +6,6 @@ apply from: "$rootDir/gradle/instrumentation.gradle" dependencies { compileOnly "javax.servlet:javax.servlet-api:3.0.1" + implementation project(':instrumentation:servlet:servlet-common:javaagent') implementation project(':instrumentation:servlet:servlet-3.0:javaagent') } diff --git a/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/LibertyHandleRequestAdvice.java b/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/LibertyHandleRequestAdvice.java index 28756b11ca0c..123f69cd1c65 100644 --- a/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/LibertyHandleRequestAdvice.java +++ b/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/LibertyHandleRequestAdvice.java @@ -9,8 +9,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; -import io.opentelemetry.javaagent.instrumentation.servlet.v3_0.TagSettingAsyncListener; -import java.util.concurrent.atomic.AtomicBoolean; +import io.opentelemetry.javaagent.instrumentation.servlet.common.service.ServletAndFilterAdviceHelper; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; @@ -33,7 +32,7 @@ public static void onEnter( // it is a bit too early to start span at this point because calling // some methods on HttpServletRequest will give a NPE // just remember the request and use it a bit later to start the span - ThreadLocalContext.startRequest(httpServletRequest); + ThreadLocalContext.startRequest(httpServletRequest, (HttpServletResponse) response); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) @@ -68,17 +67,7 @@ public static void stopSpan( return; } - AtomicBoolean responseHandled = new AtomicBoolean(false); - - // In case of async servlets wait for the actual response to be ready - if (request.isAsyncStarted()) { - request - .getAsyncContext() - .addListener(new TagSettingAsyncListener(responseHandled, context), request, response); - } - - // Check again in case the request finished before adding the listener. - if (!request.isAsyncStarted() && responseHandled.compareAndSet(false, true)) { + if (ServletAndFilterAdviceHelper.mustEndOnHandlerMethodExit(tracer(), request)) { tracer().end(context, response); } } diff --git a/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/LibertyStartSpanAdvice.java b/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/LibertyStartSpanAdvice.java index 894187a774a2..017a528342d0 100644 --- a/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/LibertyStartSpanAdvice.java +++ b/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/LibertyStartSpanAdvice.java @@ -26,5 +26,8 @@ public static void onEnter() { ctx.setContext(context); ctx.setScope(scope); + + // Must be set here since Liberty RequestProcessors can use startAsync outside of servlet scope. + tracer().setAsyncListenerResponse(ctx.getRequest(), ctx.getResponse()); } } diff --git a/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/ThreadLocalContext.java b/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/ThreadLocalContext.java index a2aafe1deefe..698887ec066d 100644 --- a/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/ThreadLocalContext.java +++ b/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/ThreadLocalContext.java @@ -8,18 +8,21 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; public class ThreadLocalContext { private static final ThreadLocal local = new ThreadLocal<>(); - private final HttpServletRequest req; + private final HttpServletRequest request; + private final HttpServletResponse response; private Context context; private Scope scope; private boolean started; - private ThreadLocalContext(HttpServletRequest req) { - this.req = req; + private ThreadLocalContext(HttpServletRequest request, HttpServletResponse response) { + this.request = request; + this.response = response; } public Context getContext() { @@ -39,7 +42,11 @@ public void setScope(Scope scope) { } public HttpServletRequest getRequest() { - return req; + return request; + } + + public HttpServletResponse getResponse() { + return response; } /** @@ -53,8 +60,8 @@ public boolean startSpan() { return !b; } - public static void startRequest(HttpServletRequest req) { - ThreadLocalContext ctx = new ThreadLocalContext(req); + public static void startRequest(HttpServletRequest request, HttpServletResponse response) { + ThreadLocalContext ctx = new ThreadLocalContext(request, response); local.set(ctx); } diff --git a/instrumentation/servlet/servlet-2.2/library/src/main/java/io/opentelemetry/instrumentation/servlet/v2_2/Servlet2Accessor.java b/instrumentation/servlet/servlet-2.2/library/src/main/java/io/opentelemetry/instrumentation/servlet/v2_2/Servlet2Accessor.java index 6705f26ab38a..018a4933e32d 100644 --- a/instrumentation/servlet/servlet-2.2/library/src/main/java/io/opentelemetry/instrumentation/servlet/v2_2/Servlet2Accessor.java +++ b/instrumentation/servlet/servlet-2.2/library/src/main/java/io/opentelemetry/instrumentation/servlet/v2_2/Servlet2Accessor.java @@ -19,14 +19,11 @@ public Integer getRequestRemotePort(HttpServletRequest httpServletRequest) { return null; } - @Override - public boolean isRequestAsyncStarted(HttpServletRequest request) { - return false; - } - @Override public void addRequestAsyncListener( - HttpServletRequest request, ServletAsyncListener listener) { + HttpServletRequest request, + ServletAsyncListener listener, + Object response) { throw new UnsupportedOperationException(); } diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java index 4b7c97221d77..6038158fa71c 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java @@ -81,6 +81,8 @@ public static void onEnter( context = tracer().startSpan(httpServletRequest, mappingResolver, servlet); scope = context.makeCurrent(); + + tracer().setAsyncListenerResponse(httpServletRequest, (HttpServletResponse) response); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3AsyncStartAdvice.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3AsyncStartAdvice.java new file mode 100644 index 000000000000..0d046357405a --- /dev/null +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3AsyncStartAdvice.java @@ -0,0 +1,40 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.v3_0; + +import static io.opentelemetry.instrumentation.servlet.v3_0.Servlet3HttpServerTracer.tracer; + +import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap; +import javax.servlet.AsyncContext; +import javax.servlet.ServletRequest; +import javax.servlet.http.HttpServletRequest; +import net.bytebuddy.asm.Advice; + +public class Servlet3AsyncStartAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void startAsyncEnter() { + // This allows to detect the outermost invocation of startAsync in method exit + CallDepthThreadLocalMap.incrementCallDepth(AsyncContext.class); + } + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void startAsyncExit(@Advice.This ServletRequest servletRequest) { + int callDepth = CallDepthThreadLocalMap.decrementCallDepth(AsyncContext.class); + + if (callDepth != 0) { + // This is not the outermost invocation, ignore. + return; + } + + if (servletRequest instanceof HttpServletRequest) { + HttpServletRequest request = (HttpServletRequest) servletRequest; + + if (!tracer().isAsyncListenerAttached(request)) { + tracer().attachAsyncListener(request); + } + } + } +} diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java index b3d8ea6f2ab9..46fbb2bd00ef 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java @@ -11,6 +11,7 @@ import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.instrumentation.servlet.common.async.AsyncContextInstrumentation; +import io.opentelemetry.javaagent.instrumentation.servlet.common.async.AsyncStartInstrumentation; import io.opentelemetry.javaagent.instrumentation.servlet.common.service.ServletAndFilterInstrumentation; import java.util.List; @@ -30,7 +31,8 @@ public List typeInstrumentations() { BASE_PACKAGE, adviceClassName(".Servlet3Advice"), adviceClassName(".Servlet3InitAdvice"), - adviceClassName(".Servlet3FilterInitAdvice"))); + adviceClassName(".Servlet3FilterInitAdvice")), + new AsyncStartInstrumentation(BASE_PACKAGE, adviceClassName(".Servlet3AsyncStartAdvice"))); } private static String adviceClassName(String suffix) { diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/TagSettingAsyncListener.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/TagSettingAsyncListener.java deleted file mode 100644 index 89019ad782f1..000000000000 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/TagSettingAsyncListener.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.servlet.v3_0; - -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.servlet.v3_0.Servlet3HttpServerTracer; -import java.util.concurrent.atomic.AtomicBoolean; -import javax.servlet.AsyncEvent; -import javax.servlet.AsyncListener; -import javax.servlet.http.HttpServletResponse; - -public class TagSettingAsyncListener implements AsyncListener { - private static final Servlet3HttpServerTracer servletHttpServerTracer = - Servlet3HttpServerTracer.tracer(); - - private final AtomicBoolean responseHandled; - private final Context context; - - public TagSettingAsyncListener(AtomicBoolean responseHandled, Context context) { - this.responseHandled = responseHandled; - this.context = context; - } - - @Override - public void onComplete(AsyncEvent event) { - if (responseHandled.compareAndSet(false, true)) { - servletHttpServerTracer.end(context, (HttpServletResponse) event.getSuppliedResponse()); - } - } - - @Override - public void onTimeout(AsyncEvent event) { - if (responseHandled.compareAndSet(false, true)) { - servletHttpServerTracer.onTimeout(context, event.getAsyncContext().getTimeout()); - } - } - - @Override - public void onError(AsyncEvent event) { - if (responseHandled.compareAndSet(false, true)) { - servletHttpServerTracer.endExceptionally( - context, event.getThrowable(), (HttpServletResponse) event.getSuppliedResponse()); - } - } - - /** Transfer the listener over to the new context. */ - @Override - public void onStartAsync(AsyncEvent event) { - event.getAsyncContext().addListener(this); - } -} diff --git a/instrumentation/servlet/servlet-3.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/v3_0/Servlet3Accessor.java b/instrumentation/servlet/servlet-3.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/v3_0/Servlet3Accessor.java index d4f74e4dcf31..7abb9ca31606 100644 --- a/instrumentation/servlet/servlet-3.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/v3_0/Servlet3Accessor.java +++ b/instrumentation/servlet/servlet-3.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/v3_0/Servlet3Accessor.java @@ -22,15 +22,18 @@ public Integer getRequestRemotePort(HttpServletRequest request) { return request.getRemotePort(); } - @Override - public boolean isRequestAsyncStarted(HttpServletRequest request) { - return request.isAsyncStarted(); - } - @Override public void addRequestAsyncListener( - HttpServletRequest request, ServletAsyncListener listener) { - request.getAsyncContext().addListener(new Listener(listener)); + HttpServletRequest request, + ServletAsyncListener listener, + Object response) { + if (response instanceof HttpServletResponse) { + request + .getAsyncContext() + .addListener(new Listener(listener), request, (HttpServletResponse) response); + } else { + request.getAsyncContext().addListener(new Listener(listener)); + } } @Override diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/JakartaServletInstrumentationModule.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/JakartaServletInstrumentationModule.java index ab6407213882..68630df17fdc 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/JakartaServletInstrumentationModule.java +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/JakartaServletInstrumentationModule.java @@ -9,6 +9,7 @@ import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.instrumentation.servlet.common.async.AsyncContextInstrumentation; +import io.opentelemetry.javaagent.instrumentation.servlet.common.async.AsyncStartInstrumentation; import io.opentelemetry.javaagent.instrumentation.servlet.common.response.HttpServletResponseInstrumentation; import io.opentelemetry.javaagent.instrumentation.servlet.common.service.ServletAndFilterInstrumentation; import java.util.Arrays; @@ -27,6 +28,7 @@ public List typeInstrumentations() { return Arrays.asList( new AsyncContextInstrumentation( BASE_PACKAGE, adviceClassName(".async.AsyncDispatchAdvice")), + new AsyncStartInstrumentation(BASE_PACKAGE, adviceClassName(".async.AsyncStartAdvice")), new ServletAndFilterInstrumentation( BASE_PACKAGE, adviceClassName(".service.JakartaServletServiceAdvice"), diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/async/AsyncStartAdvice.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/async/AsyncStartAdvice.java new file mode 100644 index 000000000000..14e110a3eba1 --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/async/AsyncStartAdvice.java @@ -0,0 +1,40 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.async; + +import static io.opentelemetry.instrumentation.servlet.jakarta.v5_0.JakartaServletHttpServerTracer.tracer; + +import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap; +import jakarta.servlet.AsyncContext; +import jakarta.servlet.http.HttpServletRequest; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.implementation.bytecode.assign.Assigner; + +public class AsyncStartAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void startAsyncEnter() { + // This allows to detect the outermost invocation of startAsync in method exit + CallDepthThreadLocalMap.incrementCallDepth(AsyncContext.class); + } + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void startAsyncExit( + @Advice.This(typing = Assigner.Typing.DYNAMIC) HttpServletRequest request) { + + int callDepth = CallDepthThreadLocalMap.decrementCallDepth(AsyncContext.class); + + if (callDepth != 0) { + // This is not the outermost invocation, ignore. + return; + } + + if (request != null) { + if (!tracer().isAsyncListenerAttached(request)) { + tracer().attachAsyncListener(request); + } + } + } +} diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java index 5936fbc6f74d..c9886ba4d028 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java @@ -82,6 +82,8 @@ public static void onEnter( context = tracer().startSpan(httpServletRequest, mappingResolver, servlet); scope = context.makeCurrent(); + + tracer().setAsyncListenerResponse(httpServletRequest, (HttpServletResponse) response); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/TagSettingAsyncListener.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/TagSettingAsyncListener.java deleted file mode 100644 index be1f98f3607b..000000000000 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/TagSettingAsyncListener.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.service; - -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.servlet.jakarta.v5_0.JakartaServletHttpServerTracer; -import jakarta.servlet.AsyncEvent; -import jakarta.servlet.AsyncListener; -import jakarta.servlet.http.HttpServletResponse; -import java.util.concurrent.atomic.AtomicBoolean; - -public class TagSettingAsyncListener implements AsyncListener { - private static final JakartaServletHttpServerTracer tracer = - JakartaServletHttpServerTracer.tracer(); - - private final AtomicBoolean responseHandled; - private final Context context; - - public TagSettingAsyncListener(AtomicBoolean responseHandled, Context context) { - this.responseHandled = responseHandled; - this.context = context; - } - - @Override - public void onComplete(AsyncEvent event) { - if (responseHandled.compareAndSet(false, true)) { - tracer.end(context, (HttpServletResponse) event.getSuppliedResponse()); - } - } - - @Override - public void onTimeout(AsyncEvent event) { - if (responseHandled.compareAndSet(false, true)) { - tracer.onTimeout(context, event.getAsyncContext().getTimeout()); - } - } - - @Override - public void onError(AsyncEvent event) { - if (responseHandled.compareAndSet(false, true)) { - tracer.endExceptionally( - context, event.getThrowable(), (HttpServletResponse) event.getSuppliedResponse()); - } - } - - /** Transfer the listener over to the new context. */ - @Override - public void onStartAsync(AsyncEvent event) { - event.getAsyncContext().addListener(this); - } -} diff --git a/instrumentation/servlet/servlet-5.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/jakarta/v5_0/JakartaServletAccessor.java b/instrumentation/servlet/servlet-5.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/jakarta/v5_0/JakartaServletAccessor.java index 99c03fd4ea23..3f67d54e106a 100644 --- a/instrumentation/servlet/servlet-5.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/jakarta/v5_0/JakartaServletAccessor.java +++ b/instrumentation/servlet/servlet-5.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/jakarta/v5_0/JakartaServletAccessor.java @@ -100,15 +100,18 @@ public Integer getRequestRemotePort(HttpServletRequest request) { return request.getRemotePort(); } - @Override - public boolean isRequestAsyncStarted(HttpServletRequest request) { - return request.isAsyncStarted(); - } - @Override public void addRequestAsyncListener( - HttpServletRequest request, ServletAsyncListener listener) { - request.getAsyncContext().addListener(new Listener(listener)); + HttpServletRequest request, + ServletAsyncListener listener, + Object response) { + if (response instanceof HttpServletResponse) { + request + .getAsyncContext() + .addListener(new Listener(listener), request, (HttpServletResponse) response); + } else { + request.getAsyncContext().addListener(new Listener(listener)); + } } @Override diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/async/AsyncStartInstrumentation.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/async/AsyncStartInstrumentation.java new file mode 100644 index 000000000000..6b247a3de568 --- /dev/null +++ b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/async/AsyncStartInstrumentation.java @@ -0,0 +1,44 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.common.async; + +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.extension.matcher.ClassLoaderMatcher.hasClassesNamed; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.returns; + +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +public class AsyncStartInstrumentation implements TypeInstrumentation { + private final String basePackageName; + private final String adviceClassName; + + public AsyncStartInstrumentation(String basePackageName, String adviceClassName) { + this.basePackageName = basePackageName; + this.adviceClassName = adviceClassName; + } + + @Override + public ElementMatcher classLoaderOptimization() { + return hasClassesNamed(basePackageName + ".Servlet"); + } + + @Override + public ElementMatcher typeMatcher() { + return implementsInterface(named(basePackageName + ".ServletRequest")); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + named("startAsync").and(returns(named(basePackageName + ".AsyncContext"))).and(isPublic()), + adviceClassName); + } +} diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/service/ServletAndFilterAdviceHelper.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/service/ServletAndFilterAdviceHelper.java index 0a8fe6a6b30d..a5c2645dbe73 100644 --- a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/service/ServletAndFilterAdviceHelper.java +++ b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/service/ServletAndFilterAdviceHelper.java @@ -8,12 +8,9 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.servlet.AppServerBridge; -import io.opentelemetry.instrumentation.servlet.ServletAccessor; import io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer; -import io.opentelemetry.instrumentation.servlet.TagSettingAsyncListener; import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap; import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; -import java.util.concurrent.atomic.AtomicBoolean; public class ServletAndFilterAdviceHelper { public static void stopSpan( @@ -49,23 +46,28 @@ public static void stopSpan( return; } - AtomicBoolean responseHandled = new AtomicBoolean(false); - ServletAccessor accessor = tracer.getServletAccessor(); - - // In case of async servlets wait for the actual response to be ready - if (accessor.isRequestAsyncStarted(request)) { - try { - accessor.addRequestAsyncListener( - request, new TagSettingAsyncListener<>(tracer, responseHandled, context)); - } catch (IllegalStateException e) { - // org.eclipse.jetty.server.Request may throw an exception here if request became - // finished after check above. We just ignore that exception and move on. - } + if (mustEndOnHandlerMethodExit(tracer, request)) { + tracer.end(context, response); } + } - // Check again in case the request finished before adding the listener. - if (!accessor.isRequestAsyncStarted(request) && responseHandled.compareAndSet(false, true)) { - tracer.end(context, response); + /** + * Helper method to determine whether the appserver handler/servlet service/servlet filter method + * that started a span must also end it, even if no error was detected. Extracted as a separate + * method to avoid duplicating the comments on the logic behind this choice. + */ + public static boolean mustEndOnHandlerMethodExit( + ServletHttpServerTracer tracer, REQUEST request) { + + if (tracer.isAsyncListenerAttached(request)) { + // This request is handled asynchronously and startAsync instrumentation has already attached + // the listener. + return false; } + + // This means that startAsync was not called (assuming startAsync instrumentation works + // correctly on this servlet engine), therefore the request was handled synchronously, and + // handler method end must also end the span. + return true; } } diff --git a/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletAccessor.java b/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletAccessor.java index 49ec785a0298..ff10d69402b8 100644 --- a/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletAccessor.java +++ b/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletAccessor.java @@ -50,9 +50,8 @@ public interface ServletAccessor { Integer getRequestRemotePort(REQUEST request); - boolean isRequestAsyncStarted(REQUEST request); - - void addRequestAsyncListener(REQUEST request, ServletAsyncListener listener); + void addRequestAsyncListener( + REQUEST request, ServletAsyncListener listener, Object response); int getResponseStatus(RESPONSE response); diff --git a/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java b/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java index 4376f6eebb6d..4ba803bc631c 100644 --- a/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java +++ b/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java @@ -23,6 +23,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.security.Principal; +import java.util.concurrent.atomic.AtomicBoolean; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,6 +32,11 @@ public abstract class ServletHttpServerTracer private static final Logger log = LoggerFactory.getLogger(ServletHttpServerTracer.class); + public static final String ASYNC_LISTENER_ATTRIBUTE = + ServletHttpServerTracer.class.getName() + ".AsyncListener"; + public static final String ASYNC_LISTENER_RESPONSE_ATTRIBUTE = + ServletHttpServerTracer.class.getName() + ".AsyncListenerResponse"; + private static final boolean CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES = Config.get() .getBooleanProperty("otel.instrumentation.servlet.experimental-span-attributes", false); @@ -138,8 +144,29 @@ protected int responseStatus(RESPONSE response) { @Override protected abstract TextMapGetter getGetter(); - public ServletAccessor getServletAccessor() { - return accessor; + /** + * Response object must be attached to a request prior to {@link #attachAsyncListener(Object)} + * being called, as otherwise in some environments it is not possible to access response from + * async event in listeners. + */ + public void setAsyncListenerResponse(REQUEST request, RESPONSE response) { + accessor.setRequestAttribute(request, ASYNC_LISTENER_RESPONSE_ATTRIBUTE, response); + } + + public void attachAsyncListener(REQUEST request) { + Context context = getServerContext(request); + + if (context != null) { + Object response = accessor.getRequestAttribute(request, ASYNC_LISTENER_RESPONSE_ATTRIBUTE); + + accessor.addRequestAsyncListener( + request, new TagSettingAsyncListener<>(this, new AtomicBoolean(), context), response); + accessor.setRequestAttribute(request, ASYNC_LISTENER_ATTRIBUTE, true); + } + } + + public boolean isAsyncListenerAttached(REQUEST request) { + return accessor.getRequestAttribute(request, ASYNC_LISTENER_ATTRIBUTE) != null; } public void addUnwrappedThrowable(Context context, Throwable throwable) { diff --git a/instrumentation/servlet/servlet-javax-common/library/src/test/java/RequestOnlyTracer.java b/instrumentation/servlet/servlet-javax-common/library/src/test/java/RequestOnlyTracer.java index 83ce43de89a8..cd3a47e91cb4 100644 --- a/instrumentation/servlet/servlet-javax-common/library/src/test/java/RequestOnlyTracer.java +++ b/instrumentation/servlet/servlet-javax-common/library/src/test/java/RequestOnlyTracer.java @@ -17,14 +17,9 @@ public Integer getRequestRemotePort(HttpServletRequest httpServletRequest) { throw new UnsupportedOperationException(); } - @Override - public boolean isRequestAsyncStarted(HttpServletRequest request) { - throw new UnsupportedOperationException(); - } - @Override public void addRequestAsyncListener( - HttpServletRequest request, ServletAsyncListener listener) { + HttpServletRequest request, ServletAsyncListener listener, Object response) { throw new UnsupportedOperationException(); } diff --git a/instrumentation/tomcat/tomcat-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/Tomcat10ServerHandlerAdvice.java b/instrumentation/tomcat/tomcat-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/Tomcat10ServerHandlerAdvice.java index dabf325fd6ff..049aea25c7f2 100644 --- a/instrumentation/tomcat/tomcat-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/Tomcat10ServerHandlerAdvice.java +++ b/instrumentation/tomcat/tomcat-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/Tomcat10ServerHandlerAdvice.java @@ -24,6 +24,7 @@ public class Tomcat10ServerHandlerAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter( @Advice.Argument(0) Request request, + @Advice.Argument(1) Response response, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { Context attachedContext = tracer().getServerContext(request); @@ -35,6 +36,12 @@ public static void onEnter( context = tracer().startServerSpan(request); scope = context.makeCurrent(); + + TomcatServerHandlerAdviceHelper.attachResponseToRequest( + Tomcat10ServletEntityProvider.INSTANCE, + JakartaServletHttpServerTracer.tracer(), + request, + response); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) @@ -47,6 +54,7 @@ public static void stopSpan( TomcatServerHandlerAdviceHelper.stopSpan( tracer(), + Tomcat10ServletEntityProvider.INSTANCE, JakartaServletHttpServerTracer.tracer(), request, response, diff --git a/instrumentation/tomcat/tomcat-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/Tomcat10ServletEntityProvider.java b/instrumentation/tomcat/tomcat-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/Tomcat10ServletEntityProvider.java new file mode 100644 index 000000000000..6ef1fad4b3e4 --- /dev/null +++ b/instrumentation/tomcat/tomcat-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/Tomcat10ServletEntityProvider.java @@ -0,0 +1,41 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.tomcat.v10_0; + +import io.opentelemetry.javaagent.instrumentation.tomcat.common.TomcatServletEntityProvider; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.apache.coyote.Request; +import org.apache.coyote.Response; + +public class Tomcat10ServletEntityProvider + implements TomcatServletEntityProvider { + public static final Tomcat10ServletEntityProvider INSTANCE = new Tomcat10ServletEntityProvider(); + + private Tomcat10ServletEntityProvider() {} + + @Override + public HttpServletRequest getServletRequest(Request request) { + Object note = request.getNote(1); + + if (note instanceof HttpServletRequest) { + return (HttpServletRequest) note; + } else { + return null; + } + } + + @Override + public HttpServletResponse getServletResponse(Response response) { + Object note = response.getNote(1); + + if (note instanceof HttpServletResponse) { + return (HttpServletResponse) note; + } else { + return null; + } + } +} diff --git a/instrumentation/tomcat/tomcat-7.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/Tomcat7ServerHandlerAdvice.java b/instrumentation/tomcat/tomcat-7.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/Tomcat7ServerHandlerAdvice.java index 550d949bc1f8..340cd9a13ffb 100644 --- a/instrumentation/tomcat/tomcat-7.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/Tomcat7ServerHandlerAdvice.java +++ b/instrumentation/tomcat/tomcat-7.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/Tomcat7ServerHandlerAdvice.java @@ -24,6 +24,7 @@ public class Tomcat7ServerHandlerAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter( @Advice.Argument(0) Request request, + @Advice.Argument(1) Response response, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { Context attachedContext = tracer().getServerContext(request); @@ -35,6 +36,12 @@ public static void onEnter( context = tracer().startServerSpan(request); scope = context.makeCurrent(); + + TomcatServerHandlerAdviceHelper.attachResponseToRequest( + Tomcat7ServletEntityProvider.INSTANCE, + Servlet3HttpServerTracer.tracer(), + request, + response); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) @@ -46,6 +53,13 @@ public static void stopSpan( @Advice.Local("otelScope") Scope scope) { TomcatServerHandlerAdviceHelper.stopSpan( - tracer(), Servlet3HttpServerTracer.tracer(), request, response, throwable, context, scope); + tracer(), + Tomcat7ServletEntityProvider.INSTANCE, + Servlet3HttpServerTracer.tracer(), + request, + response, + throwable, + context, + scope); } } diff --git a/instrumentation/tomcat/tomcat-7.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/Tomcat7ServletEntityProvider.java b/instrumentation/tomcat/tomcat-7.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/Tomcat7ServletEntityProvider.java new file mode 100644 index 000000000000..644e8471ad1d --- /dev/null +++ b/instrumentation/tomcat/tomcat-7.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/Tomcat7ServletEntityProvider.java @@ -0,0 +1,41 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.tomcat.v7_0; + +import io.opentelemetry.javaagent.instrumentation.tomcat.common.TomcatServletEntityProvider; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.apache.coyote.Request; +import org.apache.coyote.Response; + +public class Tomcat7ServletEntityProvider + implements TomcatServletEntityProvider { + public static final Tomcat7ServletEntityProvider INSTANCE = new Tomcat7ServletEntityProvider(); + + private Tomcat7ServletEntityProvider() {} + + @Override + public HttpServletRequest getServletRequest(Request request) { + Object note = request.getNote(1); + + if (note instanceof HttpServletRequest) { + return (HttpServletRequest) note; + } else { + return null; + } + } + + @Override + public HttpServletResponse getServletResponse(Response response) { + Object note = response.getNote(1); + + if (note instanceof HttpServletResponse) { + return (HttpServletResponse) note; + } else { + return null; + } + } +} diff --git a/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatServerHandlerAdviceHelper.java b/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatServerHandlerAdviceHelper.java index a5c648935fb9..5583e137fb4e 100644 --- a/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatServerHandlerAdviceHelper.java +++ b/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatServerHandlerAdviceHelper.java @@ -8,9 +8,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer; -import io.opentelemetry.instrumentation.servlet.TagSettingAsyncListener; -import java.util.concurrent.atomic.AtomicBoolean; -import net.bytebuddy.asm.Advice; +import io.opentelemetry.javaagent.instrumentation.servlet.common.service.ServletAndFilterAdviceHelper; import org.apache.coyote.Request; import org.apache.coyote.Response; @@ -27,12 +25,13 @@ public class TomcatServerHandlerAdviceHelper { */ public static void stopSpan( TomcatTracer tracer, + TomcatServletEntityProvider servletEntityProvider, ServletHttpServerTracer servletTracer, Request request, Response response, - @Advice.Thrown Throwable throwable, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { + Throwable throwable, + Context context, + Scope scope) { if (scope != null) { scope.close(); } @@ -57,35 +56,29 @@ public static void stopSpan( return; } - Object note = request.getNote(1); - if (note instanceof org.apache.catalina.connector.Request) { - AtomicBoolean responseHandled = new AtomicBoolean(false); + REQUEST servletRequest = servletEntityProvider.getServletRequest(request); - org.apache.catalina.connector.Request servletRequest = - (org.apache.catalina.connector.Request) note; - if (servletRequest.isAsync()) { - try { - // The Catalina Request always implements the HttpServletRequest (either javax or jakarta) - // which is also what REQUEST generic parameter is. We cannot cast normally without a - // generic because this class is compiled against javax.servlet which would make it try - // to use request from javax.servlet when REQUEST is actually from jakarta.servlet. - //noinspection unchecked - servletTracer - .getServletAccessor() - .addRequestAsyncListener( - (REQUEST) servletRequest, - new TagSettingAsyncListener<>(servletTracer, responseHandled, context)); - } catch (IllegalStateException e) { - // thrown by newer tomcats if request was already handled while setting the listener. - tracer.end(context, response); - return; - } - } + if (servletRequest != null + && ServletAndFilterAdviceHelper.mustEndOnHandlerMethodExit(servletTracer, servletRequest)) { + tracer.end(context, response); + } + } - // In case the request finished before adding the listener on older tomcats... - if (!servletRequest.isAsyncStarted() && responseHandled.compareAndSet(false, true)) { - tracer.end(context, response); - } + /** + * Must be attached in Tomcat instrumentations since Tomcat valves can use startAsync outside of + * servlet scope. + */ + public static void attachResponseToRequest( + TomcatServletEntityProvider servletEntityProvider, + ServletHttpServerTracer servletTracer, + Request request, + Response response) { + + REQUEST servletRequest = servletEntityProvider.getServletRequest(request); + RESPONSE servletResponse = servletEntityProvider.getServletResponse(response); + + if (servletRequest != null && servletResponse != null) { + servletTracer.setAsyncListenerResponse(servletRequest, servletResponse); } } } diff --git a/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatServletEntityProvider.java b/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatServletEntityProvider.java new file mode 100644 index 000000000000..fe05238313c3 --- /dev/null +++ b/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatServletEntityProvider.java @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.tomcat.common; + +import org.apache.coyote.Request; +import org.apache.coyote.Response; + +/** + * Used to access servlet request/response classes from their Apache Coyote counterparts. As the + * Coyote classes are the same for all Tomcat versions, but newer Tomcat uses jakarta.servlet + * instead of javax.servlet, this allows accessing the servlet entities without unchecked casts in + * shared code where HttpServletRequest and/or HttpServletResponse are used as generic parameters. + * + * @param HttpServletRequest instance + * @param HttpServletResponse instance + */ +public interface TomcatServletEntityProvider { + REQUEST getServletRequest(Request request); + + RESPONSE getServletResponse(Response response); +} diff --git a/instrumentation/tomcat/tomcat-common/javaagent/tomcat-common-javaagent.gradle b/instrumentation/tomcat/tomcat-common/javaagent/tomcat-common-javaagent.gradle index 43b3581037a8..6776fe89bed8 100644 --- a/instrumentation/tomcat/tomcat-common/javaagent/tomcat-common-javaagent.gradle +++ b/instrumentation/tomcat/tomcat-common/javaagent/tomcat-common-javaagent.gradle @@ -2,5 +2,6 @@ apply from: "$rootDir/gradle/instrumentation.gradle" dependencies { api(project(':instrumentation:servlet:servlet-common:library')) + implementation(project(':instrumentation:servlet:servlet-common:javaagent')) compileOnly "org.apache.tomcat.embed:tomcat-embed-core:7.0.4" } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcher.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcher.java index 64ad434e525f..6e6bc9d0d5d7 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcher.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcher.java @@ -178,6 +178,7 @@ public boolean matches(TypeDescription target) { if (name.startsWith("org.springframework.web.")) { if (name.startsWith("org.springframework.web.servlet.") || name.startsWith("org.springframework.web.filter.") + || name.startsWith("org.springframework.web.multipart.") || name.startsWith("org.springframework.web.reactive.") || name.startsWith("org.springframework.web.context.request.async.") || name.equals( diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/GlobalIgnoresMatcher.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/GlobalIgnoresMatcher.java index f70d8564ea17..c9a6db93142d 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/GlobalIgnoresMatcher.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/GlobalIgnoresMatcher.java @@ -196,7 +196,11 @@ public boolean matches(TypeDescription target) { // want to instrument these methods in generated classes as this would create spans that // have the generated class name in them instead of the actual class that handles the call. || name.contains("__EJB31_Generated__") - || name.startsWith("org.springframework.core.$Proxy")) { + || name.startsWith("org.springframework.core.$Proxy") + // Tapestry Proxy, check only specific class that we know would be instrumented since there + // is no common prefix for its proxies other than "$". ByteBuddy fails to instrument this + // proxy, and as there is no reason why it should be instrumented anyway, exclude it. + || name.startsWith("$HttpServletRequest_")) { return true; }