Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Record exception for async servlet invocations #4677

Merged
merged 3 commits into from
Nov 22, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
@@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -34,12 +35,14 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
public List<TypeInstrumentation> 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,13 @@ abstract class JettyServlet3Test extends AbstractServlet3Test<Server, ServletCon
ServletException
}

boolean isAsyncTest() {
false
}

@Override
boolean hasResponseSpan(ServerEndpoint endpoint) {
return (IS_BEFORE_94 && endpoint == EXCEPTION) || super.hasResponseSpan(endpoint)
return (IS_BEFORE_94 && endpoint == EXCEPTION && !isAsyncTest()) || super.hasResponseSpan(endpoint)
}

@Override
Expand Down Expand Up @@ -140,9 +144,8 @@ class JettyServlet3TestAsync extends JettyServlet3Test {
}

@Override
boolean testException() {
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807
return false
boolean isAsyncTest() {
true
}
}

Expand All @@ -152,12 +155,6 @@ class JettyServlet3TestFakeAsync extends JettyServlet3Test {
Class<Servlet> servlet() {
TestServlet3.FakeAsync
}

@Override
boolean testException() {
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807
return false
}
}

class JettyServlet3TestForward extends JettyDispatchTest {
Expand Down Expand Up @@ -223,6 +220,11 @@ class JettyServlet3TestDispatchImmediate extends JettyDispatchTest {
TestServlet3.Sync
}

@Override
boolean isAsyncTest() {
true
}

@Override
protected void setupServlets(ServletContextHandler context) {
super.setupServlets(context)
Expand All @@ -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 {
Expand All @@ -251,6 +247,11 @@ class JettyServlet3TestDispatchAsync extends JettyDispatchTest {
TestServlet3.Async
}

@Override
boolean isAsyncTest() {
true
}

@Override
protected void setupServlets(ServletContextHandler context) {
super.setupServlets(context)
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -333,12 +327,6 @@ class TomcatServlet3TestFakeAsync extends TomcatServlet3Test {
Class<Servlet> servlet() {
TestServlet3.FakeAsync
}

@Override
boolean testException() {
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807
return false
}
}

class TomcatServlet3TestForward extends TomcatDispatchTest {
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -28,6 +29,8 @@ public List<TypeInstrumentation> 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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Expand All @@ -134,12 +128,6 @@ class JettyServlet5TestFakeAsync extends JettyServlet5Test {
Class<Servlet> servlet() {
TestServlet5.FakeAsync
}

@Override
boolean testException() {
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807
return false
}
}

@IgnoreIf({ !jvm.java11Compatible })
Expand Down Expand Up @@ -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 })
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -332,12 +326,6 @@ class TomcatServlet5TestFakeAsync extends TomcatServlet5Test {
Class<Servlet> servlet() {
TestServlet5.FakeAsync
}

@Override
boolean testException() {
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807
return false
}
}

class TomcatServlet5TestForward extends TomcatDispatchTest {
Expand Down Expand Up @@ -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 {
Expand Down
Loading