diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/advice/AdviceForwardLookupSupplier.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/advice/AdviceForwardLookupSupplier.java new file mode 100644 index 000000000000..9905bc2e32ed --- /dev/null +++ b/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 { + + @Override + public MethodHandles.Lookup get() { + return MethodHandles.lookup(); + } +} diff --git a/javaagent-tooling/build.gradle.kts b/javaagent-tooling/build.gradle.kts index dc1017d14d5a..5434028a60a0 100644 --- a/javaagent-tooling/build.gradle.kts +++ b/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")) diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java index a0924738e44b..fbc7b8aad68a 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java +++ b/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; diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/ForwardIndyAdviceTransformer.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/ForwardIndyAdviceTransformer.java new file mode 100644 index 000000000000..3512266e5ecf --- /dev/null +++ b/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 fieldList, + MethodList methodList, + int writerFlags, + int readerFlags) { + + return new ClassVisitor(AsmApi.VERSION, classVisitor) { + final Map> 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 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 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(); + }; + } +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/PatchByteCodeVersionTransformer.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/PatchByteCodeVersionTransformer.java deleted file mode 100644 index cc5a4f9d7245..000000000000 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/PatchByteCodeVersionTransformer.java +++ /dev/null @@ -1,103 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.tooling.instrumentation.indy; - -import static org.objectweb.asm.ClassWriter.COMPUTE_FRAMES; - -import java.security.ProtectionDomain; -import net.bytebuddy.ClassFileVersion; -import net.bytebuddy.agent.builder.AgentBuilder; -import net.bytebuddy.asm.Advice; -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.MethodVisitor; -import org.objectweb.asm.Opcodes; -import org.objectweb.asm.commons.JSRInlinerAdapter; - -/** - * Patches the class file version to 51 (Java 7) in order to support injecting {@code INVOKEDYNAMIC} - * instructions via {@link Advice.WithCustomMapping#bootstrap} which is important for indy plugins. - */ -public class PatchByteCodeVersionTransformer implements AgentBuilder.Transformer { - - 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) { - - if (isAtLeastJava7(typeDescription)) { - // we can avoid the expensive stack frame re-computation if stack frames are already present - // in the bytecode. - return builder; - } - return builder.visit( - new AsmVisitorWrapper.AbstractBase() { - @Override - public ClassVisitor wrap( - TypeDescription typeDescription, - ClassVisitor classVisitor, - Implementation.Context context, - TypePool typePool, - FieldList fieldList, - MethodList methodList, - int writerFlags, - int readerFlags) { - - return new ClassVisitor(Opcodes.ASM7, classVisitor) { - - @Override - public void visit( - int version, - int access, - String name, - String signature, - String superName, - String[] interfaces) { - - super.visit(Opcodes.V1_7, access, name, signature, superName, interfaces); - } - - @Override - public MethodVisitor visitMethod( - int access, - String name, - String descriptor, - String signature, - String[] exceptions) { - - MethodVisitor methodVisitor = - super.visitMethod(access, name, descriptor, signature, exceptions); - return new JSRInlinerAdapter( - methodVisitor, access, name, descriptor, signature, exceptions); - } - }; - } - - @Override - public int mergeWriter(int flags) { - // class files with version < Java 7 don't require a stack frame map - // as we're patching the version to at least 7, we have to compute the frames - return flags | COMPUTE_FRAMES; - } - }); - } -} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/ClassLoaderMap.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/ClassLoaderMap.java index 2a6fe77f8d86..bb5179f5f89d 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/ClassLoaderMap.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/ClassLoaderMap.java @@ -20,9 +20,12 @@ class ClassLoaderMap { private static final Cache>> data = Cache.weak(); private static final Map 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); }; diff --git a/javaagent-tooling/src/testPatchBytecodeVersion/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/ComputeFramesAsmVisitorWrapper.java b/javaagent-tooling/src/testPatchBytecodeVersion/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/ComputeFramesAsmVisitorWrapper.java deleted file mode 100644 index 52aa114ce938..000000000000 --- a/javaagent-tooling/src/testPatchBytecodeVersion/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/ComputeFramesAsmVisitorWrapper.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.tooling.instrumentation.indy; - -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.jar.asm.ClassWriter; -import net.bytebuddy.pool.TypePool; -import org.objectweb.asm.ClassVisitor; - -public class ComputeFramesAsmVisitorWrapper extends AsmVisitorWrapper.AbstractBase { - @Override - public int mergeWriter(int flags) { - return super.mergeWriter(flags | ClassWriter.COMPUTE_FRAMES); - } - - @Override - public ClassVisitor wrap( - TypeDescription instrumentedType, - ClassVisitor classVisitor, - Implementation.Context implementationContext, - TypePool typePool, - FieldList fields, - MethodList methods, - int writerFlags, - int readerFlags) { - return classVisitor; - } -} diff --git a/javaagent-tooling/src/testPatchBytecodeVersion/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/OldBytecode.java b/javaagent-tooling/src/testPatchBytecodeVersion/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/OldBytecode.java deleted file mode 100644 index 86070a0106c0..000000000000 --- a/javaagent-tooling/src/testPatchBytecodeVersion/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/OldBytecode.java +++ /dev/null @@ -1,150 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.tooling.instrumentation.indy; - -import java.lang.reflect.Modifier; -import net.bytebuddy.ByteBuddy; -import net.bytebuddy.ClassFileVersion; -import net.bytebuddy.asm.AsmVisitorWrapper; -import net.bytebuddy.description.method.MethodDescription; -import net.bytebuddy.dynamic.DynamicType; -import net.bytebuddy.dynamic.scaffold.InstrumentedType; -import net.bytebuddy.implementation.Implementation; -import net.bytebuddy.implementation.bytecode.ByteCodeAppender; -import org.objectweb.asm.Label; -import org.objectweb.asm.MethodVisitor; -import org.objectweb.asm.Opcodes; -import org.objectweb.asm.Type; - -public class OldBytecode { - - private OldBytecode() {} - - /** - * Generates and run a simple class with a {@link #toString()} implementation as if it had been - * compiled on an older java compiler - * - * @param className class name - * @param version bytecode version - * @return "toString" - */ - public static String generateAndRun(String className, ClassFileVersion version) { - try (DynamicType.Unloaded unloadedClass = makeClass(className, version)) { - Class generatedClass = unloadedClass.load(OldBytecode.class.getClassLoader()).getLoaded(); - - return generatedClass.getConstructor().newInstance().toString(); - - } catch (Throwable e) { - throw new RuntimeException(e); - } - } - - private static DynamicType.Unloaded makeClass( - String className, ClassFileVersion version) { - return new ByteBuddy(version) - .subclass(Object.class) - // required otherwise stack frames aren't computed when needed - .visit( - version.isAtLeast(ClassFileVersion.JAVA_V7) - ? new ComputeFramesAsmVisitorWrapper() - : AsmVisitorWrapper.NoOp.INSTANCE) - .name(className) - .defineMethod("toString", String.class, Modifier.PUBLIC) - .intercept(new ToStringMethod()) - .make(); - } - - private static class ToStringMethod implements Implementation, ByteCodeAppender { - - @Override - public ByteCodeAppender appender(Target implementationTarget) { - return this; - } - - @Override - public InstrumentedType prepare(InstrumentedType instrumentedType) { - return instrumentedType; - } - - @Override - public Size apply( - MethodVisitor methodVisitor, - Context implementationContext, - MethodDescription instrumentedMethod) { - - // Bytecode archeology: - // - // JSR and RET bytecode instructions were used to create "subroutines". Those were used - // in try/catch blocks as an attempt to avoid some bytecode duplication, this was later - // replaced with inlining. - // Starting from Java 5, no java compiler is expected to issue bytecode containing them and - // the JVM bytecode validation will reject it. - // - // Java 7 bytecode introduced the concept of "stack map frames", which describe the types of - // the objects that are stored on the stack during method body execution. - // - // As a consequence, the code below allows to test the following combinations: - // - java 1 to java 4 bytecode with JSR/RET opcodes - // - java 5 and java 6 bytecode without stack map frames - // - java 7 and later bytecode with stack map frames, those are automatically added by the - // ComputeFramesAsmVisitorWrapper. - // - boolean useJsrRet = - implementationContext.getClassFileVersion().isLessThan(ClassFileVersion.JAVA_V5); - - if (useJsrRet) { - // return "toString"; - // - // using obsolete JSR/RET instructions - Label target = new Label(); - methodVisitor.visitJumpInsn(Opcodes.JSR, target); - - methodVisitor.visitVarInsn(Opcodes.ALOAD, 1); - methodVisitor.visitInsn(Opcodes.ARETURN); - methodVisitor.visitLabel(target); - methodVisitor.visitVarInsn(Opcodes.ASTORE, 2); - methodVisitor.visitLdcInsn("toString"); - methodVisitor.visitVarInsn(Opcodes.ASTORE, 1); - methodVisitor.visitVarInsn(Opcodes.RET, 2); - return new Size(1, 3); - } else { - // try { - // return "toString"; - // } catch (Throwable e) { - // return e.getMessage(); - // } - // - // the Throwable exception is added to stack map frames with java7+, and needs to be - // added when upgrading the bytecode - Label start = new Label(); - Label end = new Label(); - Label handler = new Label(); - - methodVisitor.visitTryCatchBlock( - start, end, handler, Type.getInternalName(Throwable.class)); - methodVisitor.visitLabel(start); - methodVisitor.visitLdcInsn("toString"); - methodVisitor.visitLabel(end); - - methodVisitor.visitInsn(Opcodes.ARETURN); - - methodVisitor.visitLabel(handler); - methodVisitor.visitVarInsn(Opcodes.ASTORE, 1); - methodVisitor.visitVarInsn(Opcodes.ALOAD, 1); - - methodVisitor.visitMethodInsn( - Opcodes.INVOKEVIRTUAL, - Type.getInternalName(Throwable.class), - "getMessage", - Type.getMethodDescriptor(Type.getType(String.class)), - false); - methodVisitor.visitInsn(Opcodes.ARETURN); - - return new Size(1, 2); - } - } - } -} diff --git a/javaagent-tooling/src/testPatchBytecodeVersion/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/PatchBytecodeVersionTest.java b/javaagent-tooling/src/testPatchBytecodeVersion/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/PatchBytecodeVersionTest.java deleted file mode 100644 index f8b1a4297f03..000000000000 --- a/javaagent-tooling/src/testPatchBytecodeVersion/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/PatchBytecodeVersionTest.java +++ /dev/null @@ -1,172 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.tooling.instrumentation.indy; - -import static net.bytebuddy.matcher.ElementMatchers.isMethod; -import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; -import static net.bytebuddy.matcher.ElementMatchers.named; -import static org.assertj.core.api.Assertions.assertThat; - -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.Arrays; -import java.util.List; -import java.util.stream.Stream; -import net.bytebuddy.ClassFileVersion; -import net.bytebuddy.agent.ByteBuddyAgent; -import net.bytebuddy.agent.builder.AgentBuilder; -import net.bytebuddy.agent.builder.ResettableClassFileTransformer; -import net.bytebuddy.description.method.MethodDescription; -import net.bytebuddy.dynamic.ClassFileLocator; -import net.bytebuddy.matcher.ElementMatcher; -import net.bytebuddy.utility.JavaModule; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.io.TempDir; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.MethodSource; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -class PatchBytecodeVersionTest { - - private static final String BYTEBUDDY_DUMP = "net.bytebuddy.dump"; - public static final String ORIGINAL_SUFFIX = "-original"; - public static final String CLASSNAME_PREFIX = "oldbytecode_"; - private static ResettableClassFileTransformer transformer; - - private static final Logger logger = LoggerFactory.getLogger(PatchBytecodeVersionTest.class); - - private static Path tempDir; - - @BeforeAll - static void setUp(@TempDir Path temp) { - - assertThat(temp).isEmptyDirectory(); - tempDir = temp; - System.setProperty(BYTEBUDDY_DUMP, temp.toString()); - - AgentBuilder builder = - new AgentBuilder.Default() - .disableClassFormatChanges() - .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION) - .with( - new AgentBuilder.Listener.Adapter() { - @Override - public void onError( - String typeName, - ClassLoader classLoader, - JavaModule module, - boolean loaded, - Throwable throwable) { - logger.error("Transformation error", throwable); - } - }) - .type(nameStartsWith(CLASSNAME_PREFIX)) - .transform(new PatchByteCodeVersionTransformer()) - .transform(transformerFor(isMethod().and(named("toString")))); - - ByteBuddyAgent.install(); - transformer = builder.installOn(ByteBuddyAgent.getInstrumentation()); - } - - @AfterAll - static void tearDown() { - transformer.reset( - ByteBuddyAgent.getInstrumentation(), AgentBuilder.RedefinitionStrategy.RETRANSFORMATION); - - System.clearProperty(BYTEBUDDY_DUMP); - } - - @ParameterizedTest - @MethodSource("bytecodeVersions") - void upgradeBytecode(ClassFileVersion version) { - - String className = CLASSNAME_PREFIX + version.getMinorMajorVersion(); - - int startCount = PatchTestAdvice.invocationCount.get(); - assertThat(PatchTestAdvice.invocationCount.get()).isEqualTo(startCount); - - assertThat(OldBytecode.generateAndRun(className, version)).isEqualTo("toString"); - - assertThat(PatchTestAdvice.invocationCount.get()).isEqualTo(startCount + 1); - - Path instrumentedClass; - Path instrumentedClassOriginal; - - try (Stream files = - Files.find( - tempDir, - 1, - (path, attr) -> { - String fileName = path.getFileName().toString(); - return Files.isRegularFile(path) - && fileName.startsWith(className) - && fileName.contains(ORIGINAL_SUFFIX); - })) { - - instrumentedClassOriginal = files.findFirst().orElseThrow(IllegalStateException::new); - String upgradedClassFileName = - instrumentedClassOriginal.getFileName().toString().replace(ORIGINAL_SUFFIX, ""); - instrumentedClass = instrumentedClassOriginal.resolveSibling(upgradedClassFileName); - - } catch (IOException e) { - throw new IllegalStateException(e); - } - - assertThat(instrumentedClass).exists(); - assertThat(instrumentedClassOriginal).exists(); - - assertThat(getBytecodeVersion(instrumentedClassOriginal)) - .describedAs( - "expected original bytecode for class '%s' in '%s' should have been compiled for %s", - className, instrumentedClassOriginal.toAbsolutePath(), version) - .isEqualTo(version); - - if (version.isLessThan(ClassFileVersion.JAVA_V7)) { - assertThat(getBytecodeVersion(instrumentedClass)) - .describedAs( - "expected instrumented bytecode for class '%s' in '%s' should have been upgraded to Java 7", - className, instrumentedClass.toAbsolutePath()) - .isEqualTo(ClassFileVersion.JAVA_V7); - } else { - assertThat(getBytecodeVersion(instrumentedClass)) - .describedAs("original bytecode version shouldn't be altered") - .isEqualTo(version); - } - } - - static List bytecodeVersions() { - return Arrays.asList( - ClassFileVersion.JAVA_V1, - ClassFileVersion.JAVA_V2, - ClassFileVersion.JAVA_V3, - ClassFileVersion.JAVA_V4, - ClassFileVersion.JAVA_V5, - ClassFileVersion.JAVA_V6, - // Java 7 and later should not be upgraded - ClassFileVersion.JAVA_V7, - ClassFileVersion.JAVA_V8); - } - - private static ClassFileVersion getBytecodeVersion(Path file) { - try { - return ClassFileVersion.ofClassFile(Files.readAllBytes(file)); - } catch (IOException e) { - throw new IllegalStateException(e); - } - } - - private static AgentBuilder.Transformer.ForAdvice transformerFor( - ElementMatcher.Junction methodMatcher) { - return new AgentBuilder.Transformer.ForAdvice() - .with( - new AgentBuilder.LocationStrategy.Simple( - ClassFileLocator.ForClassLoader.of(PatchTestAdvice.class.getClassLoader()))) - .advice(methodMatcher, PatchTestAdvice.class.getName()); - } -} diff --git a/javaagent-tooling/src/testPatchBytecodeVersion/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/PatchTestAdvice.java b/javaagent-tooling/src/testPatchBytecodeVersion/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/PatchTestAdvice.java deleted file mode 100644 index d0ce6000c730..000000000000 --- a/javaagent-tooling/src/testPatchBytecodeVersion/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/PatchTestAdvice.java +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.tooling.instrumentation.indy; - -import java.util.concurrent.atomic.AtomicInteger; -import net.bytebuddy.asm.Advice; - -@SuppressWarnings("unused") -public class PatchTestAdvice { - - public static final AtomicInteger invocationCount = new AtomicInteger(0); - - @Advice.OnMethodExit(suppress = Throwable.class) - public static void onExit() { - invocationCount.incrementAndGet(); - } -} 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 f382e2e691b9..46b055e971d7 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java @@ -13,6 +13,7 @@ import io.opentelemetry.javaagent.bootstrap.HelperResources; import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper; import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper.HelperClassInfo; +import io.opentelemetry.javaagent.bootstrap.advice.AdviceForwardLookupSupplier; import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldLookupSupplier; import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode; import io.opentelemetry.javaagent.instrumentation.executors.ExecutorLookupSupplier; @@ -289,7 +290,7 @@ private void injectHelperClasses( if (isBootClassLoader(classLoader)) { injectBootstrapClassLoader(classnameToBytes); } - } catch (Exception e) { + } catch (RuntimeException e) { if (logger.isLoggable(SEVERE)) { logger.log( SEVERE, @@ -297,18 +298,22 @@ private void injectHelperClasses( new Object[] {typeDescription, requestingName, classLoader}, e); } - throw new IllegalStateException(e); + throw e; } } - public static void injectHelperClasses( + public void injectHelperClasses( ClassLoader classLoader, Map> classNameToBytes) { - if (isBootClassLoader(classLoader)) { - throw new UnsupportedOperationException("boot loader not supported"); - } if (classNameToBytes.isEmpty()) { return; } + if (classLoader == null) { + if (instrumentation == null) { + throw new UnsupportedOperationException("boot loader not supported"); + } + injectBootstrapClassLoader(classNameToBytes); + return; + } Map map = helperClasses.computeIfAbsent(classLoader, (unused) -> new ConcurrentHashMap<>()); @@ -339,6 +344,7 @@ private static Map resolve(Map> classes // because all generated virtual field classes are in the same package we can use lookup to // define them addPackageLookup(new VirtualFieldLookupSupplier()); + addPackageLookup(new AdviceForwardLookupSupplier()); } private static void addPackageLookup(Supplier supplier) { @@ -350,7 +356,7 @@ private static ClassInjector getClassInjector(String packageName) { return lookup != null ? ClassInjector.UsingLookup.of(lookup) : null; } - private void injectBootstrapClassLoader(Map> inject) throws IOException { + private void injectBootstrapClassLoader(Map> inject) { Map classnameToBytes = resolve(inject); if (helperInjectorListener != null) { helperInjectorListener.onInjection(classnameToBytes); @@ -404,7 +410,7 @@ private void injectBootstrapClassLoader(Map> inject) th // a reference count -- but for now, starting simple. // Failures to create a tempDir are propagated as IOException and handled by transform - if (!classnameToBytes.isEmpty()) { + if (!classnameToBytes.isEmpty() && instrumentation != null) { File tempDir = createTempDir(); try { ClassInjector.UsingInstrumentation.of( @@ -417,8 +423,12 @@ private void injectBootstrapClassLoader(Map> inject) th } } - private static File createTempDir() throws IOException { - return Files.createTempDirectory("opentelemetry-temp-jars").toFile(); + private static File createTempDir() { + try { + return Files.createTempDirectory("opentelemetry-temp-jars").toFile(); + } catch (IOException exception) { + throw new IllegalStateException("Failed to create temporary directory.", exception); + } } private static void deleteTempDir(File file) {