diff --git a/instrumentation/grails-3.0/javaagent/src/test/groovy/test/GrailsTest.groovy b/instrumentation/grails-3.0/javaagent/src/test/groovy/test/GrailsTest.groovy index 120a8a11b6..af0ec4d750 100644 --- a/instrumentation/grails-3.0/javaagent/src/test/groovy/test/GrailsTest.groovy +++ b/instrumentation/grails-3.0/javaagent/src/test/groovy/test/GrailsTest.groovy @@ -103,7 +103,7 @@ class GrailsTest extends HttpServerTest implemen @Override int getErrorPageSpansCount(ServerEndpoint endpoint) { - endpoint == NOT_FOUND ? 1 : 2 + endpoint == NOT_FOUND ? 0 : 1 } @Override @@ -113,9 +113,8 @@ class GrailsTest extends HttpServerTest implemen @Override void errorPageSpans(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint) { - forwardSpan(trace, index, trace.span(0)) if (endpoint != NOT_FOUND) { - trace.span(index + 1) { + trace.span(index) { name "ErrorController.index" kind INTERNAL attributes { 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 1444a41a2b..762c00112e 100644 --- a/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationBasicTests.groovy +++ b/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationBasicTests.groovy @@ -323,7 +323,7 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 4) { + trace(0, 3) { span(0) { hasNoParent() name "/$jspWebappContext/includes/includeHtml.jsp" @@ -354,10 +354,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { "jsp.requestURL" reqUrl } } - span(3) { - childOf span(2) - name "ApplicationDispatcher.include" - } } } res.code() == 200 @@ -376,7 +372,7 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 9) { + trace(0, 7) { span(0) { hasNoParent() name "/$jspWebappContext/includes/includeMulti.jsp" @@ -409,37 +405,29 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification { } span(3) { childOf span(2) - name "ApplicationDispatcher.include" - } - span(4) { - childOf span(3) name "Compile /common/javaLoopH2.jsp" attributes { "jsp.classFQCN" "org.apache.jsp.common.javaLoopH2_jsp" "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" } } - span(5) { - childOf span(3) + span(4) { + childOf span(2) name "Render /common/javaLoopH2.jsp" attributes { "jsp.requestURL" reqUrl } } - span(6) { + span(5) { childOf span(2) - name "ApplicationDispatcher.include" - } - span(7) { - childOf span(6) name "Compile /common/javaLoopH2.jsp" attributes { "jsp.classFQCN" "org.apache.jsp.common.javaLoopH2_jsp" "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" } } - span(8) { - childOf span(6) + span(6) { + childOf span(2) name "Render /common/javaLoopH2.jsp" attributes { "jsp.requestURL" reqUrl 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 d69fd87a1c..5de0e769d3 100644 --- a/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy +++ b/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy @@ -81,7 +81,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 6) { + trace(0, 5) { span(0) { hasNoParent() name "/$jspWebappContext/$forwardFromFileName" @@ -114,18 +114,14 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { } span(3) { childOf span(2) - name "ApplicationDispatcher.forward" - } - span(4) { - childOf span(3) name "Compile /$forwardDestFileName" attributes { "jsp.classFQCN" "org.apache.jsp.$jspForwardDestClassPrefix$jspForwardDestClassName" "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" } } - span(5) { - childOf span(3) + span(4) { + childOf span(2) name "Render /$forwardDestFileName" attributes { "jsp.forwardOrigin" "/$forwardFromFileName" @@ -155,7 +151,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 4) { + trace(0, 3) { span(0) { hasNoParent() name "/$jspWebappContext/forwards/forwardToHtml.jsp" @@ -186,10 +182,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { "jsp.requestURL" reqUrl } } - span(3) { - childOf span(2) - name "ApplicationDispatcher.forward" - } } } res.code() == 200 @@ -208,7 +200,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 12) { + trace(0, 9) { span(0) { hasNoParent() name "/$jspWebappContext/forwards/forwardToIncludeMulti.jsp" @@ -241,30 +233,38 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { } span(3) { childOf span(2) - name "ApplicationDispatcher.forward" - } - span(4) { - childOf span(3) name "Compile /includes/includeMulti.jsp" attributes { "jsp.classFQCN" "org.apache.jsp.includes.includeMulti_jsp" "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" } } - span(5) { - childOf span(3) + span(4) { + childOf span(2) name "Render /includes/includeMulti.jsp" attributes { "jsp.forwardOrigin" "/forwards/forwardToIncludeMulti.jsp" "jsp.requestURL" baseUrl + "/includes/includeMulti.jsp" } } + span(5) { + childOf span(4) + name "Compile /common/javaLoopH2.jsp" + attributes { + "jsp.classFQCN" "org.apache.jsp.common.javaLoopH2_jsp" + "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" + } + } span(6) { - childOf span(5) - name "ApplicationDispatcher.include" + childOf span(4) + name "Render /common/javaLoopH2.jsp" + attributes { + "jsp.forwardOrigin" "/forwards/forwardToIncludeMulti.jsp" + "jsp.requestURL" baseUrl + "/includes/includeMulti.jsp" + } } span(7) { - childOf span(6) + childOf span(4) name "Compile /common/javaLoopH2.jsp" attributes { "jsp.classFQCN" "org.apache.jsp.common.javaLoopH2_jsp" @@ -272,27 +272,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { } } span(8) { - childOf span(6) - name "Render /common/javaLoopH2.jsp" - attributes { - "jsp.forwardOrigin" "/forwards/forwardToIncludeMulti.jsp" - "jsp.requestURL" baseUrl + "/includes/includeMulti.jsp" - } - } - span(9) { - childOf span(5) - name "ApplicationDispatcher.include" - } - span(10) { - childOf span(9) - name "Compile /common/javaLoopH2.jsp" - attributes { - "jsp.classFQCN" "org.apache.jsp.common.javaLoopH2_jsp" - "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" - } - } - span(11) { - childOf span(9) + childOf span(4) name "Render /common/javaLoopH2.jsp" attributes { "jsp.forwardOrigin" "/forwards/forwardToIncludeMulti.jsp" @@ -310,14 +290,14 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { def "non-erroneous GET forward to another forward (2 forwards)"() { setup: String reqUrl = baseUrl + "/forwards/forwardToJspForward.jsp" - Request req = new Request.Builder().url(new URL(reqUrl)).get().build() + Request req = new Request.Builder().url(new URL(reqUrl)).get() build() when: Response res = client.newCall(req).execute() then: assertTraces(1) { - trace(0, 9) { + trace(0, 7) { span(0) { hasNoParent() name "/$jspWebappContext/forwards/forwardToJspForward.jsp" @@ -350,38 +330,30 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { } span(3) { childOf span(2) - name "ApplicationDispatcher.forward" - } - span(4) { - childOf span(3) name "Compile /forwards/forwardToSimpleJava.jsp" attributes { "jsp.classFQCN" "org.apache.jsp.forwards.forwardToSimpleJava_jsp" "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" } } - span(5) { - childOf span(3) + span(4) { + childOf span(2) name "Render /forwards/forwardToSimpleJava.jsp" attributes { "jsp.forwardOrigin" "/forwards/forwardToJspForward.jsp" "jsp.requestURL" baseUrl + "/forwards/forwardToSimpleJava.jsp" } } - span(6) { - childOf span(5) - name "ApplicationDispatcher.forward" - } - span(7) { - childOf span(6) + span(5) { + childOf span(4) name "Compile /common/loop.jsp" attributes { "jsp.classFQCN" "org.apache.jsp.common.loop_jsp" "jsp.compiler" "org.apache.jasper.compiler.JDTCompiler" } } - span(8) { - childOf span(6) + span(6) { + childOf span(4) name "Render /common/loop.jsp" attributes { "jsp.forwardOrigin" "/forwards/forwardToJspForward.jsp" @@ -406,7 +378,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 5) { + trace(0, 4) { span(0) { hasNoParent() name "/$jspWebappContext/forwards/forwardToCompileError.jsp" @@ -443,12 +415,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { } span(3) { childOf span(2) - name "ApplicationDispatcher.forward" - status ERROR - errorEvent(JasperException, String) - } - span(4) { - childOf span(3) name "Compile /compileError.jsp" status ERROR errorEvent(JasperException, String) @@ -475,7 +441,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 5) { + trace(0, 4) { span(0) { hasNoParent() name "/$jspWebappContext/forwards/forwardToNonExistent.jsp" @@ -509,10 +475,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { } span(3) { childOf span(2) - name "ApplicationDispatcher.forward" - } - span(4) { - childOf span(3) name "ResponseFacade.sendError" } } 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 2fa4f47649..07346dc6cf 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 @@ -168,11 +168,6 @@ class JettyServlet3TestForward extends JettyDispatchTest { TestServlet3.Sync // dispatch to sync servlet } - @Override - boolean hasForwardSpan() { - true - } - @Override protected void setupServlets(ServletContextHandler context) { super.setupServlets(context) @@ -202,11 +197,6 @@ class JettyServlet3TestInclude extends JettyDispatchTest { false } - @Override - boolean hasIncludeSpan() { - true - } - @Override protected void setupServlets(ServletContextHandler context) { super.setupServlets(context) 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 6135f8b6a6..34df55bd09 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 @@ -145,26 +145,10 @@ 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, expectedCount) { + trace(it, 2) { serverSpan(it, 0, null, null, "GET", SUCCESS.body.length()) - 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)) + controllerSpan(it, 1, span(0)) } assert loggedTraces.contains(traces[it][0].traceId) @@ -193,17 +177,10 @@ abstract class TomcatServlet3Test extends AbstractServlet3Test if (errorEndpointUsesSendError()) { spanCount++ } - if (hasForwardSpan()) { - spanCount++ - } assertTraces(1) { trace(0, spanCount) { serverSpan(it, 0, null, null, method, response.body().contentLength(), ERROR) def spanIndex = 1 - if (hasForwardSpan()) { - forwardSpan(it, spanIndex, span(spanIndex - 1)) - spanIndex++ - } controllerSpan(it, spanIndex, span(spanIndex - 1)) spanIndex++ if (errorEndpointUsesSendError()) { @@ -369,11 +346,6 @@ class TomcatServlet3TestForward extends TomcatDispatchTest { false } - @Override - boolean hasForwardSpan() { - true - } - @Override protected void setupServlets(Context context) { super.setupServlets(context) @@ -408,11 +380,6 @@ class TomcatServlet3TestInclude extends TomcatDispatchTest { false } - @Override - boolean hasIncludeSpan() { - true - } - @Override protected void setupServlets(Context context) { super.setupServlets(context) diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/dispatcher/RequestDispatcherAdvice.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/dispatcher/RequestDispatcherAdvice.java index 3fc209056a..0ac70cb385 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/dispatcher/RequestDispatcherAdvice.java +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/dispatcher/RequestDispatcherAdvice.java @@ -6,7 +6,6 @@ package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.dispatcher; import static io.opentelemetry.instrumentation.api.tracer.HttpServerTracer.CONTEXT_ATTRIBUTE; -import static io.opentelemetry.javaagent.instrumentation.servlet.v5_0.dispatcher.RequestDispatcherTracer.tracer; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; @@ -33,17 +32,15 @@ public class RequestDispatcherAdvice { Object requestContextAttr = request.getAttribute(CONTEXT_ATTRIBUTE); requestContext = requestContextAttr instanceof Context ? (Context) requestContextAttr : null; - Context parentContext = - RequestDispatcherAdviceHelper.getStartParentContext(currentContext, requestContext); - if (parentContext == null) { + context = RequestDispatcherAdviceHelper.getStartParentContext(currentContext, requestContext); + if (context == null) { return; } - context = tracer().startSpan(parentContext, method); - // this tells the dispatched servlet to use the current span as the parent for its work request.setAttribute(CONTEXT_ATTRIBUTE, context); + // TODO (trask) do we need this, since doing manual propagation above? scope = context.makeCurrent(); } @@ -61,11 +58,5 @@ public class RequestDispatcherAdvice { // restore the original request context request.setAttribute(CONTEXT_ATTRIBUTE, requestContext); } - - if (throwable != null) { - tracer().endExceptionally(context, throwable); - } else { - tracer().end(context); - } } } diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/dispatcher/RequestDispatcherTracer.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/dispatcher/RequestDispatcherTracer.java deleted file mode 100644 index a632487bed..0000000000 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/dispatcher/RequestDispatcherTracer.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.dispatcher; - -import io.opentelemetry.api.trace.SpanKind; -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import java.lang.reflect.Method; - -public class RequestDispatcherTracer extends BaseTracer { - private static final RequestDispatcherTracer TRACER = new RequestDispatcherTracer(); - - public static RequestDispatcherTracer tracer() { - return TRACER; - } - - @Override - protected String getInstrumentationName() { - return "io.opentelemetry.javaagent.servlet-5.0"; - } - - public Context startSpan(Context parentContext, Method method) { - return startSpan(parentContext, spanNameForMethod(method), SpanKind.INTERNAL); - } -} diff --git a/instrumentation/servlet/servlet-javax-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/javax/dispatcher/RequestDispatcherAdvice.java b/instrumentation/servlet/servlet-javax-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/javax/dispatcher/RequestDispatcherAdvice.java index 06644e288e..cc557beb41 100644 --- a/instrumentation/servlet/servlet-javax-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/javax/dispatcher/RequestDispatcherAdvice.java +++ b/instrumentation/servlet/servlet-javax-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/javax/dispatcher/RequestDispatcherAdvice.java @@ -6,7 +6,6 @@ package io.opentelemetry.javaagent.instrumentation.servlet.javax.dispatcher; import static io.opentelemetry.instrumentation.api.tracer.HttpServerTracer.CONTEXT_ATTRIBUTE; -import static io.opentelemetry.javaagent.instrumentation.servlet.javax.dispatcher.RequestDispatcherTracer.tracer; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; @@ -31,14 +30,11 @@ public class RequestDispatcherAdvice { Object requestContextAttr = request.getAttribute(CONTEXT_ATTRIBUTE); requestContext = requestContextAttr instanceof Context ? (Context) requestContextAttr : null; - Context parentContext = - RequestDispatcherAdviceHelper.getStartParentContext(currentContext, requestContext); - if (parentContext == null) { + context = RequestDispatcherAdviceHelper.getStartParentContext(currentContext, requestContext); + if (context == null) { return; } - context = tracer().startSpan(parentContext, method); - // this tells the dispatched servlet to use the current span as the parent for its work request.setAttribute(CONTEXT_ATTRIBUTE, context); @@ -60,11 +56,5 @@ public class RequestDispatcherAdvice { // restore the original request context request.setAttribute(CONTEXT_ATTRIBUTE, requestContext); } - - if (throwable != null) { - tracer().endExceptionally(context, throwable); - } else { - tracer().end(context); - } } } diff --git a/instrumentation/servlet/servlet-javax-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/javax/dispatcher/RequestDispatcherTracer.java b/instrumentation/servlet/servlet-javax-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/javax/dispatcher/RequestDispatcherTracer.java deleted file mode 100644 index fd53d46c43..0000000000 --- a/instrumentation/servlet/servlet-javax-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/javax/dispatcher/RequestDispatcherTracer.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.servlet.javax.dispatcher; - -import io.opentelemetry.api.trace.SpanKind; -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import java.lang.reflect.Method; - -public class RequestDispatcherTracer extends BaseTracer { - private static final RequestDispatcherTracer TRACER = new RequestDispatcherTracer(); - - public static RequestDispatcherTracer tracer() { - return TRACER; - } - - @Override - protected String getInstrumentationName() { - return "io.opentelemetry.javaagent.servlet-javax-common"; - } - - public Context startSpan(Context parentContext, Method method) { - return startSpan(parentContext, spanNameForMethod(method), SpanKind.INTERNAL); - } -} diff --git a/instrumentation/servlet/servlet-javax-common/javaagent/src/test/groovy/RequestDispatcherTest.groovy b/instrumentation/servlet/servlet-javax-common/javaagent/src/test/groovy/RequestDispatcherTest.groovy index 9331bc477c..35c9a1bda9 100644 --- a/instrumentation/servlet/servlet-javax-common/javaagent/src/test/groovy/RequestDispatcherTest.groovy +++ b/instrumentation/servlet/servlet-javax-common/javaagent/src/test/groovy/RequestDispatcherTest.groovy @@ -3,7 +3,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import static io.opentelemetry.api.trace.StatusCode.ERROR import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace @@ -26,6 +25,7 @@ class RequestDispatcherTest extends AgentInstrumentationSpecification { then: assertTraces(2) { + orderByRootSpanName("forward-child", "include-child") trace(0, 1) { basicSpan(it, 0, "forward-child") } @@ -43,13 +43,9 @@ class RequestDispatcherTest extends AgentInstrumentationSpecification { then: assertTraces(1) { - trace(0, 3) { + trace(0, 2) { basicSpan(it, 0, "parent") - span(1) { - name "TestDispatcher.$operation" - childOf span(0) - } - basicSpan(it, 2, "$operation-child", span(1)) + basicSpan(it, 1, "$operation-child", span(0)) } } @@ -79,13 +75,10 @@ class RequestDispatcherTest extends AgentInstrumentationSpecification { then: assertTraces(2) { - trace(0, 3) { + orderByRootSpanName("parent", "notParent") + trace(0, 2) { basicSpan(it, 0, "parent") - span(1) { - name "TestDispatcher.$operation" - childOf span(0) - } - basicSpan(it, 2, "$operation-child", span(1)) + basicSpan(it, 1, "$operation-child", span(0)) } trace(1, 1) { basicSpan(it, 0, "notParent") @@ -117,15 +110,9 @@ class RequestDispatcherTest extends AgentInstrumentationSpecification { th == ex assertTraces(1) { - trace(0, 3) { + trace(0, 2) { basicSpan(it, 0, "parent", null, ex) - span(1) { - name "TestDispatcher.$operation" - childOf span(0) - status ERROR - errorEvent(ex.class, ex.message) - } - basicSpan(it, 2, "$operation-child", span(1)) + basicSpan(it, 1, "$operation-child", span(0)) } } 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 44b90d6c01..eddb15d9b9 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 @@ -79,11 +79,6 @@ class SpringBootBasedTest extends HttpServerTest endpoint == NOT_FOUND } - @Override - int getErrorPageSpansCount(ServerEndpoint endpoint) { - 2 - } - @Override String expectedServerSpanName(ServerEndpoint endpoint) { switch (endpoint) { @@ -113,11 +108,10 @@ class SpringBootBasedTest extends HttpServerTest and: assertTraces(1) { - trace(0, 4) { + trace(0, 3) { serverSpan(it, 0, null, null, "GET", null, AUTH_ERROR) sendErrorSpan(it, 1, span(0)) - forwardSpan(it, 2, span(0)) - errorPageSpans(it, 3, null) + errorPageSpans(it, 2, null) } } } @@ -154,10 +148,6 @@ class SpringBootBasedTest extends HttpServerTest @Override void errorPageSpans(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { - if (endpoint == NOT_FOUND) { - forwardSpan(trace, index, trace.span(0)) - index++ - } trace.span(index) { name "BasicErrorController.error" kind INTERNAL diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/filter/ServletFilterTest.groovy b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/filter/ServletFilterTest.groovy index 99686b8eb6..3bf08af55c 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/filter/ServletFilterTest.groovy +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/filter/ServletFilterTest.groovy @@ -46,11 +46,6 @@ class ServletFilterTest extends HttpServerTest i endpoint == ERROR || endpoint == EXCEPTION || endpoint == NOT_FOUND } - @Override - int getErrorPageSpansCount(ServerEndpoint endpoint) { - 2 - } - @Override boolean hasResponseSpan(ServerEndpoint endpoint) { endpoint == REDIRECT || endpoint == ERROR || endpoint == NOT_FOUND @@ -96,18 +91,11 @@ class ServletFilterTest extends HttpServerTest i @Override void errorPageSpans(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { trace.span(index) { - name "ApplicationDispatcher.forward" + name "BasicErrorController.error" kind INTERNAL childOf((SpanData) parent) attributes { } } - trace.span(index + 1) { - name "BasicErrorController.error" - kind INTERNAL - childOf(trace.span(index)) - 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 3ec0d61fd4..3ea5556c66 100644 --- a/instrumentation/struts-2.3/javaagent/src/test/groovy/Struts2ActionSpanTest.groovy +++ b/instrumentation/struts-2.3/javaagent/src/test/groovy/Struts2ActionSpanTest.groovy @@ -136,10 +136,9 @@ class Struts2ActionSpanTest extends HttpServerTest implements AgentTestT and: assertTraces(1) { - trace(0, 3) { + trace(0, 2) { basicServerSpan(it, 0, getContextPath() + "/dispatch", null) basicSpan(it, 1, "GreetingAction.dispatch_servlet", span(0)) - basicSpan(it, 2, "Dispatcher.forward", span(0)) } } } diff --git a/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy index 5adc98017c..ad2c27e9a5 100644 --- a/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy +++ b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin14Test.groovy @@ -31,33 +31,28 @@ abstract class AbstractVaadin14Test extends AbstractVaadinTest { void assertFirstRequest() { assertTraces(VAADIN_14_4 ? 5 : 4) { def handlers = getRequestHandlers("BootstrapHandler") - trace(0, 3 + handlers.size()) { + trace(0, 2 + handlers.size()) { serverSpan(it, 0, getContextPath() + "/main") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) - basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) + basicSpan(it, 1, "SpringVaadinServletService.handleRequest", span(0)) - int spanIndex = 3 + int spanIndex = 2 handlers.each { handler -> - basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) + basicSpan(it, spanIndex++, handler + ".handleRequest", span(1)) } } // following traces are for javascript files used on page - trace(1, 2) { + trace(1, 1) { serverSpan(it, 0, getContextPath() + "/*") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } - trace(2, 2) { + trace(2, 1) { serverSpan(it, 0, getContextPath() + "/*") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } - trace(3, 2) { + trace(3, 1) { serverSpan(it, 0, getContextPath() + "/*") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } if (VAADIN_14_4) { - trace(4, 2) { + trace(4, 1) { serverSpan(it, 0, getContextPath() + "/*") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } } } @@ -67,14 +62,13 @@ abstract class AbstractVaadin14Test extends AbstractVaadinTest { void assertButtonClick() { assertTraces(1) { def handlers = getRequestHandlers("UidlRequestHandler") - trace(0, 3 + handlers.size() + 1) { + trace(0, 2 + handlers.size() + 1) { serverSpan(it, 0, getContextPath() + "/main") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) - basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) + basicSpan(it, 1, "SpringVaadinServletService.handleRequest", span(0)) - int spanIndex = 3 + int spanIndex = 2 handlers.each { handler -> - basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) + basicSpan(it, spanIndex++, handler + ".handleRequest", span(1)) } basicSpan(it, spanIndex, "EventRpcHandler.handle/click", span(spanIndex - 1)) } diff --git a/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy index 1398d41659..bc9de5ee6c 100644 --- a/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy +++ b/instrumentation/vaadin-14.2/testing/src/main/groovy/test/vaadin/AbstractVaadin16Test.groovy @@ -34,67 +34,58 @@ abstract class AbstractVaadin16Test extends AbstractVaadinTest { void assertFirstRequest() { assertTraces(VAADIN_17 ? 9 : 8) { def handlers = getRequestHandlers("IndexHtmlRequestHandler") - trace(0, 3 + handlers.size()) { + trace(0, 2 + handlers.size()) { serverSpan(it, 0, "IndexHtmlRequestHandler.handleRequest") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) - basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) - int spanIndex = 3 + basicSpan(it, 1, "SpringVaadinServletService.handleRequest", span(0)) + int spanIndex = 2 handlers.each { handler -> - basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) + basicSpan(it, spanIndex++, handler + ".handleRequest", span(1)) } } // /xyz/VAADIN/build/vaadin-bundle-*.cache.js - trace(1, 2) { + trace(1, 1) { serverSpan(it, 0, getContextPath() + "/*") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } if (VAADIN_17) { // /xyz/VAADIN/build/vaadin-devmodeGizmo-*.cache.js - trace(2, 2) { + trace(2, 1) { serverSpan(it, 0, getContextPath() + "/*") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } } int traceIndex = VAADIN_17 ? 3 : 2 handlers = getRequestHandlers("JavaScriptBootstrapHandler") - trace(traceIndex, 3 + handlers.size()) { + trace(traceIndex, 2 + handlers.size()) { serverSpan(it, 0, getContextPath()) - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) - basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) - int spanIndex = 3 + basicSpan(it, 1, "SpringVaadinServletService.handleRequest", span(0)) + int spanIndex = 2 handlers.each { handler -> - basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) + basicSpan(it, spanIndex++, handler + ".handleRequest", span(1)) } } // /xyz/VAADIN/build/vaadin-?-*.cache.js - trace(traceIndex + 1, 2) { + trace(traceIndex + 1, 1) { serverSpan(it, 0, getContextPath() + "/*") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } // /xyz/VAADIN/build/vaadin-?-*.cache.js - trace(traceIndex + 2, 2) { + trace(traceIndex + 2, 1) { serverSpan(it, 0, getContextPath() + "/*") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } // /xyz/VAADIN/build/vaadin-?-*.cache.js - trace(traceIndex + 3, 2) { + trace(traceIndex + 3, 1) { serverSpan(it, 0, getContextPath() + "/*") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } // /xyz/VAADIN/build/vaadin-?-*.cache.js - trace(traceIndex + 4, 2) { + trace(traceIndex + 4, 1) { serverSpan(it, 0, getContextPath() + "/*") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) } handlers = getRequestHandlers("UidlRequestHandler") - trace(traceIndex + 5, 3 + handlers.size() + 2) { + trace(traceIndex + 5, 2 + handlers.size() + 2) { serverSpan(it, 0, getContextPath() + "/main") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) - basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) + basicSpan(it, 1, "SpringVaadinServletService.handleRequest", span(0)) - int spanIndex = 3 + int spanIndex = 2 handlers.each { handler -> - basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) + basicSpan(it, spanIndex++, handler + ".handleRequest", span(1)) } basicSpan(it, spanIndex, "PublishedServerEventHandlerRpcHandler.handle", span(spanIndex - 1)) @@ -107,14 +98,13 @@ abstract class AbstractVaadin16Test extends AbstractVaadinTest { void assertButtonClick() { assertTraces(1) { def handlers = getRequestHandlers("UidlRequestHandler") - trace(0, 3 + handlers.size() + 1) { + trace(0, 2 + handlers.size() + 1) { serverSpan(it, 0, getContextPath() + "/main") - basicSpan(it, 1, "ApplicationDispatcher.forward", span(0)) - basicSpan(it, 2, "SpringVaadinServletService.handleRequest", span(1)) + basicSpan(it, 1, "SpringVaadinServletService.handleRequest", span(0)) - int spanIndex = 3 + int spanIndex = 2 handlers.each { handler -> - basicSpan(it, spanIndex++, handler + ".handleRequest", span(2)) + basicSpan(it, spanIndex++, handler + ".handleRequest", span(1)) } basicSpan(it, spanIndex, "EventRpcHandler.handle/click", span(spanIndex - 1)) 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 c22456e954..3a1ee239cd 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 @@ -64,14 +64,6 @@ abstract class HttpServerTest extends InstrumentationSpecification imple false } - boolean hasForwardSpan() { - false - } - - boolean hasIncludeSpan() { - false - } - int getErrorPageSpansCount(ServerEndpoint endpoint) { 1 } @@ -462,12 +454,6 @@ abstract class HttpServerTest extends InstrumentationSpecification imple if (hasRenderSpan(endpoint)) { spanCount++ } - if (hasForwardSpan()) { - spanCount++ - } - if (hasIncludeSpan()) { - spanCount++ - } } if (hasErrorPageSpans(endpoint)) { spanCount += getErrorPageSpansCount(endpoint) @@ -485,14 +471,6 @@ abstract class HttpServerTest extends InstrumentationSpecification imple if (hasHandlerSpan(endpoint)) { 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) @@ -557,30 +535,6 @@ abstract class HttpServerTest extends InstrumentationSpecification imple } } - void forwardSpan(TraceAssert trace, int index, Object parent, String errorMessage = null, Class exceptionClass = Exception) { - trace.span(index) { - name ~/\.forward$/ - kind SpanKind.INTERNAL - if (errorMessage) { - status StatusCode.ERROR - 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 SpanKind.INTERNAL - if (errorMessage) { - status StatusCode.ERROR - 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) { def extraAttributes = extraAttributes() @@ -596,7 +550,7 @@ abstract class HttpServerTest extends InstrumentationSpecification imple } else { hasNoParent() } - if (endpoint == EXCEPTION && hasExceptionOnServerSpan()) { + if (endpoint == EXCEPTION && hasExceptionOnServerSpan(endpoint)) { event(0) { eventName(SemanticAttributes.EXCEPTION_EVENT_NAME) attributes {