Add code attributes to spring webmvc controller spans (#12839)

trask · web-flow · commit 0c32f85d99a6 · 2024-12-07T09:11:05.000-08:00
diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/boot/SpringBootBasedTest.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/boot/SpringBootBasedTest.java
@@ -11,6 +11,8 @@
 import static io.opentelemetry.semconv.ExceptionAttributes.EXCEPTION_MESSAGE;
 import static io.opentelemetry.semconv.ExceptionAttributes.EXCEPTION_STACKTRACE;
 import static io.opentelemetry.semconv.ExceptionAttributes.EXCEPTION_TYPE;
+import static io.opentelemetry.semconv.incubating.CodeIncubatingAttributes.CODE_FUNCTION;
+import static io.opentelemetry.semconv.incubating.CodeIncubatingAttributes.CODE_NAMESPACE;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import com.google.common.collect.ImmutableMap;
@@ -27,6 +29,7 @@
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.springframework.boot.SpringApplication;
 import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.web.servlet.resource.ResourceHttpRequestHandler;
 
 class SpringBootBasedTest extends AbstractSpringBootBasedTest {
 
@@ -73,6 +76,9 @@ protected SpanDataAssert assertHandlerSpan(
       span.hasName(handlerSpanName)
           .hasKind(SpanKind.INTERNAL)
           .hasStatus(StatusData.error())
+          .hasAttributesSatisfyingExactly(
+              equalTo(CODE_NAMESPACE, ResourceHttpRequestHandler.class.getName()),
+              equalTo(CODE_FUNCTION, "handleRequest"))
           .hasEventsSatisfyingExactly(
               event ->
                   event
diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/HandlerCodeAttributesGetter.java b/instrumentation/spring/spring-webmvc/spring-webmvc-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/HandlerCodeAttributesGetter.java
@@ -5,48 +5,50 @@
 
 package io.opentelemetry.javaagent.instrumentation.spring.webmvc;
 
-import io.opentelemetry.instrumentation.api.incubator.semconv.util.SpanNames;
-import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
+import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesGetter;
 import java.lang.reflect.Method;
 import javax.annotation.Nullable;
 import org.springframework.web.HttpRequestHandler;
 import org.springframework.web.method.HandlerMethod;
 import org.springframework.web.servlet.mvc.Controller;
 
-public class HandlerSpanNameExtractor implements SpanNameExtractor<Object> {
+public class HandlerCodeAttributesGetter implements CodeAttributesGetter<Object> {
 
   @Nullable private static final Class<?> JAVAX_SERVLET = loadOrNull("javax.servlet.Servlet");
   @Nullable private static final Class<?> JAKARTA_SERVLET = loadOrNull("jakarta.servlet.Servlet");
 
+  @Nullable
   @Override
-  public String extract(Object handler) {
-    Class<?> clazz;
-    String methodName;
+  public Class<?> getCodeClass(Object handler) {
+    if (handler instanceof HandlerMethod) {
+      // name span based on the class and method name defined in the handler
+      Method method = ((HandlerMethod) handler).getMethod();
+      return method.getDeclaringClass();
+    } else {
+      return handler.getClass();
+    }
+  }
 
+  @Nullable
+  @Override
+  public String getMethodName(Object handler) {
     if (handler instanceof HandlerMethod) {
       // name span based on the class and method name defined in the handler
       Method method = ((HandlerMethod) handler).getMethod();
-      clazz = method.getDeclaringClass();
-      methodName = method.getName();
+      return method.getName();
     } else if (handler instanceof HttpRequestHandler) {
       // org.springframework.web.servlet.mvc.HttpRequestHandlerAdapter
-      clazz = handler.getClass();
-      methodName = "handleRequest";
+      return "handleRequest";
     } else if (handler instanceof Controller) {
       // org.springframework.web.servlet.mvc.SimpleControllerHandlerAdapter
-      clazz = handler.getClass();
-      methodName = "handleRequest";
+      return "handleRequest";
     } else if (isServlet(handler)) {
       // org.springframework.web.servlet.handler.SimpleServletHandlerAdapter
-      clazz = handler.getClass();
-      methodName = "service";
+      return "service";
     } else {
       // perhaps org.springframework.web.servlet.mvc.annotation.AnnotationMethodHandlerAdapter
-      clazz = handler.getClass();
-      methodName = "<annotation>";
+      return "<annotation>";
     }
-
-    return SpanNames.fromMethod(clazz, methodName);
   }
 
   private static boolean isServlet(Object handler) {
diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/SpringWebMvcInstrumenterFactory.java b/instrumentation/spring/spring-webmvc/spring-webmvc-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/SpringWebMvcInstrumenterFactory.java
@@ -6,6 +6,8 @@
 package io.opentelemetry.javaagent.instrumentation.spring.webmvc;
 
 import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesExtractor;
+import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeSpanNameExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.javaagent.bootstrap.internal.ExperimentalConfig;
 import org.springframework.web.servlet.ModelAndView;
@@ -19,8 +21,12 @@ public SpringWebMvcInstrumenterFactory(String instrumentationName) {
   }
 
   public Instrumenter<Object, Void> createHandlerInstrumenter() {
+    HandlerCodeAttributesGetter codeAttributesGetter = new HandlerCodeAttributesGetter();
     return Instrumenter.<Object, Void>builder(
-            GlobalOpenTelemetry.get(), instrumentationName, new HandlerSpanNameExtractor())
+            GlobalOpenTelemetry.get(),
+            instrumentationName,
+            CodeSpanNameExtractor.create(codeAttributesGetter))
+        .addAttributesExtractor(CodeAttributesExtractor.create(codeAttributesGetter))
         .setEnabled(ExperimentalConfig.get().controllerTelemetryEnabled())
         .buildInstrumenter();
   }
diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-common/testing/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/boot/AbstractSpringBootBasedTest.java b/instrumentation/spring/spring-webmvc/spring-webmvc-common/testing/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/boot/AbstractSpringBootBasedTest.java
@@ -24,7 +24,6 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import io.opentelemetry.api.common.AttributeKey;
-import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.instrumentation.api.internal.HttpConstants;
 import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest;
@@ -46,6 +45,7 @@
 import org.junit.jupiter.params.provider.ValueSource;
 import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.security.web.util.OnCommittedResponseWrapper;
+import org.springframework.web.servlet.resource.ResourceHttpRequestHandler;
 import org.springframework.web.servlet.view.RedirectView;
 
 public abstract class AbstractSpringBootBasedTest
@@ -149,7 +149,9 @@ protected List<Consumer<SpanDataAssert>> errorPageSpanAssertions(
         span ->
             span.hasName("BasicErrorController.error")
                 .hasKind(SpanKind.INTERNAL)
-                .hasAttributes(Attributes.empty()));
+                .hasAttributesSatisfyingExactly(
+                    satisfies(CODE_NAMESPACE, v -> v.endsWith(".BasicErrorController")),
+                    equalTo(CODE_FUNCTION, "error")));
     return spanAssertions;
   }
 
@@ -196,10 +198,16 @@ protected SpanDataAssert assertRenderSpan(
   protected SpanDataAssert assertHandlerSpan(
       SpanDataAssert span, String method, ServerEndpoint endpoint) {
     String handlerSpanName = getHandlerSpanName(endpoint);
+    String codeNamespace = TestController.class.getName();
     if (endpoint == NOT_FOUND) {
       handlerSpanName = "ResourceHttpRequestHandler.handleRequest";
+      codeNamespace = ResourceHttpRequestHandler.class.getName();
     }
-    span.hasName(handlerSpanName).hasKind(SpanKind.INTERNAL);
+    String codeFunction = handlerSpanName.substring(handlerSpanName.indexOf('.') + 1);
+    span.hasName(handlerSpanName)
+        .hasKind(SpanKind.INTERNAL)
+        .hasAttributesSatisfyingExactly(
+            equalTo(CODE_NAMESPACE, codeNamespace), equalTo(CODE_FUNCTION, codeFunction));
     if (endpoint == EXCEPTION) {
       span.hasStatus(StatusData.error());
       span.hasEventsSatisfyingExactly(
diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-common/testing/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/filter/AbstractServletFilterTest.java b/instrumentation/spring/spring-webmvc/spring-webmvc-common/testing/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/filter/AbstractServletFilterTest.java
@@ -13,9 +13,12 @@
 import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.PATH_PARAM;
 import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.QUERY_PARAM;
 import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.REDIRECT;
+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies;
+import static io.opentelemetry.semconv.incubating.CodeIncubatingAttributes.CODE_FUNCTION;
+import static io.opentelemetry.semconv.incubating.CodeIncubatingAttributes.CODE_NAMESPACE;
 import static org.assertj.core.api.Assertions.assertThat;
 
-import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.instrumentation.api.internal.HttpConstants;
 import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest;
@@ -97,7 +100,9 @@ protected List<Consumer<SpanDataAssert>> errorPageSpanAssertions(
         span ->
             span.hasName("BasicErrorController.error")
                 .hasKind(SpanKind.INTERNAL)
-                .hasAttributes(Attributes.empty()));
+                .hasAttributesSatisfyingExactly(
+                    satisfies(CODE_NAMESPACE, v -> v.endsWith(".BasicErrorController")),
+                    equalTo(CODE_FUNCTION, "error")));
     return spanAssertions;
   }
 
