Use HttpRouteHolder in vertx-web instrumentation (#5240)

* Use HttpRouteHolder in vertx-web instrumentation * Fix vertx-reactive tests
open-telemetry · Jan 27, 2022 · 6c855d9 · 6c855d9
1 parent 8edd8f4
commit 6c855d9
Show file tree

Hide file tree

Showing 6 changed files with 18 additions and 82 deletions.
diff --git a/...vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/VertxReactivePropagationTest.groovy b/...vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/VertxReactivePropagationTest.groovy
@@ -74,6 +74,7 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
             "$SemanticAttributes.HTTP_SCHEME" "http"
             "$SemanticAttributes.HTTP_FLAVOR" "1.1"
             "$SemanticAttributes.HTTP_USER_AGENT" String
+            "$SemanticAttributes.HTTP_ROUTE" "/listProducts"
           }
         }
         span(1) {
@@ -164,6 +165,7 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
               "$SemanticAttributes.HTTP_SCHEME" "http"
               "$SemanticAttributes.HTTP_FLAVOR" "1.1"
               "$SemanticAttributes.HTTP_USER_AGENT" String
+              "$SemanticAttributes.HTTP_ROUTE" "/listProducts"
               "${TEST_REQUEST_ID_ATTRIBUTE}" requestId
             }
           }

diff --git a/...vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/server/VertxRxHttpServerTest.groovy b/...vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/server/VertxRxHttpServerTest.groovy
@@ -5,10 +5,9 @@
 
 package server
 
-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 io.vertx.core.DeploymentOptions
 import io.vertx.core.Promise
 import io.vertx.core.Vertx
@@ -24,7 +23,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.INDEXED_CHILD
-import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT
@@ -64,25 +62,6 @@ class VertxRxHttpServerTest extends HttpServerTest<Vertx> implements AgentTestTr
     return true
   }
 
-  @Override
-  Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
-    def attributes = super.httpAttributes(endpoint)
-    attributes.remove(SemanticAttributes.HTTP_ROUTE)
-    attributes
-  }
-
-  @Override
-  String expectedServerSpanName(ServerEndpoint endpoint, String method) {
-    switch (endpoint) {
-      case PATH_PARAM:
-        return "/path/:id/param"
-      case NOT_FOUND:
-        return "HTTP GET"
-      default:
-        return endpoint.getPath()
-    }
-  }
-
   @Override
   boolean verifyServerSpanEndTime() {
     // server spans are ended inside of the controller spans

diff --git a/...vertx-reactive-3.5/javaagent/src/version35Test/groovy/VertxReactivePropagationTest.groovy b/...vertx-reactive-3.5/javaagent/src/version35Test/groovy/VertxReactivePropagationTest.groovy
@@ -74,6 +74,7 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
             "$SemanticAttributes.HTTP_SCHEME" "http"
             "$SemanticAttributes.HTTP_FLAVOR" "1.1"
             "$SemanticAttributes.HTTP_USER_AGENT" String
+            "$SemanticAttributes.HTTP_ROUTE" "/listProducts"
           }
         }
         span(1) {
@@ -164,6 +165,7 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
               "$SemanticAttributes.HTTP_SCHEME" "http"
               "$SemanticAttributes.HTTP_FLAVOR" "1.1"
               "$SemanticAttributes.HTTP_USER_AGENT" String
+              "$SemanticAttributes.HTTP_ROUTE" "/listProducts"
               "${TEST_REQUEST_ID_ATTRIBUTE}" requestId
             }
           }

diff --git a/...vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxHttpServerTest.groovy b/...vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxHttpServerTest.groovy
@@ -5,10 +5,9 @@
 
 package server
 
-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 io.vertx.core.DeploymentOptions
 import io.vertx.core.Future
 import io.vertx.core.Vertx
@@ -24,7 +23,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.INDEXED_CHILD
-import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT
@@ -64,25 +62,6 @@ class VertxRxHttpServerTest extends HttpServerTest<Vertx> implements AgentTestTr
     return true
   }
 
-  @Override
-  Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
-    def attributes = super.httpAttributes(endpoint)
-    attributes.remove(SemanticAttributes.HTTP_ROUTE)
-    attributes
-  }
-
-  @Override
-  String expectedServerSpanName(ServerEndpoint endpoint, String method) {
-    switch (endpoint) {
-      case PATH_PARAM:
-        return "/path/:id/param"
-      case NOT_FOUND:
-        return "HTTP GET"
-      default:
-        return endpoint.getPath()
-    }
-  }
-
   @Override
   boolean verifyServerSpanEndTime() {
     // server spans are ended inside of the controller spans

diff --git a/...n/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java b/...n/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java
@@ -7,21 +7,19 @@
 
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder;
+import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource;
 import io.opentelemetry.instrumentation.api.server.ServerSpan;
 import io.vertx.core.Handler;
 import io.vertx.ext.web.RoutingContext;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.UndeclaredThrowableException;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.ExecutionException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /** 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 Logger logger = LoggerFactory.getLogger(RoutingContextHandlerWrapper.class);
-
   private final Handler<RoutingContext> handler;
 
   public RoutingContextHandlerWrapper(Handler<RoutingContext> handler) {
@@ -30,25 +28,25 @@ public RoutingContextHandlerWrapper(Handler<RoutingContext> handler) {
 
   @Override
   public void handle(RoutingContext context) {
-    Span serverSpan = ServerSpan.fromContextOrNull(Context.current());
-    try {
-      if (serverSpan != null) {
-        // TODO should update SERVER span name/route using ServerSpanNaming
-        serverSpan.updateName(context.currentRoute().getPath());
-      }
-    } catch (RuntimeException ex) {
-      logger.error("Failed to update server span name with vert.x route", ex);
-    }
+    Context otelContext = Context.current();
+    HttpRouteHolder.updateHttpRoute(
+        otelContext, HttpRouteSource.CONTROLLER, RoutingContextHandlerWrapper::getRoute, context);
+
     try {
       handler.handle(context);
     } catch (Throwable throwable) {
+      Span serverSpan = ServerSpan.fromContextOrNull(otelContext);
       if (serverSpan != null) {
         serverSpan.recordException(unwrapThrowable(throwable));
       }
       throw throwable;
     }
   }
 
+  private static String getRoute(Context otelContext, RoutingContext routingContext) {
+    return routingContext.currentRoute().getPath();
+  }
+
   private static Throwable unwrapThrowable(Throwable throwable) {
     if (throwable.getCause() != null
         && (throwable instanceof ExecutionException

diff --git a/...ion/vertx/vertx-web-3.0/testing/src/main/groovy/server/AbstractVertxHttpServerTest.groovy b/...ion/vertx/vertx-web-3.0/testing/src/main/groovy/server/AbstractVertxHttpServerTest.groovy
@@ -5,10 +5,9 @@
 
 package server
 
-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 io.vertx.core.AbstractVerticle
 import io.vertx.core.DeploymentOptions
 import io.vertx.core.Vertx
@@ -18,9 +17,6 @@ import io.vertx.core.json.JsonObject
 import java.util.concurrent.CompletableFuture
 import java.util.concurrent.TimeUnit
 
-import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
-import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
-
 abstract class AbstractVertxHttpServerTest extends HttpServerTest<Vertx> implements AgentTestTrait {
   @Override
   Vertx startServer(int port) {
@@ -60,24 +56,4 @@ abstract class AbstractVertxHttpServerTest extends HttpServerTest<Vertx> impleme
     // server spans are ended inside of the controller spans
     return false
   }
-
-  @Override
-  Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
-    def attributes = super.httpAttributes(endpoint)
-    attributes.remove(SemanticAttributes.HTTP_ROUTE)
-    attributes
-  }
-
-  @Override
-  String expectedServerSpanName(ServerEndpoint endpoint, String method) {
-    switch (endpoint) {
-      case PATH_PARAM:
-        return "/path/:id/param"
-      case NOT_FOUND:
-        return "HTTP GET"
-      default:
-        return endpoint.getPath()
-    }
-  }
-
 }