Implement invokedynamic advice bootstrapping

conventions/src/main/kotlin/otel.errorprone-conventions.gradle.kts

@@ -105,8 +105,9 @@ tasks {
         // some moving.
         disable("DefaultPackage")
 
-        // we use modified OtelPrivateConstructorForUtilityClass which ignores *Advice classes
+        // we use modified Otel* checks which ignore *Advice classes
         disable("PrivateConstructorForUtilityClass")
+        disable("CanIgnoreReturnValueSuggester")
 
         // TODO(anuraaga): Remove this, probably after instrumenter API migration instead of dealing
         // with older APIs.
@@ -125,9 +126,9 @@ tasks {
           // Allow underscore in test-type method names
           disable("MemberName")
         }
-        if (project.path.endsWith(":testing") || name.contains("Test")) {
+        if ((project.path.endsWith(":testing") || name.contains("Test")) && !project.name.equals("custom-checks")) {
           // This check causes too many failures, ignore the ones in tests
-          disable("CanIgnoreReturnValueSuggester")
+          disable("OtelCanIgnoreReturnValueSuggester")
         }
       }
     }

custom-checks/src/main/java/io/opentelemetry/javaagent/customchecks/OtelCanIgnoreReturnValueSuggester.java

@@ -0,0 +1,57 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.customchecks;
+
+import static com.google.errorprone.matchers.Description.NO_MATCH;
+
+import com.google.auto.service.AutoService;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.checkreturnvalue.CanIgnoreReturnValueSuggester;
+import com.google.errorprone.matchers.Description;
+import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.MethodTree;
+import com.sun.source.util.TreePath;
+
+@AutoService(BugChecker.class)
+@BugPattern(
+    summary =
+        "Methods with ignorable return values (including methods that always 'return this') should be annotated with @com.google.errorprone.annotations.CanIgnoreReturnValue",
+    severity = BugPattern.SeverityLevel.WARNING)
+public class OtelCanIgnoreReturnValueSuggester extends BugChecker
+    implements BugChecker.MethodTreeMatcher {
+
+  private static final long serialVersionUID = 1L;
+
+  private final CanIgnoreReturnValueSuggester delegate = new CanIgnoreReturnValueSuggester();
+
+  @Override
+  public Description matchMethod(MethodTree methodTree, VisitorState visitorState) {
+    ClassTree containerClass = findContainingClass(visitorState.getPath());
+    if (containerClass.getSimpleName().toString().endsWith("Advice")) {
+      return NO_MATCH;
+    }
+    Description description = delegate.matchMethod(methodTree, visitorState);
+    if (description == NO_MATCH) {
+      return description;
+    }
+    return describeMatch(methodTree);
+  }
+
+  private static ClassTree findContainingClass(TreePath path) {
+    TreePath parent = path.getParentPath();
+    while (parent != null && !(parent.getLeaf() instanceof ClassTree)) {
+      parent = parent.getParentPath();
+    }
+    if (parent == null) {
+      throw new IllegalStateException(
+          "Method is expected to be contained in a class, something must be wrong");
+    }
+    ClassTree containerClass = (ClassTree) parent.getLeaf();
+    return containerClass;
+  }
+}

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SupportabilityMetrics.java

@@ -82,7 +82,7 @@ void report() {
   }
 
   // this private method is designed for assignment of the return value
-  @SuppressWarnings("CanIgnoreReturnValueSuggester")
+  @SuppressWarnings("OtelCanIgnoreReturnValueSuggester")
   private SupportabilityMetrics start() {
     if (agentDebugEnabled) {
       ScheduledExecutorService executor =

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/cache/weaklockfree/WeakConcurrentMap.java

@@ -145,7 +145,7 @@ static final class LookupKey<K> {
     private K key;
     private int hashCode;
 
-    @SuppressWarnings("CanIgnoreReturnValueSuggester")
+    @SuppressWarnings("OtelCanIgnoreReturnValueSuggester")
     LookupKey<K> withValue(K key) {
       this.key = key;
       hashCode = System.identityHashCode(key);

instrumentation/micrometer/micrometer-1.5/library/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/DistributionStatisticConfigModifier.java

@@ -7,7 +7,7 @@
 
 import io.micrometer.core.instrument.distribution.DistributionStatisticConfig;
 
-@SuppressWarnings("CanIgnoreReturnValueSuggester")
+@SuppressWarnings("OtelCanIgnoreReturnValueSuggester")
 enum DistributionStatisticConfigModifier {
   DISABLE_HISTOGRAM_GAUGES {
     @Override

javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyBootstrapDispatcher.java

@@ -0,0 +1,80 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.bootstrap;
+
+import java.lang.invoke.CallSite;
+import java.lang.invoke.ConstantCallSite;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.reflect.Array;
+
+/**
+ * Contains the bootstrap method for initializing invokedynamic callsites which are added via agent
+ * instrumentation.
+ */
+public class IndyBootstrapDispatcher {
+
+  private static volatile MethodHandle bootstrap;
+
+  private IndyBootstrapDispatcher() {}
+
+  /**
+   * Initialized the invokedynamic bootstrapping method to which this class will delegate.
+   *
+   * @param bootstrapMethod the method to delegate to. Must have the same type as {@link
+   *     #bootstrap}.
+   */
+  public static void init(MethodHandle bootstrapMethod) {
+    bootstrap = bootstrapMethod;
+  }
+
+  public static CallSite bootstrap(
+      MethodHandles.Lookup lookup,
+      String adviceMethodName,
+      MethodType adviceMethodType,
+      Object... args) {
+    CallSite callSite = null;
+    if (bootstrap != null) {
+      try {
+        callSite = (CallSite) bootstrap.invoke(lookup, adviceMethodName, adviceMethodType, args);
+      } catch (Throwable e) {
+        ExceptionLogger.logSuppressedError("Error bootstrapping indy instruction", e);
+      }
+    }
+    if (callSite == null) {
+      // The MethodHandle pointing to the Advice could not be created for some reason,
+      // fallback to a Noop MethodHandle to not crash the application
+      MethodHandle noop = generateNoopMethodHandle(adviceMethodType);
+      callSite = new ConstantCallSite(noop);
+    }
+    return callSite;
+  }
+
+  // package visibility for testing
+  static MethodHandle generateNoopMethodHandle(MethodType methodType) {
+    Class<?> returnType = methodType.returnType();
+    MethodHandle noopNoArg;
+    if (returnType == void.class) {
+      noopNoArg =
+          MethodHandles.constant(Void.class, null).asType(MethodType.methodType(void.class));
+    } else {
+      noopNoArg = MethodHandles.constant(returnType, getDefaultValue(returnType));
+    }
+    return MethodHandles.dropArguments(noopNoArg, 0, methodType.parameterList());
+  }
+
+  private static Object getDefaultValue(Class<?> classOrPrimitive) {
+    if (classOrPrimitive.isPrimitive()) {
+      // arrays of primitives are initialized with the correct primitive default value (e.g. 0 for
+      // int.class)
+      // we use this fact to generate the correct default value reflectively
+      return Array.get(Array.newInstance(classOrPrimitive, 1), 0);
+    } else {
+      return null; // null is the default value for reference types
+    }
+  }
+}

javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/IndyBootstrapDispatcherTest.java

@@ -0,0 +1,57 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.bootstrap;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodType;
+import org.junit.jupiter.api.Test;
+
+public class IndyBootstrapDispatcherTest {
+
+  @Test
+  void testVoidNoopMethodHandle() throws Throwable {
+    MethodHandle noArg = generateAndCheck(MethodType.methodType(void.class));
+    noArg.invokeExact();
+
+    MethodHandle intArg = generateAndCheck(MethodType.methodType(void.class, int.class));
+    intArg.invokeExact(42);
+  }
+
+  @Test
+  void testIntNoopMethodHandle() throws Throwable {
+    MethodHandle noArg = generateAndCheck(MethodType.methodType(int.class));
+    assertThat((int) noArg.invokeExact()).isEqualTo(0);
+
+    MethodHandle intArg = generateAndCheck(MethodType.methodType(int.class, int.class));
+    assertThat((int) intArg.invokeExact(42)).isEqualTo(0);
+  }
+
+  @Test
+  void testBooleanNoopMethodHandle() throws Throwable {
+    MethodHandle noArg = generateAndCheck(MethodType.methodType(boolean.class));
+    assertThat((boolean) noArg.invokeExact()).isEqualTo(false);
+
+    MethodHandle intArg = generateAndCheck(MethodType.methodType(boolean.class, int.class));
+    assertThat((boolean) intArg.invokeExact(42)).isEqualTo(false);
+  }
+
+  @Test
+  void testReferenceNoopMethodHandle() throws Throwable {
+    MethodHandle noArg = generateAndCheck(MethodType.methodType(Runnable.class));
+    assertThat((Runnable) noArg.invokeExact()).isEqualTo(null);
+
+    MethodHandle intArg = generateAndCheck(MethodType.methodType(Runnable.class, int.class));
+    assertThat((Runnable) intArg.invokeExact(42)).isEqualTo(null);
+  }
+
+  private static MethodHandle generateAndCheck(MethodType type) {
+    MethodHandle mh = IndyBootstrapDispatcher.generateNoopMethodHandle(type);
+    assertThat(mh.type()).isEqualTo(type);
+    return mh;
+  }
+}

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java

@@ -20,6 +20,9 @@
 import io.opentelemetry.javaagent.tooling.config.AgentConfig;
 import io.opentelemetry.javaagent.tooling.field.VirtualFieldImplementationInstaller;
 import io.opentelemetry.javaagent.tooling.field.VirtualFieldImplementationInstallerFactory;
+import io.opentelemetry.javaagent.tooling.instrumentation.indy.IndyModuleRegistry;
+import io.opentelemetry.javaagent.tooling.instrumentation.indy.IndyTypeTransformerImpl;
+import io.opentelemetry.javaagent.tooling.instrumentation.indy.PatchByteCodeVersionTransformer;
 import io.opentelemetry.javaagent.tooling.muzzle.HelperResourceBuilderImpl;
 import io.opentelemetry.javaagent.tooling.muzzle.InstrumentationModuleMuzzle;
 import io.opentelemetry.javaagent.tooling.util.IgnoreFailedTypeMatcher;
@@ -62,6 +65,52 @@ AgentBuilder install(
           FINE, "Instrumentation {0} is disabled", instrumentationModule.instrumentationName());
       return parentAgentBuilder;
     }
+
+    if (instrumentationModule.isIndyModule()) {
+      return installIndyModule(instrumentationModule, parentAgentBuilder);
+    } else {
+      return installInjectingModule(instrumentationModule, parentAgentBuilder, config);
+    }
+  }
+
+  private AgentBuilder installIndyModule(
+      InstrumentationModule instrumentationModule, AgentBuilder parentAgentBuilder) {
+
+    IndyModuleRegistry.registerIndyModule(instrumentationModule);
+
+    // TODO (Jonas): Adapt MuzzleMatcher to use the same type lookup strategy as the
+    // InstrumentationModuleClassLoader
+    // MuzzleMatcher muzzleMatcher = new MuzzleMatcher(logger, instrumentationModule, config);
+    VirtualFieldImplementationInstaller contextProvider =
+        virtualFieldInstallerFactory.create(instrumentationModule);
+
+    AgentBuilder agentBuilder = parentAgentBuilder;
+    for (TypeInstrumentation typeInstrumentation : instrumentationModule.typeInstrumentations()) {
+      AgentBuilder.Identified.Extendable extendableAgentBuilder =
+          setTypeMatcher(agentBuilder, instrumentationModule, typeInstrumentation)
+              .transform(new PatchByteCodeVersionTransformer());
+
+      IndyTypeTransformerImpl typeTransformer =
+          new IndyTypeTransformerImpl(extendableAgentBuilder, instrumentationModule);
+      typeInstrumentation.transform(typeTransformer);
+      extendableAgentBuilder = typeTransformer.getAgentBuilder();
+      // TODO (Jonas): make instrumentation of bytecode older than 1.4 opt-in via a config option
+      // TODO (Jonas): we are not calling
+      // contextProvider.rewriteVirtualFieldsCalls(extendableAgentBuilder) anymore
+      // As a result the advices should store `VirtualFields` as static variables instead of having
+      // the lookup inline
+      // We need to update our documentation on that
+      extendableAgentBuilder = contextProvider.injectFields(extendableAgentBuilder);
+
+      agentBuilder = extendableAgentBuilder;
+    }
+    return agentBuilder;
+  }
+
+  private AgentBuilder installInjectingModule(
+      InstrumentationModule instrumentationModule,
+      AgentBuilder parentAgentBuilder,
+      ConfigProperties config) {
     List<String> helperClassNames =
         InstrumentationModuleMuzzle.getHelperClassNames(instrumentationModule);
     HelperResourceBuilderImpl helperResourceBuilder = new HelperResourceBuilderImpl();
@@ -78,8 +127,6 @@ AgentBuilder install(
       return parentAgentBuilder;
     }
 
-    ElementMatcher.Junction<ClassLoader> moduleClassLoaderMatcher =
-        instrumentationModule.classLoaderMatcher();
     MuzzleMatcher muzzleMatcher = new MuzzleMatcher(logger, instrumentationModule, config);
     AgentBuilder.Transformer helperInjector =
         new HelperInjector(
@@ -93,32 +140,9 @@ AgentBuilder install(
 
     AgentBuilder agentBuilder = parentAgentBuilder;
     for (TypeInstrumentation typeInstrumentation : typeInstrumentations) {
-      ElementMatcher<TypeDescription> typeMatcher =
-          new NamedMatcher<>(
-              instrumentationModule.getClass().getSimpleName()
-                  + "#"
-                  + typeInstrumentation.getClass().getSimpleName(),
-              new IgnoreFailedTypeMatcher(typeInstrumentation.typeMatcher()));
-      ElementMatcher<ClassLoader> classLoaderMatcher =
-          new NamedMatcher<>(
-              instrumentationModule.getClass().getSimpleName()
-                  + "#"
-                  + typeInstrumentation.getClass().getSimpleName(),
-              moduleClassLoaderMatcher.and(typeInstrumentation.classLoaderOptimization()));
 
       AgentBuilder.Identified.Extendable extendableAgentBuilder =
-          agentBuilder
-              .type(
-                  new LoggingFailSafeMatcher<>(
-                      typeMatcher,
-                      "Instrumentation type matcher unexpected exception: " + typeMatcher),
-                  new LoggingFailSafeMatcher<>(
-                      classLoaderMatcher,
-                      "Instrumentation class loader matcher unexpected exception: "
-                          + classLoaderMatcher))
-              .and(
-                  (typeDescription, classLoader, module, classBeingRedefined, protectionDomain) ->
-                      classLoader == null || NOT_DECORATOR_MATCHER.matches(typeDescription))
+          setTypeMatcher(agentBuilder, instrumentationModule, typeInstrumentation)
               .and(muzzleMatcher)
               .transform(ConstantAdjuster.instance())
               .transform(helperInjector);
@@ -133,4 +157,37 @@ AgentBuilder install(
 
     return agentBuilder;
   }
+
+  private static AgentBuilder.Identified.Narrowable setTypeMatcher(
+      AgentBuilder agentBuilder,
+      InstrumentationModule instrumentationModule,
+      TypeInstrumentation typeInstrumentation) {
+
+    ElementMatcher.Junction<ClassLoader> moduleClassLoaderMatcher =
+        instrumentationModule.classLoaderMatcher();
+
+    ElementMatcher<TypeDescription> typeMatcher =
+        new NamedMatcher<>(
+            instrumentationModule.getClass().getSimpleName()
+                + "#"
+                + typeInstrumentation.getClass().getSimpleName(),
+            new IgnoreFailedTypeMatcher(typeInstrumentation.typeMatcher()));
+    ElementMatcher<ClassLoader> classLoaderMatcher =
+        new NamedMatcher<>(
+            instrumentationModule.getClass().getSimpleName()
+                + "#"
+                + typeInstrumentation.getClass().getSimpleName(),
+            moduleClassLoaderMatcher.and(typeInstrumentation.classLoaderOptimization()));
+
+    return agentBuilder
+        .type(
+            new LoggingFailSafeMatcher<>(
+                typeMatcher, "Instrumentation type matcher unexpected exception: " + typeMatcher),
+            new LoggingFailSafeMatcher<>(
+                classLoaderMatcher,
+                "Instrumentation class loader matcher unexpected exception: " + classLoaderMatcher))
+        .and(
+            (typeDescription, classLoader, module, classBeingRedefined, protectionDomain) ->
+                classLoader == null || NOT_DECORATOR_MATCHER.matches(typeDescription));
+  }
 }