Span kind for method instrumentation / Declarative configuration tooling (#14014)

Co-authored-by: Lauri Tulmin <[email protected]>
Co-authored-by: otelbot <[email protected]>

Author: 3 people

instrumentation-api-incubator/build.gradle.kts

@@ -13,7 +13,7 @@ group = "io.opentelemetry.instrumentation"
 dependencies {
   api("io.opentelemetry.semconv:opentelemetry-semconv")
   api(project(":instrumentation-api"))
-  implementation("io.opentelemetry:opentelemetry-api-incubator")
+  api("io.opentelemetry:opentelemetry-api-incubator")
 
   compileOnly("com.google.auto.value:auto-value-annotations")
   annotationProcessor("com.google.auto.value:auto-value")

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/config/internal/InstrumentationConfig.java

@@ -7,6 +7,7 @@
 
 import static java.util.Collections.emptyList;
 
+import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
 import java.time.Duration;
 import java.util.List;
 import java.util.Map;
@@ -107,4 +108,18 @@ default List<String> getList(String name) {
    * {@code key=value,anotherKey=anotherValue}. The returned map is unmodifiable.
    */
   Map<String, String> getMap(String name, Map<String, String> defaultValue);
+
+  /**
+   * Returns a {@link DeclarativeConfigProperties} for the given instrumentation name, or {@code
+   * null} if no declarative configuration is available for that instrumentation.
+   *
+   * <p>Declarative configuration is used to configure instrumentation properties in a declarative
+   * way, such as through YAML or JSON files.
+   *
+   * @param instrumentationName the name of the instrumentation
+   * @return the declarative configuration properties for the given instrumentation name, or {@code
+   *     null} if not available
+   */
+  @Nullable
+  DeclarativeConfigProperties getDeclarativeConfig(String instrumentationName);
 }

instrumentation/methods/javaagent/build.gradle.kts

@@ -13,8 +13,30 @@ dependencies {
   compileOnly(project(":instrumentation-annotations-support"))
 }
 
-tasks.withType<Test>().configureEach {
+tasks.test {
   jvmArgs(
     "-Dotel.instrumentation.methods.include=io.opentelemetry.javaagent.instrumentation.methods.MethodTest\$ConfigTracedCallable[call];io.opentelemetry.javaagent.instrumentation.methods.MethodTest\$ConfigTracedCompletableFuture[getResult];javax.naming.directory.InitialDirContext[search]"
   )
 }
+
+testing {
+  suites {
+    val declarativeConfigTest by registering(JvmTestSuite::class) {
+      targets {
+        all {
+          testTask.configure {
+            jvmArgs(
+              "-Dotel.experimental.config.file=$projectDir/src/declarativeConfigTest/resources/declarative-config.yaml"
+            )
+          }
+        }
+      }
+    }
+  }
+}
+
+tasks {
+  check {
+    dependsOn(testing.suites)
+  }
+}

instrumentation/methods/javaagent/src/declarativeConfigTest/java/io/opentelemetry/javaagent/instrumentation/methods/MethodTest.java

@@ -0,0 +1,45 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.methods;
+
+import static io.opentelemetry.instrumentation.testing.junit.code.SemconvCodeStabilityUtil.codeFunctionAssertions;
+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
+
+import io.opentelemetry.api.trace.SpanKind;
+import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
+import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
+import java.util.concurrent.Callable;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+@SuppressWarnings("deprecation") // using deprecated semconv
+class MethodTest {
+
+  @RegisterExtension
+  static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
+
+  @SuppressWarnings("deprecation") // using deprecated semconv
+  @Test
+  void methodTraced() {
+    assertThat(new ConfigTracedCallable().call()).isEqualTo("Hello!");
+    testing.waitAndAssertTraces(
+        trace ->
+            trace.hasSpansSatisfyingExactly(
+                span ->
+                    span.hasName("ConfigTracedCallable.call")
+                        .hasKind(SpanKind.SERVER)
+                        .hasAttributesSatisfyingExactly(
+                            codeFunctionAssertions(ConfigTracedCallable.class, "call"))));
+  }
+
+  static class ConfigTracedCallable implements Callable<String> {
+
+    @Override
+    public String call() {
+      return "Hello!";
+    }
+  }
+}

instrumentation/methods/javaagent/src/declarativeConfigTest/resources/declarative-config.yaml

@@ -0,0 +1,33 @@
+file_format: "0.4"
+tracer_provider:
+  processors:
+    - simple:
+        exporter:
+          test:
+    - simple:
+        exporter:
+          console:
+
+logger_provider:
+  processors:
+    - simple:
+        exporter:
+          test:
+
+meter_provider:
+  readers:
+    - periodic:
+        # Set really long interval. We'll call forceFlush when we need the metrics
+        # instead of collecting them periodically.
+        interval: 1000000
+        exporter:
+          test:
+
+instrumentation/development:
+  java:
+    methods:
+      include:
+        - class: io.opentelemetry.javaagent.instrumentation.methods.MethodTest$ConfigTracedCallable
+          methods:
+            - name: call
+              span_kind: SERVER

instrumentation/methods/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/methods/MethodAndType.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.methods;
+
+import io.opentelemetry.api.trace.SpanKind;
+import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod;
+
+public class MethodAndType {
+  private final ClassAndMethod classAndMethod;
+  private final SpanKind spanKind;
+
+  private MethodAndType(ClassAndMethod classAndMethod, SpanKind spanKind) {
+    this.classAndMethod = classAndMethod;
+    this.spanKind = spanKind;
+  }
+
+  public static MethodAndType create(ClassAndMethod classAndMethod, SpanKind spanKind) {
+    return new MethodAndType(classAndMethod, spanKind);
+  }
+
+  public ClassAndMethod getClassAndMethod() {
+    return classAndMethod;
+  }
+
+  public SpanKind getSpanKind() {
+    return spanKind;
+  }
+}

instrumentation/methods/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/methods/MethodInstrumentation.java

@@ -10,33 +10,41 @@
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasSuperType;
 import static io.opentelemetry.javaagent.instrumentation.methods.MethodSingletons.getBootstrapLoader;
 import static io.opentelemetry.javaagent.instrumentation.methods.MethodSingletons.instrumenter;
+import static net.bytebuddy.matcher.ElementMatchers.any;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 import static net.bytebuddy.matcher.ElementMatchers.namedOneOf;
+import static net.bytebuddy.matcher.ElementMatchers.none;
 
+import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndSupport;
 import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
-import java.util.Set;
+import java.util.Collection;
+import java.util.Map;
 import net.bytebuddy.asm.Advice;
+import net.bytebuddy.description.enumeration.EnumerationDescription;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.implementation.bytecode.assign.Assigner;
 import net.bytebuddy.matcher.ElementMatcher;
 
 public class MethodInstrumentation implements TypeInstrumentation {
   private final String className;
-  private final Set<String> methodNames;
+  private final Map<SpanKind, Collection<String>> methodNames;
 
-  public MethodInstrumentation(String className, Set<String> methodNames) {
+  public MethodInstrumentation(String className, Map<SpanKind, Collection<String>> methodNames) {
     this.className = className;
     this.methodNames = methodNames;
   }
 
   @Override
   public ElementMatcher<ClassLoader> classLoaderOptimization() {
+    if (className == null) {
+      return any();
+    }
     ElementMatcher<ClassLoader> delegate = hasClassesNamed(className);
     return target -> {
       // hasClassesNamed does not support null class loader, so we provide a custom loader that
@@ -50,37 +58,51 @@ public ElementMatcher<ClassLoader> classLoaderOptimization() {
 
   @Override
   public ElementMatcher<TypeDescription> typeMatcher() {
-    return hasSuperType(named(className));
+    return className == null ? none() : hasSuperType(named(className));
   }
 
   @Override
   public void transform(TypeTransformer transformer) {
-    transformer.applyAdviceToMethod(
-        namedOneOf(methodNames.toArray(new String[0])).and(isMethod()),
-        mapping ->
-            mapping.bind(
-                MethodReturnType.class,
-                (instrumentedType, instrumentedMethod, assigner, argumentHandler, sort) ->
-                    Advice.OffsetMapping.Target.ForStackManipulation.of(
-                        instrumentedMethod.getReturnType().asErasure())),
-        MethodInstrumentation.class.getName() + "$MethodAdvice");
+    for (Map.Entry<SpanKind, Collection<String>> entry : methodNames.entrySet()) {
+      SpanKind spanKind = entry.getKey();
+      Collection<String> names = entry.getValue();
+      transformer.applyAdviceToMethod(
+          namedOneOf(names.toArray(new String[0])).and(isMethod()),
+          mapping ->
+              mapping
+                  .bind(
+                      MethodReturnType.class,
+                      (instrumentedType, instrumentedMethod, assigner, argumentHandler, sort) ->
+                          Advice.OffsetMapping.Target.ForStackManipulation.of(
+                              instrumentedMethod.getReturnType().asErasure()))
+                  .bind(
+                      MethodSpanKind.class,
+                      new EnumerationDescription.ForLoadedEnumeration(spanKind)),
+          MethodInstrumentation.class.getName() + "$MethodAdvice");
+    }
   }
 
   // custom annotation that represents the return type of the method
   @interface MethodReturnType {}
 
+  // custom annotation that represents the SpanKind of the method
+  @interface MethodSpanKind {}
+
   @SuppressWarnings("unused")
   public static class MethodAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
     public static void onEnter(
+        @MethodSpanKind SpanKind spanKind,
         @Advice.Origin("#t") Class<?> declaringClass,
         @Advice.Origin("#m") String methodName,
-        @Advice.Local("otelMethod") ClassAndMethod classAndMethod,
+        @Advice.Local("otelMethod") MethodAndType classAndMethod,
         @Advice.Local("otelContext") Context context,
         @Advice.Local("otelScope") Scope scope) {
       Context parentContext = currentContext();
-      classAndMethod = ClassAndMethod.create(declaringClass, methodName);
+      classAndMethod =
+          MethodAndType.create(ClassAndMethod.create(declaringClass, methodName), spanKind);
+
       if (!instrumenter().shouldStart(parentContext, classAndMethod)) {
         return;
       }
@@ -92,7 +114,7 @@ public static void onEnter(
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void stopSpan(
         @MethodReturnType Class<?> methodReturnType,
-        @Advice.Local("otelMethod") ClassAndMethod classAndMethod,
+        @Advice.Local("otelMethod") MethodAndType classAndMethod,
         @Advice.Local("otelContext") Context context,
         @Advice.Local("otelScope") Scope scope,
         @Advice.Return(typing = Assigner.Typing.DYNAMIC, readOnly = false) Object returnValue,

instrumentation/methods/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/methods/MethodInstrumentationModule.java

@@ -6,13 +6,16 @@
 package io.opentelemetry.javaagent.instrumentation.methods;
 
 import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
+import static java.util.Collections.singletonMap;
 
 import com.google.auto.service.AutoService;
+import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
+import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.tooling.config.MethodsConfigurationParser;
-import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -27,27 +30,35 @@ public class MethodInstrumentationModule extends InstrumentationModule {
 
   public MethodInstrumentationModule() {
     super("methods");
+    typeInstrumentations = createInstrumentations();
+  }
+
+  private static List<TypeInstrumentation> createInstrumentations() {
+    DeclarativeConfigProperties methods =
+        AgentInstrumentationConfig.get().getDeclarativeConfig("methods");
+    List<TypeInstrumentation> list =
+        methods != null ? MethodsConfig.parseDeclarativeConfig(methods) : parseConfigProperties();
+    // ensure that there is at least one instrumentation so that muzzle reference collection could
+    // work
+    if (list.isEmpty()) {
+      return singletonList(
+          new MethodInstrumentation(null, singletonMap(SpanKind.INTERNAL, emptyList())));
+    }
+    return list;
+  }
 
+  private static List<TypeInstrumentation> parseConfigProperties() {
     Map<String, Set<String>> classMethodsToTrace =
         MethodsConfigurationParser.parse(
             AgentInstrumentationConfig.get().getString(TRACE_METHODS_CONFIG));
 
-    typeInstrumentations =
-        classMethodsToTrace.entrySet().stream()
-            .filter(e -> !e.getValue().isEmpty())
-            .map(e -> new MethodInstrumentation(e.getKey(), e.getValue()))
-            .collect(Collectors.toList());
-  }
-
-  // the default configuration has empty "otel.instrumentation.methods.include", and so doesn't
-  // generate any TypeInstrumentation for muzzle to analyze
-  @Override
-  public List<String> getAdditionalHelperClassNames() {
-    return typeInstrumentations.isEmpty()
-        ? emptyList()
-        : Arrays.asList(
-            "io.opentelemetry.javaagent.instrumentation.methods.MethodSingletons",
-            "io.opentelemetry.javaagent.instrumentation.methods.MethodSingletons$BootstrapLoader");
+    return classMethodsToTrace.entrySet().stream()
+        .filter(e -> !e.getValue().isEmpty())
+        .map(
+            e ->
+                new MethodInstrumentation(
+                    e.getKey(), singletonMap(SpanKind.INTERNAL, e.getValue())))
+        .collect(Collectors.toList());
   }
 
   @Override