Jersey and cxf server span naming (#2919)

open-telemetry · May 6, 2021 · 8dca278 · 8dca278
1 parent 53c38ac
commit 8dca278
Show file tree

Hide file tree

Showing 15 changed files with 250 additions and 36 deletions.
diff --git a/...nt/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsPathUtil.java b/...nt/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsPathUtil.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.jaxrs.v2_0;
+
+public final class JaxRsPathUtil {
+  private JaxRsPathUtil() {}
+
+  public static String normalizePath(String path) {
+    // ensure that non-empty path starts with /
+    if (path == null || "/".equals(path)) {
+      path = "";
+    } else if (!path.startsWith("/")) {
+      path = "/" + path;
+    }
+    // remove trailing /
+    if (path.endsWith("/")) {
+      path = path.substring(0, path.length() - 1);
+    }
+
+    return path;
+  }
+}
diff --git a/.../java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfInstrumentationModule.java b/.../java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfInstrumentationModule.java
@@ -23,6 +23,7 @@ public List<TypeInstrumentation> typeInstrumentations() {
     return asList(
         new CxfRequestContextInstrumentation(),
         new CxfServletControllerInstrumentation(),
-        new CxfRsHttpListenerInstrumentation());
+        new CxfRsHttpListenerInstrumentation(),
+        new CxfJaxRsInvokerInstrumentation());
   }
 }
diff --git a/...io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfJaxRsInvokerInstrumentation.java b/...io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfJaxRsInvokerInstrumentation.java
@@ -0,0 +1,57 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.jaxrs.v2_0;
+
+import static java.util.Collections.singletonMap;
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
+
+import io.opentelemetry.context.Context;
+import io.opentelemetry.context.Scope;
+import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
+import java.util.Map;
+import net.bytebuddy.asm.Advice;
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.cxf.message.Exchange;
+
+public class CxfJaxRsInvokerInstrumentation implements TypeInstrumentation {
+
+  @Override
+  public ElementMatcher<TypeDescription> typeMatcher() {
+    return named("org.apache.cxf.jaxrs.JAXRSInvoker");
+  }
+
+  @Override
+  public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
+    return singletonMap(
+        named("invoke")
+            .and(takesArgument(0, named("org.apache.cxf.message.Exchange")))
+            .and(takesArgument(1, Object.class))
+            .and(takesArgument(2, Object.class)),
+        CxfJaxRsInvokerInstrumentation.class.getName() + "$InvokeAdvice");
+  }
+
+  public static class InvokeAdvice {
+
+    @Advice.OnMethodEnter(suppress = Throwable.class)
+    public static void onEnter(
+        @Advice.Argument(0) Exchange exchange, @Advice.Local("otelScope") Scope scope) {
+      Context context = CxfTracingUtil.updateServerSpanName(exchange);
+      if (context != null) {
+        scope = context.makeCurrent();
+      }
+    }
+
+    @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
+    public static void stopSpan(@Advice.Local("otelScope") Scope scope) {
+      if (scope != null) {
+        scope.close();
+      }
+    }
+  }
+}
diff --git a/...t/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfTracingUtil.java b/...t/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfTracingUtil.java
@@ -0,0 +1,61 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.jaxrs.v2_0;
+
+import static io.opentelemetry.javaagent.instrumentation.jaxrs.v2_0.JaxRsPathUtil.normalizePath;
+
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming;
+import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
+import io.opentelemetry.instrumentation.api.tracer.ServerSpan;
+import io.opentelemetry.javaagent.instrumentation.api.jaxrs.JaxrsContextPath;
+import org.apache.cxf.jaxrs.model.ClassResourceInfo;
+import org.apache.cxf.jaxrs.model.OperationResourceInfo;
+import org.apache.cxf.jaxrs.model.URITemplate;
+import org.apache.cxf.message.Exchange;
+
+public final class CxfTracingUtil {
+
+  private CxfTracingUtil() {}
+
+  public static Context updateServerSpanName(Exchange exchange) {
+    Context context = Context.current();
+    Span serverSpan = ServerSpan.fromContextOrNull(context);
+    if (serverSpan == null) {
+      return null;
+    }
+
+    OperationResourceInfo ori = exchange.get(OperationResourceInfo.class);
+    ClassResourceInfo cri = ori.getClassResourceInfo();
+    String name = getName(cri.getURITemplate(), ori.getURITemplate());
+    if (name.isEmpty()) {
+      return null;
+    }
+
+    serverSpan.updateName(
+        ServletContextPath.prepend(context, JaxrsContextPath.prepend(context, name)));
+    // mark span name as updated from controller to avoid JaxRsAnnotationsTracer updating it
+    ServerSpanNaming.updateSource(context, ServerSpanNaming.Source.CONTROLLER);
+
+    return JaxrsContextPath.init(context, JaxrsContextPath.prepend(context, name));
+  }
+
+  private static String getName(URITemplate classTemplate, URITemplate operationTemplate) {
+    String classPath = normalize(classTemplate);
+    String operationPath = normalize(operationTemplate);
+
+    return classPath + operationPath;
+  }
+
+  private static String normalize(URITemplate uriTemplate) {
+    if (uriTemplate == null) {
+      return "";
+    }
+
+    return normalizePath(uriTemplate.getValue());
+  }
+}
diff --git a/...tion/jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/test/groovy/CxfHttpServerTest.groovy b/...tion/jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/test/groovy/CxfHttpServerTest.groovy
@@ -39,9 +39,4 @@ class CxfHttpServerTest extends JaxRsHttpServerTest<Server> {
   void stopServer(Server httpServer) {
     httpServer.stop()
   }
-
-  @Override
-  boolean hasFrameworkInstrumentation() {
-    false
-  }
 }
diff --git a/...jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/test/groovy/CxfJettyHttpServerTest.groovy b/...jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/test/groovy/CxfJettyHttpServerTest.groovy
@@ -4,9 +4,4 @@
  */
 
 class CxfJettyHttpServerTest extends JaxRsJettyHttpServerTest {
-
-  @Override
-  boolean hasFrameworkInstrumentation() {
-    false
-  }
 }
diff --git a/...va/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java b/...va/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java
@@ -21,6 +21,8 @@ public JerseyInstrumentationModule() {
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(
-        new JerseyRequestContextInstrumentation(), new JerseyServletContainerInstrumentation());
+        new JerseyRequestContextInstrumentation(),
+        new JerseyServletContainerInstrumentation(),
+        new JerseyResourceMethodDispatcherInstrumentation());
   }
 }
diff --git a/...y/javaagent/instrumentation/jaxrs/v2_0/JerseyResourceMethodDispatcherInstrumentation.java b/...y/javaagent/instrumentation/jaxrs/v2_0/JerseyResourceMethodDispatcherInstrumentation.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.jaxrs.v2_0;
+
+import static java.util.Collections.singletonMap;
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
+
+import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
+import java.util.Map;
+import javax.ws.rs.core.Request;
+import net.bytebuddy.asm.Advice;
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+
+public class JerseyResourceMethodDispatcherInstrumentation implements TypeInstrumentation {
+
+  @Override
+  public ElementMatcher<TypeDescription> typeMatcher() {
+    return named("org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher");
+  }
+
+  @Override
+  public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
+    return singletonMap(
+        named("dispatch")
+            .and(
+                takesArgument(
+                    1,
+                    named("javax.ws.rs.core.Request")
+                        .or(named("org.glassfish.jersey.server.ContainerRequest")))),
+        JerseyResourceMethodDispatcherInstrumentation.class.getName() + "$DispatchAdvice");
+  }
+
+  public static class DispatchAdvice {
+
+    @Advice.OnMethodEnter(suppress = Throwable.class)
+    public static void onEnter(@Advice.Argument(1) Request request) {
+      JerseyTracingUtil.updateServerSpanName(request);
+    }
+  }
+}
diff --git a/...rc/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyTracingUtil.java b/...rc/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyTracingUtil.java
@@ -0,0 +1,47 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.jaxrs.v2_0;
+
+import static io.opentelemetry.javaagent.instrumentation.jaxrs.v2_0.JaxRsPathUtil.normalizePath;
+
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming;
+import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
+import io.opentelemetry.instrumentation.api.tracer.ServerSpan;
+import io.opentelemetry.javaagent.instrumentation.api.jaxrs.JaxrsContextPath;
+import java.util.Optional;
+import javax.ws.rs.core.Request;
+import javax.ws.rs.core.UriInfo;
+import org.glassfish.jersey.server.ContainerRequest;
+import org.glassfish.jersey.server.ExtendedUriInfo;
+
+public class JerseyTracingUtil {
+
+  public static void updateServerSpanName(Request request) {
+    Context context = Context.current();
+    Span serverSpan = ServerSpan.fromContextOrNull(context);
+    if (serverSpan == null) {
+      return;
+    }
+
+    ContainerRequest containerRequest = (ContainerRequest) request;
+    UriInfo uriInfo = containerRequest.getUriInfo();
+    ExtendedUriInfo extendedUriInfo = (ExtendedUriInfo) uriInfo;
+    Optional<String> name =
+        extendedUriInfo.getMatchedTemplates().stream()
+            .map((uriTemplate) -> normalizePath(uriTemplate.getTemplate()))
+            .reduce((a, b) -> b + a);
+    if (!name.isPresent()) {
+      return;
+    }
+
+    serverSpan.updateName(
+        ServletContextPath.prepend(context, JaxrsContextPath.prepend(context, name.get())));
+    // mark span name as updated from controller to avoid JaxRsAnnotationsTracer updating it
+    ServerSpanNaming.updateSource(context, ServerSpanNaming.Source.CONTROLLER);
+  }
+}
diff --git a/...axrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/test/groovy/JerseyHttpServerTest.groovy b/...axrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/test/groovy/JerseyHttpServerTest.groovy
@@ -38,7 +38,8 @@ class JerseyHttpServerTest extends JaxRsHttpServerTest<Server> {
   }
 
   @Override
-  boolean hasFrameworkInstrumentation() {
+  boolean testInterfaceMethodWithPath() {
+    // disables a test that jersey deems invalid
     false
   }
 }
diff --git a/...jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/test/groovy/JerseyJettyHttpServerTest.groovy b/...jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/test/groovy/JerseyJettyHttpServerTest.groovy
@@ -11,7 +11,8 @@ class JerseyJettyHttpServerTest extends JaxRsJettyHttpServerTest {
   }
 
   @Override
-  boolean hasFrameworkInstrumentation() {
+  boolean testInterfaceMethodWithPath() {
+    // disables a test that jersey deems invalid
     false
   }
 }
diff --git a/.../jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/jaxrs-2.0-resteasy-common-javaagent.gradle b/.../jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/jaxrs-2.0-resteasy-common-javaagent.gradle
@@ -3,4 +3,6 @@ apply from: "$rootDir/gradle/instrumentation.gradle"
 dependencies {
   compileOnly group: 'javax.ws.rs', name: 'javax.ws.rs-api', version: '2.0'
   compileOnly group: 'org.jboss.resteasy', name: 'resteasy-jaxrs', version: '3.1.0.Final'
+
+  implementation project(':instrumentation:jaxrs:jaxrs-2.0:jaxrs-2.0-common:javaagent')
 }
diff --git a/...entelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyRootNodeTypeInstrumentation.java b/...entelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyRootNodeTypeInstrumentation.java
@@ -48,7 +48,7 @@ public static class AddInvokerAdvice {
     public static void addInvoker(
         @Advice.Argument(0) String path,
         @Advice.Argument(value = 1, typing = Assigner.Typing.DYNAMIC) Object invoker) {
-      String normalizedPath = ResteasyTracingUtil.normalizePath(path);
+      String normalizedPath = JaxRsPathUtil.normalizePath(path);
       if (invoker instanceof ResourceLocatorInvoker) {
         ResourceLocatorInvoker resourceLocatorInvoker = (ResourceLocatorInvoker) invoker;
         InstrumentationContext.get(ResourceLocatorInvoker.class, String.class)

diff --git a/.../main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyTracingUtil.java b/.../main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyTracingUtil.java
@@ -16,21 +16,6 @@ public final class ResteasyTracingUtil {
 
   private ResteasyTracingUtil() {}
 
-  public static String normalizePath(String path) {
-    // ensure that non-empty path starts with /
-    if (path == null || "/".equals(path)) {
-      path = "";
-    } else if (!path.startsWith("/")) {
-      path = "/" + path;
-    }
-    // remove trailing /
-    if (path.endsWith("/")) {
-      path = path.substring(0, path.length() - 1);
-    }
-
-    return path;
-  }
-
   public static void updateServerSpanName(Context context, String name) {
     if (name == null || name.isEmpty()) {
       return;

diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy
@@ -30,8 +30,6 @@ import test.JaxRsTestResource
 abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> implements AgentTestTrait {
 
   def "test super method without @Path"() {
-    assumeTrue(hasFrameworkInstrumentation())
-
     given:
     def url = HttpUrl.get(address.resolve("test-resource-super")).newBuilder()
       .build()
@@ -58,7 +56,7 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> implements Agent
   }
 
   def "test interface method with @Path"() {
-    assumeTrue(hasFrameworkInstrumentation())
+    assumeTrue(testInterfaceMethodWithPath())
 
     given:
     def url = HttpUrl.get(address.resolve("test-resource-interface/call")).newBuilder()
@@ -86,8 +84,6 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> implements Agent
   }
 
   def "test sub resource locator"() {
-    assumeTrue(hasFrameworkInstrumentation())
-
     given:
     def url = HttpUrl.get(address.resolve("test-sub-resource-locator/call/sub")).newBuilder()
       .build()
@@ -219,7 +215,7 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> implements Agent
     true
   }
 
-  boolean hasFrameworkInstrumentation() {
+  boolean testInterfaceMethodWithPath() {
     true
   }