try anoter approach

laurit · laurit · commit a611bf761a94 · 2025-10-03T16:15:22.000+03:00
diff --git a/instrumentation/internal/internal-class-loader/compile-stub/build.gradle.kts b/instrumentation/internal/internal-class-loader/compile-stub/build.gradle.kts
@@ -0,0 +1,3 @@
+plugins {
+  id("otel.java-conventions")
+}
diff --git a/instrumentation/internal/internal-class-loader/compile-stub/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/stub/ClassLoader.java b/instrumentation/internal/internal-class-loader/compile-stub/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/stub/ClassLoader.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.internal.classloader.stub;
+
+import java.security.ProtectionDomain;
+
+/**
+ * A placeholder for java.lang.ClassLoader to allow compilation of advice classes that invoke
+ * protected methods of ClassLoader (like defineClass and findLoadedClass). During the build we'll
+ * use shadow plugin to replace reference to this class with the real java.lang.ClassLoader.
+ */
+@SuppressWarnings("JavaLangClash")
+public abstract class ClassLoader {
+  public abstract Class<?> findLoadedClass(String name);
+
+  public abstract Class<?> defineClass(
+      String name, byte[] b, int off, int len, ProtectionDomain protectionDomain);
+}
diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/build.gradle.kts b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/build.gradle.kts
@@ -6,5 +6,5 @@ dependencies {
   compileOnly("org.apache.commons:commons-lang3:3.12.0")
   testImplementation("org.apache.commons:commons-lang3:3.12.0")
 
-  testInstrumentation(project(":instrumentation:internal:internal-class-loader:javaagent"))
+  testInstrumentation(project(":instrumentation:internal:internal-class-loader:javaagent", configuration = "shadow"))
 }
diff --git a/instrumentation/internal/internal-class-loader/javaagent/build.gradle.kts b/instrumentation/internal/internal-class-loader/javaagent/build.gradle.kts
@@ -5,6 +5,7 @@ plugins {
 dependencies {
   compileOnly(project(":javaagent-bootstrap"))
   compileOnly(project(":javaagent-tooling"))
+  compileOnly(project(":instrumentation:internal:internal-class-loader:compile-stub"))
 
   testImplementation(project(":javaagent-bootstrap"))
 
@@ -21,3 +22,9 @@ dependencies {
   testImplementation("org.eclipse.platform:org.eclipse.osgi:3.13.200")
   testImplementation("org.apache.felix:org.apache.felix.framework:6.0.2")
 }
+
+tasks {
+  shadowJar {
+    relocate("io.opentelemetry.javaagent.instrumentation.internal.classloader.stub", "java.lang")
+  }
+}
diff --git a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java
@@ -35,9 +35,8 @@ public List<String> injectedClassNames() {
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(
-        // added first so it would get applied after other instrumentations
-        new LoadInjectedClassInstrumentation(),
         new BootDelegationInstrumentation(),
+        new LoadInjectedClassInstrumentation(),
         new ResourceInjectionInstrumentation(),
         new DefineClassInstrumentation());
   }
diff --git a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java
@@ -6,28 +6,24 @@
 package io.opentelemetry.javaagent.instrumentation.internal.classloader;
 
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.extendsClass;
+import static io.opentelemetry.javaagent.instrumentation.internal.classloader.AdviceUtil.applyInlineAdvice;
+import static net.bytebuddy.matcher.ElementMatchers.isMethod;
+import static net.bytebuddy.matcher.ElementMatchers.isProtected;
+import static net.bytebuddy.matcher.ElementMatchers.isPublic;
+import static net.bytebuddy.matcher.ElementMatchers.isStatic;
 import static net.bytebuddy.matcher.ElementMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.not;
+import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
+import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
 
 import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper;
-import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper.HelperClassInfo;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
-import io.opentelemetry.javaagent.extension.instrumentation.internal.AsmApi;
-import net.bytebuddy.ClassFileVersion;
-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.asm.Advice;
+import net.bytebuddy.asm.Advice.AssignReturned;
+import net.bytebuddy.description.method.MethodDescription;
 import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.implementation.Implementation;
 import net.bytebuddy.matcher.ElementMatcher;
-import net.bytebuddy.pool.TypePool;
-import org.objectweb.asm.ClassVisitor;
-import org.objectweb.asm.ClassWriter;
-import org.objectweb.asm.Label;
-import org.objectweb.asm.MethodVisitor;
-import org.objectweb.asm.Opcodes;
-import org.objectweb.asm.Type;
 
 /**
  * This instrumentation inserts loading of our injected helper classes at the start of {@code
@@ -42,220 +38,63 @@ public ElementMatcher<TypeDescription> typeMatcher() {
 
   @Override
   public void transform(TypeTransformer transformer) {
-    transformer.applyTransformer(
-        (builder, typeDescription, classLoader, module, protectionDomain) ->
-            builder.visit(
-                new AsmVisitorWrapper() {
-                  @Override
-                  public int mergeWriter(int flags) {
-                    return flags | ClassWriter.COMPUTE_MAXS;
-                  }
-
-                  @Override
-                  public int mergeReader(int flags) {
-                    return flags;
-                  }
-
-                  @Override
-                  public ClassVisitor wrap(
-                      TypeDescription instrumentedType,
-                      ClassVisitor classVisitor,
-                      Implementation.Context implementationContext,
-                      TypePool typePool,
-                      FieldList<FieldDescription.InDefinedShape> fields,
-                      MethodList<?> methods,
-                      int writerFlags,
-                      int readerFlags) {
-                    return new ClassLoaderClassVisitor(classVisitor);
-                  }
-                }));
+    ElementMatcher.Junction<MethodDescription> methodMatcher =
+        isMethod()
+            .and(named("loadClass"))
+            .and(
+                takesArguments(1)
+                    .and(takesArgument(0, String.class))
+                    .or(
+                        takesArguments(2)
+                            .and(takesArgument(0, String.class))
+                            .and(takesArgument(1, boolean.class))))
+            .and(isPublic().or(isProtected()))
+            .and(not(isStatic()));
+    // Inline instrumentation to prevent problems with invokedynamic-recursion
+    applyInlineAdvice(transformer, methodMatcher, this.getClass().getName() + "$LoadClassAdvice");
   }
 
-  private static class ClassLoaderClassVisitor extends ClassVisitor implements Opcodes {
-    private String internalClassName;
-    private boolean frames;
-
-    ClassLoaderClassVisitor(ClassVisitor classVisitor) {
-      super(AsmApi.VERSION, classVisitor);
-    }
-
-    @Override
-    public void visit(
-        int version,
-        int access,
-        String name,
-        String signature,
-        String superName,
-        String[] interfaces) {
-      super.visit(version, access, name, signature, superName, interfaces);
-      internalClassName = name;
-      frames = ClassFileVersion.ofMinorMajor(version).isAtLeast(ClassFileVersion.JAVA_V6);
-    }
-
-    @Override
-    public MethodVisitor visitMethod(
-        int access, String name, String descriptor, String signature, String[] exceptions) {
-      MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions);
-      boolean loadClassMethod =
-          "loadClass".equals(name)
-              && ("(Ljava/lang/String;)Ljava/lang/Class;".equals(descriptor)
-                  || "(Ljava/lang/String;Z)Ljava/lang/Class;".equals(descriptor));
-      if (!loadClassMethod) {
-        return mv;
-      }
-
-      int argumentCount = Type.getArgumentTypes(descriptor).length;
-      return new MethodVisitor(api, mv) {
-        @Override
-        public void visitCode() {
-          super.visitCode();
-
-          // inserts the following at the start of the loadClass method:
-          /*
-           InjectedClassHelper.HelperClassInfo helperClassInfo = InjectedClassHelper.getHelperClassInfo(this, name);
-           if (helperClassInfo != null) {
-               Class<?> clazz = findLoadedClass(name);
-               if (clazz != null) {
-                   return clazz;
-               }
-               try {
-                   byte[] bytes = helperClassInfo.getClassBytes();
-                   return defineClass(name, bytes, 0, bytes.length, helperClassInfo.getProtectionDomain());
-               } catch (LinkageError error) {
-                   clazz = findLoadedClass(name);
-                   if (clazz != null) {
-                       return clazz;
-                   }
-                   throw error;
-               }
-           }
-          */
-
-          Label startTry = new Label();
-          Label endTry = new Label();
-          Label handler = new Label();
-          mv.visitTryCatchBlock(startTry, endTry, handler, "java/lang/LinkageError");
-          // InjectedClassHelper.HelperClassInfo helperClassInfo = InjectedClassHelper
-          //   .getHelperClassInfo(this, name);
-          mv.visitVarInsn(ALOAD, 0);
-          mv.visitVarInsn(ALOAD, 1);
-          mv.visitMethodInsn(
-              INVOKESTATIC,
-              Type.getInternalName(InjectedClassHelper.class),
-              "getHelperClassInfo",
-              "(Ljava/lang/ClassLoader;Ljava/lang/String;)"
-                  + Type.getDescriptor(HelperClassInfo.class),
-              false);
-          mv.visitVarInsn(ASTORE, argumentCount + 1); // store helperClassInfo
-          mv.visitVarInsn(ALOAD, argumentCount + 1);
-          Label notHelperClass = new Label();
-          mv.visitJumpInsn(IFNULL, notHelperClass);
-
-          // getHelperClassInfo returned non-null
-          // Class<?> clazz = findLoadedClass(name);
-          mv.visitVarInsn(ALOAD, 0);
-          mv.visitVarInsn(ALOAD, 1);
-          mv.visitMethodInsn(
-              INVOKEVIRTUAL,
-              internalClassName,
-              "findLoadedClass",
-              "(Ljava/lang/String;)Ljava/lang/Class;",
-              false);
-          mv.visitVarInsn(ASTORE, argumentCount + 2); // store clazz
-          mv.visitVarInsn(ALOAD, argumentCount + 2);
-          mv.visitJumpInsn(IFNULL, startTry);
-
-          // findLoadedClass returned non-null
-          // return clazz
-          mv.visitVarInsn(ALOAD, argumentCount + 2);
-          mv.visitInsn(ARETURN);
-
-          mv.visitLabel(startTry);
-          if (frames) {
-            mv.visitFrame(
-                Opcodes.F_APPEND,
-                2,
-                new Object[] {Type.getInternalName(HelperClassInfo.class), "java/lang/Class"},
-                0,
-                null);
-          }
-          // byte[] bytes = helperClassInfo.getClassBytes();
-          mv.visitVarInsn(ALOAD, argumentCount + 1);
-          mv.visitMethodInsn(
-              INVOKEINTERFACE,
-              Type.getInternalName(HelperClassInfo.class),
-              "getClassBytes",
-              "()[B",
-              true);
-          mv.visitVarInsn(ASTORE, argumentCount + 3); // store bytes
-
-          // return defineClass(name, bytes, 0, bytes.length,
-          //   helperClassInfo.getProtectionDomain());
-          mv.visitVarInsn(ALOAD, 0);
-          mv.visitVarInsn(ALOAD, 1);
-          mv.visitVarInsn(ALOAD, argumentCount + 3);
-          mv.visitInsn(ICONST_0);
-          mv.visitVarInsn(ALOAD, argumentCount + 3);
-          mv.visitInsn(ARRAYLENGTH);
-          mv.visitVarInsn(ALOAD, argumentCount + 1);
-          mv.visitMethodInsn(
-              INVOKEINTERFACE,
-              Type.getInternalName(HelperClassInfo.class),
-              "getProtectionDomain",
-              "()Ljava/security/ProtectionDomain;",
-              true);
-          mv.visitMethodInsn(
-              INVOKEVIRTUAL,
-              internalClassName,
-              "defineClass",
-              "(Ljava/lang/String;[BIILjava/security/ProtectionDomain;)Ljava/lang/Class;",
-              false);
-          mv.visitLabel(endTry);
-          mv.visitInsn(ARETURN);
-
-          mv.visitLabel(handler);
-          if (frames) {
-            mv.visitFrame(Opcodes.F_SAME1, 0, null, 1, new Object[] {"java/lang/LinkageError"});
-          }
-          mv.visitVarInsn(ASTORE, argumentCount + 3); // store LinkageError
-          // clazz = findLoadedClass(name);
-          mv.visitVarInsn(ALOAD, 0);
-          mv.visitVarInsn(ALOAD, 1);
-          mv.visitMethodInsn(
-              INVOKEVIRTUAL,
-              internalClassName,
-              "findLoadedClass",
-              "(Ljava/lang/String;)Ljava/lang/Class;",
-              false);
-          mv.visitVarInsn(ASTORE, argumentCount + 2); // score clazz
-          mv.visitVarInsn(ALOAD, argumentCount + 2);
-          Label throwError = new Label();
-          mv.visitJumpInsn(IFNULL, throwError);
-          // return clazz
-          mv.visitVarInsn(ALOAD, argumentCount + 2);
-          mv.visitInsn(ARETURN);
-          mv.visitLabel(throwError);
-          if (frames) {
-            mv.visitFrame(Opcodes.F_APPEND, 1, new Object[] {"java/lang/LinkageError"}, 0, null);
-          }
-          // throw error
-          mv.visitVarInsn(ALOAD, argumentCount + 3);
-          mv.visitInsn(ATHROW);
-
-          mv.visitLabel(notHelperClass);
-          if (frames) {
-            mv.visitFrame(Opcodes.F_CHOP, 3, null, 0, null);
-            // ensure there aren't two frames at the same location
-            mv.visitInsn(NOP);
+  @SuppressWarnings("unused")
+  public static class LoadClassAdvice {
+
+    // Class loader stub is shaded back to the real class loader class. It is used to call protected
+    // method from the advice that the complier won't let us call directly. During runtime it is
+    // fine since this code is inlined into subclasses of ClassLoader that can call protected
+    // methods.
+    @Advice.OnMethodEnter(skipOn = Advice.OnNonDefaultValue.class)
+    public static Class<?> onEnter(
+        @Advice.This java.lang.ClassLoader classLoader,
+        @Advice.This
+            io.opentelemetry.javaagent.instrumentation.internal.classloader.stub.ClassLoader
+                classLoaderStub,
+        @Advice.Argument(0) String name) {
+      InjectedClassHelper.HelperClassInfo helperClassInfo =
+          InjectedClassHelper.getHelperClassInfo(classLoader, name);
+      if (helperClassInfo != null) {
+        Class<?> clazz = classLoaderStub.findLoadedClass(name);
+        if (clazz != null) {
+          return clazz;
+        }
+        try {
+          byte[] bytes = helperClassInfo.getClassBytes();
+          return classLoaderStub.defineClass(
+              name, bytes, 0, bytes.length, helperClassInfo.getProtectionDomain());
+        } catch (LinkageError error) {
+          clazz = classLoaderStub.findLoadedClass(name);
+          if (clazz != null) {
+            return clazz;
           }
+          throw error;
         }
+      }
+      return null;
+    }
 
-        @Override
-        public void visitMaxs(int maxStack, int maxLocals) {
-          // minimally we have argumentCount parameters + this + 3 locals added by us
-          super.visitMaxs(maxStack, Math.max(maxLocals, argumentCount + 1 + 3));
-        }
-      };
+    @AssignReturned.ToReturned
+    @Advice.OnMethodExit(onThrowable = Throwable.class)
+    public static Class<?> onExit(
+        @Advice.Return Class<?> originalResult, @Advice.Enter Class<?> loadedClass) {
+      return loadedClass != null ? loadedClass : originalResult;
     }
   }
 }
diff --git a/javaagent/build.gradle.kts b/javaagent/build.gradle.kts
@@ -94,7 +94,7 @@ dependencies {
   baseJavaagentLibs(project(":instrumentation:opentelemetry-instrumentation-annotations-1.16:javaagent"))
   baseJavaagentLibs(project(":instrumentation:executors:javaagent"))
   baseJavaagentLibs(project(":instrumentation:internal:internal-application-logger:javaagent"))
-  baseJavaagentLibs(project(":instrumentation:internal:internal-class-loader:javaagent"))
+  baseJavaagentLibs(project(":instrumentation:internal:internal-class-loader:javaagent", configuration = "shadow"))
   baseJavaagentLibs(project(":instrumentation:internal:internal-eclipse-osgi-3.6:javaagent"))
   baseJavaagentLibs(project(":instrumentation:internal:internal-lambda:javaagent"))
   baseJavaagentLibs(project(":instrumentation:internal:internal-reflection:javaagent"))
diff --git a/settings.gradle.kts b/settings.gradle.kts
@@ -264,6 +264,7 @@ include(":instrumentation:hystrix-1.4:javaagent")
 include(":instrumentation:influxdb-2.4:javaagent")
 include(":instrumentation:internal:internal-application-logger:bootstrap")
 include(":instrumentation:internal:internal-application-logger:javaagent")
+include(":instrumentation:internal:internal-class-loader:compile-stub")
 include(":instrumentation:internal:internal-class-loader:javaagent")
 include(":instrumentation:internal:internal-class-loader:javaagent-integration-tests")
 include(":instrumentation:internal:internal-eclipse-osgi-3.6:javaagent")

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+plugins {`
	`2`	`+ id("otel.java-conventions")`
	`3`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -6,5 +6,5 @@ dependencies {`
`6`	`6`	`compileOnly("org.apache.commons:commons-lang3:3.12.0")`
`7`	`7`	`testImplementation("org.apache.commons:commons-lang3:3.12.0")`
`8`	`8`
`9`		`- testInstrumentation(project(":instrumentation:internal:internal-class-loader:javaagent"))`
	`9`	`+ testInstrumentation(project(":instrumentation:internal:internal-class-loader:javaagent", configuration = "shadow"))`
`10`	`10`	`}`