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 a1268d659c..3ae41bff2a 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 @@ -11,6 +11,7 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS import javax.servlet.Servlet +import javax.servlet.ServletException import javax.servlet.http.HttpServletRequest import org.eclipse.jetty.server.Server import org.eclipse.jetty.server.handler.ErrorHandler @@ -23,6 +24,11 @@ abstract class JettyServlet3Test extends AbstractServlet3Test expectedExceptionClass() { + ServletException + } + @Override Server startServer(int port) { def jettyServer = new Server(port) @@ -123,55 +129,53 @@ class JettyServlet3TestFakeAsync extends JettyServlet3Test { } } -// FIXME: not working right now... -//class JettyServlet3TestForward extends JettyDispatchTest { -// @Override -// Class servlet() { -// TestServlet3.Sync // dispatch to sync servlet -// } -// -// @Override -// boolean testNotFound() { -// false -// } -// -// @Override -// protected void setupServlets(ServletContextHandler context) { -// super.setupServlets(context) -// -// addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Forward) -// addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Forward) -// addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Forward) -// addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Forward) -// addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Forward) -// addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Forward) -// } -//} +class JettyServlet3TestForward extends JettyDispatchTest { + @Override + Class servlet() { + TestServlet3.Sync // dispatch to sync servlet + } -// FIXME: not working right now... -//class JettyServlet3TestInclude extends JettyDispatchTest { -// @Override -// Class servlet() { -// TestServlet3.Sync // dispatch to sync servlet -// } -// -// @Override -// boolean testNotFound() { -// false -// } -// -// @Override -// protected void setupServlets(ServletContextHandler context) { -// super.setupServlets(context) -// -// addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Include) -// addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Include) -// addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Include) -// addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Include) -// addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Include) -// addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Include) -// } -//} + @Override + protected void setupServlets(ServletContextHandler context) { + super.setupServlets(context) + + addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Forward) + addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Forward) + addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Forward) + addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Forward) + addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Forward) + addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Forward) + } +} + +class JettyServlet3TestInclude extends JettyDispatchTest { + @Override + Class servlet() { + TestServlet3.Sync // dispatch to sync servlet + } + + @Override + boolean testRedirect() { + false + } + + @Override + boolean testError() { + false + } + + @Override + protected void setupServlets(ServletContextHandler context) { + super.setupServlets(context) + + addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Include) + addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Include) + addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Include) + addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Include) + addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Include) + addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Include) + } +} class JettyServlet3TestDispatchImmediate extends JettyDispatchTest { diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServletHandlerTest.groovy b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServletHandlerTest.groovy index a7b47ebf9e..57baf71664 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServletHandlerTest.groovy +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServletHandlerTest.groovy @@ -4,6 +4,7 @@ */ import javax.servlet.Servlet +import javax.servlet.ServletException import javax.servlet.http.HttpServletRequest import org.eclipse.jetty.server.Server import org.eclipse.jetty.server.handler.ErrorHandler @@ -52,4 +53,9 @@ class JettyServletHandlerTest extends AbstractServlet3Test expectedExceptionClass() { + ServletException + } } 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 f4acfa23bd..c02c308563 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 @@ -12,6 +12,8 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn import groovy.servlet.AbstractHttpServlet import io.opentelemetry.instrumentation.test.base.HttpServerTest import java.util.concurrent.Phaser +import javax.servlet.RequestDispatcher +import javax.servlet.ServletException import javax.servlet.annotation.WebServlet import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse @@ -22,7 +24,11 @@ class TestServlet3 { static class Sync extends AbstractHttpServlet { @Override protected void service(HttpServletRequest req, HttpServletResponse resp) { - HttpServerTest.ServerEndpoint endpoint = HttpServerTest.ServerEndpoint.forPath(req.servletPath) + String servletPath = req.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH) + if (servletPath == null) { + servletPath = req.servletPath + } + HttpServerTest.ServerEndpoint endpoint = HttpServerTest.ServerEndpoint.forPath(servletPath) HttpServerTest.controller(endpoint) { resp.contentType = "text/plain" switch (endpoint) { @@ -41,7 +47,7 @@ class TestServlet3 { resp.sendError(endpoint.status, endpoint.body) break case EXCEPTION: - throw new Exception(endpoint.body) + throw new ServletException(endpoint.body) } } } 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 54a4daac32..7622ea279e 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 @@ -9,6 +9,7 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS +import static org.junit.Assume.assumeTrue import com.google.common.io.Files import javax.servlet.Servlet @@ -29,6 +30,16 @@ import spock.lang.Unroll @Unroll abstract class TomcatServlet3Test extends AbstractServlet3Test { + @Override + boolean testExceptionBody() { + return false + } + + @Override + Class expectedExceptionClass() { + ServletException + } + @Shared def accessLogValue = new TestAccessLogValve() @@ -130,6 +141,7 @@ abstract class TomcatServlet3Test extends AbstractServlet3Test def "access log has ids for error request"() { setup: + assumeTrue(testError()) def request = request(ERROR, method, body).build() def response = client.newCall(request).execute() @@ -265,55 +277,63 @@ class TomcatServlet3TestFakeAsync extends TomcatServlet3Test { } } -// FIXME: not working right now... -//class TomcatServlet3TestForward extends TomcatDispatchTest { -// @Override -// Class servlet() { -// TestServlet3.Sync // dispatch to sync servlet -// } -// -// @Override -// boolean testNotFound() { -// false -// } -// -// @Override -// protected void setupServlets(Context context) { -// super.setupServlets(context) -// -// addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Forward) -// addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Forward) -// addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Forward) -// addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Forward) -// addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Forward) -// addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Forward) -// } -//} +class TomcatServlet3TestForward extends TomcatDispatchTest { + @Override + Class servlet() { + TestServlet3.Sync // dispatch to sync servlet + } -// FIXME: not working right now... -//class TomcatServlet3TestInclude extends TomcatDispatchTest { -// @Override -// Class servlet() { -// TestServlet3.Sync // dispatch to sync servlet -// } -// -// @Override -// boolean testNotFound() { -// false -// } -// -// @Override -// protected void setupServlets(Context context) { -// super.setupServlets(context) -// -// addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Include) -// addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Include) -// addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Include) -// addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Include) -// addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Include) -// addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Include) -// } -//} + @Override + boolean testNotFound() { + false + } + + @Override + protected void setupServlets(Context context) { + super.setupServlets(context) + + addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Forward) + addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Forward) + addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Forward) + addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Forward) + addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Forward) + addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Forward) + } +} + +class TomcatServlet3TestInclude extends TomcatDispatchTest { + @Override + Class servlet() { + TestServlet3.Sync // dispatch to sync servlet + } + + @Override + boolean testNotFound() { + false + } + + @Override + boolean testRedirect() { + false + } + + @Override + boolean testError() { + false + } + + @Override + protected void setupServlets(Context context) { + super.setupServlets(context) + + addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Include) + addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Include) + addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Include) + addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Include) + addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Include) + addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Include) + } +} class TomcatServlet3TestDispatchImmediate extends TomcatDispatchTest { @Override diff --git a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy index 767cf407ec..e7d9d3c9eb 100644 --- a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy +++ b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy @@ -132,6 +132,18 @@ abstract class HttpServerTest extends AgentTestRunner { true } + Class expectedExceptionClass() { + Exception + } + + boolean testRedirect() { + true + } + + boolean testError() { + true + } + enum ServerEndpoint { SUCCESS("success", 200, "success"), REDIRECT("redirect", 302, "/redirected"), @@ -282,6 +294,7 @@ abstract class HttpServerTest extends AgentTestRunner { def "test redirect"() { setup: + assumeTrue(testRedirect()) def request = request(REDIRECT, method, body).build() def response = client.newCall(request).execute() @@ -301,6 +314,7 @@ abstract class HttpServerTest extends AgentTestRunner { def "test error"() { setup: + assumeTrue(testError()) def request = request(ERROR, method, body).build() def response = client.newCall(request).execute() @@ -402,9 +416,9 @@ abstract class HttpServerTest extends AgentTestRunner { } if (endpoint != NOT_FOUND) { if (hasHandlerSpan()) { - controllerSpan(it, spanIndex++, span(1), errorMessage) + controllerSpan(it, spanIndex++, span(1), errorMessage, expectedExceptionClass()) } else { - controllerSpan(it, spanIndex++, span(0), errorMessage) + controllerSpan(it, spanIndex++, span(0), errorMessage, expectedExceptionClass()) } if (hasRenderSpan(endpoint)) { renderSpan(it, spanIndex++, span(0), method, endpoint) @@ -422,12 +436,12 @@ abstract class HttpServerTest extends AgentTestRunner { } } - void controllerSpan(TraceAssert trace, int index, Object parent, String errorMessage = null) { + void controllerSpan(TraceAssert trace, int index, Object parent, String errorMessage = null, Class exceptionClass = Exception) { trace.span(index) { name "controller" errored errorMessage != null if (errorMessage) { - errorEvent(Exception, errorMessage) + errorEvent(exceptionClass, errorMessage) } childOf((SpanData) parent) } @@ -465,8 +479,8 @@ abstract class HttpServerTest extends AgentTestRunner { event(0) { eventName(SemanticAttributes.EXCEPTION_EVENT_NAME) attributes { - "${SemanticAttributes.EXCEPTION_TYPE.key}" { it == null || it == Exception.name } - "${SemanticAttributes.EXCEPTION_MESSAGE.key}" { it == null || it == EXCEPTION.body } + "${SemanticAttributes.EXCEPTION_TYPE.key}" { it == null || it == expectedExceptionClass().name } + "${SemanticAttributes.EXCEPTION_MESSAGE.key}" { it == null || it == endpoint.body } "${SemanticAttributes.EXCEPTION_STACKTRACE.key}" { it == null || it instanceof String } } }