Skip to content

Commit

Permalink
Use HttpRouteHolder in spring-webflux instrumentation (open-telemetry…
Browse files Browse the repository at this point in the history 
…#5239)
  • Loading branch information
@RashmiRam
Mateusz Rzeszutek authored and RashmiRam committed May 23, 2022
1 parent b431a75 commit bfdc693
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 43 deletions.
29 changes: 29 additions & 0 deletions ...src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,24 @@ public static <REQUEST> ContextCustomizer<REQUEST> get() {

private HttpRouteHolder() {}

/**
* Updates the {@code http.route} attribute in the received {@code context}.
*
* <p>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.
*
* <p>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}.
*
Expand Down Expand Up @@ -154,4 +172,15 @@ public String get(Context context, T arg, HttpRouteGetter<T> httpRouteGetter) {
return httpRouteGetter.get(context, arg);
}
}

private static final class ConstantAdapter implements HttpRouteGetter<String> {

private static final ConstantAdapter INSTANCE = new ConstantAdapter();

@Nullable
@Override
public String get(Context context, String route) {
return route;
}
}
}
1 change: 0 additions & 1 deletion instrumentation/spring/spring-webflux-5.0/javaagent/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
18 changes: 5 additions & 13 deletions ...emetry/javaagent/instrumentation/spring/webflux/server/HandlerAdapterInstrumentation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {

Expand Down Expand Up @@ -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;
Expand Down
30 changes: 12 additions & 18 deletions .../opentelemetry/javaagent/instrumentation/spring/webflux/server/RouteOnSuccessOrError.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -21,30 +21,20 @@ public class RouteOnSuccessOrError implements BiConsumer<HandlerFunction<?>, 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
Expand All @@ -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
Expand Down
12 changes: 12 additions & 0 deletions ...a/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/WebfluxSingletons.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -33,5 +37,13 @@ public static Instrumenter<Object, Void> instrumenter() {
return INSTRUMENTER;
}

public static HttpRouteGetter<ServerWebExchange> httpRouteGetter() {
return (context, exchange) -> {
PathPattern bestPattern =
exchange.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE);
return bestPattern == null ? null : bestPattern.getPatternString();
};
}

private WebfluxSingletons() {}
}
9 changes: 9 additions & 0 deletions instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/SpringWebfluxTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
13 changes: 2 additions & 11 deletions ...g/spring-webflux-5.0/javaagent/src/test/groovy/server/base/SpringWebFluxServerTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -36,21 +34,14 @@ abstract class SpringWebFluxServerTest extends HttpServerTest<ConfigurableApplic
}

@Override
Set<AttributeKey<?>> 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)
}
}

Expand Down

0 comments on commit bfdc693

Please sign in to comment.