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 index ea87471503c7..33fc4e2c6dbd 100644 --- a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/build.gradle.kts +++ b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/build.gradle.kts @@ -3,6 +3,7 @@ plugins { } 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")) diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/main/java/instrumentation/TestFailableCallable.java b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/main/java/instrumentation/TestFailableCallable.java new file mode 100644 index 000000000000..7ce45826964d --- /dev/null +++ b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/main/java/instrumentation/TestFailableCallable.java @@ -0,0 +1,10 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package instrumentation; + +import org.apache.commons.lang3.function.FailableCallable; + +public interface TestFailableCallable extends FailableCallable {} diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule2.java b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule2.java new file mode 100644 index 000000000000..31a38a5e512d --- /dev/null +++ b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule2.java @@ -0,0 +1,64 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package instrumentation; + +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static java.util.Collections.singletonList; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.none; + +import com.google.auto.service.AutoService; +import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import java.util.List; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(InstrumentationModule.class) +public class TestInstrumentationModule2 extends InstrumentationModule { + public TestInstrumentationModule2() { + super("test-instrumentation2"); + } + + @Override + public List typeInstrumentations() { + return singletonList(new TestTypeInstrumentation()); + } + + @Override + public boolean isHelperClass(String className) { + return "instrumentation.TestFailableCallable".equals(className); + } + + public static class TestTypeInstrumentation implements TypeInstrumentation { + @Override + public ElementMatcher classLoaderOptimization() { + return hasClassesNamed("org.apache.commons.lang3.function.FailableCallable"); + } + + @Override + public ElementMatcher typeMatcher() { + return implementsInterface(named("org.apache.commons.lang3.function.FailableCallable")); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + none(), TestInstrumentationModule2.class.getName() + "$TestAdvice"); + } + } + + @SuppressWarnings("unused") + public static class TestAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static String onEnter() { + return TestFailableCallable.class.getName(); + } + } +} diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/groovy/RegressionTest.groovy b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/groovy/RegressionTest.groovy new file mode 100644 index 000000000000..8a4601927225 --- /dev/null +++ b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/groovy/RegressionTest.groovy @@ -0,0 +1,19 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification + +class RegressionTest extends AgentInstrumentationSpecification { + + // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/5155 + // loading a class that is extended/implemented by a helper class causes + // java.lang.LinkageError: loader 'app' (instance of jdk.internal.loader.ClassLoaders$AppClassLoader) attempted duplicate interface definition for org.apache.commons.lang3.function.FailableCallable + // this test verifies that the duplicate class definition LinkageError is not thrown into + // application code + def "test no duplicate class definition"() { + expect: + Class.forName("org.apache.commons.lang3.function.FailableCallable") != null + } +} 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 index b544abcef43e..39a6fdb00d15 100644 --- 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 @@ -6,6 +6,7 @@ package io.opentelemetry.javaagent.instrumentation.internal.classloader; import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; import com.google.auto.service.AutoService; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; @@ -29,8 +30,17 @@ public boolean isHelperClass(String className) { return className.equals("io.opentelemetry.javaagent.tooling.Constants"); } + @Override + public List getAdditionalHelperClassNames() { + return singletonList( + "io.opentelemetry.javaagent.instrumentation.internal.classloader.DefineClassUtil"); + } + @Override public List typeInstrumentations() { - return asList(new ClassLoaderInstrumentation(), new ResourceInjectionInstrumentation()); + return asList( + new ClassLoaderInstrumentation(), + new ResourceInjectionInstrumentation(), + new DefineClassInstrumentation()); } } diff --git a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/DefineClassInstrumentation.java b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/DefineClassInstrumentation.java new file mode 100644 index 000000000000..48496bf3b12c --- /dev/null +++ b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/DefineClassInstrumentation.java @@ -0,0 +1,187 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.internal.classloader; + +import static net.bytebuddy.matcher.ElementMatchers.named; + +import io.opentelemetry.javaagent.bootstrap.DefineClassContext; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +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.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; + +public class DefineClassInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return named("java.lang.ClassLoader"); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyTransformer( + (builder, typeDescription, classLoader, module) -> + 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 fields, + MethodList methods, + int writerFlags, + int readerFlags) { + return new ClassLoaderClassVisitor(classVisitor); + } + })); + } + + private static class ClassLoaderClassVisitor extends ClassVisitor { + + ClassLoaderClassVisitor(ClassVisitor cv) { + super(Opcodes.ASM7, cv); + } + + @Override + public MethodVisitor visitMethod( + int access, String name, String descriptor, String signature, String[] exceptions) { + MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions); + // apply the following transformation to defineClass method + /* + DefineClassContext.enter(); + try { + // original method body + DefineClassContext.exit(); + return result; + } catch (LinkageError error) { + boolean helpersInjected = DefineClassContext.exitAndGet(); + Class loaded = findLoadedClass(className); + return DefineClassUtil.handleLinkageError(error, helpersInjected, loaded); + } catch (Throwable throwable) { + DefineClassContext.exit(); + throw throwable; + } + */ + if ("defineClass".equals(name) + && ("(Ljava/lang/String;[BIILjava/security/ProtectionDomain;)Ljava/lang/Class;" + .equals(descriptor) + || "(Ljava/lang/String;Ljava/nio/ByteBuffer;Ljava/security/ProtectionDomain;)Ljava/lang/Class;" + .equals(descriptor))) { + mv = + new MethodVisitor(api, mv) { + Label start = new Label(); + Label end = new Label(); + Label handler = new Label(); + + @Override + public void visitCode() { + mv.visitMethodInsn( + Opcodes.INVOKESTATIC, + Type.getInternalName(DefineClassContext.class), + "enter", + "()V", + false); + mv.visitTryCatchBlock(start, end, end, "java/lang/LinkageError"); + // catch other exceptions + mv.visitTryCatchBlock(start, end, handler, null); + mv.visitLabel(start); + + super.visitCode(); + } + + @Override + public void visitInsn(int opcode) { + if (opcode == Opcodes.ARETURN) { + mv.visitMethodInsn( + Opcodes.INVOKESTATIC, + Type.getInternalName(DefineClassContext.class), + "exit", + "()V", + false); + } + super.visitInsn(opcode); + } + + @Override + public void visitMaxs(int maxStack, int maxLocals) { + // handle LinkageError + mv.visitLabel(end); + mv.visitFrame( + Opcodes.F_FULL, + 2, + new Object[] {"java/lang/ClassLoader", "java/lang/String"}, + 1, + new Object[] {"java/lang/LinkageError"}); + mv.visitMethodInsn( + Opcodes.INVOKESTATIC, + Type.getInternalName(DefineClassContext.class), + "exitAndGet", + "()Z", + false); + mv.visitVarInsn(Opcodes.ALOAD, 0); + mv.visitVarInsn(Opcodes.ALOAD, 1); + mv.visitMethodInsn( + Opcodes.INVOKEVIRTUAL, + "java/lang/ClassLoader", + "findLoadedClass", + "(Ljava/lang/String;)Ljava/lang/Class;", + false); + mv.visitMethodInsn( + Opcodes.INVOKESTATIC, + Type.getInternalName(DefineClassUtil.class), + "handleLinkageError", + "(Ljava/lang/LinkageError;ZLjava/lang/Class;)Ljava/lang/Class;", + false); + mv.visitInsn(Opcodes.ARETURN); + + // handle Throwable + mv.visitLabel(handler); + mv.visitFrame( + Opcodes.F_FULL, + 2, + new Object[] {"java/lang/ClassLoader", "java/lang/String"}, + 1, + new Object[] {"java/lang/Throwable"}); + mv.visitMethodInsn( + Opcodes.INVOKESTATIC, + Type.getInternalName(DefineClassContext.class), + "exit", + "()V", + false); + mv.visitInsn(Opcodes.ATHROW); + + super.visitMaxs(maxStack, maxLocals); + } + }; + } + return mv; + } + } +} diff --git a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/DefineClassUtil.java b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/DefineClassUtil.java new file mode 100644 index 000000000000..33b4771d531d --- /dev/null +++ b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/DefineClassUtil.java @@ -0,0 +1,41 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.internal.classloader; + +@SuppressWarnings("unused") +public final class DefineClassUtil { + private DefineClassUtil() {} + + /** + * Handle LinkageError in ClassLoader.defineClass. Call to this method is inserted into + * ClassLoader.defineClass by DefineClassInstrumentation. + * + * @param linkageError LinkageError that happened in defineClass + * @param helpersInjected whether helpers were injected during defineClass call + * @param clazz Class that is being defined if it is already loaded + * @return give Class if LinkageError was a duplicate class definition error + */ + public static Class handleLinkageError( + LinkageError linkageError, boolean helpersInjected, Class clazz) { + // only attempt to recover from duplicate class definition if helpers were injected during + // the defineClass call + if (!helpersInjected + // if exception was duplicate class definition we'll have access to the loaded class + || clazz == null + // duplicate class definition throws LinkageError, we can ignore its subclasses + || linkageError.getClass() != LinkageError.class) { + throw linkageError; + } + // check that the exception is a duplicate class or interface definition + String message = linkageError.getMessage(); + if (message == null + || !(message.contains("duplicate interface definition") + || message.contains("duplicate class definition"))) { + throw linkageError; + } + return clazz; + } +} diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/DefineClassContext.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/DefineClassContext.java new file mode 100644 index 000000000000..e0a3fade5046 --- /dev/null +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/DefineClassContext.java @@ -0,0 +1,59 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.bootstrap; + +/** Context for tracking whether helper classes were injected during defining class. */ +public final class DefineClassContext { + private static final ThreadLocal contextThreadLocal = new ThreadLocal<>(); + + private int counter; + private boolean helpersInjected; + + private DefineClassContext() {} + + /** + * Start defining class. Instrumentation inserts call to this method into ClassLoader.defineClass. + */ + public static void enter() { + DefineClassContext context = contextThreadLocal.get(); + if (context == null) { + context = new DefineClassContext(); + contextThreadLocal.set(context); + } + context.counter++; + } + + /** + * Finish defining class. Instrumentation inserts call to this method into + * ClassLoader.defineClass. + */ + public static void exit() { + exitAndGet(); + } + + /** + * Finish defining class. Instrumentation inserts call to this method into + * ClassLoader.defineClass. + * + * @return true if helper classes were injected + */ + public static boolean exitAndGet() { + DefineClassContext context = contextThreadLocal.get(); + context.counter--; + if (context.counter == 0) { + contextThreadLocal.remove(); + } + return context.helpersInjected; + } + + /** Called when helper classes are injected. */ + public static void helpersInjected() { + DefineClassContext context = contextThreadLocal.get(); + if (context != null) { + context.helpersInjected = true; + } + } +} diff --git a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java index 8cffc39d1a3b..988c5d90532e 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java @@ -6,6 +6,7 @@ package io.opentelemetry.javaagent.tooling; import io.opentelemetry.instrumentation.api.cache.Cache; +import io.opentelemetry.javaagent.bootstrap.DefineClassContext; import io.opentelemetry.javaagent.bootstrap.HelperResources; import io.opentelemetry.javaagent.tooling.muzzle.HelperResource; import java.io.File; @@ -195,6 +196,7 @@ private void injectHelperClasses( cl -> { try { logger.debug("Injecting classes onto classloader {} -> {}", cl, helperClassNames); + DefineClassContext.helpersInjected(); Map classnameToBytes = getHelperMap(); Map> classes;