diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/AppServerBridge.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/AppServerBridge.java index 3ac1096a54..34af542379 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/AppServerBridge.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/AppServerBridge.java @@ -8,7 +8,6 @@ package io.opentelemetry.instrumentation.api.servlet; import io.opentelemetry.context.Context; import io.opentelemetry.context.ContextKey; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; /** * Helper container for Context attributes for transferring certain information between servlet @@ -40,15 +39,7 @@ public class AppServerBridge { * @return new context with AppServerBridge attached. */ public static Context init(Context ctx, boolean shouldRecordException) { - Context context = - ctx.with(AppServerBridge.CONTEXT_KEY, new AppServerBridge(shouldRecordException)); - // Add context for storing servlet context path. Servlet context path is updated from servlet - // integration. - if (context.get(ServletContextPath.CONTEXT_KEY) == null) { - context = context.with(ServletContextPath.CONTEXT_KEY, new AtomicReference<>(null)); - } - - return context; + return ctx.with(AppServerBridge.CONTEXT_KEY, new AppServerBridge(shouldRecordException)); } private final AtomicBoolean servletUpdatedServerSpanName = new AtomicBoolean(false); diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServletContextPath.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServletContextPath.java index fcfcbafb89..d400349334 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServletContextPath.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServletContextPath.java @@ -7,7 +7,6 @@ package io.opentelemetry.instrumentation.api.servlet; import io.opentelemetry.context.Context; import io.opentelemetry.context.ContextKey; -import java.util.concurrent.atomic.AtomicReference; /** * The context key here is used to propagate the servlet context path throughout the request, so @@ -23,12 +22,11 @@ public class ServletContextPath { // Keeps track of the servlet context path that needs to be prepended to the route when updating // the span name - public static final ContextKey> CONTEXT_KEY = + public static final ContextKey CONTEXT_KEY = ContextKey.named("opentelemetry-servlet-context-path-key"); public static String prepend(Context context, String spanName) { - AtomicReference valueReference = context.get(CONTEXT_KEY); - String value = valueReference != null ? valueReference.get() : null; + String value = context.get(CONTEXT_KEY); // checking isEmpty just to avoid unnecessary string concat / allocation if (value != null && !value.isEmpty()) { return value + spanName; diff --git a/instrumentation-core/servlet-2.2/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java b/instrumentation-core/servlet-2.2/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java index ea89803fb4..03b960751d 100644 --- a/instrumentation-core/servlet-2.2/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java +++ b/instrumentation-core/servlet-2.2/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java @@ -9,13 +9,11 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.TextMapPropagator.Getter; import io.opentelemetry.instrumentation.api.servlet.AppServerBridge; -import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; import io.opentelemetry.instrumentation.api.tracer.HttpServerTracer; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.net.URI; import java.net.URISyntaxException; import java.security.Principal; -import java.util.concurrent.atomic.AtomicReference; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import org.slf4j.Logger; @@ -27,16 +25,7 @@ public abstract class ServletHttpServerTracer private static final Logger log = LoggerFactory.getLogger(ServletHttpServerTracer.class); public Context startSpan(HttpServletRequest request) { - Context context = startSpan(request, request, request, getSpanName(request)); - return addContextPathContext(context, request); - } - - protected Context addContextPathContext(Context context, HttpServletRequest request) { - String contextPath = request.getContextPath(); - if (contextPath != null && !contextPath.isEmpty() && !contextPath.equals("/")) { - context = context.with(ServletContextPath.CONTEXT_KEY, new AtomicReference<>(contextPath)); - } - return context; + return startSpan(request, request, request, getSpanName(request)); } @Override @@ -158,41 +147,19 @@ public abstract class ServletHttpServerTracer return contextPath + servletPath; } - /** - * When server spans are managed by app server instrumentation servlet instrumentation should set - * information that was not available from app server instrumentation. - */ - public void updateServerSpan(Context attachedContext, HttpServletRequest request) { - updateServerSpanName(attachedContext, request); - updateContextPath(attachedContext, request); - } - /** * When server spans are managed by app server instrumentation, servlet must update server span * name only once and only during the first pass through the servlet stack. There are potential * forward and other scenarios, where servlet path may change, but we don't want this to be * reflected in the span name. */ - private void updateServerSpanName(Context attachedContext, HttpServletRequest request) { - // Update name only when server span wasn't created by servlet integration - // and has not been already updated + public void updateServerSpanNameOnce(Context attachedContext, HttpServletRequest request) { if (AppServerBridge.shouldUpdateServerSpanName(attachedContext)) { updateSpanName(Span.fromContext(attachedContext), request); AppServerBridge.setServletUpdatedServerSpanName(attachedContext, true); } } - private void updateContextPath(Context attachedContext, HttpServletRequest request) { - // Update context path if it isn't already set - AtomicReference reference = attachedContext.get(ServletContextPath.CONTEXT_KEY); - if (reference != null && reference.get() == null) { - String contextPath = request.getContextPath(); - if (contextPath != null && !contextPath.isEmpty() && !contextPath.equals("/")) { - reference.compareAndSet(null, contextPath); - } - } - } - public void updateSpanName(HttpServletRequest request) { updateSpanName(getServerSpan(request), request); } diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/test/groovy/JerseyHttpServerTest.groovy b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/test/groovy/JerseyHttpServerTest.groovy index 79898e8cca..2a65c98306 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/test/groovy/JerseyHttpServerTest.groovy +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/test/groovy/JerseyHttpServerTest.groovy @@ -30,4 +30,9 @@ class JerseyHttpServerTest extends JaxRsHttpServerTest { void stopServer(Server httpServer) { httpServer.stop() } + + @Override + boolean asyncCancelHasSendError() { + true + } } \ No newline at end of file diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy index 4c9adb183f..3083ea41f7 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy @@ -49,10 +49,18 @@ abstract class JaxRsHttpServerTest extends HttpServerTest { assert response.code() == statusCode assert bodyPredicate(response.body().string()) + def spanCount = 2 + def hasSendError = asyncCancelHasSendError() && action == "cancel" + if (hasSendError) { + spanCount++ + } assertTraces(1) { - trace(0, 2) { + trace(0, spanCount) { asyncServerSpan(it, 0, url, statusCode) handlerSpan(it, 1, span(0), "asyncOp", isCancelled, isError, errorMessage) + if (hasSendError) { + sendErrorSpan(it, 2, span(1)) + } } } @@ -117,6 +125,10 @@ abstract class JaxRsHttpServerTest extends HttpServerTest { true } + boolean asyncCancelHasSendError() { + false + } + private static boolean shouldTestCompletableStageAsync() { Boolean.getBoolean("testLatestDeps") } diff --git a/instrumentation/jetty-8.0/javaagent/src/test/groovy/JettyHandlerTest.groovy b/instrumentation/jetty-8.0/javaagent/src/test/groovy/JettyHandlerTest.groovy index 9013d465e3..f731ce7aa6 100644 --- a/instrumentation/jetty-8.0/javaagent/src/test/groovy/JettyHandlerTest.groovy +++ b/instrumentation/jetty-8.0/javaagent/src/test/groovy/JettyHandlerTest.groovy @@ -10,6 +10,7 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS +import io.opentelemetry.instrumentation.test.asserts.TraceAssert import io.opentelemetry.instrumentation.test.base.HttpServerTest import javax.servlet.DispatcherType import javax.servlet.ServletException @@ -62,6 +63,23 @@ class JettyHandlerTest extends HttpServerTest { false } + @Override + boolean hasResponseSpan(ServerEndpoint endpoint) { + endpoint == REDIRECT || endpoint == ERROR + } + + @Override + void responseSpan(TraceAssert trace, int index, Object parent, String method, ServerEndpoint endpoint) { + switch (endpoint) { + case REDIRECT: + redirectSpan(trace, index, parent) + break + case ERROR: + sendErrorSpan(trace, index, parent) + break + } + } + static void handleRequest(Request request, HttpServletResponse response) { ServerEndpoint endpoint = ServerEndpoint.forPath(request.requestURI) controller(endpoint) { diff --git a/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationBasicTests.groovy b/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationBasicTests.groovy index 079ed8b8ff..2edcd1d6a6 100644 --- a/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationBasicTests.groovy +++ b/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationBasicTests.groovy @@ -332,7 +332,7 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 3) { + trace(0, 4) { span(0) { hasNoParent() name "/$jspWebappContext/includes/includeHtml.jsp" @@ -366,6 +366,11 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { "jsp.requestURL" reqUrl } } + span(3) { + childOf span(2) + name "ApplicationDispatcher.include" + errored false + } } } res.code() == 200 @@ -384,7 +389,7 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 7) { + trace(0, 9) { span(0) { hasNoParent() name "/$jspWebappContext/includes/includeMulti.jsp" @@ -420,6 +425,11 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { } span(3) { childOf span(2) + name "ApplicationDispatcher.include" + errored false + } + span(4) { + childOf span(3) name "Compile /common/javaLoopH2.jsp" errored false attributes { @@ -427,16 +437,21 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" } } - span(4) { - childOf span(2) + span(5) { + childOf span(3) name "Render /common/javaLoopH2.jsp" errored false attributes { "jsp.requestURL" reqUrl } } - span(5) { + span(6) { childOf span(2) + name "ApplicationDispatcher.include" + errored false + } + span(7) { + childOf span(6) name "Compile /common/javaLoopH2.jsp" errored false attributes { @@ -444,8 +459,8 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" } } - span(6) { - childOf span(2) + span(8) { + childOf span(6) name "Render /common/javaLoopH2.jsp" errored false attributes { diff --git a/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy b/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy index bff59a5b45..e9ba904f50 100644 --- a/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy +++ b/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy @@ -80,7 +80,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 5) { + trace(0, 6) { span(0) { hasNoParent() name "/$jspWebappContext/$forwardFromFileName" @@ -116,6 +116,11 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { } span(3) { childOf span(2) + name "ApplicationDispatcher.forward" + errored false + } + span(4) { + childOf span(3) name "Compile /$forwardDestFileName" errored false attributes { @@ -123,8 +128,8 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" } } - span(4) { - childOf span(2) + span(5) { + childOf span(3) name "Render /$forwardDestFileName" errored false attributes { @@ -155,7 +160,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 3) { + trace(0, 4) { span(0) { hasNoParent() name "/$jspWebappContext/forwards/forwardToHtml.jsp" @@ -189,6 +194,11 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { "jsp.requestURL" reqUrl } } + span(3) { + childOf span(2) + name "ApplicationDispatcher.forward" + errored false + } } } res.code() == 200 @@ -207,7 +217,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 9) { + trace(0, 12) { span(0) { hasNoParent() name "/$jspWebappContext/forwards/forwardToIncludeMulti.jsp" @@ -243,6 +253,11 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { } span(3) { childOf span(2) + name "ApplicationDispatcher.forward" + errored false + } + span(4) { + childOf span(3) name "Compile /includes/includeMulti.jsp" errored false attributes { @@ -250,8 +265,8 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" } } - span(4) { - childOf span(2) + span(5) { + childOf span(3) name "Render /includes/includeMulti.jsp" errored false attributes { @@ -259,26 +274,13 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { "jsp.requestURL" baseUrl + "/includes/includeMulti.jsp" } } - span(5) { - childOf span(4) - name "Compile /common/javaLoopH2.jsp" - errored false - attributes { - "jsp.classFQCN" "org.apache.jsp.common.javaLoopH2_jsp" - "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" - } - } span(6) { - childOf span(4) - name "Render /common/javaLoopH2.jsp" + childOf span(5) + name "ApplicationDispatcher.include" errored false - attributes { - "jsp.forwardOrigin" "/forwards/forwardToIncludeMulti.jsp" - "jsp.requestURL" baseUrl + "/includes/includeMulti.jsp" - } } span(7) { - childOf span(4) + childOf span(6) name "Compile /common/javaLoopH2.jsp" errored false attributes { @@ -287,7 +289,30 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { } } span(8) { - childOf span(4) + childOf span(6) + name "Render /common/javaLoopH2.jsp" + errored false + attributes { + "jsp.forwardOrigin" "/forwards/forwardToIncludeMulti.jsp" + "jsp.requestURL" baseUrl + "/includes/includeMulti.jsp" + } + } + span(9) { + childOf span(5) + name "ApplicationDispatcher.include" + errored false + } + span(10) { + childOf span(9) + name "Compile /common/javaLoopH2.jsp" + errored false + attributes { + "jsp.classFQCN" "org.apache.jsp.common.javaLoopH2_jsp" + "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" + } + } + span(11) { + childOf span(9) name "Render /common/javaLoopH2.jsp" errored false attributes { @@ -313,7 +338,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 7) { + trace(0, 9) { span(0) { hasNoParent() name "/$jspWebappContext/forwards/forwardToJspForward.jsp" @@ -349,6 +374,11 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { } span(3) { childOf span(2) + name "ApplicationDispatcher.forward" + errored false + } + span(4) { + childOf span(3) name "Compile /forwards/forwardToSimpleJava.jsp" errored false attributes { @@ -356,8 +386,8 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" } } - span(4) { - childOf span(2) + span(5) { + childOf span(3) name "Render /forwards/forwardToSimpleJava.jsp" errored false attributes { @@ -365,8 +395,13 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { "jsp.requestURL" baseUrl + "/forwards/forwardToSimpleJava.jsp" } } - span(5) { - childOf span(4) + span(6) { + childOf span(5) + name "ApplicationDispatcher.forward" + errored false + } + span(7) { + childOf span(6) name "Compile /common/loop.jsp" errored false attributes { @@ -374,8 +409,8 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" } } - span(6) { - childOf span(4) + span(8) { + childOf span(6) name "Render /common/loop.jsp" errored false attributes { @@ -401,7 +436,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 4) { + trace(0, 5) { span(0) { hasNoParent() name "/$jspWebappContext/forwards/forwardToCompileError.jsp" @@ -439,6 +474,12 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { } span(3) { childOf span(2) + name "ApplicationDispatcher.forward" + errored true + errorEvent(JasperException, String) + } + span(4) { + childOf span(3) name "Compile /compileError.jsp" errored true errorEvent(JasperException, String) @@ -465,7 +506,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 3) { + trace(0, 5) { span(0) { hasNoParent() name "/$jspWebappContext/forwards/forwardToNonExistent.jsp" @@ -499,6 +540,14 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { "jsp.requestURL" reqUrl } } + span(3) { + childOf span(2) + name "ApplicationDispatcher.forward" + } + span(4) { + childOf span(3) + name "ResponseFacade.sendError" + } } } res.code() == 404 diff --git a/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/LibertyHttpServerTracer.java b/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/LibertyHttpServerTracer.java index 72f86b9ac1..3571da564a 100644 --- a/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/LibertyHttpServerTracer.java +++ b/instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/LibertyHttpServerTracer.java @@ -21,8 +21,7 @@ public class LibertyHttpServerTracer extends Servlet3HttpServerTracer { // using request method as span name as server isn't ready for calling request.getServletPath() // span name will be updated a bit later when calling request.getServletPath() works // see LibertyUpdateSpanAdvice - Context context = startSpan(request, request, request, "HTTP " + request.getMethod()); - return addContextPathContext(context, request); + return startSpan(request, request, request, "HTTP " + request.getMethod()); } @Override diff --git a/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy b/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy index 8e8b461936..92880c84d4 100644 --- a/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy +++ b/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy @@ -3,6 +3,11 @@ * SPDX-License-Identifier: Apache-2.0 */ +import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR +import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND +import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT + +import io.opentelemetry.instrumentation.test.asserts.TraceAssert import io.opentelemetry.instrumentation.test.base.HttpServerTest import org.glassfish.embeddable.BootstrapProperties import org.glassfish.embeddable.Deployer @@ -63,4 +68,22 @@ class GlassFishServerTest extends HttpServerTest { boolean redirectHasBody() { true } + + @Override + boolean hasResponseSpan(ServerEndpoint endpoint) { + endpoint == REDIRECT || endpoint == ERROR || endpoint == NOT_FOUND + } + + @Override + void responseSpan(TraceAssert trace, int index, Object parent, String method, ServerEndpoint endpoint) { + switch (endpoint) { + case REDIRECT: + redirectSpan(trace, index, parent) + break + case ERROR: + case NOT_FOUND: + sendErrorSpan(trace, index, parent) + break + } + } } diff --git a/instrumentation/servlet/servlet-2.2/javaagent/servlet-2.2-javaagent.gradle b/instrumentation/servlet/servlet-2.2/javaagent/servlet-2.2-javaagent.gradle index 4d902b8e61..626c63828a 100644 --- a/instrumentation/servlet/servlet-2.2/javaagent/servlet-2.2-javaagent.gradle +++ b/instrumentation/servlet/servlet-2.2/javaagent/servlet-2.2-javaagent.gradle @@ -18,6 +18,7 @@ muzzle { dependencies { compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.2' api(project(':instrumentation-core:servlet-2.2')) + api(project(':instrumentation:servlet:servlet-common:javaagent')) testImplementation(project(':testing-common')) { exclude group: 'org.eclipse.jetty', module: 'jetty-server' diff --git a/instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Advice.java b/instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Advice.java index fccd427897..5496e2ab6e 100644 --- a/instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Advice.java +++ b/instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Advice.java @@ -37,7 +37,7 @@ public class Servlet2Advice { Context serverContext = tracer().getServerContext(httpServletRequest); if (serverContext != null) { - tracer().updateServerSpan(serverContext, httpServletRequest); + tracer().updateServerSpanNameOnce(serverContext, httpServletRequest); return; } diff --git a/instrumentation/servlet/servlet-2.2/javaagent/src/test/groovy/JettyServlet2Test.groovy b/instrumentation/servlet/servlet-2.2/javaagent/src/test/groovy/JettyServlet2Test.groovy index e33f70ce91..b7620e706b 100644 --- a/instrumentation/servlet/servlet-2.2/javaagent/src/test/groovy/JettyServlet2Test.groovy +++ b/instrumentation/servlet/servlet-2.2/javaagent/src/test/groovy/JettyServlet2Test.groovy @@ -70,10 +70,15 @@ class JettyServlet2Test extends HttpServerTest { false } + @Override + boolean hasResponseSpan(ServerEndpoint endpoint) { + endpoint == REDIRECT || endpoint == ERROR + } + @Override void responseSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { trace.span(index) { - name endpoint == REDIRECT ? "HttpServletResponse.sendRedirect" : "HttpServletResponse.sendError" + name endpoint == REDIRECT ? "Response.sendRedirect" : "Response.sendError" kind INTERNAL errored false childOf((SpanData) parent) diff --git a/instrumentation/servlet/servlet-3.0/javaagent/servlet-3.0-javaagent.gradle b/instrumentation/servlet/servlet-3.0/javaagent/servlet-3.0-javaagent.gradle index 487b75aa54..82ec5599f6 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/servlet-3.0-javaagent.gradle +++ b/instrumentation/servlet/servlet-3.0/javaagent/servlet-3.0-javaagent.gradle @@ -17,6 +17,7 @@ muzzle { dependencies { compileOnly group: 'javax.servlet', name: 'javax.servlet-api', version: '3.0.1' api(project(':instrumentation-core:servlet-2.2')) + api(project(':instrumentation:servlet:servlet-common:javaagent')) testInstrumentation project(':instrumentation:jetty-8.0:javaagent') diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java index 877620951e..c871350cd7 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java @@ -40,14 +40,14 @@ public class Servlet3Advice { scope = attachedContext.makeCurrent(); } - tracer().updateServerSpan(attachedContext, httpServletRequest); + tracer().updateServerSpanNameOnce(attachedContext, httpServletRequest); // We are inside nested servlet/filter/app-server span, don't create new span return; } Context parentContext = Java8BytecodeBridge.currentContext(); if (parentContext != null && Java8BytecodeBridge.spanFromContext(parentContext).isRecording()) { - tracer().updateServerSpan(parentContext, httpServletRequest); + tracer().updateServerSpanNameOnce(parentContext, httpServletRequest); // We are inside nested servlet/filter/app-server span, don't create new span return; } diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3Test.groovy b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3Test.groovy index 87a755157f..a6e0313085 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3Test.groovy +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3Test.groovy @@ -10,6 +10,7 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS +import io.opentelemetry.instrumentation.test.asserts.TraceAssert import io.opentelemetry.instrumentation.test.base.HttpServerTest import javax.servlet.Servlet import okhttp3.Request @@ -49,4 +50,25 @@ abstract class AbstractServlet3Test extends HttpServerTest ServletException } + @Override + boolean hasResponseSpan(ServerEndpoint endpoint) { + endpoint == NOT_FOUND || super.hasResponseSpan(endpoint) + } + + @Override + void responseSpan(TraceAssert trace, int index, Object parent, String method, ServerEndpoint endpoint) { + switch (endpoint) { + case NOT_FOUND: + sendErrorSpan(trace, index, parent) + break + } + super.responseSpan(trace, index, parent, method, endpoint) + } + @Shared def accessLogValue = new TestAccessLogValve() @@ -122,10 +139,26 @@ abstract class TomcatServlet3Test extends AbstractServlet3Test def loggedTraces = accessLogValue.loggedIds*.first def loggedSpans = accessLogValue.loggedIds*.second + def expectedCount = 2 + if (hasIncludeSpan()) { + expectedCount++ + } + if (hasForwardSpan()) { + expectedCount++ + } (0..count - 1).each { - trace(it, 2) { + trace(it, expectedCount) { serverSpan(it, 0, null, null, "GET", SUCCESS.body.length()) - controllerSpan(it, 1, span(0)) + def controllerIndex = 1 + if (hasIncludeSpan()) { + includeSpan(it, 1, span(0)) + controllerIndex++ + } + if (hasForwardSpan()) { + forwardSpan(it, 1, span(0)) + controllerIndex++ + } + controllerSpan(it, controllerIndex, span(controllerIndex - 1)) } assert loggedTraces.contains(traces[it][0].traceId) @@ -150,10 +183,27 @@ abstract class TomcatServlet3Test extends AbstractServlet3Test response.body().string() == ERROR.body and: + def spanCount = 2 + if (errorEndpointUsesSendError()) { + spanCount++ + } + if (hasForwardSpan()) { + spanCount++ + } assertTraces(1) { - trace(0, 2) { + trace(0, spanCount) { serverSpan(it, 0, null, null, method, response.body().contentLength(), ERROR) - controllerSpan(it, 1, span(0)) + def spanIndex = 1 + if (hasForwardSpan()) { + forwardSpan(it, spanIndex, span(spanIndex - 1)) + spanIndex++ + } + controllerSpan(it, spanIndex, span(spanIndex - 1)) + spanIndex++ + if (errorEndpointUsesSendError()) { + sendErrorSpan(it, spanIndex, span(spanIndex - 1)) + spanIndex++ + } } def (String traceId, String spanId) = accessLogValue.loggedIds[0] @@ -256,6 +306,11 @@ class TomcatServlet3TestAsync extends TomcatServlet3Test { TestServlet3.Async } + @Override + boolean errorEndpointUsesSendError() { + false + } + @Override boolean testException() { // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 @@ -288,6 +343,11 @@ class TomcatServlet3TestForward extends TomcatDispatchTest { false } + @Override + boolean hasForwardSpan() { + true + } + @Override protected void setupServlets(Context context) { super.setupServlets(context) @@ -322,6 +382,11 @@ class TomcatServlet3TestInclude extends TomcatDispatchTest { false } + @Override + boolean hasIncludeSpan() { + true + } + @Override protected void setupServlets(Context context) { super.setupServlets(context) @@ -379,6 +444,11 @@ class TomcatServlet3TestDispatchAsync extends TomcatDispatchTest { addServlet(context, "/dispatch/recursive", TestServlet3.DispatchRecursive) } + @Override + boolean errorEndpointUsesSendError() { + false + } + @Override boolean testException() { // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/contextpath/ServletContextPathInstrumentationModule.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/contextpath/ServletContextPathInstrumentationModule.java new file mode 100644 index 0000000000..8af40d1ff8 --- /dev/null +++ b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/contextpath/ServletContextPathInstrumentationModule.java @@ -0,0 +1,100 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.servlet.contextpath; + +import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.safeHasSuperType; +import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.ClassLoaderMatcher.hasClassesNamed; +import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf; +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; +import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.tooling.InstrumentationModule; +import io.opentelemetry.javaagent.tooling.TypeInstrumentation; +import java.util.List; +import java.util.Map; +import javax.servlet.ServletRequest; +import javax.servlet.http.HttpServletRequest; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(InstrumentationModule.class) +public class ServletContextPathInstrumentationModule extends InstrumentationModule { + public ServletContextPathInstrumentationModule() { + super("servlet", "servlet-context-path"); + } + + @Override + public List typeInstrumentations() { + return singletonList(new ServletContextPathInstrumentation()); + } + + public static class ServletContextPathInstrumentation implements TypeInstrumentation { + @Override + public ElementMatcher classLoaderOptimization() { + return hasClassesNamed("javax.servlet.Filter"); + } + + @Override + public ElementMatcher typeMatcher() { + return safeHasSuperType(namedOneOf("javax.servlet.Filter", "javax.servlet.Servlet")); + } + + @Override + public Map, String> transformers() { + return singletonMap( + namedOneOf("doFilter", "service") + .and(takesArgument(0, named("javax.servlet.ServletRequest"))) + .and(takesArgument(1, named("javax.servlet.ServletResponse"))) + .and(isPublic()), + ServletContextPathAdvice.class.getName()); + } + } + + public static class ServletContextPathAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.Argument(0) ServletRequest request, @Advice.Local("otelScope") Scope scope) { + + if (!(request instanceof HttpServletRequest)) { + return; + } + + int callDepth = CallDepthThreadLocalMap.incrementCallDepth(ServletContextPath.class); + if (callDepth > 0) { + return; + } + + HttpServletRequest httpServletRequest = (HttpServletRequest) request; + + String contextPath = httpServletRequest.getContextPath(); + if (contextPath != null && !contextPath.isEmpty() && !contextPath.equals("/")) { + Context context = + Java8BytecodeBridge.currentContext().with(ServletContextPath.CONTEXT_KEY, contextPath); + scope = context.makeCurrent(); + } + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stop(@Advice.Local("otelScope") Scope scope) { + if (scope != null) { + CallDepthThreadLocalMap.reset(ServletContextPath.class); + scope.close(); + } + } + } +} diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerAdapterInstrumentation.java b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerAdapterInstrumentation.java index cca8a4fe0a..b70352d912 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerAdapterInstrumentation.java +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerAdapterInstrumentation.java @@ -64,10 +64,10 @@ public class HandlerAdapterInstrumentation implements TypeInstrumentation { if (serverSpan != null) { // Name the parent span based on the matching pattern tracer().onRequest(context, serverSpan, request); - // Now create a span for handler/controller execution. - span = tracer().startHandlerSpan(handler); - scope = context.with(span).makeCurrent(); } + // Now create a span for handler/controller execution. + span = tracer().startHandlerSpan(handler); + scope = context.with(span).makeCurrent(); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/boot/SpringBootBasedTest.groovy b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/boot/SpringBootBasedTest.groovy index d8db857de3..2c2fde4001 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/boot/SpringBootBasedTest.groovy +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/boot/SpringBootBasedTest.groovy @@ -58,6 +58,11 @@ class SpringBootBasedTest extends HttpServerTest endpoint == REDIRECT } + @Override + boolean hasResponseSpan(ServerEndpoint endpoint) { + endpoint == REDIRECT + } + @Override boolean testNotFound() { // FIXME: the instrumentation adds an extra controller span which is not consistent. @@ -84,9 +89,11 @@ class SpringBootBasedTest extends HttpServerTest and: assertTraces(1) { - trace(0, 2) { + trace(0, 4) { serverSpan(it, 0, null, null, "GET", null, AUTH_ERROR) - errorPageSpans(it, 1, null) + sendErrorSpan(it, 1, span(0)) + forwardSpan(it, 2, span(0)) + errorPageSpans(it, 3, null) } } } @@ -111,8 +118,9 @@ class SpringBootBasedTest extends HttpServerTest and: assertTraces(1) { - trace(0, 1) { + trace(0, 2) { serverSpan(it, 0, null, null, "POST", response.body()?.contentLength(), LOGIN) + redirectSpan(it, 1, span(0)) } } @@ -134,7 +142,7 @@ class SpringBootBasedTest extends HttpServerTest @Override void responseSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { trace.span(index) { - name "HttpServletResponse.sendRedirect" + name "OnCommittedResponseWrapper.sendRedirect" kind INTERNAL errored false attributes { diff --git a/instrumentation/struts-2.3/javaagent/src/test/groovy/Struts2ActionSpanTest.groovy b/instrumentation/struts-2.3/javaagent/src/test/groovy/Struts2ActionSpanTest.groovy index a465273c48..5ca6b0e000 100644 --- a/instrumentation/struts-2.3/javaagent/src/test/groovy/Struts2ActionSpanTest.groovy +++ b/instrumentation/struts-2.3/javaagent/src/test/groovy/Struts2ActionSpanTest.groovy @@ -3,8 +3,10 @@ * SPDX-License-Identifier: Apache-2.0 */ +import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM +import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT import io.opentelemetry.api.trace.SpanKind import io.opentelemetry.instrumentation.test.asserts.TraceAssert @@ -46,6 +48,24 @@ class Struts2ActionSpanTest extends HttpServerTest { return true } + @Override + boolean hasResponseSpan(ServerEndpoint endpoint) { + endpoint == REDIRECT || endpoint == ERROR || endpoint == EXCEPTION + } + + @Override + void responseSpan(TraceAssert trace, int index, Object controllerSpan, Object handlerSpan, String method, ServerEndpoint endpoint) { + switch (endpoint) { + case REDIRECT: + redirectSpan(trace, index, handlerSpan) + break + case ERROR: + case EXCEPTION: + sendErrorSpan(trace, index, handlerSpan) + break + } + } + String expectedServerSpanName(ServerEndpoint endpoint) { return endpoint == PATH_PARAM ? getContextPath() + "/path/{id}/param" : endpoint.resolvePath(address).path } diff --git a/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TestServlet.java b/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TestServlet.java index f8f831426e..1a4598bb54 100644 --- a/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TestServlet.java +++ b/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TestServlet.java @@ -19,21 +19,22 @@ public class TestServlet extends HttpServlet { HttpServerTest.ServerEndpoint serverEndpoint = HttpServerTest.ServerEndpoint.forPath(path); if (serverEndpoint != null) { - if (serverEndpoint == HttpServerTest.ServerEndpoint.EXCEPTION) { - HttpServerTest.controller( - serverEndpoint, - () -> { + HttpServerTest.controller( + serverEndpoint, + () -> { + if (serverEndpoint == HttpServerTest.ServerEndpoint.EXCEPTION) { throw new Exception(serverEndpoint.getBody()); - }); - } else { - resp.getWriter().print(HttpServerTest.controller(serverEndpoint, serverEndpoint::getBody)); - } - - if (serverEndpoint == HttpServerTest.ServerEndpoint.REDIRECT) { - resp.sendRedirect(serverEndpoint.getBody()); - } else { - resp.setStatus(serverEndpoint.getStatus()); - } + } + resp.getWriter().print(serverEndpoint.getBody()); + if (serverEndpoint == HttpServerTest.ServerEndpoint.REDIRECT) { + resp.sendRedirect(serverEndpoint.getBody()); + } else if (serverEndpoint == HttpServerTest.ServerEndpoint.ERROR) { + resp.sendError(serverEndpoint.getStatus()); + } else { + resp.setStatus(serverEndpoint.getStatus()); + } + return null; + }); } else if ("/errorPage".equals(path)) { resp.getWriter().print(HttpServerTest.ServerEndpoint.EXCEPTION.getBody()); resp.setStatus(500); diff --git a/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TomcatHandlerTest.groovy b/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TomcatHandlerTest.groovy index c4f4bc8d17..0f0b2751a0 100644 --- a/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TomcatHandlerTest.groovy +++ b/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TomcatHandlerTest.groovy @@ -5,7 +5,15 @@ package io.opentelemetry.javaagent.instrumentation.tomcat7 +import static io.opentelemetry.api.trace.Span.Kind.INTERNAL +import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR +import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS + +import io.opentelemetry.instrumentation.test.asserts.TraceAssert import io.opentelemetry.instrumentation.test.base.HttpServerTest +import io.opentelemetry.sdk.trace.data.SpanData import org.apache.catalina.Context import org.apache.catalina.startup.Tomcat import org.apache.tomcat.util.descriptor.web.ErrorPage @@ -53,4 +61,42 @@ class TomcatHandlerTest extends HttpServerTest { tomcat.getServer().stop() } + @Override + boolean testExceptionBody() { + false + } + + @Override + boolean hasErrorPageSpans(ServerEndpoint endpoint) { + endpoint == ERROR || endpoint == EXCEPTION + } + + @Override + void errorPageSpans(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { + trace.span(index) { + name "ApplicationDispatcher.forward" + kind INTERNAL + errored false + childOf((SpanData) parent) + attributes { + } + } + } + + @Override + boolean hasResponseSpan(ServerEndpoint endpoint) { + endpoint == REDIRECT || endpoint == ERROR + } + + @Override + void responseSpan(TraceAssert trace, int index, Object parent, String method, ServerEndpoint endpoint) { + switch (endpoint) { + case REDIRECT: + redirectSpan(trace, index, parent) + break + case ERROR: + sendErrorSpan(trace, index, parent) + break + } + } } 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 a21f9ded79..939ad91a06 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 @@ -51,6 +51,14 @@ abstract class HttpServerTest extends AgentInstrumentationSpecification false } + boolean hasForwardSpan() { + false + } + + boolean hasIncludeSpan() { + false + } + boolean hasErrorPageSpans(ServerEndpoint endpoint) { false } @@ -341,12 +349,18 @@ abstract class HttpServerTest extends AgentInstrumentationSpecification if (hasHandlerSpan()) { spanCount++ } + if (hasResponseSpan(endpoint)) { + spanCount++ + } if (endpoint != NOT_FOUND) { spanCount++ // controller span if (hasRenderSpan(endpoint)) { spanCount++ } - if (hasResponseSpan(endpoint)) { + if (hasForwardSpan()) { + spanCount++ + } + if (hasIncludeSpan()) { spanCount++ } if (hasErrorPageSpans(endpoint)) { @@ -362,21 +376,31 @@ abstract class HttpServerTest extends AgentInstrumentationSpecification handlerSpan(it, spanIndex++, span(0), method, endpoint) } if (endpoint != NOT_FOUND) { + def controllerSpanIndex = 0 if (hasHandlerSpan()) { - controllerSpan(it, spanIndex++, span(1), errorMessage, expectedExceptionClass()) - } else { - controllerSpan(it, spanIndex++, span(0), errorMessage, expectedExceptionClass()) + controllerSpanIndex++ } + if (hasForwardSpan()) { + forwardSpan(it, spanIndex++, span(0), errorMessage, expectedExceptionClass()) + controllerSpanIndex++ + } + if (hasIncludeSpan()) { + includeSpan(it, spanIndex++, span(0), errorMessage, expectedExceptionClass()) + controllerSpanIndex++ + } + controllerSpan(it, spanIndex++, span(controllerSpanIndex), errorMessage, expectedExceptionClass()) if (hasRenderSpan(endpoint)) { renderSpan(it, spanIndex++, span(0), method, endpoint) } if (hasResponseSpan(endpoint)) { - responseSpan(it, spanIndex, span(spanIndex - 1), method, endpoint) + responseSpan(it, spanIndex, span(spanIndex - 1), span(0), method, endpoint) spanIndex++ } if (hasErrorPageSpans(endpoint)) { errorPageSpans(it, spanIndex, span(0), method, endpoint) } + } else if (hasResponseSpan(endpoint)) { + responseSpan(it, 1, span(0), span(0), method, endpoint) } } } @@ -402,6 +426,10 @@ abstract class HttpServerTest extends AgentInstrumentationSpecification throw new UnsupportedOperationException("renderSpan not implemented in " + getClass().name) } + void responseSpan(TraceAssert trace, int index, Object controllerSpan, Object handlerSpan, String method = "GET", ServerEndpoint endpoint = SUCCESS) { + responseSpan(trace, index, controllerSpan, method, endpoint) + } + void responseSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { throw new UnsupportedOperationException("responseSpan not implemented in " + getClass().name) } @@ -410,6 +438,46 @@ abstract class HttpServerTest extends AgentInstrumentationSpecification throw new UnsupportedOperationException("errorPageSpans not implemented in " + getClass().name) } + void redirectSpan(TraceAssert trace, int index, Object parent) { + trace.span(index) { + name ~/\.sendRedirect$/ + kind Span.Kind.INTERNAL + childOf((SpanData) parent) + } + } + + void sendErrorSpan(TraceAssert trace, int index, Object parent) { + trace.span(index) { + name ~/\.sendError$/ + kind Span.Kind.INTERNAL + childOf((SpanData) parent) + } + } + + void forwardSpan(TraceAssert trace, int index, Object parent, String errorMessage = null, Class exceptionClass = Exception) { + trace.span(index) { + name ~/\.forward$/ + kind Span.Kind.INTERNAL + errored errorMessage != null + if (errorMessage) { + errorEvent(exceptionClass, errorMessage) + } + childOf((SpanData) parent) + } + } + + void includeSpan(TraceAssert trace, int index, Object parent, String errorMessage = null, Class exceptionClass = Exception) { + trace.span(index) { + name ~/\.include$/ + kind Span.Kind.INTERNAL + errored errorMessage != null + if (errorMessage) { + errorEvent(exceptionClass, errorMessage) + } + childOf((SpanData) parent) + } + } + // parent span must be cast otherwise it breaks debugging classloading (junit loads it early) void serverSpan(TraceAssert trace, int index, String traceID = null, String parentID = null, String method = "GET", Long responseContentLength = null, ServerEndpoint endpoint = SUCCESS) { trace.span(index) {