Removed eager advice loading

Author: JonasKunz

instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java

@@ -34,20 +34,6 @@ public boolean isHelperClass(String className) {
     return className.equals("io.opentelemetry.javaagent.tooling.Constants");
   }
 
-  @Override
-  public boolean loadAdviceClassesEagerly() {
-    // This is required due to DefineClassInstrumentation
-    // Without this, we would get an infinite recursion on bootstrapping of that instrumentation:
-    // 1. ClassLoader.defineClass is invoked somewhere
-    // 2. The inserted invokedynamic for the instrumentation is reached
-    // 3. To bootstrap the advice, IndyBootstrap is invoked
-    // 4. IndyBootstrap tries to load the DefineClassInstrumentation Advice class
-    // 5. The loading calls ClassLoader.defineClass -> recursion, BOOM!
-    // We avoid this recursion by ensuring that the DefineClassInstrumentation Advice class
-    // is loaded eagerly before the corresponding invokedynamic is reached
-    return true;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/DefineClassInstrumentation.java

@@ -52,17 +52,10 @@ public static DefineClassContext onEnter(
           classLoader, className, classBytes, offset, length);
     }
 
-    // TODO: the ToReturned does nothing except for signaling the AdviceTransformer that it must
-    // not touch this advice
-    // this is done to ensure that ClassLoaderInstrumentationModule.loadAdviceClassesEagerly works
-    // correctly
-    // we can therefore remove it, as soon as the AdviceTransformer is not applied anymore
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
-    @Advice.AssignReturned.ToReturned
-    public static Class<?> onExit(
+    public static void onExit(
         @Advice.Enter DefineClassContext context, @Advice.Return Class<?> returned) {
       DefineClassHelper.afterDefineClass(context);
-      return returned;
     }
   }
 
@@ -76,17 +69,10 @@ public static DefineClassContext onEnter(
       return DefineClassHelper.beforeDefineClass(classLoader, className, classBytes);
     }
 
-    // TODO: the ToReturned does nothing except for signaling the AdviceTransformer that it must
-    // not touch this advice
-    // this is done to ensure that ClassLoaderInstrumentationModule.loadAdviceClassesEagerly works
-    // correctly
-    // we can therefore remove it, as soon as the AdviceTransformer is not applied anymore
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
-    @Advice.AssignReturned.ToReturned
-    public static Class<?> onExit(
+    public static void onExit(
         @Advice.Enter DefineClassContext context, @Advice.Return Class<?> returned) {
       DefineClassHelper.afterDefineClass(context);
-      return returned;
     }
   }
 }

javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/ExperimentalInstrumentationModule.java

@@ -61,19 +61,4 @@ default String getModuleGroup() {
   default List<String> agentPackagesToHide() {
     return Collections.emptyList();
   }
-
-  /**
-   * By default, Advice classes are loaded lazily the first time the corresponding instrumented
-   * method is invoked. This function allows to change the behaviour so that all Advice classes are
-   * loaded and initialized as soon as a matching type is instrumented.
-   *
-   * <p>Note: this functionality currently does not work together with the AdviceTransformer.
-   * Therefore you should make your Advices indy-compatible (use @Advice.AssignReturned) before
-   * using this feature.
-   *
-   * @return true, if Advice classes should be loaded on instrumentation instead of first execution
-   */
-  default boolean loadAdviceClassesEagerly() {
-    return false;
-  }
 }

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/InstrumentationModuleClassLoader.java

@@ -25,7 +25,6 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
@@ -158,21 +157,6 @@ public synchronized void installModule(InstrumentationModule module) {
       ExperimentalInstrumentationModule experimentalModule =
           (ExperimentalInstrumentationModule) module;
       hiddenAgentPackages.addAll(experimentalModule.agentPackagesToHide());
-      if (experimentalModule.loadAdviceClassesEagerly()) {
-        for (String adviceClass : getModuleAdviceNames(module)) {
-          try {
-            this.loadClass(adviceClass, true);
-          } catch (ClassNotFoundException e) {
-            logger.log(
-                Level.SEVERE,
-                "Failed to eagerly load advice class {0}",
-                new Object[] {adviceClass, e});
-          }
-        }
-        // We also eagerly load the LookupExposer, because that is also required for invokedynamic
-        // bootstrapping
-        getLookup();
-      }
     }
   }