Recover from duplicate class definition errors

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"))

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<R, E extends Throwable> extends FailableCallable<R, E> {}

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<TypeInstrumentation> 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<ClassLoader> classLoaderOptimization() {
+ return hasClassesNamed("org.apache.commons.lang3.function.FailableCallable");
+ }
+
+ @Override
+ public ElementMatcher<TypeDescription> 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();
+ }
+ }
+}

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:/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
+ }
+}

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<String> getAdditionalHelperClassNames() {
+ return singletonList(
+ "io.opentelemetry.javaagent.instrumentation.internal.classloader.DefineClassUtil");
+ }
+
  @Override
  public List<TypeInstrumentation> typeInstrumentations() {
- return asList(new ClassLoaderInstrumentation(), new ResourceInjectionInstrumentation());
+ return asList(
+ new ClassLoaderInstrumentation(),
+ new ResourceInjectionInstrumentation(),
+ new DefineClassInstrumentation());
  }
 }

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<TypeDescription> 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<FieldDescription.InDefinedShape> 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;
+ }
+ }
+}

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;
+ }
+}