From 454f408182b044115f854eaeb38d2fb110412082 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Mon, 22 Nov 2021 21:17:59 +0200 Subject: [PATCH] Record exception for async servlet invocations (#4677) * Record exception for asyn servlet invocations * add back accidentally commented out line * rearrange test so that it passes on both jetty and tomcat --- .../v3_0/Servlet3AsyncContextStartAdvice.java | 27 ++++++++++++ .../v3_0/Servlet3InstrumentationModule.java | 7 ++- .../src/test/groovy/JettyServlet3Test.groovy | 39 ++++++++--------- .../src/test/groovy/TestServlet3.groovy | 11 +++-- .../src/test/groovy/TomcatServlet3Test.groovy | 18 -------- .../JakartaServletInstrumentationModule.java | 3 ++ .../v5_0/async/AsyncContextStartAdvice.java | 27 ++++++++++++ .../src/test/groovy/JettyServlet5Test.groovy | 24 ----------- .../src/test/groovy/TestServlet5.groovy | 10 +++-- .../src/test/groovy/TomcatServlet5Test.groovy | 18 -------- .../AsyncRequestCompletionListener.java | 6 ++- .../servlet/AsyncRunnableWrapper.java | 37 ++++++++++++++++ .../servlet/ServletHelper.java | 19 ++++++++ .../AsyncContextStartInstrumentation.java | 43 +++++++++++++++++++ .../tomcat/v10_0/AsyncServlet.groovy | 19 ++++---- .../tomcat/v10_0/TomcatAsyncTest.groovy | 6 --- .../tomcat/v7_0/AsyncServlet.groovy | 15 +++++-- .../tomcat/v7_0/TomcatAsyncTest.groovy | 6 --- 18 files changed, 217 insertions(+), 118 deletions(-) create mode 100644 instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3AsyncContextStartAdvice.java create mode 100644 instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/async/AsyncContextStartAdvice.java create mode 100644 instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/AsyncRunnableWrapper.java create mode 100644 instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/async/AsyncContextStartInstrumentation.java diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3AsyncContextStartAdvice.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3AsyncContextStartAdvice.java new file mode 100644 index 000000000000..fcc8cb995104 --- /dev/null +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3AsyncContextStartAdvice.java @@ -0,0 +1,27 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.v3_0; + +import static io.opentelemetry.javaagent.instrumentation.servlet.v3_0.Servlet3Singletons.helper; + +import javax.servlet.AsyncContext; +import javax.servlet.ServletRequest; +import javax.servlet.http.HttpServletRequest; +import net.bytebuddy.asm.Advice; + +@SuppressWarnings("unused") +public class Servlet3AsyncContextStartAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void start( + @Advice.This AsyncContext asyncContext, + @Advice.Argument(value = 0, readOnly = false) Runnable runnable) { + ServletRequest request = asyncContext.getRequest(); + if (request instanceof HttpServletRequest) { + runnable = helper().wrapAsyncRunnable((HttpServletRequest) request, runnable); + } + } +} 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 80d635590ed0..1025ac76a39a 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 @@ -12,6 +12,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.AsyncContextStartInstrumentation; import io.opentelemetry.javaagent.instrumentation.servlet.common.async.AsyncStartInstrumentation; import io.opentelemetry.javaagent.instrumentation.servlet.common.service.ServletAndFilterInstrumentation; import java.util.List; @@ -34,12 +35,14 @@ public ElementMatcher.Junction classLoaderMatcher() { public List typeInstrumentations() { return asList( new AsyncContextInstrumentation(BASE_PACKAGE, adviceClassName(".AsyncDispatchAdvice")), + new AsyncContextStartInstrumentation( + BASE_PACKAGE, adviceClassName(".Servlet3AsyncContextStartAdvice")), + new AsyncStartInstrumentation(BASE_PACKAGE, adviceClassName(".Servlet3AsyncStartAdvice")), new ServletAndFilterInstrumentation( BASE_PACKAGE, adviceClassName(".Servlet3Advice"), adviceClassName(".Servlet3InitAdvice"), - adviceClassName(".Servlet3FilterInitAdvice")), - new AsyncStartInstrumentation(BASE_PACKAGE, adviceClassName(".Servlet3AsyncStartAdvice"))); + adviceClassName(".Servlet3FilterInitAdvice"))); } private static String adviceClassName(String suffix) { diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServlet3Test.groovy b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServlet3Test.groovy index d113cb004764..187c9095bab5 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServlet3Test.groovy +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServlet3Test.groovy @@ -42,9 +42,13 @@ abstract class JettyServlet3Test extends AbstractServlet3Test servlet() { TestServlet3.FakeAsync } - - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } } class JettyServlet3TestForward extends JettyDispatchTest { @@ -223,6 +220,11 @@ class JettyServlet3TestDispatchImmediate extends JettyDispatchTest { TestServlet3.Sync } + @Override + boolean isAsyncTest() { + true + } + @Override protected void setupServlets(ServletContextHandler context) { super.setupServlets(context) @@ -237,12 +239,6 @@ class JettyServlet3TestDispatchImmediate extends JettyDispatchTest { addServlet(context, "/dispatch" + INDEXED_CHILD.path, TestServlet3.DispatchImmediate) addServlet(context, "/dispatch/recursive", TestServlet3.DispatchRecursive) } - - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } } class JettyServlet3TestDispatchAsync extends JettyDispatchTest { @@ -251,6 +247,11 @@ class JettyServlet3TestDispatchAsync extends JettyDispatchTest { TestServlet3.Async } + @Override + boolean isAsyncTest() { + true + } + @Override protected void setupServlets(ServletContextHandler context) { super.setupServlets(context) @@ -270,12 +271,6 @@ class JettyServlet3TestDispatchAsync extends JettyDispatchTest { boolean errorEndpointUsesSendError() { false } - - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } } abstract class JettyDispatchTest extends JettyServlet3Test { diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TestServlet3.groovy b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TestServlet3.groovy index 71d9ba8c7f74..17bbdb1936da 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TestServlet3.groovy +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TestServlet3.groovy @@ -5,7 +5,6 @@ import groovy.servlet.AbstractHttpServlet import io.opentelemetry.instrumentation.test.base.HttpServerTest - import javax.servlet.RequestDispatcher import javax.servlet.ServletException import javax.servlet.annotation.WebServlet @@ -72,6 +71,9 @@ class TestServlet3 { HttpServerTest.ServerEndpoint endpoint = HttpServerTest.ServerEndpoint.forPath(req.servletPath) def latch = new CountDownLatch(1) def context = req.startAsync() + if (endpoint == EXCEPTION) { + context.setTimeout(5000) + } context.start { try { HttpServerTest.controller(endpoint) { @@ -111,8 +113,7 @@ class TestServlet3 { case EXCEPTION: resp.status = endpoint.status resp.writer.print(endpoint.body) - context.complete() - throw new Exception(endpoint.body) + throw new ServletException(endpoint.body) } } } finally { @@ -157,7 +158,9 @@ class TestServlet3 { resp.sendError(endpoint.status, endpoint.body) break case EXCEPTION: - throw new Exception(endpoint.body) + resp.status = endpoint.status + resp.writer.print(endpoint.body) + throw new ServletException(endpoint.body) } } } finally { diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TomcatServlet3Test.groovy b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TomcatServlet3Test.groovy index 9d5e8756a495..a1ba6f1eb10f 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TomcatServlet3Test.groovy +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TomcatServlet3Test.groovy @@ -319,12 +319,6 @@ class TomcatServlet3TestAsync extends TomcatServlet3Test { boolean errorEndpointUsesSendError() { false } - - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } } class TomcatServlet3TestFakeAsync extends TomcatServlet3Test { @@ -333,12 +327,6 @@ class TomcatServlet3TestFakeAsync extends TomcatServlet3Test { Class servlet() { TestServlet3.FakeAsync } - - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } } class TomcatServlet3TestForward extends TomcatDispatchTest { @@ -459,12 +447,6 @@ class TomcatServlet3TestDispatchAsync extends TomcatDispatchTest { boolean errorEndpointUsesSendError() { false } - - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } } abstract class TomcatDispatchTest extends TomcatServlet3Test { 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 68630df17fdc..3fbba69b2496 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.AsyncContextStartInstrumentation; 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; @@ -28,6 +29,8 @@ public List typeInstrumentations() { return Arrays.asList( new AsyncContextInstrumentation( BASE_PACKAGE, adviceClassName(".async.AsyncDispatchAdvice")), + new AsyncContextStartInstrumentation( + BASE_PACKAGE, adviceClassName(".async.AsyncContextStartAdvice")), new AsyncStartInstrumentation(BASE_PACKAGE, adviceClassName(".async.AsyncStartAdvice")), new ServletAndFilterInstrumentation( BASE_PACKAGE, diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/async/AsyncContextStartAdvice.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/async/AsyncContextStartAdvice.java new file mode 100644 index 000000000000..dbde8946adcd --- /dev/null +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/async/AsyncContextStartAdvice.java @@ -0,0 +1,27 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.async; + +import static io.opentelemetry.javaagent.instrumentation.servlet.v5_0.Servlet5Singletons.helper; + +import jakarta.servlet.AsyncContext; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.http.HttpServletRequest; +import net.bytebuddy.asm.Advice; + +@SuppressWarnings("unused") +public class AsyncContextStartAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void start( + @Advice.This AsyncContext asyncContext, + @Advice.Argument(value = 0, readOnly = false) Runnable runnable) { + ServletRequest request = asyncContext.getRequest(); + if (request instanceof HttpServletRequest) { + runnable = helper().wrapAsyncRunnable((HttpServletRequest) request, runnable); + } + } +} diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/JettyServlet5Test.groovy b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/JettyServlet5Test.groovy index c9399a76bda8..9242348fe12f 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/JettyServlet5Test.groovy +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/JettyServlet5Test.groovy @@ -119,12 +119,6 @@ class JettyServlet5TestAsync extends JettyServlet5Test { boolean errorEndpointUsesSendError() { false } - - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } } @IgnoreIf({ !jvm.java11Compatible }) @@ -134,12 +128,6 @@ class JettyServlet5TestFakeAsync extends JettyServlet5Test { Class servlet() { TestServlet5.FakeAsync } - - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } } @IgnoreIf({ !jvm.java11Compatible }) @@ -222,12 +210,6 @@ class JettyServlet5TestDispatchImmediate extends JettyDispatchTest { addServlet(context, "/dispatch" + INDEXED_CHILD.path, TestServlet5.DispatchImmediate) addServlet(context, "/dispatch/recursive", TestServlet5.DispatchRecursive) } - - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } } @IgnoreIf({ !jvm.java11Compatible }) @@ -256,12 +238,6 @@ class JettyServlet5TestDispatchAsync extends JettyDispatchTest { boolean errorEndpointUsesSendError() { false } - - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } } abstract class JettyDispatchTest extends JettyServlet5Test { diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TestServlet5.groovy b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TestServlet5.groovy index 44a2b75484d9..93ac9ef48a8a 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TestServlet5.groovy +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TestServlet5.groovy @@ -72,6 +72,9 @@ class TestServlet5 { HttpServerTest.ServerEndpoint endpoint = HttpServerTest.ServerEndpoint.forPath(req.servletPath) def latch = new CountDownLatch(1) def context = req.startAsync() + if (endpoint == EXCEPTION) { + context.setTimeout(5000) + } context.start { try { HttpServerTest.controller(endpoint) { @@ -111,8 +114,7 @@ class TestServlet5 { case EXCEPTION: resp.status = endpoint.status resp.writer.print(endpoint.body) - context.complete() - throw new Exception(endpoint.body) + throw new ServletException(endpoint.body) } } } finally { @@ -157,7 +159,9 @@ class TestServlet5 { resp.sendError(endpoint.status, endpoint.body) break case EXCEPTION: - throw new Exception(endpoint.body) + resp.status = endpoint.status + resp.writer.print(endpoint.body) + throw new ServletException(endpoint.body) } } } finally { diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TomcatServlet5Test.groovy b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TomcatServlet5Test.groovy index ee9bcbb5f6ba..0d673603dd69 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TomcatServlet5Test.groovy +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TomcatServlet5Test.groovy @@ -318,12 +318,6 @@ class TomcatServlet5TestAsync extends TomcatServlet5Test { boolean errorEndpointUsesSendError() { false } - - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } } class TomcatServlet5TestFakeAsync extends TomcatServlet5Test { @@ -332,12 +326,6 @@ class TomcatServlet5TestFakeAsync extends TomcatServlet5Test { Class servlet() { TestServlet5.FakeAsync } - - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } } class TomcatServlet5TestForward extends TomcatDispatchTest { @@ -458,12 +446,6 @@ class TomcatServlet5TestDispatchAsync extends TomcatDispatchTest { boolean errorEndpointUsesSendError() { false } - - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } } abstract class TomcatDispatchTest extends TomcatServlet5Test { diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/AsyncRequestCompletionListener.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/AsyncRequestCompletionListener.java index a1d3ac3839f5..fb739a8b6e5f 100644 --- a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/AsyncRequestCompletionListener.java +++ b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/AsyncRequestCompletionListener.java @@ -35,7 +35,8 @@ public void onComplete(RESPONSE response) { if (responseHandled.compareAndSet(false, true)) { ServletResponseContext responseContext = new ServletResponseContext<>(response, null); - instrumenter.end(context, requestContext, responseContext, null); + Throwable throwable = servletHelper.getAsyncException(requestContext.request()); + instrumenter.end(context, requestContext, responseContext, throwable); } } @@ -46,7 +47,8 @@ public void onTimeout(long timeout) { ServletResponseContext responseContext = new ServletResponseContext<>(response, null); responseContext.setTimeout(timeout); - instrumenter.end(context, requestContext, responseContext, null); + Throwable throwable = servletHelper.getAsyncException(requestContext.request()); + instrumenter.end(context, requestContext, responseContext, throwable); } } diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/AsyncRunnableWrapper.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/AsyncRunnableWrapper.java new file mode 100644 index 000000000000..41899d94ddf1 --- /dev/null +++ b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/AsyncRunnableWrapper.java @@ -0,0 +1,37 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet; + +public class AsyncRunnableWrapper implements Runnable { + private final ServletHelper helper; + private final REQUEST request; + private final Runnable runnable; + + private AsyncRunnableWrapper( + ServletHelper helper, REQUEST request, Runnable runnable) { + this.helper = helper; + this.request = request; + this.runnable = runnable; + } + + public static Runnable wrap( + ServletHelper helper, REQUEST request, Runnable runnable) { + if (runnable == null || runnable instanceof AsyncRunnableWrapper) { + return runnable; + } + return new AsyncRunnableWrapper(helper, request, runnable); + } + + @Override + public void run() { + try { + runnable.run(); + } catch (Throwable throwable) { + helper.recordAsyncException(request, throwable); + throw throwable; + } + } +} diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletHelper.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletHelper.java index ae37476ebdb8..5b0b96be00e0 100644 --- a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletHelper.java +++ b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletHelper.java @@ -15,6 +15,8 @@ public class ServletHelper extends BaseServletHelper requestContext) public boolean isAsyncListenerAttached(REQUEST request) { return accessor.getRequestAttribute(request, ASYNC_LISTENER_ATTRIBUTE) != null; } + + public Runnable wrapAsyncRunnable(REQUEST request, Runnable runnable) { + return AsyncRunnableWrapper.wrap(this, request, runnable); + } + + public void recordAsyncException(REQUEST request, Throwable throwable) { + accessor.setRequestAttribute(request, ASYNC_EXCEPTION_ATTRIBUTE, throwable); + } + + public Throwable getAsyncException(REQUEST request) { + return (Throwable) accessor.getRequestAttribute(request, ASYNC_EXCEPTION_ATTRIBUTE); + } } diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/async/AsyncContextStartInstrumentation.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/async/AsyncContextStartInstrumentation.java new file mode 100644 index 000000000000..496079e290fa --- /dev/null +++ b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/async/AsyncContextStartInstrumentation.java @@ -0,0 +1,43 @@ +/* + * 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.hasClassesNamed; +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +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 AsyncContextStartInstrumentation implements TypeInstrumentation { + private final String basePackageName; + private final String adviceClassName; + + public AsyncContextStartInstrumentation(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 + ".AsyncContext")); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + named("start").and(takesArguments(Runnable.class)).and(isPublic()), adviceClassName); + } +} diff --git a/instrumentation/tomcat/tomcat-10.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/AsyncServlet.groovy b/instrumentation/tomcat/tomcat-10.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/AsyncServlet.groovy index a0b395834d06..5b8c10b46543 100644 --- a/instrumentation/tomcat/tomcat-10.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/AsyncServlet.groovy +++ b/instrumentation/tomcat/tomcat-10.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/AsyncServlet.groovy @@ -6,6 +6,7 @@ package io.opentelemetry.javaagent.instrumentation.tomcat.v10_0 import io.opentelemetry.instrumentation.test.base.HttpServerTest +import jakarta.servlet.ServletException import jakarta.servlet.annotation.WebServlet import jakarta.servlet.http.HttpServlet import jakarta.servlet.http.HttpServletRequest @@ -28,6 +29,9 @@ class AsyncServlet extends HttpServlet { HttpServerTest.ServerEndpoint endpoint = HttpServerTest.ServerEndpoint.forPath(req.servletPath) def latch = new CountDownLatch(1) def context = req.startAsync() + if (endpoint == EXCEPTION) { + context.setTimeout(5000) + } context.start { try { HttpServerTest.controller(endpoint) { @@ -36,41 +40,36 @@ class AsyncServlet extends HttpServlet { case SUCCESS: resp.status = endpoint.status resp.writer.print(endpoint.body) - context.complete() break case INDEXED_CHILD: endpoint.collectSpanAttributes { req.getParameter(it) } resp.status = endpoint.status - context.complete() break case QUERY_PARAM: resp.status = endpoint.status resp.writer.print(req.queryString) - context.complete() break case REDIRECT: resp.sendRedirect(endpoint.body) - context.complete() break case CAPTURE_HEADERS: resp.setHeader("X-Test-Response", req.getHeader("X-Test-Request")) resp.status = endpoint.status resp.writer.print(endpoint.body) - context.complete() break case ERROR: resp.status = endpoint.status resp.writer.print(endpoint.body) - context.complete() break case EXCEPTION: - resp.status = endpoint.status - resp.writer.print(endpoint.body) - context.complete() - throw new Exception(endpoint.body) + throw new ServletException(endpoint.body) } } } finally { + // complete at the end so the server span will end after the controller span + if (endpoint != EXCEPTION) { + context.complete() + } latch.countDown() } } diff --git a/instrumentation/tomcat/tomcat-10.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/TomcatAsyncTest.groovy b/instrumentation/tomcat/tomcat-10.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/TomcatAsyncTest.groovy index dfe113c52860..061339682624 100644 --- a/instrumentation/tomcat/tomcat-10.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/TomcatAsyncTest.groovy +++ b/instrumentation/tomcat/tomcat-10.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/TomcatAsyncTest.groovy @@ -108,12 +108,6 @@ class TomcatAsyncTest extends HttpServerTest implements AgentTestTrait { ] } - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } - @Override Class expectedExceptionClass() { ServletException diff --git a/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/AsyncServlet.groovy b/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/AsyncServlet.groovy index ee4dcf5ec6b2..03a920be4265 100644 --- a/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/AsyncServlet.groovy +++ b/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/AsyncServlet.groovy @@ -7,7 +7,8 @@ package io.opentelemetry.javaagent.instrumentation.tomcat.v7_0 import groovy.servlet.AbstractHttpServlet import io.opentelemetry.instrumentation.test.base.HttpServerTest - +import java.util.concurrent.CountDownLatch +import javax.servlet.ServletException import javax.servlet.annotation.WebServlet import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse @@ -25,7 +26,11 @@ class AsyncServlet extends AbstractHttpServlet { @Override protected void service(HttpServletRequest req, HttpServletResponse resp) { HttpServerTest.ServerEndpoint endpoint = HttpServerTest.ServerEndpoint.forPath(req.servletPath) + def latch = new CountDownLatch(1) def context = req.startAsync() + if (endpoint == EXCEPTION) { + context.setTimeout(5000) + } context.start { try { HttpServerTest.controller(endpoint) { @@ -58,13 +63,17 @@ class AsyncServlet extends AbstractHttpServlet { case EXCEPTION: resp.status = endpoint.status resp.writer.print(endpoint.body) - throw new Exception(endpoint.body) + throw new ServletException(endpoint.body) } } } finally { // complete at the end so the server span will end after the controller span - context.complete() + if (endpoint != EXCEPTION) { + context.complete() + } + latch.countDown() } } + latch.await() } } diff --git a/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/TomcatAsyncTest.groovy b/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/TomcatAsyncTest.groovy index 3d5b3009b9a4..6ff261d90e22 100644 --- a/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/TomcatAsyncTest.groovy +++ b/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/TomcatAsyncTest.groovy @@ -108,12 +108,6 @@ class TomcatAsyncTest extends HttpServerTest implements AgentTestTrait { ] } - @Override - boolean testException() { - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 - return false - } - @Override Class expectedExceptionClass() { ServletException