Add code attributes to several instrumentations (open-telemetry#6365)

* Add code attributes to several instrumentations * fix servlet tests * fix tests * fix elasticsearch tests * fix context bridge test * fix rxjava tests * fix spring webmvc tests
helios · Oct 23, 2022 · ebaf6c7 · ebaf6c7
1 parent b61d33b
commit ebaf6c7
Show file tree

Hide file tree

Showing 28 changed files with 585 additions and 79 deletions.
diff --git a/...java/io/opentelemetry/javaagent/instrumentation/otelannotations/AbstractWithSpanTest.java b/...java/io/opentelemetry/javaagent/instrumentation/otelannotations/AbstractWithSpanTest.java
@@ -5,11 +5,13 @@
 
 package io.opentelemetry.javaagent.instrumentation.otelannotations;
 
-import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.attributeEntry;
+import static io.opentelemetry.api.common.AttributeKey.booleanKey;
+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.CODE_FUNCTION;
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.CODE_NAMESPACE;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.catchThrowable;
 
-import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
@@ -42,7 +44,8 @@ protected final InstrumentationExtension testing() {
 
   @Test
   void success() {
-    T future = newTraced().completable();
+    AbstractTraced<T, U> traced = newTraced();
+    T future = traced.completable();
     complete(future, AbstractTraced.SUCCESS_VALUE);
 
     assertThat(getCompleted(future)).isEqualTo(AbstractTraced.SUCCESS_VALUE);
@@ -54,12 +57,15 @@ void success() {
                     span.hasName("Traced.completable")
                         .hasKind(SpanKind.INTERNAL)
                         .hasNoParent()
-                        .hasAttributes(Attributes.empty())));
+                        .hasAttributesSatisfyingExactly(
+                            equalTo(CODE_NAMESPACE, traced.getClass().getName()),
+                            equalTo(CODE_FUNCTION, "completable"))));
   }
 
   @Test
   void failure() {
-    T future = newTraced().completable();
+    AbstractTraced<T, U> traced = newTraced();
+    T future = traced.completable();
     fail(future, AbstractTraced.FAILURE);
 
     Throwable thrown = catchThrowable(() -> getCompleted(future));
@@ -74,12 +80,15 @@ void failure() {
                         .hasNoParent()
                         .hasStatus(StatusData.error())
                         .hasException(AbstractTraced.FAILURE)
-                        .hasAttributes(Attributes.empty())));
+                        .hasAttributesSatisfyingExactly(
+                            equalTo(CODE_NAMESPACE, traced.getClass().getName()),
+                            equalTo(CODE_FUNCTION, "completable"))));
   }
 
   @Test
   void canceled() {
-    T future = newTraced().completable();
+    AbstractTraced<T, U> traced = newTraced();
+    T future = traced.completable();
     cancel(future);
 
     testing.waitAndAssertTraces(
@@ -89,13 +98,16 @@ void canceled() {
                     span.hasName("Traced.completable")
                         .hasKind(SpanKind.INTERNAL)
                         .hasNoParent()
-                        .hasAttributes(attributeEntry(canceledKey(), true))));
+                        .hasAttributesSatisfyingExactly(
+                            equalTo(CODE_NAMESPACE, traced.getClass().getName()),
+                            equalTo(CODE_FUNCTION, "completable"),
+                            equalTo(booleanKey(canceledKey()), true))));
   }
 
   @Test
   void immediateSuccess() {
-    assertThat(getCompleted(newTraced().alreadySucceeded()))
-        .isEqualTo(AbstractTraced.SUCCESS_VALUE);
+    AbstractTraced<T, U> traced = newTraced();
+    assertThat(getCompleted(traced.alreadySucceeded())).isEqualTo(AbstractTraced.SUCCESS_VALUE);
 
     testing.waitAndAssertTraces(
         trace ->
@@ -104,12 +116,15 @@ void immediateSuccess() {
                     span.hasName("Traced.alreadySucceeded")
                         .hasKind(SpanKind.INTERNAL)
                         .hasNoParent()
-                        .hasAttributes(Attributes.empty())));
+                        .hasAttributesSatisfyingExactly(
+                            equalTo(CODE_NAMESPACE, traced.getClass().getName()),
+                            equalTo(CODE_FUNCTION, "alreadySucceeded"))));
   }
 
   @Test
   void immediateFailure() {
-    Throwable error = catchThrowable(() -> getCompleted(newTraced().alreadyFailed()));
+    AbstractTraced<T, U> traced = newTraced();
+    Throwable error = catchThrowable(() -> getCompleted(traced.alreadyFailed()));
     assertThat(unwrapError(error)).isEqualTo(AbstractTraced.FAILURE);
 
     testing.waitAndAssertTraces(
@@ -121,6 +136,8 @@ void immediateFailure() {
                         .hasNoParent()
                         .hasStatus(StatusData.error())
                         .hasException(AbstractTraced.FAILURE)
-                        .hasAttributes(Attributes.empty())));
+                        .hasAttributesSatisfyingExactly(
+                            equalTo(CODE_NAMESPACE, traced.getClass().getName()),
+                            equalTo(CODE_FUNCTION, "alreadyFailed"))));
   }
 }
diff --git a/...tation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/util/SpanNames.java b/...tation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/util/SpanNames.java
@@ -5,6 +5,7 @@
 
 package io.opentelemetry.instrumentation.api.util;
 
+import io.opentelemetry.instrumentation.api.instrumenter.code.CodeSpanNameExtractor;
 import io.opentelemetry.instrumentation.api.internal.cache.Cache;
 import java.lang.reflect.Method;
 import java.util.Map;
@@ -26,15 +27,22 @@ public static String fromMethod(Method method) {
   /**
    * This method is used to generate a span name based on a method. Anonymous classes are named
    * based on their parent.
+   *
+   * @deprecated Use {@link #fromMethod(Class, String)} instead.
    */
+  @Deprecated
   public static String fromMethod(Class<?> clazz, @Nullable Method method) {
     return fromMethod(clazz, method == null ? "<unknown>" : method.getName());
   }
 
   /**
    * This method is used to generate a span name based on a method. Anonymous classes are named
    * based on their parent.
+   *
+   * @deprecated Use {@link ClassAndMethod#codeAttributesGetter()} and {@link CodeSpanNameExtractor}
+   *     instead.
    */
+  @Deprecated
   public static String fromMethod(ClassAndMethod classAndMethod) {
     return fromMethod(classAndMethod.declaringClass(), classAndMethod.methodName());
   }

diff --git a/...sport-5.3/javaagent/src/test/groovy/springdata/Elasticsearch53SpringRepositoryTest.groovy b/...sport-5.3/javaagent/src/test/groovy/springdata/Elasticsearch53SpringRepositoryTest.groovy
@@ -84,6 +84,8 @@ class Elasticsearch53SpringRepositoryTest extends AgentInstrumentationSpecificat
           name "DocRepository.findAll"
           kind INTERNAL
           attributes {
+            "$SemanticAttributes.CODE_NAMESPACE" DocRepository.name
+            "$SemanticAttributes.CODE_FUNCTION" "findAll"
           }
         }
         span(1) {
@@ -120,6 +122,8 @@ class Elasticsearch53SpringRepositoryTest extends AgentInstrumentationSpecificat
           name "DocRepository.index"
           kind INTERNAL
           attributes {
+            "$SemanticAttributes.CODE_NAMESPACE" DocRepository.name
+            "$SemanticAttributes.CODE_FUNCTION" "index"
           }
         }
         span(1) {
@@ -169,6 +173,8 @@ class Elasticsearch53SpringRepositoryTest extends AgentInstrumentationSpecificat
           name "DocRepository.findById"
           kind INTERNAL
           attributes {
+            "$SemanticAttributes.CODE_NAMESPACE" DocRepository.name
+            "$SemanticAttributes.CODE_FUNCTION" "findById"
           }
         }
         span(1) {
@@ -204,6 +210,8 @@ class Elasticsearch53SpringRepositoryTest extends AgentInstrumentationSpecificat
           name "DocRepository.index"
           kind INTERNAL
           attributes {
+            "$SemanticAttributes.CODE_NAMESPACE" DocRepository.name
+            "$SemanticAttributes.CODE_FUNCTION" "index"
           }
         }
         span(1) {
@@ -245,6 +253,8 @@ class Elasticsearch53SpringRepositoryTest extends AgentInstrumentationSpecificat
           name "DocRepository.findById"
           kind INTERNAL
           attributes {
+            "$SemanticAttributes.CODE_NAMESPACE" DocRepository.name
+            "$SemanticAttributes.CODE_FUNCTION" "findById"
           }
         }
         span(1) {
@@ -279,6 +289,8 @@ class Elasticsearch53SpringRepositoryTest extends AgentInstrumentationSpecificat
           name "DocRepository.deleteById"
           kind INTERNAL
           attributes {
+            "$SemanticAttributes.CODE_NAMESPACE" DocRepository.name
+            "$SemanticAttributes.CODE_FUNCTION" "deleteById"
           }
         }
         span(1) {
@@ -320,6 +332,8 @@ class Elasticsearch53SpringRepositoryTest extends AgentInstrumentationSpecificat
           name "DocRepository.findAll"
           kind INTERNAL
           attributes {
+            "$SemanticAttributes.CODE_NAMESPACE" DocRepository.name
+            "$SemanticAttributes.CODE_FUNCTION" "findAll"
           }
         }
         span(1) {

diff --git a/...nt/src/main/java/io/opentelemetry/javaagent/instrumentation/methods/MethodSingletons.java b/...nt/src/main/java/io/opentelemetry/javaagent/instrumentation/methods/MethodSingletons.java
@@ -8,21 +8,26 @@
 import io.opentelemetry.api.GlobalOpenTelemetry;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
-import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.code.CodeAttributesExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.code.CodeAttributesGetter;
+import io.opentelemetry.instrumentation.api.instrumenter.code.CodeSpanNameExtractor;
 import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
-import io.opentelemetry.instrumentation.api.util.SpanNames;
 
 public final class MethodSingletons {
   private static final String INSTRUMENTATION_NAME = "io.opentelemetry.methods";
 
   private static final Instrumenter<ClassAndMethod, Void> INSTRUMENTER;
 
   static {
-    SpanNameExtractor<ClassAndMethod> spanName = SpanNames::fromMethod;
+    CodeAttributesGetter<ClassAndMethod> codeAttributesGetter =
+        ClassAndMethod.codeAttributesGetter();
 
     INSTRUMENTER =
         Instrumenter.<ClassAndMethod, Void>builder(
-                GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, spanName)
+                GlobalOpenTelemetry.get(),
+                INSTRUMENTATION_NAME,
+                CodeSpanNameExtractor.create(codeAttributesGetter))
+            .addAttributesExtractor(CodeAttributesExtractor.create(codeAttributesGetter))
             .buildInstrumenter(SpanKindExtractor.alwaysInternal());
   }
 

diff --git a/...avaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/methods/MethodTest.java b/...avaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/methods/MethodTest.java
@@ -6,8 +6,10 @@
 package io.opentelemetry.javaagent.instrumentation.methods;
 
 import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.CODE_FUNCTION;
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.CODE_NAMESPACE;
 
-import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
@@ -33,7 +35,9 @@ void methodTraced() {
                 span ->
                     span.hasName("ConfigTracedCallable.call")
                         .hasKind(SpanKind.INTERNAL)
-                        .hasAttributes(Attributes.empty())));
+                        .hasAttributesSatisfyingExactly(
+                            equalTo(CODE_NAMESPACE, ConfigTracedCallable.class.getName()),
+                            equalTo(CODE_FUNCTION, "call"))));
   }
 
   static class ConfigTracedCallable implements Callable<String> {
@@ -62,7 +66,9 @@ void methodTracedWithAsyncStop() throws Exception {
                 span ->
                     span.hasName("ConfigTracedCompletableFuture.getResult")
                         .hasKind(SpanKind.INTERNAL)
-                        .hasAttributes(Attributes.empty())));
+                        .hasAttributesSatisfyingExactly(
+                            equalTo(CODE_NAMESPACE, ConfigTracedCompletableFuture.class.getName()),
+                            equalTo(CODE_FUNCTION, "getResult"))));
   }
 
   static class ConfigTracedCompletableFuture {

diff --git a/...pentelemetry-api/opentelemetry-api-1.0/javaagent/src/test/groovy/ContextBridgeTest.groovy b/...pentelemetry-api/opentelemetry-api-1.0/javaagent/src/test/groovy/ContextBridgeTest.groovy
@@ -10,6 +10,7 @@ import io.opentelemetry.context.Context
 import io.opentelemetry.context.ContextKey
 import io.opentelemetry.extension.annotations.WithSpan
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
 
 import java.util.concurrent.CountDownLatch
 import java.util.concurrent.Executors
@@ -34,8 +35,8 @@ class ContextBridgeTest extends AgentInstrumentationSpecification {
   }
 
   def "application propagates agent's context"() {
-    when:
-    new Runnable() {
+    given:
+    def runnable = new Runnable() {
       @WithSpan("test")
       @Override
       void run() {
@@ -49,7 +50,10 @@ class ContextBridgeTest extends AgentInstrumentationSpecification {
           }
         }
       }
-    }.run()
+    }
+
+    when:
+    runnable.run()
 
     then:
     assertTraces(1) {
@@ -58,6 +62,8 @@ class ContextBridgeTest extends AgentInstrumentationSpecification {
           name "test"
           hasNoParent()
           attributes {
+            "$SemanticAttributes.CODE_NAMESPACE" runnable.class.name
+            "$SemanticAttributes.CODE_FUNCTION" "run"
             "cat" "yes"
           }
         }
@@ -92,8 +98,8 @@ class ContextBridgeTest extends AgentInstrumentationSpecification {
   }
 
   def "application propagates agent's span"() {
-    when:
-    new Runnable() {
+    given:
+    def runnable = new Runnable() {
       @WithSpan("test")
       @Override
       void run() {
@@ -107,7 +113,10 @@ class ContextBridgeTest extends AgentInstrumentationSpecification {
           }
         }
       }
-    }.run()
+    }
+
+    when:
+    runnable.run()
 
     then:
     assertTraces(1) {
@@ -116,6 +125,8 @@ class ContextBridgeTest extends AgentInstrumentationSpecification {
           name "test"
           hasNoParent()
           attributes {
+            "$SemanticAttributes.CODE_NAMESPACE" runnable.class.name
+            "$SemanticAttributes.CODE_FUNCTION" "run"
             "cat" "yes"
           }
         }

diff --git a/...ntelemetry/javaagent/instrumentation/extensionannotations/MethodCodeAttributesGetter.java b/...ntelemetry/javaagent/instrumentation/extensionannotations/MethodCodeAttributesGetter.java
@@ -0,0 +1,23 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.extensionannotations;
+
+import io.opentelemetry.instrumentation.api.instrumenter.code.CodeAttributesGetter;
+import java.lang.reflect.Method;
+
+enum MethodCodeAttributesGetter implements CodeAttributesGetter<Method> {
+  INSTANCE;
+
+  @Override
+  public Class<?> codeClass(Method method) {
+    return method.getDeclaringClass();
+  }
+
+  @Override
+  public String methodName(Method method) {
+    return method.getName();
+  }
+}
diff --git a/...try/javaagent/instrumentation/extensionannotations/MethodRequestCodeAttributesGetter.java b/...try/javaagent/instrumentation/extensionannotations/MethodRequestCodeAttributesGetter.java
@@ -0,0 +1,22 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.extensionannotations;
+
+import io.opentelemetry.instrumentation.api.instrumenter.code.CodeAttributesGetter;
+
+enum MethodRequestCodeAttributesGetter implements CodeAttributesGetter<MethodRequest> {
+  INSTANCE;
+
+  @Override
+  public Class<?> codeClass(MethodRequest methodRequest) {
+    return methodRequest.method().getDeclaringClass();
+  }
+
+  @Override
+  public String methodName(MethodRequest methodRequest) {
+    return methodRequest.method().getName();
+  }
+}