Remove the need to bump class version when indy advice is used

javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/advice/AdviceForwardLookupSupplier.java

@@ -0,0 +1,20 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.bootstrap.advice;
+
+import java.lang.invoke.MethodHandles;
+import java.util.function.Supplier;
+
+/**
+ * Helper class that provides a MethodHandles.Lookup that allows defining classes in this package.
+ */
+public final class AdviceForwardLookupSupplier implements Supplier<MethodHandles.Lookup> {
+
+  @Override
+  public MethodHandles.Lookup get() {
+    return MethodHandles.lookup();
+  }
+}

javaagent-tooling/build.gradle.kts

@@ -103,17 +103,6 @@ testing {
       }
     }
 
-    val testPatchBytecodeVersion by registering(JvmTestSuite::class) {
-      dependencies {
-        implementation(project(":javaagent-bootstrap"))
-        implementation(project(":javaagent-tooling"))
-        implementation("net.bytebuddy:byte-buddy-dep")
-
-        // Used by byte-buddy but not brought in as a transitive dependency.
-        compileOnly("com.google.code.findbugs:annotations")
-      }
-    }
-
     val testConfigFile by registering(JvmTestSuite::class) {
       dependencies {
         implementation(project(":javaagent-tooling"))

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

@@ -25,9 +25,9 @@
 import io.opentelemetry.javaagent.tooling.field.VirtualFieldImplementationInstaller;
 import io.opentelemetry.javaagent.tooling.field.VirtualFieldImplementationInstallerFactory;
 import io.opentelemetry.javaagent.tooling.instrumentation.indy.ClassInjectorImpl;
+import io.opentelemetry.javaagent.tooling.instrumentation.indy.ForwardIndyAdviceTransformer;
 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;
@@ -134,7 +134,7 @@ private AgentBuilder installIndyModule(
           return helpers;
         };
 
-    AgentBuilder.Transformer helperInjector =
+    HelperInjector helperInjector =
         new HelperInjector(
             instrumentationModule.instrumentationName(),
             helperGenerator,
@@ -150,13 +150,9 @@ private AgentBuilder installIndyModule(
       AgentBuilder.Identified.Extendable extendableAgentBuilder =
           setTypeMatcher(agentBuilder, instrumentationModule, typeInstrumentation)
               .and(muzzleMatcher)
-              .transform(new PatchByteCodeVersionTransformer());
+              .transform(ConstantAdjuster.instance())
+              .transform(new ForwardIndyAdviceTransformer(helperInjector));
 
-      // 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 =
           IndyModuleRegistry.initializeModuleLoaderOnMatch(
               instrumentationModule, extendableAgentBuilder);
@@ -166,7 +162,6 @@ private AgentBuilder installIndyModule(
           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
       extendableAgentBuilder = contextProvider.injectFields(extendableAgentBuilder);
 
       agentBuilder = extendableAgentBuilder;

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

@@ -0,0 +1,180 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.tooling.instrumentation.indy;
+
+import io.opentelemetry.javaagent.bootstrap.IndyBootstrapDispatcher;
+import io.opentelemetry.javaagent.bootstrap.advice.AdviceForwardLookupSupplier;
+import io.opentelemetry.javaagent.extension.instrumentation.internal.AsmApi;
+import io.opentelemetry.javaagent.tooling.HelperInjector;
+import java.security.ProtectionDomain;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Supplier;
+import net.bytebuddy.ClassFileVersion;
+import net.bytebuddy.agent.builder.AgentBuilder;
+import net.bytebuddy.asm.AsmVisitorWrapper;
+import net.bytebuddy.description.field.FieldDescription;
+import net.bytebuddy.description.field.FieldList;
+import net.bytebuddy.description.method.MethodList;
+import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.dynamic.DynamicType;
+import net.bytebuddy.implementation.Implementation;
+import net.bytebuddy.pool.TypePool;
+import net.bytebuddy.utility.JavaModule;
+import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.Handle;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.Type;
+import org.objectweb.asm.commons.GeneratorAdapter;
+
+/**
+ * Replaces {@code INVOKEDYNAMIC} instructions used for invoking advice with {@code INVOKESTATIC}
+ * instructions in a helper class that contains the original {@code INVOKEDYNAMIC} instruction in
+ * classes that do not support {@code INVOKEDYNAMIC} (i.e. pre Java 7 class files).
+ */
+public class ForwardIndyAdviceTransformer implements AgentBuilder.Transformer {
+  private static final AtomicInteger counter = new AtomicInteger();
+  private static final String bootForwardClassPackage =
+      AdviceForwardLookupSupplier.class.getPackage().getName();
+
+  private final HelperInjector helperInjector;
+
+  public ForwardIndyAdviceTransformer(HelperInjector helperInjector) {
+    this.helperInjector = helperInjector;
+  }
+
+  private static boolean isAtLeastJava7(TypeDescription typeDescription) {
+    ClassFileVersion classFileVersion = typeDescription.getClassFileVersion();
+    return classFileVersion != null && classFileVersion.getJavaVersion() >= 7;
+  }
+
+  @Override
+  public DynamicType.Builder<?> transform(
+      DynamicType.Builder<?> builder,
+      TypeDescription typeDescription,
+      ClassLoader classLoader,
+      JavaModule javaModule,
+      ProtectionDomain protectionDomain) {
+
+    // java 7+ class files already support invokedynamic
+    if (isAtLeastJava7(typeDescription)) {
+      return builder;
+    }
+
+    return builder.visit(
+        new AsmVisitorWrapper.AbstractBase() {
+          @Override
+          public ClassVisitor wrap(
+              TypeDescription typeDescription,
+              ClassVisitor classVisitor,
+              Implementation.Context context,
+              TypePool typePool,
+              FieldList<FieldDescription.InDefinedShape> fieldList,
+              MethodList<?> methodList,
+              int writerFlags,
+              int readerFlags) {
+
+            return new ClassVisitor(AsmApi.VERSION, classVisitor) {
+              final Map<String, Supplier<byte[]>> injectedClasses = new HashMap<>();
+
+              @Override
+              public void visitEnd() {
+                super.visitEnd();
+
+                // inject helper classes that forward to the advice using invokedynamic
+                if (!injectedClasses.isEmpty()) {
+                  helperInjector.injectHelperClasses(classLoader, injectedClasses);
+                }
+              }
+
+              @Override
+              public MethodVisitor visitMethod(
+                  int access,
+                  String name,
+                  String descriptor,
+                  String signature,
+                  String[] exceptions) {
+                MethodVisitor mv =
+                    super.visitMethod(access, name, descriptor, signature, exceptions);
+                return new MethodVisitor(api, mv) {
+                  @Override
+                  public void visitInvokeDynamicInsn(
+                      String name,
+                      String descriptor,
+                      Handle bootstrapMethodHandle,
+                      Object... bootstrapMethodArguments) {
+                    if (Type.getInternalName(IndyBootstrapDispatcher.class)
+                        .equals(bootstrapMethodHandle.getOwner())) {
+
+                      String adviceClassName = (String) bootstrapMethodArguments[3];
+                      String forwardClassDotName =
+                          classLoader == null
+                              ? bootForwardClassPackage + ".Forward$$" + counter.incrementAndGet()
+                              : adviceClassName + "$$Forward$$" + counter.incrementAndGet();
+                      String forwardClassSlasName = forwardClassDotName.replace('.', '/');
+
+                      Supplier<byte[]> forwardClassBytes =
+                          generateForwardClass(
+                              forwardClassSlasName,
+                              name,
+                              descriptor,
+                              bootstrapMethodHandle,
+                              bootstrapMethodArguments);
+                      injectedClasses.put(forwardClassDotName, forwardClassBytes);
+
+                      // replace invokedynamic with invokestatic to the generated forwarder class
+                      // the forwarder class will contain the original invokedynamic instruction
+                      super.visitMethodInsn(
+                          Opcodes.INVOKESTATIC, forwardClassSlasName, name, descriptor, false);
+                      return;
+                    }
+
+                    super.visitInvokeDynamicInsn(
+                        name, descriptor, bootstrapMethodHandle, bootstrapMethodArguments);
+                  }
+                };
+              }
+            };
+          }
+        });
+  }
+
+  private static Supplier<byte[]> generateForwardClass(
+      String forwardClassSlasName,
+      String methodName,
+      String methodDescriptor,
+      Handle bootstrapMethodHandle,
+      Object[] bootstrapMethodArguments) {
+    return () -> {
+      ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS);
+      cw.visit(
+          Opcodes.V1_8,
+          Opcodes.ACC_PUBLIC,
+          forwardClassSlasName,
+          null,
+          Type.getInternalName(Object.class),
+          null);
+      MethodVisitor mv =
+          cw.visitMethod(
+              Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, methodName, methodDescriptor, null, null);
+      GeneratorAdapter ga =
+          new GeneratorAdapter(
+              mv, Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, methodName, methodDescriptor);
+      ga.loadArgs();
+      mv.visitInvokeDynamicInsn(
+          methodName, methodDescriptor, bootstrapMethodHandle, bootstrapMethodArguments);
+      ga.returnValue();
+      mv.visitMaxs(0, 0);
+      mv.visitEnd();
+      cw.visitEnd();
+
+      return cw.toByteArray();
+    };
+  }
+}

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/ClassLoaderMap.java

@@ -20,9 +20,12 @@
 class ClassLoaderMap {
   private static final Cache<ClassLoader, WeakReference<Map<Object, Object>>> data = Cache.weak();
   private static final Map<Object, Object> bootLoaderData = new ConcurrentHashMap<>();
+  private static final HelperInjector helperInjector =
+      HelperInjector.forDynamicTypes(
+          ClassLoaderMap.class.getSimpleName(), Collections.emptyList(), null);
   static Injector defaultInjector =
       (classLoader, className, bytes) -> {
-        HelperInjector.injectHelperClasses(
+        helperInjector.injectHelperClasses(
             classLoader, Collections.singletonMap(className, () -> bytes));
         return Class.forName(className, false, classLoader);
       };