diff --git a/instrumentation/internal/internal-lambda-java9/javaagent/build.gradle.kts b/instrumentation/internal/internal-lambda-java9/javaagent/build.gradle.kts deleted file mode 100644 index b0997aba5d7d..000000000000 --- a/instrumentation/internal/internal-lambda-java9/javaagent/build.gradle.kts +++ /dev/null @@ -1,20 +0,0 @@ -plugins { - id("otel.javaagent-instrumentation") -} - -// We cannot use otelJava { minJavaVersionSupported.set(JavaVersion.VERSION_1_9) } because compiler -// will fail with -Xlint without providing an error message. -// We cannot use "--release" javac option because that will forbid calling methods added in jdk 9. -java { - sourceCompatibility = JavaVersion.VERSION_1_8 - targetCompatibility = JavaVersion.VERSION_1_8 - toolchain { - languageVersion.set(null as JavaLanguageVersion?) - } -} - -tasks { - withType().configureEach { - options.release.set(null as Int?) - } -} diff --git a/instrumentation/internal/internal-lambda-java9/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/Java9LambdaTransformer.java b/instrumentation/internal/internal-lambda-java9/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/Java9LambdaTransformer.java deleted file mode 100644 index 586b34b4bab5..000000000000 --- a/instrumentation/internal/internal-lambda-java9/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/Java9LambdaTransformer.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.internal.lambda; - -import java.lang.instrument.ClassFileTransformer; -import java.lang.instrument.IllegalClassFormatException; - -/** Helper class for transforming lambda class bytes using java9 api. */ -public final class Java9LambdaTransformer { - - private Java9LambdaTransformer() {} - - public static byte[] transform( - ClassFileTransformer transformer, - byte[] classBytes, - String slashClassName, - Class targetClass) - throws IllegalClassFormatException { - return transformer.transform( - targetClass.getModule(), - targetClass.getClassLoader(), - slashClassName, - null, - null, - classBytes); - } -} diff --git a/instrumentation/internal/internal-lambda/javaagent/build.gradle.kts b/instrumentation/internal/internal-lambda/javaagent/build.gradle.kts index d49082abcc5c..70f921cc7802 100644 --- a/instrumentation/internal/internal-lambda/javaagent/build.gradle.kts +++ b/instrumentation/internal/internal-lambda/javaagent/build.gradle.kts @@ -4,7 +4,6 @@ plugins { dependencies { compileOnly(project(":javaagent-bootstrap")) - implementation(project(":instrumentation:internal:internal-lambda-java9:javaagent")) testImplementation(project(":javaagent-bootstrap")) } diff --git a/instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/InnerClassLambdaMetafactoryInstrumentation.java b/instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/InnerClassLambdaMetafactoryInstrumentation.java index 07d444913afa..4386101429f1 100644 --- a/instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/InnerClassLambdaMetafactoryInstrumentation.java +++ b/instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/InnerClassLambdaMetafactoryInstrumentation.java @@ -128,7 +128,7 @@ public void visitMethodInsn( Opcodes.GETFIELD, slashClassName, "targetClass", "Ljava/lang/Class;"); mv.visitMethodInsn( Opcodes.INVOKESTATIC, - Type.getInternalName(LambdaTransformer.class), + Type.getInternalName(LambdaTransformerHelper.class), "transform", "([BLjava/lang/String;Ljava/lang/Class;)[B", false); diff --git a/instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaInstrumentationModule.java b/instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaInstrumentationModule.java index 0b0c12b82ae3..41a903f8baf0 100644 --- a/instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaInstrumentationModule.java +++ b/instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaInstrumentationModule.java @@ -10,13 +10,14 @@ 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.internal.ExperimentalInstrumentationModule; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; -import java.util.ArrayList; +import java.util.Collections; import java.util.List; -import net.bytebuddy.utility.JavaModule; @AutoService(InstrumentationModule.class) -public class LambdaInstrumentationModule extends InstrumentationModule { +public class LambdaInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public LambdaInstrumentationModule() { super("internal-lambda"); } @@ -28,21 +29,16 @@ public boolean defaultEnabled(ConfigProperties config) { } @Override - public boolean isIndyModule() { - return false; + public List injectedClassNames() { + return Collections.singletonList( + "io.opentelemetry.javaagent.instrumentation.internal.lambda.LambdaTransformerHelper"); } @Override public List getAdditionalHelperClassNames() { // this instrumentation uses ASM not ByteBuddy so muzzle doesn't automatically add helper // classes - List classNames = new ArrayList<>(); - classNames.add("io.opentelemetry.javaagent.instrumentation.internal.lambda.LambdaTransformer"); - if (JavaModule.isSupported()) { - classNames.add( - "io.opentelemetry.javaagent.instrumentation.internal.lambda.Java9LambdaTransformer"); - } - return classNames; + return injectedClassNames(); } @Override diff --git a/instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaTransformer.java b/instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaTransformer.java deleted file mode 100644 index a5a393c06740..000000000000 --- a/instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaTransformer.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.internal.lambda; - -import io.opentelemetry.javaagent.bootstrap.ClassFileTransformerHolder; -import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper; -import java.lang.instrument.ClassFileTransformer; - -/** Helper class for transforming lambda class bytes. */ -public final class LambdaTransformer { - private static final boolean IS_JAVA_9 = isJava9(); - - private LambdaTransformer() {} - - private static boolean isJava9() { - try { - Class.forName("java.lang.Module", false, null); - return true; - } catch (ClassNotFoundException exception) { - return false; - } - } - - /** - * Called from {@code java.lang.invoke.InnerClassLambdaMetafactory} to transform lambda class - * bytes. - */ - @SuppressWarnings("unused") - public static byte[] transform(byte[] classBytes, String slashClassName, Class targetClass) { - // Skip transforming lambdas of injected helper classes. - if (InjectedClassHelper.isHelperClass(targetClass)) { - return classBytes; - } - - ClassFileTransformer transformer = ClassFileTransformerHolder.getClassFileTransformer(); - if (transformer != null) { - try { - byte[] result; - if (IS_JAVA_9) { - result = - Java9LambdaTransformer.transform( - transformer, classBytes, slashClassName, targetClass); - } else { - result = - transformer.transform( - targetClass.getClassLoader(), slashClassName, null, null, classBytes); - } - if (result != null) { - return result; - } - } catch (Throwable throwable) { - // sun.instrument.TransformerManager catches Throwable from ClassFileTransformer and ignores - // it, we do the same. - } - } - - return classBytes; - } -} diff --git a/instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaTransformerHelper.java b/instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaTransformerHelper.java new file mode 100644 index 000000000000..78ae91346e93 --- /dev/null +++ b/instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaTransformerHelper.java @@ -0,0 +1,43 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.internal.lambda; + +import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper; +import io.opentelemetry.javaagent.bootstrap.LambdaTransformer; +import io.opentelemetry.javaagent.bootstrap.LambdaTransformerHolder; + +/** Helper class for transforming lambda class bytes. */ +public final class LambdaTransformerHelper { + + private LambdaTransformerHelper() {} + + /** + * Called from {@code java.lang.invoke.InnerClassLambdaMetafactory} to transform lambda class + * bytes. + */ + @SuppressWarnings("unused") + public static byte[] transform(byte[] classBytes, String slashClassName, Class targetClass) { + // Skip transforming lambdas of injected helper classes. + if (InjectedClassHelper.isHelperClass(targetClass)) { + return classBytes; + } + LambdaTransformer transformer = LambdaTransformerHolder.getLambdaTransformer(); + if (transformer == null) { + return classBytes; + } + + try { + byte[] result = transformer.transform(slashClassName, targetClass, classBytes); + if (result != null) { + classBytes = result; + } + } catch (Throwable throwable) { + // sun.instrument.TransformerManager catches Throwable from ClassFileTransformer and ignores + // it, we do the same. + } + return classBytes; + } +} diff --git a/instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/AnnotationInstrumentationHelper.java b/instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/AnnotationInstrumentationHelper.java index a0f9c004f8d3..3a3fba544790 100644 --- a/instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/AnnotationInstrumentationHelper.java +++ b/instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/AnnotationInstrumentationHelper.java @@ -15,12 +15,20 @@ import kotlin.coroutines.Continuation; import kotlin.coroutines.intrinsics.IntrinsicsKt; +/** + * Instrumentation helper that is called through bytecode instrumentation. When using invokedynamic + * instrumentation this class is called through an injected proxy, and thus it should not pull any + * other class references than the ones that are already present in the target classloader or the + * bootstrap classloader. This is why the {@link MethodRequest} class is here passed as an {@link + * Object} as it allows to avoid having to inject extra classes in the target classloader + */ +@SuppressWarnings("unused") // methods calls injected through bytecode instrumentation public final class AnnotationInstrumentationHelper { private static final VirtualField, Context> contextField = VirtualField.find(Continuation.class, Context.class); - public static MethodRequest createMethodRequest( + public static Object createMethodRequest( Class declaringClass, String methodName, String withSpanValue, String spanKindString) { SpanKind spanKind = SpanKind.INTERNAL; if (spanKindString != null) { @@ -34,11 +42,10 @@ public static MethodRequest createMethodRequest( return MethodRequest.create(declaringClass, methodName, withSpanValue, spanKind); } - public static Context enterCoroutine( - int label, Continuation continuation, MethodRequest request) { + public static Context enterCoroutine(int label, Continuation continuation, Object request) { // label 0 means that coroutine is started, any other label means that coroutine is resumed if (label == 0) { - Context context = instrumenter().start(Context.current(), request); + Context context = instrumenter().start(Context.current(), (MethodRequest) request); // null continuation means that this method is not going to be resumed, and we don't need to // store the context if (continuation != null) { @@ -55,18 +62,14 @@ public static Scope openScope(Context context) { } public static void exitCoroutine( - Object result, - MethodRequest request, - Continuation continuation, - Context context, - Scope scope) { + Object result, Object request, Continuation continuation, Context context, Scope scope) { exitCoroutine(null, result, request, continuation, context, scope); } public static void exitCoroutine( Throwable error, Object result, - MethodRequest request, + Object request, Continuation continuation, Context context, Scope scope) { @@ -78,7 +81,7 @@ public static void exitCoroutine( // end the span when this method can not be resumed (coroutine is null) or if it has reached // final state (returns anything else besides COROUTINE_SUSPENDED) if (continuation == null || result != IntrinsicsKt.getCOROUTINE_SUSPENDED()) { - instrumenter().end(context, request, null, error); + instrumenter().end(context, (MethodRequest) request, null, error); } } diff --git a/instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/AnnotationInstrumentationModule.java b/instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/AnnotationInstrumentationModule.java index 84f4ab0c6338..1a36d4f418dc 100644 --- a/instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/AnnotationInstrumentationModule.java +++ b/instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/AnnotationInstrumentationModule.java @@ -11,12 +11,16 @@ 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.internal.ExperimentalInstrumentationModule; +import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.ClassInjector; +import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode; import java.util.List; import net.bytebuddy.matcher.ElementMatcher; /** Instrumentation for methods annotated with {@code WithSpan} annotation. */ @AutoService(InstrumentationModule.class) -public class AnnotationInstrumentationModule extends InstrumentationModule { +public class AnnotationInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public AnnotationInstrumentationModule() { super( @@ -26,12 +30,6 @@ public AnnotationInstrumentationModule() { "opentelemetry-instrumentation-annotations"); } - @Override - public boolean isIndyModule() { - // needs helper classes in the same class loader - return false; - } - @Override public int order() { // Run first to ensure other automatic instrumentation is added after and therefore is executed @@ -50,4 +48,17 @@ public ElementMatcher.Junction classLoaderMatcher() { public List typeInstrumentations() { return singletonList(new WithSpanInstrumentation()); } + + @Override + public void injectClasses(ClassInjector injector) { + // AnnotationInstrumentationHelper is called directly in the instrumented bytecode. + // + // With invokedynamic instrumentation a proxy class can be used as long as it does not pull + // extra types in the method signatures (which would require those types to also be available + // in the instrumented code). + injector + .proxyBuilder( + "io.opentelemetry.javaagent.instrumentation.kotlinxcoroutines.instrumentationannotations.AnnotationInstrumentationHelper") + .inject(InjectionMode.CLASS_ONLY); + } } diff --git a/instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/WithSpanInstrumentation.java b/instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/WithSpanInstrumentation.java index e98a11fef3fe..bf8c9bdfb1e9 100644 --- a/instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/WithSpanInstrumentation.java +++ b/instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/WithSpanInstrumentation.java @@ -232,7 +232,7 @@ private static MethodVisitor instrument( public void visitCode() { super.visitCode(); // add our local variables after method arguments, this will shift rest of the locals - requestLocal = newLocal(Type.getType(MethodRequest.class)); + requestLocal = newLocal(Type.getType(Object.class)); ourContinuationLocal = newLocal(Type.getType(Continuation.class)); contextLocal = newLocal(Type.getType(Context.class)); scopeLocal = newLocal(Type.getType(Scope.class)); @@ -324,7 +324,8 @@ public void visitEnd() { // initialize our local variables, start span and open scope { MethodNode temp = new MethodNode(); - // insert + // insert the following code + // // request = // AnnotationInstrumentationHelper.createMethodRequest(InstrumentedClass.class, // instrumentedMethodName, withSpanValue, withSpanKind) @@ -360,32 +361,26 @@ public void visitEnd() { } else { temp.visitInsn(Opcodes.ACONST_NULL); } - temp.visitMethodInsn( - Opcodes.INVOKESTATIC, - Type.getInternalName(AnnotationInstrumentationHelper.class), + visitInvokeHelperMethod( + temp, "createMethodRequest", - "(Ljava/lang/Class;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)" - + Type.getDescriptor(MethodRequest.class), - false); + "(Ljava/lang/Class;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Ljava/lang/Object;"); temp.visitInsn(Opcodes.DUP); temp.visitVarInsn(Opcodes.ASTORE, requestLocal); - temp.visitMethodInsn( - Opcodes.INVOKESTATIC, - Type.getInternalName(AnnotationInstrumentationHelper.class), + visitInvokeHelperMethod( + temp, "enterCoroutine", - "(ILkotlin/coroutines/Continuation;" - + Type.getDescriptor(MethodRequest.class) - + ")" - + Type.getDescriptor(Context.class), - false); + "(ILkotlin/coroutines/Continuation;Ljava/lang/Object;)" + + Type.getDescriptor(Context.class)); temp.visitInsn(Opcodes.DUP); temp.visitVarInsn(Opcodes.ASTORE, contextLocal); - temp.visitMethodInsn( - Opcodes.INVOKESTATIC, - Type.getInternalName(AnnotationInstrumentationHelper.class), + visitInvokeHelperMethod( + temp, "openScope", - "(" + Type.getDescriptor(Context.class) + ")" + Type.getDescriptor(Scope.class), - false); + "(" + + Type.getDescriptor(Context.class) + + ")" + + Type.getDescriptor(Scope.class)); temp.visitVarInsn(Opcodes.ASTORE, scopeLocal); // @SpanAttribute handling for (Parameter parameter : annotatedParameters) { @@ -396,14 +391,12 @@ public void visitEnd() { boolean primitive = parameter.type.getSort() != Type.ARRAY && parameter.type.getSort() != Type.OBJECT; - temp.visitMethodInsn( - Opcodes.INVOKESTATIC, - Type.getInternalName(AnnotationInstrumentationHelper.class), + visitInvokeHelperMethod( + temp, "setSpanAttribute", "(ILjava/lang/String;" + (primitive ? parameter.type.getDescriptor() : "Ljava/lang/Object;") - + ")V", - false); + + ")V"); } // pop label temp.visitInsn(Opcodes.POP); @@ -446,7 +439,7 @@ public void visitEnd() { // in this handler we are using only the locals we added, we don't care about method // arguments and this, so we don't list them in the stack frame Arrays.fill(locals, Opcodes.TOP); - locals[requestLocal] = Type.getInternalName(MethodRequest.class); + locals[requestLocal] = Type.getInternalName(Object.class); locals[ourContinuationLocal] = Type.getInternalName(Continuation.class); locals[contextLocal] = Type.getInternalName(Context.class); locals[scopeLocal] = Type.getInternalName(Scope.class); @@ -463,17 +456,15 @@ public void visitEnd() { temp.visitVarInsn(Opcodes.ALOAD, ourContinuationLocal); temp.visitVarInsn(Opcodes.ALOAD, contextLocal); temp.visitVarInsn(Opcodes.ALOAD, scopeLocal); - temp.visitMethodInsn( - Opcodes.INVOKESTATIC, - Type.getInternalName(AnnotationInstrumentationHelper.class), + visitInvokeHelperMethod( + temp, "exitCoroutine", "(Ljava/lang/Throwable;Ljava/lang/Object;" - + Type.getDescriptor(MethodRequest.class) + + "Ljava/lang/Object;" + Type.getDescriptor(Continuation.class) + Type.getDescriptor(Context.class) + Type.getDescriptor(Scope.class) - + ")V", - false); + + ")V"); // rethrow the exception temp.visitInsn(Opcodes.ATHROW); @@ -498,17 +489,15 @@ public void visitEnd() { temp.visitVarInsn(Opcodes.ALOAD, ourContinuationLocal); temp.visitVarInsn(Opcodes.ALOAD, contextLocal); temp.visitVarInsn(Opcodes.ALOAD, scopeLocal); - temp.visitMethodInsn( - Opcodes.INVOKESTATIC, - Type.getInternalName(AnnotationInstrumentationHelper.class), + visitInvokeHelperMethod( + temp, "exitCoroutine", "(Ljava/lang/Object;" - + Type.getDescriptor(MethodRequest.class) + + "Ljava/lang/Object;" + Type.getDescriptor(Continuation.class) + Type.getDescriptor(Context.class) + Type.getDescriptor(Scope.class) - + ")V", - false); + + ")V"); methodNode.instructions.insertBefore(instruction, temp.instructions); } } @@ -519,5 +508,15 @@ public void visitEnd() { return generatorAdapter; } + + private static void visitInvokeHelperMethod( + MethodNode methodNode, String methodName, String descriptor) { + methodNode.visitMethodInsn( + Opcodes.INVOKESTATIC, + Type.getInternalName(AnnotationInstrumentationHelper.class), + methodName, + descriptor, + false); + } } } diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/ClassFileTransformerHolder.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/ClassFileTransformerHolder.java deleted file mode 100644 index c4c22f444fe8..000000000000 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/ClassFileTransformerHolder.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.bootstrap; - -import java.lang.instrument.ClassFileTransformer; - -/** - * Holder for {@link ClassFileTransformer} used by the instrumentation. Calling transform on this - * class file transformer processes given bytes the same way as they would be processed during - * loading of the class. - */ -public final class ClassFileTransformerHolder { - - private static volatile ClassFileTransformer classFileTransformer; - - public static ClassFileTransformer getClassFileTransformer() { - return classFileTransformer; - } - - public static void setClassFileTransformer(ClassFileTransformer transformer) { - classFileTransformer = transformer; - } - - private ClassFileTransformerHolder() {} -} diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/LambdaTransformer.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/LambdaTransformer.java new file mode 100644 index 000000000000..a85b6c687d31 --- /dev/null +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/LambdaTransformer.java @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.bootstrap; + +import java.lang.instrument.IllegalClassFormatException; + +/** Transformer for lambda bytecode */ +public interface LambdaTransformer { + + /** + * Transforms lambda bytecode for instrumentation + * + * @param className class name in JVM format with slashes + * @param targetClass target class, must not be {@literal null} + * @param classfileBuffer target class bytecode + * @return instrumented lambda bytecode + * @throws IllegalClassFormatException if bytecode is invalid + */ + byte[] transform(String className, Class targetClass, byte[] classfileBuffer) + throws IllegalClassFormatException; +} diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/LambdaTransformerHolder.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/LambdaTransformerHolder.java new file mode 100644 index 000000000000..3376791a1253 --- /dev/null +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/LambdaTransformerHolder.java @@ -0,0 +1,36 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.bootstrap; + +/** + * Holder for {@link LambdaTransformer} used by the instrumentation. Calling transform on this + * transformer processes given bytes the same way as they would be processed during loading of the + * class. + */ +public final class LambdaTransformerHolder { + + private static volatile LambdaTransformer lambdaTransformer; + + /** + * get lambda transformer + * + * @return class transformer for defining lambdas + */ + public static LambdaTransformer getLambdaTransformer() { + return lambdaTransformer; + } + + /** + * set lambda transformer + * + * @param transformer transformer + */ + public static void setLambdaTransformer(LambdaTransformer transformer) { + lambdaTransformer = transformer; + } + + private LambdaTransformerHolder() {} +} diff --git a/javaagent-tooling/javaagent-tooling-java9/src/main/java/io/opentelemetry/javaagent/tooling/Java9LambdaTransformer.java b/javaagent-tooling/javaagent-tooling-java9/src/main/java/io/opentelemetry/javaagent/tooling/Java9LambdaTransformer.java new file mode 100644 index 000000000000..f74ce866a4f4 --- /dev/null +++ b/javaagent-tooling/javaagent-tooling-java9/src/main/java/io/opentelemetry/javaagent/tooling/Java9LambdaTransformer.java @@ -0,0 +1,36 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling; + +import io.opentelemetry.javaagent.bootstrap.LambdaTransformer; +import java.lang.instrument.ClassFileTransformer; +import java.lang.instrument.IllegalClassFormatException; + +/** lambda transformer with java9 jpms module compatibility */ +public class Java9LambdaTransformer implements LambdaTransformer { + + private final ClassFileTransformer delegate; + + public Java9LambdaTransformer(ClassFileTransformer delegate) { + this.delegate = delegate; + } + + @Override + public byte[] transform(String className, Class targetClass, byte[] classfileBuffer) + throws IllegalClassFormatException { + + // lambda instrumentation happens only when the lambda is being defined, so the targetClass + // argument should not be passed to the transformer otherwise we get a partial instrumentation, + // for example virtual fields are not properly applied + return delegate.transform( + targetClass.getModule(), + targetClass.getClassLoader(), + className, + null, + null, + classfileBuffer); + } +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java index 4d13036dab24..0b4dff0553e9 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java @@ -21,9 +21,10 @@ import io.opentelemetry.instrumentation.api.internal.EmbeddedInstrumentationProperties; import io.opentelemetry.javaagent.bootstrap.AgentClassLoader; import io.opentelemetry.javaagent.bootstrap.BootstrapPackagePrefixesHolder; -import io.opentelemetry.javaagent.bootstrap.ClassFileTransformerHolder; import io.opentelemetry.javaagent.bootstrap.DefineClassHelper; import io.opentelemetry.javaagent.bootstrap.InstrumentedTaskClasses; +import io.opentelemetry.javaagent.bootstrap.LambdaTransformer; +import io.opentelemetry.javaagent.bootstrap.LambdaTransformerHolder; import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseCustomizer; import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseCustomizerHolder; import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseMutator; @@ -48,6 +49,7 @@ import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; import io.opentelemetry.sdk.autoconfigure.SdkAutoconfigureAccess; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import java.lang.instrument.ClassFileTransformer; import java.lang.instrument.Instrumentation; import java.util.ArrayList; import java.util.Collections; @@ -63,7 +65,6 @@ import net.bytebuddy.ByteBuddy; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.agent.builder.AgentBuilderUtil; -import net.bytebuddy.agent.builder.ResettableClassFileTransformer; import net.bytebuddy.description.type.TypeDefinition; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.dynamic.DynamicType; @@ -199,9 +200,18 @@ private static void installBytebuddyAgent( logger.log(FINE, "Installed {0} extension(s)", numberOfLoadedExtensions); agentBuilder = AgentBuilderUtil.optimize(agentBuilder); - ResettableClassFileTransformer resettableClassFileTransformer = agentBuilder.installOn(inst); + ClassFileTransformer transformer = agentBuilder.installOn(inst); + LambdaTransformer lambdaTransformer; + if (JavaModule.isSupported()) { + // wrapping in a JPMS compliant implementation + lambdaTransformer = new Java9LambdaTransformer(transformer); + } else { + // wrapping in a java 8 compliant transformer + lambdaTransformer = new Java8LambdaTransformer(transformer); + } + LambdaTransformerHolder.setLambdaTransformer(lambdaTransformer); + instrumentationInstalled = true; - ClassFileTransformerHolder.setClassFileTransformer(resettableClassFileTransformer); addHttpServerResponseCustomizers(extensionClassLoader); diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/Java8LambdaTransformer.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/Java8LambdaTransformer.java new file mode 100644 index 000000000000..6c7956409c2a --- /dev/null +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/Java8LambdaTransformer.java @@ -0,0 +1,30 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling; + +import io.opentelemetry.javaagent.bootstrap.LambdaTransformer; +import java.lang.instrument.ClassFileTransformer; +import java.lang.instrument.IllegalClassFormatException; + +/** lambda transformer for java < 9 without jpms modules support */ +public class Java8LambdaTransformer implements LambdaTransformer { + + private final ClassFileTransformer delegate; + + public Java8LambdaTransformer(ClassFileTransformer delegate) { + this.delegate = delegate; + } + + @Override + public byte[] transform(String className, Class targetClass, byte[] classfileBuffer) + throws IllegalClassFormatException { + + // lambda instrumentation happens only when the lambda is being defined, so the targetClass + // argument should not be passed to the transformer otherwise we get a partial instrumentation, + // for example virtual fields are not properly applied + return delegate.transform(targetClass.getClassLoader(), className, null, null, classfileBuffer); + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index e366013c333a..e0535bf94eab 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -259,7 +259,6 @@ 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") include(":instrumentation:internal:internal-lambda:javaagent") -include(":instrumentation:internal:internal-lambda-java9:javaagent") include(":instrumentation:internal:internal-reflection:javaagent") include(":instrumentation:internal:internal-reflection:javaagent-integration-tests") include(":instrumentation:internal:internal-url-class-loader:javaagent") diff --git a/testing-common/integration-tests/build.gradle.kts b/testing-common/integration-tests/build.gradle.kts index 3379dcfab84b..49acf4af7b2b 100644 --- a/testing-common/integration-tests/build.gradle.kts +++ b/testing-common/integration-tests/build.gradle.kts @@ -52,7 +52,6 @@ tasks { includeTestsMatching("InstrumentOldBytecode") } include("**/InstrumentOldBytecode.*") - jvmArgs("-Dotel.instrumentation.inline-ibm-resource-level.enabled=false") } val testInlineModuleOldBytecodeInstrumentation by registering(Test::class) { @@ -60,7 +59,6 @@ tasks { includeTestsMatching("InstrumentOldBytecode") } include("**/InstrumentOldBytecode.*") - jvmArgs("-Dotel.instrumentation.indy-ibm-resource-level.enabled=false") } test { diff --git a/testing-common/integration-tests/src/main/java/InlineIbmResourceLevelInstrumentationModule.java b/testing-common/integration-tests/src/main/java/IbmResourceLevelInstrumentationModule.java similarity index 66% rename from testing-common/integration-tests/src/main/java/InlineIbmResourceLevelInstrumentationModule.java rename to testing-common/integration-tests/src/main/java/IbmResourceLevelInstrumentationModule.java index 4e06e733b6f5..d223a0ac56e0 100644 --- a/testing-common/integration-tests/src/main/java/InlineIbmResourceLevelInstrumentationModule.java +++ b/testing-common/integration-tests/src/main/java/IbmResourceLevelInstrumentationModule.java @@ -11,13 +11,13 @@ import java.util.List; @AutoService(InstrumentationModule.class) -public class InlineIbmResourceLevelInstrumentationModule extends InstrumentationModule { - public InlineIbmResourceLevelInstrumentationModule() { - super("inline-ibm-resource-level"); +public class IbmResourceLevelInstrumentationModule extends InstrumentationModule { + public IbmResourceLevelInstrumentationModule() { + super("ibm-resource-level"); } @Override public List typeInstrumentations() { - return singletonList(new InlineResourceLevelInstrumentation()); + return singletonList(new ResourceLevelInstrumentation()); } } diff --git a/testing-common/integration-tests/src/main/java/IndyIbmResourceLevelInstrumentationModule.java b/testing-common/integration-tests/src/main/java/IndyIbmResourceLevelInstrumentationModule.java deleted file mode 100644 index 9dee76b8ac1b..000000000000 --- a/testing-common/integration-tests/src/main/java/IndyIbmResourceLevelInstrumentationModule.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -import static java.util.Collections.singletonList; - -import com.google.auto.service.AutoService; -import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; -import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; -import java.util.List; - -@AutoService(InstrumentationModule.class) -public class IndyIbmResourceLevelInstrumentationModule extends InstrumentationModule { - public IndyIbmResourceLevelInstrumentationModule() { - super("indy-ibm-resource-level"); - } - - @Override - public boolean isIndyModule() { - return true; - } - - @Override - public List typeInstrumentations() { - return singletonList(new IndyResourceLevelInstrumentation()); - } -} diff --git a/testing-common/integration-tests/src/main/java/InlineResourceLevelInstrumentation.java b/testing-common/integration-tests/src/main/java/InlineResourceLevelInstrumentation.java deleted file mode 100644 index 1e4f074e94af..000000000000 --- a/testing-common/integration-tests/src/main/java/InlineResourceLevelInstrumentation.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -import static net.bytebuddy.matcher.ElementMatchers.named; - -import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; -import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import net.bytebuddy.asm.Advice; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; - -public class InlineResourceLevelInstrumentation implements TypeInstrumentation { - @Override - public ElementMatcher typeMatcher() { - return named("com.ibm.as400.resource.ResourceLevel"); - } - - @Override - public void transform(TypeTransformer transformer) { - transformer.applyAdviceToMethod( - named("toString"), this.getClass().getName() + "$ToStringAdvice"); - } - - @SuppressWarnings("unused") - public static class ToStringAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - static void toStringReplace(@Advice.Return(readOnly = false) String ret) { - ret = "instrumented"; - } - } -} diff --git a/testing-common/integration-tests/src/main/java/IndyResourceLevelInstrumentation.java b/testing-common/integration-tests/src/main/java/ResourceLevelInstrumentation.java similarity index 86% rename from testing-common/integration-tests/src/main/java/IndyResourceLevelInstrumentation.java rename to testing-common/integration-tests/src/main/java/ResourceLevelInstrumentation.java index d66fe5b9a992..6714fc539cf7 100644 --- a/testing-common/integration-tests/src/main/java/IndyResourceLevelInstrumentation.java +++ b/testing-common/integration-tests/src/main/java/ResourceLevelInstrumentation.java @@ -11,7 +11,7 @@ import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; -public class IndyResourceLevelInstrumentation implements TypeInstrumentation { +public class ResourceLevelInstrumentation implements TypeInstrumentation { @Override public ElementMatcher typeMatcher() { return named("com.ibm.as400.resource.ResourceLevel"); @@ -25,7 +25,7 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class ToStringAdvice { - @Advice.OnMethodExit(suppress = Throwable.class, inline = false) + @Advice.OnMethodExit(suppress = Throwable.class) @Advice.AssignReturned.ToReturned public static String toStringReplace() { return "instrumented";