Use HttpRouteHolder in spark instrumentation (#5241)

open-telemetry · Jan 27, 2022 · 817b3d2 · 817b3d2
1 parent 9ff7b92
commit 817b3d2
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 11 deletions.
diff --git a/...main/java/io/opentelemetry/javaagent/instrumentation/sparkjava/RoutesInstrumentation.java b/...main/java/io/opentelemetry/javaagent/instrumentation/sparkjava/RoutesInstrumentation.java
@@ -10,10 +10,8 @@
 import static net.bytebuddy.matcher.ElementMatchers.returns;
 import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
 
-import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
-import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
@@ -40,12 +38,7 @@ public static class FindAdvice {
 
     @Advice.OnMethodExit(suppress = Throwable.class)
     public static void routeMatchEnricher(@Advice.Return RouteMatch routeMatch) {
-
-      Span span = Java8BytecodeBridge.currentSpan();
-      if (span != null && routeMatch != null) {
-        // TODO should update SERVER span name/route using ServerSpanNaming
-        span.updateName(routeMatch.getMatchUri());
-      }
+      SparkRouteUpdater.updateHttpRoute(routeMatch);
     }
   }
 }
diff --git a/...src/main/java/io/opentelemetry/javaagent/instrumentation/sparkjava/SparkRouteUpdater.java b/...src/main/java/io/opentelemetry/javaagent/instrumentation/sparkjava/SparkRouteUpdater.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.sparkjava;
+
+import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder;
+import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource;
+import javax.annotation.Nullable;
+import spark.routematch.RouteMatch;
+
+public final class SparkRouteUpdater {
+
+  public static void updateHttpRoute(@Nullable RouteMatch routeMatch) {
+    if (routeMatch != null) {
+      Context context = Context.current();
+      HttpRouteHolder.updateHttpRoute(
+          context, HttpRouteSource.CONTROLLER, (c, r) -> r.getMatchUri(), routeMatch);
+    }
+  }
+
+  private SparkRouteUpdater() {}
+}
diff --git a/instrumentation/spark-2.3/javaagent/src/test/groovy/SparkJavaBasedTest.groovy b/instrumentation/spark-2.3/javaagent/src/test/groovy/SparkJavaBasedTest.groovy
@@ -3,8 +3,6 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP
-
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
 import io.opentelemetry.instrumentation.test.utils.PortUtils
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
@@ -13,6 +11,7 @@ import spark.Spark
 import spock.lang.Shared
 
 import static io.opentelemetry.api.trace.SpanKind.SERVER
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP
 
 class SparkJavaBasedTest extends AgentInstrumentationSpecification {
 
@@ -48,6 +47,7 @@ class SparkJavaBasedTest extends AgentInstrumentationSpecification {
           kind SERVER
           hasNoParent()
           attributes {
+            "$SemanticAttributes.NET_TRANSPORT" IP_TCP
             "$SemanticAttributes.NET_PEER_IP" "127.0.0.1"
             "$SemanticAttributes.NET_PEER_PORT" Long
             "$SemanticAttributes.HTTP_SCHEME" "http"
@@ -58,7 +58,7 @@ class SparkJavaBasedTest extends AgentInstrumentationSpecification {
             "$SemanticAttributes.HTTP_FLAVOR" "1.1"
             "$SemanticAttributes.HTTP_USER_AGENT" String
             "$SemanticAttributes.HTTP_SERVER_NAME" String
-            "$SemanticAttributes.NET_TRANSPORT" IP_TCP
+            "$SemanticAttributes.HTTP_ROUTE" "/param/:param"
           }
         }
       }