Enable servlet forward and include tests (#1968)

This commit is contained in:
Lauri Tulmin 2021-01-06 21:01:47 +02:00 committed by GitHub
parent 589732bef6
commit 5f816c5d43
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 154 additions and 104 deletions

View File

@ -11,6 +11,7 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS
import javax.servlet.Servlet import javax.servlet.Servlet
import javax.servlet.ServletException
import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletRequest
import org.eclipse.jetty.server.Server import org.eclipse.jetty.server.Server
import org.eclipse.jetty.server.handler.ErrorHandler import org.eclipse.jetty.server.handler.ErrorHandler
@ -23,6 +24,11 @@ abstract class JettyServlet3Test extends AbstractServlet3Test<Server, ServletCon
false false
} }
@Override
Class<?> expectedExceptionClass() {
ServletException
}
@Override @Override
Server startServer(int port) { Server startServer(int port) {
def jettyServer = new Server(port) def jettyServer = new Server(port)
@ -123,55 +129,53 @@ class JettyServlet3TestFakeAsync extends JettyServlet3Test {
} }
} }
// FIXME: not working right now... class JettyServlet3TestForward extends JettyDispatchTest {
//class JettyServlet3TestForward extends JettyDispatchTest { @Override
// @Override Class<Servlet> servlet() {
// Class<Servlet> servlet() { TestServlet3.Sync // dispatch to sync 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)
// }
//}
// FIXME: not working right now... @Override
//class JettyServlet3TestInclude extends JettyDispatchTest { protected void setupServlets(ServletContextHandler context) {
// @Override super.setupServlets(context)
// Class<Servlet> servlet() {
// TestServlet3.Sync // dispatch to sync servlet addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Forward)
// } addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Forward)
// addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Forward)
// @Override addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Forward)
// boolean testNotFound() { addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Forward)
// false addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Forward)
// } }
// }
// @Override
// protected void setupServlets(ServletContextHandler context) { class JettyServlet3TestInclude extends JettyDispatchTest {
// super.setupServlets(context) @Override
// Class<Servlet> servlet() {
// addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Include) TestServlet3.Sync // dispatch to sync servlet
// addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Include) }
// addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Include)
// addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Include) @Override
// addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Include) boolean testRedirect() {
// addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Include) 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 { class JettyServlet3TestDispatchImmediate extends JettyDispatchTest {

View File

@ -4,6 +4,7 @@
*/ */
import javax.servlet.Servlet import javax.servlet.Servlet
import javax.servlet.ServletException
import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletRequest
import org.eclipse.jetty.server.Server import org.eclipse.jetty.server.Server
import org.eclipse.jetty.server.handler.ErrorHandler import org.eclipse.jetty.server.handler.ErrorHandler
@ -52,4 +53,9 @@ class JettyServletHandlerTest extends AbstractServlet3Test<Server, ServletHandle
boolean testNotFound() { boolean testNotFound() {
false false
} }
@Override
Class<?> expectedExceptionClass() {
ServletException
}
} }

View File

@ -12,6 +12,8 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
import groovy.servlet.AbstractHttpServlet import groovy.servlet.AbstractHttpServlet
import io.opentelemetry.instrumentation.test.base.HttpServerTest import io.opentelemetry.instrumentation.test.base.HttpServerTest
import java.util.concurrent.Phaser import java.util.concurrent.Phaser
import javax.servlet.RequestDispatcher
import javax.servlet.ServletException
import javax.servlet.annotation.WebServlet import javax.servlet.annotation.WebServlet
import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletRequest
import javax.servlet.http.HttpServletResponse import javax.servlet.http.HttpServletResponse
@ -22,7 +24,11 @@ class TestServlet3 {
static class Sync extends AbstractHttpServlet { static class Sync extends AbstractHttpServlet {
@Override @Override
protected void service(HttpServletRequest req, HttpServletResponse resp) { 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) { HttpServerTest.controller(endpoint) {
resp.contentType = "text/plain" resp.contentType = "text/plain"
switch (endpoint) { switch (endpoint) {
@ -41,7 +47,7 @@ class TestServlet3 {
resp.sendError(endpoint.status, endpoint.body) resp.sendError(endpoint.status, endpoint.body)
break break
case EXCEPTION: case EXCEPTION:
throw new Exception(endpoint.body) throw new ServletException(endpoint.body)
} }
} }
} }

View File

@ -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.QUERY_PARAM
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS
import static org.junit.Assume.assumeTrue
import com.google.common.io.Files import com.google.common.io.Files
import javax.servlet.Servlet import javax.servlet.Servlet
@ -29,6 +30,16 @@ import spock.lang.Unroll
@Unroll @Unroll
abstract class TomcatServlet3Test extends AbstractServlet3Test<Tomcat, Context> { abstract class TomcatServlet3Test extends AbstractServlet3Test<Tomcat, Context> {
@Override
boolean testExceptionBody() {
return false
}
@Override
Class<?> expectedExceptionClass() {
ServletException
}
@Shared @Shared
def accessLogValue = new TestAccessLogValve() def accessLogValue = new TestAccessLogValve()
@ -130,6 +141,7 @@ abstract class TomcatServlet3Test extends AbstractServlet3Test<Tomcat, Context>
def "access log has ids for error request"() { def "access log has ids for error request"() {
setup: setup:
assumeTrue(testError())
def request = request(ERROR, method, body).build() def request = request(ERROR, method, body).build()
def response = client.newCall(request).execute() def response = client.newCall(request).execute()
@ -265,55 +277,63 @@ class TomcatServlet3TestFakeAsync extends TomcatServlet3Test {
} }
} }
// FIXME: not working right now... class TomcatServlet3TestForward extends TomcatDispatchTest {
//class TomcatServlet3TestForward extends TomcatDispatchTest { @Override
// @Override Class<Servlet> servlet() {
// Class<Servlet> servlet() { TestServlet3.Sync // dispatch to sync 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)
// }
//}
// FIXME: not working right now... @Override
//class TomcatServlet3TestInclude extends TomcatDispatchTest { boolean testNotFound() {
// @Override false
// Class<Servlet> servlet() { }
// TestServlet3.Sync // dispatch to sync servlet
// } @Override
// protected void setupServlets(Context context) {
// @Override super.setupServlets(context)
// boolean testNotFound() {
// false addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Forward)
// } addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Forward)
// addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Forward)
// @Override addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Forward)
// protected void setupServlets(Context context) { addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Forward)
// super.setupServlets(context) addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Forward)
// }
// addServlet(context, "/dispatch" + SUCCESS.path, RequestDispatcherServlet.Include) }
// addServlet(context, "/dispatch" + QUERY_PARAM.path, RequestDispatcherServlet.Include)
// addServlet(context, "/dispatch" + REDIRECT.path, RequestDispatcherServlet.Include) class TomcatServlet3TestInclude extends TomcatDispatchTest {
// addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Include) @Override
// addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Include) Class<Servlet> servlet() {
// addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Include) 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 { class TomcatServlet3TestDispatchImmediate extends TomcatDispatchTest {
@Override @Override

View File

@ -132,6 +132,18 @@ abstract class HttpServerTest<SERVER> extends AgentTestRunner {
true true
} }
Class<?> expectedExceptionClass() {
Exception
}
boolean testRedirect() {
true
}
boolean testError() {
true
}
enum ServerEndpoint { enum ServerEndpoint {
SUCCESS("success", 200, "success"), SUCCESS("success", 200, "success"),
REDIRECT("redirect", 302, "/redirected"), REDIRECT("redirect", 302, "/redirected"),
@ -282,6 +294,7 @@ abstract class HttpServerTest<SERVER> extends AgentTestRunner {
def "test redirect"() { def "test redirect"() {
setup: setup:
assumeTrue(testRedirect())
def request = request(REDIRECT, method, body).build() def request = request(REDIRECT, method, body).build()
def response = client.newCall(request).execute() def response = client.newCall(request).execute()
@ -301,6 +314,7 @@ abstract class HttpServerTest<SERVER> extends AgentTestRunner {
def "test error"() { def "test error"() {
setup: setup:
assumeTrue(testError())
def request = request(ERROR, method, body).build() def request = request(ERROR, method, body).build()
def response = client.newCall(request).execute() def response = client.newCall(request).execute()
@ -402,9 +416,9 @@ abstract class HttpServerTest<SERVER> extends AgentTestRunner {
} }
if (endpoint != NOT_FOUND) { if (endpoint != NOT_FOUND) {
if (hasHandlerSpan()) { if (hasHandlerSpan()) {
controllerSpan(it, spanIndex++, span(1), errorMessage) controllerSpan(it, spanIndex++, span(1), errorMessage, expectedExceptionClass())
} else { } else {
controllerSpan(it, spanIndex++, span(0), errorMessage) controllerSpan(it, spanIndex++, span(0), errorMessage, expectedExceptionClass())
} }
if (hasRenderSpan(endpoint)) { if (hasRenderSpan(endpoint)) {
renderSpan(it, spanIndex++, span(0), method, 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) { trace.span(index) {
name "controller" name "controller"
errored errorMessage != null errored errorMessage != null
if (errorMessage) { if (errorMessage) {
errorEvent(Exception, errorMessage) errorEvent(exceptionClass, errorMessage)
} }
childOf((SpanData) parent) childOf((SpanData) parent)
} }
@ -465,8 +479,8 @@ abstract class HttpServerTest<SERVER> extends AgentTestRunner {
event(0) { event(0) {
eventName(SemanticAttributes.EXCEPTION_EVENT_NAME) eventName(SemanticAttributes.EXCEPTION_EVENT_NAME)
attributes { attributes {
"${SemanticAttributes.EXCEPTION_TYPE.key}" { it == null || it == Exception.name } "${SemanticAttributes.EXCEPTION_TYPE.key}" { it == null || it == expectedExceptionClass().name }
"${SemanticAttributes.EXCEPTION_MESSAGE.key}" { it == null || it == EXCEPTION.body } "${SemanticAttributes.EXCEPTION_MESSAGE.key}" { it == null || it == endpoint.body }
"${SemanticAttributes.EXCEPTION_STACKTRACE.key}" { it == null || it instanceof String } "${SemanticAttributes.EXCEPTION_STACKTRACE.key}" { it == null || it instanceof String }
} }
} }