Enable servlet forward and include tests
MODIFY
instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServlet3Test.groovy
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<Server, ServletCon
false
}
+ @Override
+ Class<?> 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> 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> servlet() {
+ TestServlet3.Sync // dispatch to sync servlet
+ }
-// FIXME: not working right now...
-//class JettyServlet3TestInclude extends JettyDispatchTest {
-// @Override
-// Class<Servlet> 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> 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 {

MODIFY
instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServletHandlerTest.groovy
instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServletHandlerTest.groovy
*/
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<Server, ServletHandle
boolean testNotFound() {
false
}
+
+ @Override
+ Class<?> expectedExceptionClass() {
+ ServletException
+ }
}

MODIFY
instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TestServlet3.groovy
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)
}
}
}

MODIFY
instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TomcatServlet3Test.groovy
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<Tomcat, Context> {
+ @Override
+ boolean testExceptionBody() {
+ return false
+ }
+
+ @Override
+ Class<?> expectedExceptionClass() {
+ ServletException
+ }
+
@Shared
def accessLogValue = new TestAccessLogValve()
@@ -130,6 +141,7 @@ abstract class TomcatServlet3Test extends AbstractServlet3Test<Tomcat, Context>
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> 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> servlet() {
+ TestServlet3.Sync // dispatch to sync servlet
+ }
-// FIXME: not working right now...
-//class TomcatServlet3TestInclude extends TomcatDispatchTest {
-// @Override
-// Class<Servlet> 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> 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

MODIFY
testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy
testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy
@@ -132,6 +132,18 @@ abstract class HttpServerTest<SERVER> 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<SERVER> 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<SERVER> 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<SERVER> 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<SERVER> 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<SERVER> 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 }
}
}

Enable servlet forward and include tests (#1968)