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 index 0ccf9623e729..c7d4c24970b6 100644 --- 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 @@ -50,7 +50,9 @@ private ServerSpanNaming(Source initialSource) { public static void updateServerSpanName( Context context, Source source, Supplier serverSpanName) { Span serverSpan = ServerSpan.fromContextOrNull(context); - if (serverSpan == null) { + // checking isRecording() is a helpful optimization for more expensive suppliers + // (e.g. Spring MVC instrumentation's HandlerAdapterInstrumentation) + if (serverSpan == null || !serverSpan.isRecording()) { return; } ServerSpanNaming serverSpanNaming = context.get(CONTEXT_KEY); 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 20ace6889670..b0e4cabfd8df 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 @@ -5,6 +5,7 @@ package io.opentelemetry.javaagent.instrumentation.springwebmvc; +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; import static io.opentelemetry.javaagent.instrumentation.springwebmvc.SpringWebMvcSingletons.handlerInstrumenter; @@ -18,6 +19,7 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.instrumentation.api.tracer.ServerSpan; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; @@ -69,7 +71,10 @@ public static void nameResourceAndStartSpan( // TODO (trask) is it important to check serverSpan != null here? if (serverSpan != null) { // Name the parent span based on the matching pattern - ServerNameUpdater.updateServerSpanName(parentContext, request); + ServerSpanNaming.updateServerSpanName( + parentContext, + CONTROLLER, + SpringWebMvcServerSpanNaming.getServerSpanNameSupplier(parentContext, request)); // Now create a span for handler/controller execution. context = handlerInstrumenter().start(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 7a333d31d540..98f8e499bf7a 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 @@ -5,7 +5,10 @@ package io.opentelemetry.javaagent.instrumentation.springwebmvc; +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER; + import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -38,19 +41,20 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha return; } - Context context = Context.current(); - 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. - ServerNameUpdater.updateServerSpanName(context, (HttpServletRequest) request); - } - } catch (Exception ignored) { - // mapping.getHandler() threw exception. Ignore - } + Context context = Context.current(); + ServerSpanNaming.updateServerSpanName( + context, + CONTROLLER, + () -> { + 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. + return SpringWebMvcServerSpanNaming.getServerSpanName( + context, (HttpServletRequest) request); + } + return null; + }); } filterChain.doFilter(request, response); @@ -64,12 +68,16 @@ public void destroy() {} * as an attribute on the request. This attribute is read by SpringWebMvcDecorator.onRequest and * set as the resource name. */ - private boolean findMapping(HttpServletRequest request) throws Exception { - for (HandlerMapping mapping : handlerMappings) { - HandlerExecutionChain handler = mapping.getHandler(request); - if (handler != null) { - return true; + private boolean findMapping(HttpServletRequest request) { + try { + for (HandlerMapping mapping : handlerMappings) { + HandlerExecutionChain handler = mapping.getHandler(request); + if (handler != null) { + return true; + } } + } catch (Exception ignored) { + // mapping.getHandler() threw exception. Ignore } return false; } diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/ServerNameUpdater.java b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/ServerNameUpdater.java deleted file mode 100644 index 06421eec92f3..000000000000 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/ServerNameUpdater.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.springwebmvc; - -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 javax.servlet.http.HttpServletRequest; -import org.springframework.web.servlet.HandlerMapping; - -public class ServerNameUpdater { - - public static void updateServerSpanName(Context context, HttpServletRequest request) { - if (request != null) { - Object bestMatchingPattern = - request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); - if (bestMatchingPattern != null) { - ServerSpanNaming.updateServerSpanName( - context, - CONTROLLER, - () -> ServletContextPath.prepend(context, bestMatchingPattern.toString())); - } - } - } -} diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcServerSpanNaming.java b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcServerSpanNaming.java new file mode 100644 index 000000000000..4cf635c6aa1a --- /dev/null +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcServerSpanNaming.java @@ -0,0 +1,29 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.springwebmvc; + +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; +import java.util.function.Supplier; +import javax.servlet.http.HttpServletRequest; +import org.springframework.web.servlet.HandlerMapping; + +public class SpringWebMvcServerSpanNaming { + + public static Supplier getServerSpanNameSupplier( + Context context, HttpServletRequest request) { + return () -> getServerSpanName(context, request); + } + + public static String getServerSpanName(Context context, HttpServletRequest request) { + Object bestMatchingPattern = + request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); + if (bestMatchingPattern != null) { + return ServletContextPath.prepend(context, bestMatchingPattern.toString()); + } + return null; + } +}