Fix vert.x route containing dupicate segments when RoutingContext.next is used (#12260)

Author: laurit

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<RouteHolder> KEY = named("opentelemetry-vertx-route");
+
+  private String route;
+
+  private RouteHolder(String route) {
+    this.route = route;
+  }
+
+  public static Context init(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);
+  }
+}

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<RoutingContext> {
 
-  private static final ContextKey<String> ROUTE_KEY = named("opentelemetry-vertx-route");
-
   private final Handler<RoutingContext> handler;
 
   public RoutingContextHandlerWrapper(Handler<RoutingContext> handler) {
@@ -35,13 +30,15 @@ public RoutingContextHandlerWrapper(Handler<RoutingContext> 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.init(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;
   }
 

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<ClassLoader> classLoaderOptimization() {
+    return hasClassesNamed("io.vertx.ext.web.RoutingContext");
+  }
+
+  @Override
+  public ElementMatcher<TypeDescription> 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);
+      }
+    }
+  }
+}

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<RoutingContext, String> 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() {}
+}

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<TypeInstrumentation> typeInstrumentations() {
-    return singletonList(new RouteInstrumentation());
+    return asList(new RouteInstrumentation(), new RoutingContextInstrumentation());
   }
 }

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())