diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolder.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolder.java index ddf3ed74b0e8..f26a3f3f452b 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolder.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolder.java @@ -46,6 +46,24 @@ public static ContextCustomizer get() { private HttpRouteHolder() {} + /** + * Updates the {@code http.route} attribute in the received {@code context}. + * + *

If there is a server span in the context, and the context has been customized with a {@link + * HttpRouteHolder}, then this method will update the route using the provided {@code httpRoute} + * if and only if the last {@link HttpRouteSource} to update the route using this method has + * strictly lower priority than the provided {@link HttpRouteSource}, and the pased value is + * non-null. + * + *

If there is a server span in the context, and the context has NOT been customized with a + * {@link HttpRouteHolder}, then this method will update the route using the provided value if it + * is non-null. + */ + public static void updateHttpRoute( + Context context, HttpRouteSource source, @Nullable String httpRoute) { + updateHttpRoute(context, source, ConstantAdapter.INSTANCE, httpRoute); + } + /** * Updates the {@code http.route} attribute in the received {@code context}. * @@ -154,4 +172,15 @@ public String get(Context context, T arg, HttpRouteGetter httpRouteGetter) { return httpRouteGetter.get(context, arg); } } + + private static final class ConstantAdapter implements HttpRouteGetter { + + private static final ConstantAdapter INSTANCE = new ConstantAdapter(); + + @Nullable + @Override + public String get(Context context, String route) { + return route; + } + } } diff --git a/instrumentation/spring/spring-webflux-5.0/javaagent/build.gradle.kts b/instrumentation/spring/spring-webflux-5.0/javaagent/build.gradle.kts index 45d286d11fb2..f9899791d83e 100644 --- a/instrumentation/spring/spring-webflux-5.0/javaagent/build.gradle.kts +++ b/instrumentation/spring/spring-webflux-5.0/javaagent/build.gradle.kts @@ -40,7 +40,6 @@ muzzle { dependencies { implementation(project(":instrumentation:spring:spring-webflux-5.0:library")) - bootstrap(project(":instrumentation:servlet:servlet-common:bootstrap")) compileOnly("org.springframework:spring-webflux:5.0.0.RELEASE") compileOnly("io.projectreactor.ipc:reactor-netty:0.7.0.RELEASE") diff --git a/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/HandlerAdapterInstrumentation.java b/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/HandlerAdapterInstrumentation.java index d2ebddcfce06..f42403e11a00 100644 --- a/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/HandlerAdapterInstrumentation.java +++ b/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/HandlerAdapterInstrumentation.java @@ -7,6 +7,7 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.instrumentation.spring.webflux.server.WebfluxSingletons.httpRouteGetter; import static io.opentelemetry.javaagent.instrumentation.spring.webflux.server.WebfluxSingletons.instrumenter; import static net.bytebuddy.matcher.ElementMatchers.isAbstract; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -16,19 +17,16 @@ 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.context.Scope; -import io.opentelemetry.instrumentation.api.server.ServerSpan; -import io.opentelemetry.javaagent.bootstrap.servlet.ServletContextPath; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; -import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.server.ServerWebExchange; -import org.springframework.web.util.pattern.PathPattern; public class HandlerAdapterInstrumentation implements TypeInstrumentation { @@ -67,14 +65,8 @@ public static void methodEnter( Context parentContext = Context.current(); - Span serverSpan = ServerSpan.fromContextOrNull(parentContext); - - PathPattern bestPattern = - exchange.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); - if (serverSpan != null && bestPattern != null) { - serverSpan.updateName( - ServletContextPath.prepend(Context.current(), bestPattern.toString())); - } + HttpRouteHolder.updateHttpRoute( + parentContext, HttpRouteSource.CONTROLLER, httpRouteGetter(), exchange); if (handler == null) { return; diff --git a/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/RouteOnSuccessOrError.java b/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/RouteOnSuccessOrError.java index e3b886fdfcce..53f6e96c2abc 100644 --- a/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/RouteOnSuccessOrError.java +++ b/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/RouteOnSuccessOrError.java @@ -5,12 +5,12 @@ package io.opentelemetry.javaagent.instrumentation.spring.webflux.server; -import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.server.ServerSpan; -import io.opentelemetry.javaagent.bootstrap.servlet.ServletContextPath; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource; import java.util.function.BiConsumer; import java.util.regex.Pattern; +import javax.annotation.Nullable; import org.springframework.web.reactive.function.server.HandlerFunction; import org.springframework.web.reactive.function.server.RouterFunction; @@ -21,30 +21,20 @@ public class RouteOnSuccessOrError implements BiConsumer, Thr private static final Pattern METHOD_REGEX = Pattern.compile("^(GET|HEAD|POST|PUT|DELETE|CONNECT|OPTIONS|TRACE|PATCH) "); - private final RouterFunction routerFunction; + @Nullable private final String route; public RouteOnSuccessOrError(RouterFunction routerFunction) { - this.routerFunction = routerFunction; + this.route = parseRoute(parsePredicateString(routerFunction)); } @Override public void accept(HandlerFunction handler, Throwable throwable) { if (handler != null) { - String predicateString = parsePredicateString(); - if (predicateString != null) { - Context context = Context.current(); - if (context != null) { - Span serverSpan = ServerSpan.fromContextOrNull(context); - if (serverSpan != null) { - // TODO should update SERVER span name/route using ServerSpanNaming - serverSpan.updateName(ServletContextPath.prepend(context, parseRoute(predicateString))); - } - } - } + HttpRouteHolder.updateHttpRoute(Context.current(), HttpRouteSource.CONTROLLER, route); } } - private String parsePredicateString() { + private static String parsePredicateString(RouterFunction routerFunction) { String routerFunctionString = routerFunction.toString(); // Router functions containing lambda predicates should not end up in span tags since they are // confusing @@ -56,7 +46,11 @@ private String parsePredicateString() { } } - private static String parseRoute(String routerString) { + @Nullable + private static String parseRoute(@Nullable String routerString) { + if (routerString == null) { + return null; + } return METHOD_REGEX .matcher( SPACES_REGEX diff --git a/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/WebfluxSingletons.java b/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/WebfluxSingletons.java index a36078a796e9..5037a3801a9f 100644 --- a/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/WebfluxSingletons.java +++ b/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/WebfluxSingletons.java @@ -9,7 +9,11 @@ import io.opentelemetry.instrumentation.api.config.ExperimentalConfig; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteGetter; import io.opentelemetry.javaagent.instrumentation.spring.webflux.SpringWebfluxConfig; +import org.springframework.web.reactive.HandlerMapping; +import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.util.pattern.PathPattern; public final class WebfluxSingletons { private static final String INSTRUMENTATION_NAME = "io.opentelemetry.spring-webflux-5.0"; @@ -33,5 +37,13 @@ public static Instrumenter instrumenter() { return INSTRUMENTER; } + public static HttpRouteGetter httpRouteGetter() { + return (context, exchange) -> { + PathPattern bestPattern = + exchange.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); + return bestPattern == null ? null : bestPattern.getPatternString(); + }; + } + private WebfluxSingletons() {} } diff --git a/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/SpringWebfluxTest.groovy b/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/SpringWebfluxTest.groovy index 516b5aa5974b..bc60631e4309 100644 --- a/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/SpringWebfluxTest.groovy +++ b/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/SpringWebfluxTest.groovy @@ -91,6 +91,7 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification { "$SemanticAttributes.HTTP_SCHEME" "http" "$SemanticAttributes.HTTP_FLAVOR" "1.1" "$SemanticAttributes.HTTP_USER_AGENT" String + "$SemanticAttributes.HTTP_ROUTE" urlPathWithVariables } } span(1) { @@ -157,6 +158,7 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification { "$SemanticAttributes.HTTP_SCHEME" "http" "$SemanticAttributes.HTTP_FLAVOR" "1.1" "$SemanticAttributes.HTTP_USER_AGENT" String + "$SemanticAttributes.HTTP_ROUTE" urlPathWithVariables } } span(1) { @@ -243,6 +245,7 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification { "$SemanticAttributes.HTTP_SCHEME" "http" "$SemanticAttributes.HTTP_FLAVOR" "1.1" "$SemanticAttributes.HTTP_USER_AGENT" String + "$SemanticAttributes.HTTP_ROUTE" urlPathWithVariables } } span(1) { @@ -307,6 +310,7 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification { "$SemanticAttributes.HTTP_SCHEME" "http" "$SemanticAttributes.HTTP_FLAVOR" "1.1" "$SemanticAttributes.HTTP_USER_AGENT" String + "$SemanticAttributes.HTTP_ROUTE" "/**" } } span(1) { @@ -350,6 +354,7 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification { "$SemanticAttributes.HTTP_SCHEME" "http" "$SemanticAttributes.HTTP_FLAVOR" "1.1" "$SemanticAttributes.HTTP_USER_AGENT" String + "$SemanticAttributes.HTTP_ROUTE" "/echo" } } span(1) { @@ -398,6 +403,7 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification { "$SemanticAttributes.HTTP_SCHEME" "http" "$SemanticAttributes.HTTP_FLAVOR" "1.1" "$SemanticAttributes.HTTP_USER_AGENT" String + "$SemanticAttributes.HTTP_ROUTE" urlPathWithVariables } } span(1) { @@ -461,6 +467,7 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification { "$SemanticAttributes.HTTP_SCHEME" "http" "$SemanticAttributes.HTTP_FLAVOR" "1.1" "$SemanticAttributes.HTTP_USER_AGENT" String + "$SemanticAttributes.HTTP_ROUTE" "/double-greet-redirect" } } span(1) { @@ -492,6 +499,7 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification { "$SemanticAttributes.HTTP_SCHEME" "http" "$SemanticAttributes.HTTP_FLAVOR" "1.1" "$SemanticAttributes.HTTP_USER_AGENT" String + "$SemanticAttributes.HTTP_ROUTE" "/double-greet" } } span(1) { @@ -538,6 +546,7 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification { "$SemanticAttributes.HTTP_SCHEME" "http" "$SemanticAttributes.HTTP_FLAVOR" "1.1" "$SemanticAttributes.HTTP_USER_AGENT" String + "$SemanticAttributes.HTTP_ROUTE" urlPathWithVariables } } span(1) { diff --git a/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/SpringWebFluxServerTest.groovy b/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/SpringWebFluxServerTest.groovy index 8c96af8608bd..1cfa22580153 100644 --- a/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/SpringWebFluxServerTest.groovy +++ b/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/SpringWebFluxServerTest.groovy @@ -5,10 +5,8 @@ package server.base -import io.opentelemetry.api.common.AttributeKey import io.opentelemetry.instrumentation.test.AgentTestTrait import io.opentelemetry.instrumentation.test.base.HttpServerTest -import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import org.springframework.boot.SpringApplication import org.springframework.context.ConfigurableApplicationContext @@ -36,21 +34,14 @@ abstract class SpringWebFluxServerTest extends HttpServerTest> httpAttributes(ServerEndpoint endpoint) { - def attributes = super.httpAttributes(endpoint) - attributes.remove(SemanticAttributes.HTTP_ROUTE) - attributes - } - - @Override - String expectedServerSpanName(ServerEndpoint endpoint, String method) { + String expectedHttpRoute(ServerEndpoint endpoint) { switch (endpoint) { case PATH_PARAM: return getContextPath() + "/path/{id}/param" case NOT_FOUND: return "/**" default: - return endpoint.resolvePath(address).path + return super.expectedHttpRoute(endpoint) } }