From 59369680c71ec3e0615b109981d1dc738d1c5d91 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Mon, 16 Sep 2024 14:36:42 +0300 Subject: [PATCH 1/2] Fix vert.x route containing dupicate segments when RoutingContext.next is used --- .../instrumentation/vertx/RouteHolder.java | 46 ++++++++++++++++ .../vertx/RoutingContextHandlerWrapper.java | 11 ++-- .../vertx/RoutingContextInstrumentation.java | 53 +++++++++++++++++++ .../vertx/RoutingContextUtil.java | 24 +++++++++ .../vertx/VertxWebInstrumentationModule.java | 4 +- .../java/server/AbstractVertxWebServer.java | 2 + 6 files changed, 131 insertions(+), 9 deletions(-) create mode 100644 instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RouteHolder.java create mode 100644 instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextInstrumentation.java create mode 100644 instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextUtil.java diff --git a/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RouteHolder.java b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RouteHolder.java new file mode 100644 index 000000000000..f8e5d337f117 --- /dev/null +++ b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RouteHolder.java @@ -0,0 +1,46 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.vertx; + +import static io.opentelemetry.context.ContextKey.named; + +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextKey; +import io.opentelemetry.context.ImplicitContextKeyed; + +public final class RouteHolder implements ImplicitContextKeyed { + private static final ContextKey KEY = named("opentelemetry-vertx-route"); + + private String route; + + private RouteHolder(String route) { + this.route = route; + } + + public static Context with(Context context, String route) { + if (context.get(KEY) != null) { + return context; + } + return context.with(new RouteHolder(route)); + } + + public static String get(Context context) { + RouteHolder holder = context.get(KEY); + return holder != null ? holder.route : null; + } + + public static void set(Context context, String route) { + RouteHolder holder = context.get(KEY); + if (holder != null) { + holder.route = route; + } + } + + @Override + public Context storeInContext(Context context) { + return context.with(KEY, this); + } +} diff --git a/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java index 0caddc3dae4b..44839bda4304 100644 --- a/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java +++ b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java @@ -5,11 +5,8 @@ package io.opentelemetry.javaagent.instrumentation.vertx; -import static io.opentelemetry.context.ContextKey.named; - import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; -import io.opentelemetry.context.ContextKey; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan; import io.opentelemetry.instrumentation.api.semconv.http.HttpServerRoute; @@ -24,8 +21,6 @@ /** This is used to wrap Vert.x Handlers to provide nice user-friendly SERVER span names */ public final class RoutingContextHandlerWrapper implements Handler { - private static final ContextKey ROUTE_KEY = named("opentelemetry-vertx-route"); - private final Handler handler; public RoutingContextHandlerWrapper(Handler handler) { @@ -35,13 +30,15 @@ public RoutingContextHandlerWrapper(Handler handler) { @Override public void handle(RoutingContext context) { Context otelContext = Context.current(); + // remember currently set route so it could be restored + RoutingContextUtil.setRoute(context, RouteHolder.get(otelContext)); String route = getRoute(otelContext, context); if (route != null && route.endsWith("/")) { route = route.substring(0, route.length() - 1); } HttpServerRoute.update(otelContext, HttpServerRouteSource.NESTED_CONTROLLER, route); - try (Scope ignore = otelContext.with(ROUTE_KEY, route).makeCurrent()) { + try (Scope ignore = RouteHolder.with(otelContext, route).makeCurrent()) { handler.handle(context); } catch (Throwable throwable) { Span serverSpan = LocalRootSpan.fromContextOrNull(otelContext); @@ -54,7 +51,7 @@ public void handle(RoutingContext context) { private static String getRoute(Context otelContext, RoutingContext routingContext) { String route = routingContext.currentRoute().getPath(); - String existingRoute = otelContext.get(ROUTE_KEY); + String existingRoute = RouteHolder.get(otelContext); return existingRoute != null ? existingRoute + route : route; } diff --git a/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextInstrumentation.java b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextInstrumentation.java new file mode 100644 index 000000000000..859b17156d03 --- /dev/null +++ b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextInstrumentation.java @@ -0,0 +1,53 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.vertx; + +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesNoArguments; + +import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import io.vertx.ext.web.RoutingContext; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +public class RoutingContextInstrumentation implements TypeInstrumentation { + @Override + public ElementMatcher classLoaderOptimization() { + return hasClassesNamed("io.vertx.ext.web.RoutingContext"); + } + + @Override + public ElementMatcher typeMatcher() { + return implementsInterface(named("io.vertx.ext.web.RoutingContext")); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + isPublic().and(named("next")).and(takesNoArguments()), + this.getClass().getName() + "$NextAdvice"); + } + + @SuppressWarnings("unused") + public static class NextAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void next(@Advice.This RoutingContext routingContext) { + // calling next tells router to move to the next matching route + // restore remembered route to remove currently matched route from it + String previousRoute = RoutingContextUtil.getRoute(routingContext); + if (previousRoute != null) { + RouteHolder.set(Java8BytecodeBridge.currentContext(), previousRoute); + } + } + } +} diff --git a/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextUtil.java b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextUtil.java new file mode 100644 index 000000000000..2707f29afcaf --- /dev/null +++ b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextUtil.java @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.vertx; + +import io.opentelemetry.instrumentation.api.util.VirtualField; +import io.vertx.ext.web.RoutingContext; + +public final class RoutingContextUtil { + private static final VirtualField routeField = + VirtualField.find(RoutingContext.class, String.class); + + public static void setRoute(RoutingContext routingContext, String route) { + routeField.set(routingContext, route); + } + + public static String getRoute(RoutingContext routingContext) { + return routeField.get(routingContext); + } + + private RoutingContextUtil() {} +} diff --git a/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/VertxWebInstrumentationModule.java b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/VertxWebInstrumentationModule.java index e0daaf344c06..659c2dfe507a 100644 --- a/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/VertxWebInstrumentationModule.java +++ b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/VertxWebInstrumentationModule.java @@ -5,7 +5,7 @@ package io.opentelemetry.javaagent.instrumentation.vertx; -import static java.util.Collections.singletonList; +import static java.util.Arrays.asList; import com.google.auto.service.AutoService; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; @@ -21,6 +21,6 @@ public VertxWebInstrumentationModule() { @Override public List typeInstrumentations() { - return singletonList(new RouteInstrumentation()); + return asList(new RouteInstrumentation(), new RoutingContextInstrumentation()); } } diff --git a/instrumentation/vertx/vertx-web-3.0/testing/src/main/java/server/AbstractVertxWebServer.java b/instrumentation/vertx/vertx-web-3.0/testing/src/main/java/server/AbstractVertxWebServer.java index b1791f238b3c..c25ebfb9f2b4 100644 --- a/instrumentation/vertx/vertx-web-3.0/testing/src/main/java/server/AbstractVertxWebServer.java +++ b/instrumentation/vertx/vertx-web-3.0/testing/src/main/java/server/AbstractVertxWebServer.java @@ -31,6 +31,8 @@ public abstract class AbstractVertxWebServer extends AbstractVerticle { public Router buildRouter() { Router router = Router.router(vertx); + // verify that calling RoutingContext::next doesn't affect http.route + router.route(SUCCESS.getPath()).handler(RoutingContext::next); //noinspection Convert2Lambda router .route(SUCCESS.getPath()) From 6625a563b02187f6900b8b2ae571272983c6a444 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 17 Sep 2024 10:07:06 +0300 Subject: [PATCH 2/2] rename method --- .../javaagent/instrumentation/vertx/RouteHolder.java | 2 +- .../instrumentation/vertx/RoutingContextHandlerWrapper.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RouteHolder.java b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RouteHolder.java index f8e5d337f117..0fcf8807f0a6 100644 --- a/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RouteHolder.java +++ b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RouteHolder.java @@ -20,7 +20,7 @@ private RouteHolder(String route) { this.route = route; } - public static Context with(Context context, String route) { + public static Context init(Context context, String route) { if (context.get(KEY) != null) { return context; } diff --git a/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java index 44839bda4304..a9120a5ee929 100644 --- a/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java +++ b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java @@ -38,7 +38,7 @@ public void handle(RoutingContext context) { } HttpServerRoute.update(otelContext, HttpServerRouteSource.NESTED_CONTROLLER, route); - try (Scope ignore = RouteHolder.with(otelContext, route).makeCurrent()) { + try (Scope ignore = RouteHolder.init(otelContext, route).makeCurrent()) { handler.handle(context); } catch (Throwable throwable) { Span serverSpan = LocalRootSpan.fromContextOrNull(otelContext);