diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNaming.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNaming.java new file mode 100644 index 000000000000..cba442923d52 --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNaming.java @@ -0,0 +1,94 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.servlet; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextKey; +import io.opentelemetry.instrumentation.api.tracer.ServerSpan; +import java.util.function.Supplier; + +/** Helper container for tracking whether instrumentation should update server span name or not. */ +public final class ServerSpanNaming { + + private static final ContextKey<ServerSpanNaming> CONTEXT_KEY = + ContextKey.named("opentelemetry-servlet-span-naming-key"); + + public static Context init(Context context, Source initialSource) { + ServerSpanNaming serverSpanNaming = context.get(CONTEXT_KEY); + if (serverSpanNaming != null) { + // TODO (trask) does this ever happen? + serverSpanNaming.updatedBySource = initialSource; + return context; + } + return context.with(CONTEXT_KEY, new ServerSpanNaming(initialSource)); + } + + private volatile Source updatedBySource; + + private ServerSpanNaming(Source initialSource) { + this.updatedBySource = initialSource; + } + + /** + * If there is a server span in the context, and {@link #init(Context, Source)} has been called to + * populate a {@code ServerSpanName} into the context, then this method will update the server + * span name using the provided {@link Supplier} if and only if the last {@link Source} to update + * the span name using this method has strictly lower priority than the provided {@link Source}, + * and the value returned from the {@link Supplier} is non-null. + * + * <p>If there is a server span in the context, and {@link #init(Context, Source)} has NOT been + * called to populate a {@code ServerSpanName} into the context, then this method will update the + * server span name using the provided {@link Supplier} if the value returned from it is non-null. + */ + public static void updateServerSpanName( + Context context, Source source, Supplier<String> serverSpanName) { + Span serverSpan = ServerSpan.fromContextOrNull(context); + if (serverSpan == null) { + return; + } + ServerSpanNaming serverSpanNaming = context.get(CONTEXT_KEY); + if (serverSpanNaming == null) { + String name = serverSpanName.get(); + if (name != null) { + serverSpan.updateName(name); + } + return; + } + if (source.order > serverSpanNaming.updatedBySource.order) { + String name = serverSpanName.get(); + if (name != null) { + serverSpan.updateName(name); + serverSpanNaming.updatedBySource = source; + } + } + } + + // TODO (trask) migrate the one usage (ServletHttpServerTracer) to ServerSpanNaming.init() once we + // migrate to new Instrumenters (see + // https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/2814#discussion_r617351334 + // for the challenge with doing this now in the current Tracer structure, at least without some + // bigger changes, which we want to avoid in the Tracers as they are already deprecated) + @Deprecated + public static void updateSource(Context context, Source source) { + ServerSpanNaming serverSpanNaming = context.get(CONTEXT_KEY); + if (serverSpanNaming != null && source.order > serverSpanNaming.updatedBySource.order) { + serverSpanNaming.updatedBySource = source; + } + } + + public enum Source { + CONTAINER(1), + SERVLET(2), + CONTROLLER(3); + + private final int order; + + Source(int order) { + this.order = order; + } + } +} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServletSpanNaming.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServletSpanNaming.java deleted file mode 100644 index f5a88477f199..000000000000 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServletSpanNaming.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.servlet; - -import io.opentelemetry.context.Context; -import io.opentelemetry.context.ContextKey; - -/** - * Helper container for tracking whether servlet integration should update server span name or not. - */ -public class ServletSpanNaming { - - private static final ContextKey<ServletSpanNaming> CONTEXT_KEY = - ContextKey.named("opentelemetry-servlet-span-naming-key"); - - public static Context init(Context context) { - if (context.get(CONTEXT_KEY) != null) { - return context; - } - return context.with(CONTEXT_KEY, new ServletSpanNaming()); - } - - private volatile boolean servletUpdatedServerSpanName = false; - - private ServletSpanNaming() {} - - /** - * Returns true, if servlet integration should update server span name. After server span name has - * been updated with <code>setServletUpdatedServerSpanName</code> this method will return <code> - * false</code>. - * - * @param context server context - * @return <code>true</code>, if the server span name should be updated by servlet integration, or - * <code>false</code> otherwise. - */ - public static boolean shouldUpdateServerSpanName(Context context) { - ServletSpanNaming servletSpanNaming = context.get(CONTEXT_KEY); - if (servletSpanNaming != null) { - return !servletSpanNaming.servletUpdatedServerSpanName; - } - return false; - } - - /** - * Indicate that the servlet integration has updated the name for the server span. - * - * @param context server context - */ - public static void setServletUpdatedServerSpanName(Context context) { - ServletSpanNaming servletSpanNaming = context.get(CONTEXT_KEY); - if (servletSpanNaming != null) { - servletSpanNaming.servletUpdatedServerSpanName = true; - } - } -} diff --git a/instrumentation/armeria-1.3/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpServerTest.groovy b/instrumentation/armeria-1.3/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpServerTest.groovy index 4bf8da5ea904..75ba7ccdfe2c 100644 --- a/instrumentation/armeria-1.3/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpServerTest.groovy +++ b/instrumentation/armeria-1.3/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpServerTest.groovy @@ -49,6 +49,12 @@ abstract class AbstractArmeriaHttpServerTest extends HttpServerTest<Server> { ] } + @Override + boolean testNotFound() { + // currently span name is /notFound which indicates it won't be low-cardinality + false + } + @Override Server startServer(int port) { ServerBuilder sb = Server.builder() @@ -61,31 +67,31 @@ abstract class AbstractArmeriaHttpServerTest extends HttpServerTest<Server> { } } - sb.service(REDIRECT.path) {ctx, req -> + sb.service(REDIRECT.path) { ctx, req -> controller(REDIRECT) { HttpResponse.of(ResponseHeaders.of(HttpStatus.valueOf(REDIRECT.status), HttpHeaderNames.LOCATION, REDIRECT.body)) } } - sb.service(ERROR.path) {ctx, req -> + sb.service(ERROR.path) { ctx, req -> controller(ERROR) { HttpResponse.of(HttpStatus.valueOf(ERROR.status), MediaType.PLAIN_TEXT_UTF_8, ERROR.body) } } - sb.service(EXCEPTION.path) {ctx, req -> + sb.service(EXCEPTION.path) { ctx, req -> controller(EXCEPTION) { throw new Exception(EXCEPTION.body) } } - sb.service("/query") {ctx, req -> + sb.service("/query") { ctx, req -> controller(QUERY_PARAM) { HttpResponse.of(HttpStatus.valueOf(QUERY_PARAM.status), MediaType.PLAIN_TEXT_UTF_8, "some=${QueryParams.fromQueryString(ctx.query()).get("some")}") } } - sb.service("/path/:id/param") {ctx, req -> + sb.service("/path/:id/param") { ctx, req -> controller(PATH_PARAM) { HttpResponse.of(HttpStatus.valueOf(PATH_PARAM.status), MediaType.PLAIN_TEXT_UTF_8, ctx.pathParam("id")) } diff --git a/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/GrailsTracer.java b/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/GrailsTracer.java index 093f511bc777..eb7d72660b37 100644 --- a/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/GrailsTracer.java +++ b/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/GrailsTracer.java @@ -5,8 +5,10 @@ package io.opentelemetry.javaagent.instrumentation.grails; -import io.opentelemetry.api.trace.Span; +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER; + import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; import org.grails.web.mapping.mvc.GrailsControllerUrlMappingInfo; @@ -23,14 +25,17 @@ public Context startSpan(Object controller, String action) { return startSpan(spanNameForClass(controller.getClass()) + "." + action); } - public void nameServerSpan( - Context context, Span serverSpan, GrailsControllerUrlMappingInfo info) { + public void updateServerSpanName(Context context, GrailsControllerUrlMappingInfo info) { + ServerSpanNaming.updateServerSpanName( + context, CONTROLLER, () -> getServerSpanName(context, info)); + } + + private static String getServerSpanName(Context context, GrailsControllerUrlMappingInfo info) { String action = info.getActionName() != null ? info.getActionName() : info.getControllerClass().getDefaultAction(); - serverSpan.updateName( - ServletContextPath.prepend(context, "/" + info.getControllerName() + "/" + action)); + return ServletContextPath.prepend(context, "/" + info.getControllerName() + "/" + action); } @Override diff --git a/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/UrlMappingsInfoHandlerAdapterInstrumentation.java b/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/UrlMappingsInfoHandlerAdapterInstrumentation.java index 11f64072bb6a..41f717ee99a9 100644 --- a/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/UrlMappingsInfoHandlerAdapterInstrumentation.java +++ b/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/UrlMappingsInfoHandlerAdapterInstrumentation.java @@ -13,9 +13,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; -import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.tracer.ServerSpan; import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; import io.opentelemetry.javaagent.tooling.TypeInstrumentation; import java.util.Map; @@ -48,11 +46,7 @@ public static void nameSpan(@Advice.Argument(2) Object handler) { if (handler instanceof GrailsControllerUrlMappingInfo) { Context parentContext = Java8BytecodeBridge.currentContext(); - Span serverSpan = ServerSpan.fromContextOrNull(parentContext); - if (serverSpan != null) { - tracer() - .nameServerSpan(parentContext, serverSpan, (GrailsControllerUrlMappingInfo) handler); - } + tracer().updateServerSpanName(parentContext, (GrailsControllerUrlMappingInfo) handler); } } } 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 af0ec4d75081..b7916d1d9077 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 @@ -58,8 +58,8 @@ class GrailsTest extends HttpServerTest<ConfigurableApplicationContext> implemen return getContextPath() + "/test/path" } else if (endpoint == QUERY_PARAM) { return getContextPath() + "/test/query" - } else if (endpoint == ERROR || endpoint == EXCEPTION) { - return getContextPath() + "/error/index" + } else if (endpoint == ERROR) { + return getContextPath() + "/test/error" } else if (endpoint == NOT_FOUND) { return getContextPath() + "/**" } @@ -101,11 +101,6 @@ class GrailsTest extends HttpServerTest<ConfigurableApplicationContext> implemen endpoint == ERROR || endpoint == EXCEPTION || endpoint == NOT_FOUND } - @Override - int getErrorPageSpansCount(ServerEndpoint endpoint) { - endpoint == NOT_FOUND ? 0 : 1 - } - @Override boolean testPathParam() { true @@ -113,12 +108,10 @@ class GrailsTest extends HttpServerTest<ConfigurableApplicationContext> implemen @Override void errorPageSpans(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint) { - if (endpoint != NOT_FOUND) { - trace.span(index) { - name "ErrorController.index" - kind INTERNAL - attributes { - } + trace.span(index) { + name endpoint == NOT_FOUND ? "BasicErrorController.error" : "ErrorController.index" + kind INTERNAL + attributes { } } } diff --git a/instrumentation/grails-3.0/javaagent/src/test/groovy/test/UrlMappings.groovy b/instrumentation/grails-3.0/javaagent/src/test/groovy/test/UrlMappings.groovy index ef49ee310188..bb65ff069ceb 100644 --- a/instrumentation/grails-3.0/javaagent/src/test/groovy/test/UrlMappings.groovy +++ b/instrumentation/grails-3.0/javaagent/src/test/groovy/test/UrlMappings.groovy @@ -7,15 +7,14 @@ package test class UrlMappings { - static mappings = { - "/success"(controller: 'test', action: 'success') - "/query"(controller: 'test', action: 'query') - "/redirect"(controller: 'test', action: 'redirect') - "/error-status"(controller: 'test', action: 'error') - "/exception"(controller: 'test', action: 'exception') - "/path/$id/param"(controller: 'test', action: 'path') + static mappings = { + "/success"(controller: 'test', action: 'success') + "/query"(controller: 'test', action: 'query') + "/redirect"(controller: 'test', action: 'redirect') + "/error-status"(controller: 'test', action: 'error') + "/exception"(controller: 'test', action: 'exception') + "/path/$id/param"(controller: 'test', action: 'path') - "500"(controller: 'error') - "404"(controller: 'error', action: 'notFound') - } + "500"(controller: 'error') + } } diff --git a/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyFilterchainServerTest.groovy b/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyFilterchainServerTest.groovy index 315ef2acf3d0..6fe984c837b3 100644 --- a/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyFilterchainServerTest.groovy +++ b/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyFilterchainServerTest.groovy @@ -141,9 +141,6 @@ class GrizzlyFilterchainServerTest extends HttpServerTest<HttpServer> implements break case "/exception": throw new Exception(EXCEPTION.body) - case "/notFound": - endpoint = NOT_FOUND - break case "/query?some=query": endpoint = QUERY_PARAM break diff --git a/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11HttpServerTracer.java b/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11HttpServerTracer.java index 70f24f596d33..91c887523799 100644 --- a/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11HttpServerTracer.java +++ b/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11HttpServerTracer.java @@ -18,7 +18,7 @@ public static Jetty11HttpServerTracer tracer() { } public Context startServerSpan(HttpServletRequest request) { - return startSpan(request, "HTTP " + request.getMethod()); + return startSpan(request, "HTTP " + request.getMethod(), false); } @Override diff --git a/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8HttpServerTracer.java b/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8HttpServerTracer.java index b589501a44ca..de20c1b8e700 100644 --- a/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8HttpServerTracer.java +++ b/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8HttpServerTracer.java @@ -18,7 +18,7 @@ public static Jetty8HttpServerTracer tracer() { } public Context startServerSpan(HttpServletRequest request) { - return startSpan(request, "HTTP " + request.getMethod()); + return startSpan(request, "HTTP " + request.getMethod(), false); } @Override 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 be8f4cffbccd..d46a95689b1d 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 @@ -17,7 +17,7 @@ public static LibertyHttpServerTracer tracer() { } public Context startSpan(HttpServletRequest request) { - return startSpan(request, "HTTP " + request.getMethod()); + return startSpan(request, "HTTP " + request.getMethod(), false); } @Override diff --git a/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy b/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy index db5cd6278b62..5efe578d68d5 100644 --- a/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy +++ b/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy @@ -77,12 +77,4 @@ class GlassFishServerTest extends HttpServerTest<GlassFish> implements AgentTest break } } - - @Override - String expectedServerSpanName(ServerEndpoint endpoint) { - if (endpoint == NOT_FOUND) { - return getContextPath() + "/*" - } - return super.expectedServerSpanName(endpoint) - } } diff --git a/instrumentation/servlet/servlet-2.2/library/src/main/java/io/opentelemetry/instrumentation/servlet/v2_2/Servlet2HttpServerTracer.java b/instrumentation/servlet/servlet-2.2/library/src/main/java/io/opentelemetry/instrumentation/servlet/v2_2/Servlet2HttpServerTracer.java index 0900c3635c4d..dc7de3bc6415 100644 --- a/instrumentation/servlet/servlet-2.2/library/src/main/java/io/opentelemetry/instrumentation/servlet/v2_2/Servlet2HttpServerTracer.java +++ b/instrumentation/servlet/servlet-2.2/library/src/main/java/io/opentelemetry/instrumentation/servlet/v2_2/Servlet2HttpServerTracer.java @@ -5,10 +5,10 @@ package io.opentelemetry.instrumentation.servlet.v2_2; -import io.opentelemetry.api.trace.Span; +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.SERVLET; + import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.servlet.ServletSpanNaming; -import io.opentelemetry.instrumentation.api.tracer.ServerSpan; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.instrumentation.servlet.javax.JavaxServletHttpServerTracer; import javax.servlet.http.HttpServletRequest; @@ -24,20 +24,12 @@ public static Servlet2HttpServerTracer tracer() { } public Context startSpan(HttpServletRequest request) { - Context context = startSpan(request, getSpanName(request)); - // server span name shouldn't be update when server span was created by servlet 2.2 - // instrumentation - ServletSpanNaming.setServletUpdatedServerSpanName(context); - return context; + return startSpan(request, getSpanName(request), true); } + @Override public Context updateContext(Context context, HttpServletRequest request) { - Span span = ServerSpan.fromContextOrNull(context); - if (span != null && ServletSpanNaming.shouldUpdateServerSpanName(context)) { - span.updateName(getSpanName(request)); - ServletSpanNaming.setServletUpdatedServerSpanName(context); - } - + ServerSpanNaming.updateServerSpanName(context, SERVLET, () -> getSpanName(request)); return super.updateContext(context, request); } 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 34df55bd0986..e02c0a76507b 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 @@ -54,14 +54,6 @@ abstract class TomcatServlet3Test extends AbstractServlet3Test<Tomcat, Context> super.responseSpan(trace, index, parent, method, endpoint) } - @Override - String expectedServerSpanName(ServerEndpoint endpoint) { - if (endpoint == NOT_FOUND) { - return getContextPath() + "/*" - } - return super.expectedServerSpanName(endpoint) - } - @Shared def accessLogValue = new TestAccessLogValve() diff --git a/instrumentation/servlet/servlet-3.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/v3_0/Servlet3HttpServerTracer.java b/instrumentation/servlet/servlet-3.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/v3_0/Servlet3HttpServerTracer.java index 20dd96b20cbb..08a44a535c10 100644 --- a/instrumentation/servlet/servlet-3.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/v3_0/Servlet3HttpServerTracer.java +++ b/instrumentation/servlet/servlet-3.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/v3_0/Servlet3HttpServerTracer.java @@ -5,10 +5,10 @@ package io.opentelemetry.instrumentation.servlet.v3_0; -import io.opentelemetry.api.trace.Span; +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.SERVLET; + import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.servlet.ServletSpanNaming; -import io.opentelemetry.instrumentation.api.tracer.ServerSpan; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.instrumentation.servlet.MappingResolver; import io.opentelemetry.instrumentation.servlet.javax.JavaxServletHttpServerTracer; import java.util.Collection; @@ -31,13 +31,8 @@ public static Servlet3HttpServerTracer tracer() { } public Context startSpan(Object servletOrFilter, HttpServletRequest request) { - Context context = startSpan(request, getSpanName(servletOrFilter, request)); - // server span name shouldn't be update when server span was created from a call to Servlet - // if server span was created from a call to Filter then name may be updated from updateContext. - if (servletOrFilter instanceof Servlet) { - ServletSpanNaming.setServletUpdatedServerSpanName(context); - } - return context; + return startSpan( + request, getSpanName(servletOrFilter, request), servletOrFilter instanceof Servlet); } private String getSpanName(Object servletOrFilter, HttpServletRequest request) { @@ -116,15 +111,8 @@ private static MappingResolver getMappingResolver( public Context updateContext( Context context, Object servletOrFilter, HttpServletRequest request) { - Span span = ServerSpan.fromContextOrNull(context); - if (span != null && ServletSpanNaming.shouldUpdateServerSpanName(context)) { - String spanName = getSpanNameFromPath(servletOrFilter, request); - if (spanName != null) { - span.updateName(spanName); - ServletSpanNaming.setServletUpdatedServerSpanName(context); - } - } - + ServerSpanNaming.updateServerSpanName( + context, SERVLET, () -> getSpanNameFromPath(servletOrFilter, request)); return updateContext(context, request); } diff --git a/instrumentation/servlet/servlet-5.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/jakarta/v5_0/JakartaServletHttpServerTracer.java b/instrumentation/servlet/servlet-5.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/jakarta/v5_0/JakartaServletHttpServerTracer.java index 06b48ff6d7be..d4bca7c71ffe 100644 --- a/instrumentation/servlet/servlet-5.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/jakarta/v5_0/JakartaServletHttpServerTracer.java +++ b/instrumentation/servlet/servlet-5.0/library/src/main/java/io/opentelemetry/instrumentation/servlet/jakarta/v5_0/JakartaServletHttpServerTracer.java @@ -5,11 +5,11 @@ package io.opentelemetry.instrumentation.servlet.jakarta.v5_0; -import io.opentelemetry.api.trace.Span; +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.SERVLET; + import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.TextMapGetter; -import io.opentelemetry.instrumentation.api.servlet.ServletSpanNaming; -import io.opentelemetry.instrumentation.api.tracer.ServerSpan; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.instrumentation.servlet.MappingResolver; import io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer; import jakarta.servlet.RequestDispatcher; @@ -34,13 +34,8 @@ public static JakartaServletHttpServerTracer tracer() { } public Context startSpan(Object servletOrFilter, HttpServletRequest request) { - Context context = startSpan(request, getSpanName(servletOrFilter, request)); - // server span name shouldn't be update when server span was created from a call to Servlet - // if server span was created from a call to Filter then name may be updated from updateContext. - if (servletOrFilter instanceof Servlet) { - ServletSpanNaming.setServletUpdatedServerSpanName(context); - } - return context; + return startSpan( + request, getSpanName(servletOrFilter, request), servletOrFilter instanceof Servlet); } private String getSpanName(Object servletOrFilter, HttpServletRequest request) { @@ -119,15 +114,8 @@ private static MappingResolver getMappingResolver( public Context updateContext( Context context, Object servletOrFilter, HttpServletRequest request) { - Span span = ServerSpan.fromContextOrNull(context); - if (span != null && ServletSpanNaming.shouldUpdateServerSpanName(context)) { - String spanName = getSpanNameFromPath(servletOrFilter, request); - if (spanName != null) { - span.updateName(spanName); - ServletSpanNaming.setServletUpdatedServerSpanName(context); - } - } - + ServerSpanNaming.updateServerSpanName( + context, SERVLET, () -> getSpanNameFromPath(servletOrFilter, request)); return updateContext(context, request); } diff --git a/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java b/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java index 74366ea03ebb..d3b783268722 100644 --- a/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java +++ b/instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java @@ -5,6 +5,9 @@ package io.opentelemetry.instrumentation.servlet; +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTAINER; +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.SERVLET; + import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.api.trace.StatusCode; @@ -12,8 +15,8 @@ import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.instrumentation.api.config.Config; import io.opentelemetry.instrumentation.api.servlet.AppServerBridge; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; -import io.opentelemetry.instrumentation.api.servlet.ServletSpanNaming; import io.opentelemetry.instrumentation.api.tracer.HttpServerTracer; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.net.URI; @@ -37,7 +40,7 @@ public ServletHttpServerTracer(ServletAccessor<REQUEST, RESPONSE> accessor) { this.accessor = accessor; } - public Context startSpan(REQUEST request, String spanName) { + public Context startSpan(REQUEST request, String spanName, boolean servlet) { Context context = startSpan(request, request, request, spanName); SpanContext spanContext = Span.fromContext(context).getSpanContext(); @@ -45,14 +48,18 @@ public Context startSpan(REQUEST request, String spanName) { accessor.setRequestAttribute(request, "trace_id", spanContext.getTraceId()); accessor.setRequestAttribute(request, "span_id", spanContext.getSpanId()); + if (servlet) { + // server span name shouldn't be updated when server span was created from a call to Servlet + // (if created from a call to Filter then name may be updated from updateContext) + ServerSpanNaming.updateSource(context, SERVLET); + } return addServletContextPath(context, request); } @Override protected Context customizeContext(Context context, REQUEST request) { - // add context for tracking whether servlet instrumentation has updated - // server span - context = ServletSpanNaming.init(context); + // add context for tracking whether servlet instrumentation has updated the server span name + context = ServerSpanNaming.init(context, CONTAINER); // add context for current request's context path return addServletContextPath(context, request); } 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 c314b490b466..197c73b998d7 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 @@ -61,9 +61,10 @@ public static void nameResourceAndStartSpan( @Advice.Local("otelScope") Scope scope) { Context parentContext = Java8BytecodeBridge.currentContext(); Span serverSpan = ServerSpan.fromContextOrNull(parentContext); + // TODO (trask) is it important to check serverSpan != null here? if (serverSpan != null) { // Name the parent span based on the matching pattern - tracer().onRequest(parentContext, serverSpan, request); + tracer().updateServerSpanName(parentContext, request); // Now create a span for handler/controller execution. context = tracer().startHandlerSpan(parentContext, handler); if (context != null) { diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerMappingResourceNameFilter.java b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerMappingResourceNameFilter.java index a4c675544ec6..4376f52dd1b6 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerMappingResourceNameFilter.java +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerMappingResourceNameFilter.java @@ -7,9 +7,7 @@ import static io.opentelemetry.javaagent.instrumentation.springwebmvc.SpringWebMvcTracer.tracer; -import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.tracer.ServerSpan; import java.io.IOException; import java.util.List; import javax.servlet.Filter; @@ -41,15 +39,14 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha } Context context = Context.current(); - Span serverSpan = ServerSpan.fromContextOrNull(context); - if (handlerMappings != null && serverSpan != null) { + if (handlerMappings != null) { try { if (findMapping((HttpServletRequest) request)) { // Name the parent span based on the matching pattern // Let the parent span resource name be set with the attribute set in findMapping. - tracer().onRequest(context, serverSpan, (HttpServletRequest) request); + tracer().updateServerSpanName(context, (HttpServletRequest) request); } } catch (Exception ignored) { // mapping.getHandler() threw exception. Ignore diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcTracer.java b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcTracer.java index bd1e76855f27..8b4c4c7f932a 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcTracer.java +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcTracer.java @@ -6,11 +6,12 @@ package io.opentelemetry.javaagent.instrumentation.springwebmvc; import static io.opentelemetry.api.trace.SpanKind.INTERNAL; +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER; -import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.config.Config; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; import java.lang.reflect.Method; @@ -52,12 +53,15 @@ public Context startSpan(ModelAndView mv) { return parentContext.with(span.startSpan()); } - public void onRequest(Context context, Span span, HttpServletRequest request) { + public void updateServerSpanName(Context context, HttpServletRequest request) { if (request != null) { Object bestMatchingPattern = request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); if (bestMatchingPattern != null) { - span.updateName(ServletContextPath.prepend(context, bestMatchingPattern.toString())); + ServerSpanNaming.updateServerSpanName( + context, + CONTROLLER, + () -> ServletContextPath.prepend(context, bestMatchingPattern.toString())); } } } 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 eddb15d9b9d4..e98f236d9adf 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 @@ -84,9 +84,8 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext> switch (endpoint) { case PATH_PARAM: return getContextPath() + "/path/{id}/param" - case AUTH_ERROR: case NOT_FOUND: - return getContextPath() + "/error" + return getContextPath() + "/**" case LOGIN: return "HTTP POST" default: 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 3bf08af55c2d..e7c3e0d02389 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 @@ -80,12 +80,14 @@ class ServletFilterTest extends HttpServerTest<ConfigurableApplicationContext> i @Override String expectedServerSpanName(ServerEndpoint endpoint) { - if (endpoint == PATH_PARAM) { - return "/path/{id}/param" - } else if (endpoint == ERROR || endpoint == EXCEPTION || endpoint == NOT_FOUND) { - return "/error" + switch (endpoint) { + case PATH_PARAM: + return getContextPath() + "/path/{id}/param" + case NOT_FOUND: + return getContextPath() + "/**" + default: + return super.expectedServerSpanName(endpoint) } - return endpoint.resolvePath(address).path } @Override diff --git a/instrumentation/struts-2.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/struts2/Struts2Tracer.java b/instrumentation/struts-2.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/struts2/Struts2Tracer.java index cdc55d4c6a9e..cdb8db766ae1 100644 --- a/instrumentation/struts-2.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/struts2/Struts2Tracer.java +++ b/instrumentation/struts-2.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/struts2/Struts2Tracer.java @@ -6,16 +6,15 @@ package io.opentelemetry.javaagent.instrumentation.struts2; import static io.opentelemetry.api.trace.SpanKind.INTERNAL; +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER; import com.opensymphony.xwork2.ActionInvocation; import com.opensymphony.xwork2.ActionProxy; -import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; -import io.opentelemetry.instrumentation.api.servlet.ServletSpanNaming; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import io.opentelemetry.instrumentation.api.tracer.ServerSpan; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; public class Struts2Tracer extends BaseTracer { @@ -45,11 +44,11 @@ public Context startSpan(Context parentContext, ActionInvocation actionInvocatio // Handle cases where action parameters are encoded into URL path public void updateServerSpanName(Context context, ActionProxy actionProxy) { - Span serverSpan = ServerSpan.fromContextOrNull(context); - if (serverSpan == null) { - return; - } + ServerSpanNaming.updateServerSpanName( + context, CONTROLLER, () -> getServerSpanName(context, actionProxy)); + } + private static String getServerSpanName(Context context, ActionProxy actionProxy) { // We take name from the config, because it contains the path pattern from the // configuration. String result = actionProxy.getConfig().getName(); @@ -67,9 +66,7 @@ public void updateServerSpanName(Context context, ActionProxy actionProxy) { result = "/" + result; } - serverSpan.updateName(ServletContextPath.prepend(context, result)); - // prevent servlet integration from doing further updates to server span name - ServletSpanNaming.setServletUpdatedServerSpanName(context); + return ServletContextPath.prepend(context, result); } @Override diff --git a/instrumentation/tomcat/tomcat-10.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/TomcatHandlerTest.groovy b/instrumentation/tomcat/tomcat-10.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/TomcatHandlerTest.groovy index 9e6ef2c86e9e..b219226e532e 100644 --- a/instrumentation/tomcat/tomcat-10.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/TomcatHandlerTest.groovy +++ b/instrumentation/tomcat/tomcat-10.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/TomcatHandlerTest.groovy @@ -6,6 +6,7 @@ package io.opentelemetry.javaagent.instrumentation.tomcat.v10_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.AgentTestTrait @@ -30,6 +31,16 @@ class TomcatHandlerTest extends HttpServerTest<Tomcat> implements AgentTestTrait return "/app" } + @Override + String expectedServerSpanName(ServerEndpoint endpoint) { + switch (endpoint) { + case NOT_FOUND: + return "HTTP GET" + default: + return endpoint.resolvePath(address).path + } + } + @Override Tomcat startServer(int port) { Tomcat tomcat = new Tomcat() @@ -42,9 +53,11 @@ class TomcatHandlerTest extends HttpServerTest<Tomcat> implements AgentTestTrait Tomcat.addServlet(ctx, "testServlet", new TestServlet()) // Mapping servlet to /* will result in all requests have a name of just a context. - ServerEndpoint.values().each { - ctx.addServletMappingDecoded(it.path, "testServlet") - } + ServerEndpoint.values().toList().stream() + .filter { it != NOT_FOUND } + .forEach { + ctx.addServletMappingDecoded(it.path, "testServlet") + } (tomcat.host as StandardHost).errorReportValveClass = ErrorHandlerValve.name @@ -60,7 +73,7 @@ class TomcatHandlerTest extends HttpServerTest<Tomcat> implements AgentTestTrait @Override boolean hasResponseSpan(ServerEndpoint endpoint) { - endpoint == REDIRECT || endpoint == ERROR + endpoint == REDIRECT || endpoint == ERROR || endpoint == NOT_FOUND } @Override @@ -70,6 +83,7 @@ class TomcatHandlerTest extends HttpServerTest<Tomcat> implements AgentTestTrait redirectSpan(trace, index, parent) break case ERROR: + case NOT_FOUND: sendErrorSpan(trace, index, parent) break } diff --git a/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/TomcatHandlerTest.groovy b/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/TomcatHandlerTest.groovy index ef38df012275..d7af9bfc8a15 100644 --- a/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/TomcatHandlerTest.groovy +++ b/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/TomcatHandlerTest.groovy @@ -6,6 +6,7 @@ package io.opentelemetry.javaagent.instrumentation.tomcat.v7_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.AgentTestTrait @@ -30,6 +31,16 @@ class TomcatHandlerTest extends HttpServerTest<Tomcat> implements AgentTestTrait return "/app" } + @Override + String expectedServerSpanName(ServerEndpoint endpoint) { + switch (endpoint) { + case NOT_FOUND: + return "HTTP GET" + default: + return endpoint.resolvePath(address).path + } + } + @Override Tomcat startServer(int port) { Tomcat tomcat = new Tomcat() @@ -42,9 +53,11 @@ class TomcatHandlerTest extends HttpServerTest<Tomcat> implements AgentTestTrait Tomcat.addServlet(ctx, "testServlet", new TestServlet()) // Mapping servlet to /* will result in all requests have a name of just a context. - ServerEndpoint.values().each { - ctx.addServletMappingDecoded(it.path, "testServlet") - } + ServerEndpoint.values().toList().stream() + .filter { it != NOT_FOUND } + .forEach { + ctx.addServletMappingDecoded(it.path, "testServlet") + } (tomcat.host as StandardHost).errorReportValveClass = ErrorHandlerValve.name @@ -60,7 +73,7 @@ class TomcatHandlerTest extends HttpServerTest<Tomcat> implements AgentTestTrait @Override boolean hasResponseSpan(ServerEndpoint endpoint) { - endpoint == REDIRECT || endpoint == ERROR + endpoint == REDIRECT || endpoint == ERROR || endpoint == NOT_FOUND } @Override @@ -70,6 +83,7 @@ class TomcatHandlerTest extends HttpServerTest<Tomcat> implements AgentTestTrait redirectSpan(trace, index, parent) break case ERROR: + case NOT_FOUND: sendErrorSpan(trace, index, parent) break } diff --git a/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatTracer.java b/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatTracer.java index cf5270dca37f..9c1b3e0f268b 100644 --- a/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatTracer.java +++ b/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatTracer.java @@ -5,10 +5,12 @@ package io.opentelemetry.javaagent.instrumentation.tomcat.common; +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTAINER; + import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.instrumentation.api.servlet.AppServerBridge; -import io.opentelemetry.instrumentation.api.servlet.ServletSpanNaming; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.instrumentation.api.tracer.HttpServerTracer; import java.net.URI; import java.util.Collections; @@ -35,7 +37,7 @@ public Context startServerSpan(Request request) { @Override protected Context customizeContext(Context context, Request request) { - context = ServletSpanNaming.init(context); + context = ServerSpanNaming.init(context, CONTAINER); return AppServerBridge.init(context); } diff --git a/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java b/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java index a021b9aace4c..2378b5738ede 100644 --- a/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java +++ b/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java @@ -5,10 +5,12 @@ package io.opentelemetry.javaagent.instrumentation.undertow; +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTAINER; + import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.instrumentation.api.servlet.AppServerBridge; -import io.opentelemetry.instrumentation.api.servlet.ServletSpanNaming; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.instrumentation.api.tracer.HttpServerTracer; import io.opentelemetry.javaagent.instrumentation.api.undertow.KeyHolder; import io.opentelemetry.javaagent.instrumentation.api.undertow.UndertowActiveHandlers; @@ -39,7 +41,7 @@ public Context startServerSpan(HttpServerExchange exchange) { @Override protected Context customizeContext(Context context, HttpServerExchange exchange) { - context = ServletSpanNaming.init(context); + context = ServerSpanNaming.init(context, CONTAINER); // span is ended when counter reaches 0, we start from 2 which accounts for the // handler that started the span and exchange completion listener context = UndertowActiveHandlers.init(context, 2); diff --git a/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/RequestHandlerExecutorInstrumentation.java b/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/RequestHandlerExecutorInstrumentation.java index 7eb7ee0bb955..2bcb86ed5e15 100644 --- a/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/RequestHandlerExecutorInstrumentation.java +++ b/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/RequestHandlerExecutorInstrumentation.java @@ -5,13 +5,13 @@ package io.opentelemetry.javaagent.instrumentation.wicket; +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; -import io.opentelemetry.instrumentation.api.servlet.ServletSpanNaming; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.instrumentation.api.tracer.ServerSpan; import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; import io.opentelemetry.javaagent.tooling.TypeInstrumentation; @@ -23,7 +23,6 @@ import net.bytebuddy.matcher.ElementMatcher; import org.apache.wicket.core.request.handler.IPageClassRequestHandler; import org.apache.wicket.request.IRequestHandler; -import org.apache.wicket.request.cycle.RequestCycle; public class RequestHandlerExecutorInstrumentation implements TypeInstrumentation { @@ -49,14 +48,10 @@ public static void onExit(@Advice.Argument(0) IRequestHandler handler) { return; } if (handler instanceof IPageClassRequestHandler) { - // using class name as page name - String pageName = ((IPageClassRequestHandler) handler).getPageClass().getName(); - // wicket filter mapping without wildcard, if wicket filter is mapped to /* - // this will be an empty string - String filterPath = RequestCycle.get().getRequest().getFilterPath(); - serverSpan.updateName(ServletContextPath.prepend(context, filterPath + "/" + pageName)); - // prevent servlet integration from doing further updates to server span name - ServletSpanNaming.setServletUpdatedServerSpanName(context); + ServerSpanNaming.updateServerSpanName( + context, + CONTROLLER, + new ServerSpanNameSupplier(context, (IPageClassRequestHandler) handler)); } } } diff --git a/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/ServerSpanNameSupplier.java b/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/ServerSpanNameSupplier.java new file mode 100644 index 000000000000..d8e740be5e10 --- /dev/null +++ b/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/ServerSpanNameSupplier.java @@ -0,0 +1,33 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.wicket; + +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; +import java.util.function.Supplier; +import org.apache.wicket.core.request.handler.IPageClassRequestHandler; +import org.apache.wicket.request.cycle.RequestCycle; + +public class ServerSpanNameSupplier implements Supplier<String> { + + private final Context context; + private final IPageClassRequestHandler handler; + + public ServerSpanNameSupplier(Context context, IPageClassRequestHandler handler) { + this.context = context; + this.handler = handler; + } + + @Override + public String get() { + // using class name as page name + String pageName = ((IPageClassRequestHandler) handler).getPageClass().getName(); + // wicket filter mapping without wildcard, if wicket filter is mapped to /* + // this will be an empty string + String filterPath = RequestCycle.get().getRequest().getFilterPath(); + return ServletContextPath.prepend(context, filterPath + "/" + pageName); + } +} diff --git a/instrumentation/wicket-8.0/javaagent/src/test/groovy/WicketTest.groovy b/instrumentation/wicket-8.0/javaagent/src/test/groovy/WicketTest.groovy index 67e44a661924..b497ceb55c4b 100644 --- a/instrumentation/wicket-8.0/javaagent/src/test/groovy/WicketTest.groovy +++ b/instrumentation/wicket-8.0/javaagent/src/test/groovy/WicketTest.groovy @@ -82,7 +82,7 @@ class WicketTest extends AgentInstrumentationSpecification implements HttpServer assertTraces(1) { trace(0, 1) { - basicServerSpan(it, 0, getContextPath() + "/wicket-test/org.apache.wicket.markup.html.pages.InternalErrorPage", null, new Exception("test exception")) + basicServerSpan(it, 0, getContextPath() + "/wicket-test/hello.ExceptionPage", null, new Exception("test exception")) } } } 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 3a1ee239cdc7..2f01f8d04897 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 @@ -41,7 +41,14 @@ import spock.lang.Unroll abstract class HttpServerTest<SERVER> extends InstrumentationSpecification implements HttpServerTestTrait<SERVER> { String expectedServerSpanName(ServerEndpoint endpoint) { - return endpoint == PATH_PARAM ? getContextPath() + "/path/:id/param" : endpoint.resolvePath(address).path + switch (endpoint) { + case PATH_PARAM: + return getContextPath() + "/path/:id/param" + case NOT_FOUND: + return getContextPath() + "/*" + default: + return endpoint.resolvePath(address).path + } } String getContextPath() {