diff --git a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestHelperClass.java b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestHelperClass.java deleted file mode 100644 index fa310587f74f..000000000000 --- a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestHelperClass.java +++ /dev/null @@ -1,15 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package instrumentation; - -/** - * Class that will be injected in target classloader with inline instrumentation and proxied with - * indy instrumentation - */ -public class TestHelperClass { - - public TestHelperClass() {} -} diff --git a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule.java b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule.java index 1d6c1038007d..008b0c59ff58 100644 --- a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule.java +++ b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule.java @@ -5,20 +5,16 @@ package instrumentation; -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; 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; @AutoService(InstrumentationModule.class) -public class TestInstrumentationModule extends InstrumentationModule - implements ExperimentalInstrumentationModule { +public class TestInstrumentationModule extends InstrumentationModule { + public TestInstrumentationModule() { super("test-instrumentation"); } @@ -28,21 +24,8 @@ public List typeInstrumentations() { return singletonList(new TestTypeInstrumentation()); } - @Override - public List getAdditionalHelperClassNames() { - // makes the class from instrumentation from the instrumented class with inlined advice - return asList("instrumentation.TestHelperClass"); - } - @Override public boolean isHelperClass(String className) { return "instrumentation.TestSingletons".equals(className); } - - @Override - public void injectClasses(ClassInjector injector) { - injector - .proxyBuilder("instrumentation.TestHelperClass") - .inject(InjectionMode.CLASS_AND_RESOURCE); - } } diff --git a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionTest.java b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionTest.java index 5b149283e6ce..ea7a4e4f8325 100644 --- a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionTest.java +++ b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionTest.java @@ -7,8 +7,6 @@ import static org.assertj.core.api.Assertions.assertThat; -import instrumentation.TestHelperClass; -import io.opentelemetry.javaagent.bootstrap.InstrumentationProxy; import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldAccessorMarker; import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldInstalledMarker; import java.io.ObjectStreamClass; @@ -46,39 +44,7 @@ void testOurFieldsAndMethodsAreNotVisibleWithReflection() { void testGeneratedSerialVersionUid() { // expected value is computed with serialver utility that comes with jdk assertThat(ObjectStreamClass.lookup(TestClass.class).getSerialVersionUID()) - .isEqualTo(1115158932483123524L); + .isEqualTo(-1006206785953990857L); assertThat(TestClass.class.getDeclaredFields()).isEmpty(); } - - @Test - void testInjectedClassProxyUnwrap() throws Exception { - TestClass testClass = new TestClass(); - Class helperType = testClass.testHelperClass(); - assertThat(helperType).isNotNull(); - - Object instance = helperType.getConstructor().newInstance(); - if (InstrumentationProxy.class.isAssignableFrom(helperType)) { - // indy advice: must be an indy proxy - - for (Method method : helperType.getMethods()) { - assertThat(method.getName()).isNotEqualTo("__getIndyProxyDelegate"); - } - - for (Class interfaceType : helperType.getInterfaces()) { - assertThat(interfaceType).isNotEqualTo(InstrumentationProxy.class); - } - - assertThat(instance).isInstanceOf(InstrumentationProxy.class); - - Object proxyDelegate = ((InstrumentationProxy) instance).__getIndyProxyDelegate(); - assertThat(proxyDelegate).isNotInstanceOf(InstrumentationProxy.class); - - } else { - // inline advice: must be of the expected type - assertThat(helperType).isEqualTo(TestHelperClass.class); - assertThat(instance) - .isInstanceOf(TestHelperClass.class) - .isNotInstanceOf(InstrumentationProxy.class); - } - } } diff --git a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/TestClass.java b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/TestClass.java index ee4af8ac1237..9d921fba22e4 100644 --- a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/TestClass.java +++ b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/TestClass.java @@ -19,13 +19,4 @@ public String testMethod() { public String testMethod2() { return "not instrumented"; } - - public Class testHelperClass() { - try { - return Class.forName( - "instrumentation.TestHelperClass", false, TestClass.class.getClassLoader()); - } catch (ClassNotFoundException e) { - return null; - } - } } diff --git a/instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ClassInstrumentation.java b/instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ClassInstrumentation.java index dabba8e3dea8..aa45cb4179b7 100644 --- a/instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ClassInstrumentation.java +++ b/instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ClassInstrumentation.java @@ -11,6 +11,8 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.extension.instrumentation.internal.AsmApi; +import io.opentelemetry.javaagent.tooling.instrumentation.indy.IndyBootstrap; +import io.opentelemetry.javaagent.tooling.instrumentation.indy.IndyTypeTransformerImpl; import net.bytebuddy.asm.AsmVisitorWrapper; import net.bytebuddy.description.field.FieldDescription; import net.bytebuddy.description.field.FieldList; @@ -58,15 +60,18 @@ public ClassVisitor wrap( MethodList methods, int writerFlags, int readerFlags) { - return new ClassClassVisitor(classVisitor); + return new ClassClassVisitor( + classVisitor, transformer instanceof IndyTypeTransformerImpl); } })); } private static class ClassClassVisitor extends ClassVisitor { + private final boolean isIndy; - ClassClassVisitor(ClassVisitor cv) { + ClassClassVisitor(ClassVisitor cv, boolean isIndy) { super(AsmApi.VERSION, cv); + this.isIndy = isIndy; } @Override @@ -92,12 +97,23 @@ public void visitMethodInsn( && "java/lang/J9VMInternals".equals(owner) && "(Ljava/lang/Class;)[Ljava/lang/Class;".equals(descriptor))) { super.visitVarInsn(Opcodes.ALOAD, 0); - super.visitMethodInsn( - Opcodes.INVOKESTATIC, - Type.getInternalName(ReflectionHelper.class), - "filterInterfaces", - "([Ljava/lang/Class;Ljava/lang/Class;)[Ljava/lang/Class;", - false); + if (isIndy) { + // ReflectionHelper is in a separate class loader that is not visible to the + // instrumented class, so we need to invoke it via indy bootstrap method + IndyBootstrap.emitIndyStaticCall( + mv, + "filterInterfaces", + "([Ljava/lang/Class;Ljava/lang/Class;)[Ljava/lang/Class;", + ReflectionInstrumentationModule.class, + ReflectionHelper.class.getName()); + } else { + super.visitMethodInsn( + Opcodes.INVOKESTATIC, + Type.getInternalName(ReflectionHelper.class), + "filterInterfaces", + "([Ljava/lang/Class;Ljava/lang/Class;)[Ljava/lang/Class;", + false); + } } } }; diff --git a/instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionHelper.java b/instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionHelper.java index 4503f304dd7b..13734a07c891 100644 --- a/instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionHelper.java +++ b/instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionHelper.java @@ -5,7 +5,6 @@ package io.opentelemetry.javaagent.instrumentation.internal.reflection; -import io.opentelemetry.javaagent.bootstrap.InstrumentationProxy; import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldAccessorMarker; import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldDetector; import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldInstalledMarker; @@ -70,8 +69,6 @@ public static Class[] filterInterfaces(Class[] interfaces, Class contai // filter out virtual field marker and accessor interfaces if (interfaceClass == VirtualFieldInstalledMarker.class) { continue; - } else if (interfaceClass == InstrumentationProxy.class) { - continue; } else if (VirtualFieldAccessorMarker.class.isAssignableFrom(interfaceClass) && interfaceClass.isSynthetic() && interfaceClass.getName().contains("VirtualFieldAccessor$")) { @@ -87,7 +84,6 @@ public static Class[] filterInterfaces(Class[] interfaces, Class contai } private static boolean noInterfaceToHide(Class containingClass) { - return !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass) - && !InstrumentationProxy.class.isAssignableFrom(containingClass); + return !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass); } } diff --git a/instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionInstrumentationModule.java b/instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionInstrumentationModule.java index 5725dc1e8a48..9b5690181d8e 100644 --- a/instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionInstrumentationModule.java +++ b/instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionInstrumentationModule.java @@ -10,14 +10,10 @@ 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; @AutoService(InstrumentationModule.class) -public class ReflectionInstrumentationModule extends InstrumentationModule - implements ExperimentalInstrumentationModule { +public class ReflectionInstrumentationModule extends InstrumentationModule { public ReflectionInstrumentationModule() { super("internal-reflection"); } @@ -32,15 +28,4 @@ public boolean defaultEnabled() { public List typeInstrumentations() { return asList(new ClassInstrumentation(), new ReflectionInstrumentation()); } - - @Override - public void injectClasses(ClassInjector injector) { - // we do not use ByteBuddy Advice dispatching in this instrumentation - // Instead, we manually call ReflectionHelper via ASM - // Easiest solution to work with indy is to inject an indy-proxy to be invoked - injector - .proxyBuilder( - "io.opentelemetry.javaagent.instrumentation.internal.reflection.ReflectionHelper") - .inject(InjectionMode.CLASS_ONLY); - } } diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxy.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxy.java deleted file mode 100644 index b92f0994dd09..000000000000 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxy.java +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.bootstrap; - -/** Interface added to indy proxies to allow unwrapping the proxy object */ -public interface InstrumentationProxy { - - /** - * Unwraps the proxy delegate instance - * - * @return wrapped object instance - */ - // Method name does not fit common naming practices on purpose - // Also, when modifying this make sure to also update string references. - @SuppressWarnings("all") - Object __getIndyProxyDelegate(); -} diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxyHelper.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxyHelper.java deleted file mode 100644 index 2a369ab9a224..000000000000 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxyHelper.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.bootstrap; - -public class InstrumentationProxyHelper { - - private InstrumentationProxyHelper() {} - - /** - * Unwraps and casts an indy proxy, or just casts if it's not an indy proxy. - * - * @param type of object to return - * @param o object to unwrap - * @param type expected object type - * @return unwrapped proxy instance or the original object (if not a proxy) cast to the expected - * type - * @throws IllegalArgumentException if the provided object the proxied object can't be cast to the - * expected type - */ - public static T unwrapIfNeeded(Object o, Class type) { - if (o instanceof InstrumentationProxy) { - Object delegate = ((InstrumentationProxy) o).__getIndyProxyDelegate(); - if (type.isAssignableFrom(delegate.getClass())) { - return type.cast(delegate); - } - } - if (type.isAssignableFrom(o.getClass())) { - return type.cast(o); - } - - throw new IllegalArgumentException("unexpected object type"); - } -} diff --git a/javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxyHelperTest.java b/javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxyHelperTest.java deleted file mode 100644 index 1763a72c69bf..000000000000 --- a/javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxyHelperTest.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.bootstrap; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; - -import org.junit.jupiter.api.Test; - -class InstrumentationProxyHelperTest { - - @Test - void wrongType() { - assertThatThrownBy(() -> InstrumentationProxyHelper.unwrapIfNeeded("", Integer.class)) - .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> InstrumentationProxyHelper.unwrapIfNeeded(proxy(""), Integer.class)) - .isInstanceOf(IllegalArgumentException.class); - } - - @Test - void unwrap() { - - // no wrapping - Number number = InstrumentationProxyHelper.unwrapIfNeeded(42, Number.class); - assertThat(number).isEqualTo(42); - - // unwrap needed - String string = InstrumentationProxyHelper.unwrapIfNeeded(proxy("hello"), String.class); - assertThat(string).isEqualTo("hello"); - } - - private static InstrumentationProxy proxy(Object delegate) { - return () -> delegate; - } -} diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/ExperimentalInstrumentationModule.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/ExperimentalInstrumentationModule.java index 35ec426d5f3d..db9cb6a7e243 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/ExperimentalInstrumentationModule.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/ExperimentalInstrumentationModule.java @@ -9,7 +9,6 @@ import static java.util.Collections.emptyMap; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; -import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.ClassInjector; import java.util.List; import java.util.Map; import net.bytebuddy.utility.JavaModule; @@ -20,18 +19,6 @@ */ public interface ExperimentalInstrumentationModule { - /** - * Only functional for Modules where {@link InstrumentationModule#isIndyModule()} returns {@code - * true}. - * - *

Normally, helper and advice classes are loaded in a child classloader of the instrumented - * classloader. This method allows to inject classes directly into the instrumented classloader - * instead. - * - * @param injector the builder for injecting classes - */ - default void injectClasses(ClassInjector injector) {} - /** * Returns a list of helper classes that will be defined in the class loader of the instrumented * library. diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/injection/ClassInjector.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/injection/ClassInjector.java deleted file mode 100644 index 9a11cbd6c689..000000000000 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/injection/ClassInjector.java +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.extension.instrumentation.internal.injection; - -/** - * This class is internal and is hence not for public use. Its APIs are unstable and can change at - * any time. - */ -public interface ClassInjector { - - /** - * Create a builder for a proxy class which will be injected into the instrumented {@link - * ClassLoader}. The generated proxy will delegate to the original class, which is loaded in a - * separate classloader. - * - *

This removes the need for the proxied class and its dependencies to be visible (just like - * Advices) to the instrumented ClassLoader. - * - * @param classToProxy the fully qualified name of the class for which a proxy will be generated - * @param newProxyName the fully qualified name to use for the generated proxy - * @return a builder for further customizing the proxy. {@link - * ProxyInjectionBuilder#inject(InjectionMode)} must be called to actually inject the proxy. - */ - ProxyInjectionBuilder proxyBuilder(String classToProxy, String newProxyName); - - /** - * Same as invoking {@link #proxyBuilder(String, String)}, but the resulting proxy will have the - * same name as the proxied class. - * - * @param classToProxy the fully qualified name of the class for which a proxy will be generated - * @return a builder for further customizing and injecting the proxy - */ - default ProxyInjectionBuilder proxyBuilder(String classToProxy) { - return proxyBuilder(classToProxy, classToProxy); - } -} diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/injection/ProxyInjectionBuilder.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/injection/ProxyInjectionBuilder.java deleted file mode 100644 index 23d5237aabda..000000000000 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/injection/ProxyInjectionBuilder.java +++ /dev/null @@ -1,15 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.extension.instrumentation.internal.injection; - -/** - * This class is internal and is hence not for public use. Its APIs are unstable and can change at - * any time. - */ -public interface ProxyInjectionBuilder { - - void inject(InjectionMode mode); -} 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 fa5ad09fa5c3..efde839c18a8 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 @@ -16,16 +16,15 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.internal.AgentDistributionConfig; import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule; -import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode; import io.opentelemetry.javaagent.tooling.HelperClassDefinition; import io.opentelemetry.javaagent.tooling.HelperInjector; +import io.opentelemetry.javaagent.tooling.InjectionMode; import io.opentelemetry.javaagent.tooling.ModuleOpener; import io.opentelemetry.javaagent.tooling.TransformSafeLogger; import io.opentelemetry.javaagent.tooling.Utils; import io.opentelemetry.javaagent.tooling.bytebuddy.LoggingFailSafeMatcher; 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; @@ -113,12 +112,6 @@ private AgentBuilder installIndyModule( injectedHelperClassNames = emptyList(); } - ClassInjectorImpl injectedClassesCollector = new ClassInjectorImpl(instrumentationModule); - if (instrumentationModule instanceof ExperimentalInstrumentationModule) { - ((ExperimentalInstrumentationModule) instrumentationModule) - .injectClasses(injectedClassesCollector); - } - MuzzleMatcher muzzleMatcher = new MuzzleMatcher( logger, @@ -133,8 +126,7 @@ private AgentBuilder installIndyModule( Function> helperGenerator = cl -> { - List helpers = - new ArrayList<>(injectedClassesCollector.getClassesToInject(cl)); + List helpers = new ArrayList<>(); for (String helperName : injectedHelperClassNames) { helpers.add( HelperClassDefinition.create( diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/ClassInjectorImpl.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/ClassInjectorImpl.java deleted file mode 100644 index 681dfe5a9fac..000000000000 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/ClassInjectorImpl.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.tooling.instrumentation.indy; - -import static java.util.stream.Collectors.toList; - -import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; -import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.ClassInjector; -import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode; -import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.ProxyInjectionBuilder; -import io.opentelemetry.javaagent.tooling.HelperClassDefinition; -import io.opentelemetry.javaagent.tooling.muzzle.AgentTooling; -import java.util.ArrayList; -import java.util.List; -import java.util.function.Function; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.dynamic.DynamicType; -import net.bytebuddy.pool.TypePool; - -public class ClassInjectorImpl implements ClassInjector { - - private final InstrumentationModule instrumentationModule; - - private final List> classesToInject; - - private final IndyProxyFactory proxyFactory; - - public ClassInjectorImpl(InstrumentationModule module) { - instrumentationModule = module; - classesToInject = new ArrayList<>(); - proxyFactory = IndyBootstrap.getProxyFactory(module); - } - - public List getClassesToInject(ClassLoader instrumentedCl) { - return classesToInject.stream() - .map(generator -> generator.apply(instrumentedCl)) - .collect(toList()); - } - - @Override - public ProxyInjectionBuilder proxyBuilder(String classToProxy, String newProxyName) { - return new ProxyBuilder(classToProxy, newProxyName); - } - - private class ProxyBuilder implements ProxyInjectionBuilder { - - private final String classToProxy; - private final String proxyClassName; - - ProxyBuilder(String classToProxy, String proxyClassName) { - this.classToProxy = classToProxy; - this.proxyClassName = proxyClassName; - } - - @Override - public void inject(InjectionMode mode) { - classesToInject.add( - cl -> { - InstrumentationModuleClassLoader moduleCl = - IndyModuleRegistry.getInstrumentationClassLoader(instrumentationModule, cl); - TypePool typePool = - AgentTooling.poolStrategy() - .typePool(AgentTooling.locationStrategy().classFileLocator(moduleCl), moduleCl); - TypeDescription proxiedType = typePool.describe(classToProxy).resolve(); - DynamicType.Unloaded proxy = proxyFactory.generateProxy(proxiedType, proxyClassName); - return HelperClassDefinition.create(proxy, mode); - }); - } - } -} 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 index 3512266e5ecf..e0515a73a1fc 100644 --- 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 @@ -112,7 +112,7 @@ public void visitInvokeDynamicInsn( if (Type.getInternalName(IndyBootstrapDispatcher.class) .equals(bootstrapMethodHandle.getOwner())) { - String adviceClassName = (String) bootstrapMethodArguments[3]; + String adviceClassName = (String) bootstrapMethodArguments[2]; String forwardClassDotName = classLoader == null ? bootForwardClassPackage + ".Forward$$" + counter.incrementAndGet() diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyBootstrap.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyBootstrap.java index 3f09dfe7fdbf..6537a4bfafa5 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyBootstrap.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyBootstrap.java @@ -23,6 +23,10 @@ import javax.annotation.Nullable; import net.bytebuddy.asm.Advice; import net.bytebuddy.utility.JavaConstant; +import org.objectweb.asm.Handle; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; /** * We instruct Byte Buddy (via {@link Advice.WithCustomMapping#bootstrap(java.lang.reflect.Method)}) @@ -65,13 +69,6 @@ public class IndyBootstrap { private static final Method indyBootstrapMethod; - private static final String BOOTSTRAP_KIND_ADVICE = "advice"; - private static final String BOOTSTRAP_KIND_PROXY = "proxy"; - - private static final String PROXY_KIND_STATIC = "static"; - private static final String PROXY_KIND_CONSTRUCTOR = "constructor"; - private static final String PROXY_KIND_VIRTUAL = "virtual"; - static { try { indyBootstrapMethod = @@ -130,29 +127,14 @@ private static CallSite internalBootstrap( MethodType adviceMethodType, Object[] args) { try { - String kind = (String) args[0]; - switch (kind) { - case BOOTSTRAP_KIND_ADVICE: - // See the getAdviceBootstrapArguments method for the argument definitions - return bootstrapAdvice( - lookup, - adviceMethodName, - adviceMethodType, - (String) args[1], - (String) args[2], - (String) args[3]); - case BOOTSTRAP_KIND_PROXY: - // See getProxyFactory for the argument definitions - return bootstrapProxyMethod( - lookup, - adviceMethodName, - adviceMethodType, - (String) args[1], - (String) args[2], - (String) args[3]); - default: - throw new IllegalArgumentException("Unknown bootstrapping kind: " + kind); - } + // See the getAdviceBootstrapArguments method for the argument definitions + return bootstrapAdvice( + lookup, + adviceMethodName, + adviceMethodType, + (String) args[0], + (String) args[1], + (String) args[2]); } catch (Exception e) { logger.log(SEVERE, e.getMessage(), e); return null; @@ -227,86 +209,29 @@ static Advice.BootstrapArgumentResolver.Factory getAdviceBootstrapArguments( return (adviceMethod, exit) -> (instrumentedType, instrumentedMethod) -> asList( - JavaConstant.Simple.ofLoaded(BOOTSTRAP_KIND_ADVICE), JavaConstant.Simple.ofLoaded(moduleName), JavaConstant.Simple.ofLoaded(adviceMethod.getDescriptor()), JavaConstant.Simple.ofLoaded(adviceMethod.getDeclaringType().getName())); } - private static ConstantCallSite bootstrapProxyMethod( - MethodHandles.Lookup lookup, - String proxyMethodName, - MethodType expectedMethodType, - String moduleClassName, - String proxyClassName, - String methodKind) - throws NoSuchMethodException, IllegalAccessException, ClassNotFoundException { - InstrumentationModuleClassLoader instrumentationClassloader = - IndyModuleRegistry.getInstrumentationClassLoader( - moduleClassName, lookup.lookupClass().getClassLoader()); - - Class proxiedClass = instrumentationClassloader.loadClass(proxyClassName); - - MethodHandle target; - switch (methodKind) { - case PROXY_KIND_STATIC: - target = - MethodHandles.publicLookup() - .findStatic(proxiedClass, proxyMethodName, expectedMethodType); - break; - case PROXY_KIND_CONSTRUCTOR: - target = - MethodHandles.publicLookup() - .findConstructor(proxiedClass, expectedMethodType.changeReturnType(void.class)) - .asType(expectedMethodType); // return type is the proxied class, but proxies expect - // Object - break; - case PROXY_KIND_VIRTUAL: - target = - MethodHandles.publicLookup() - .findVirtual( - proxiedClass, proxyMethodName, expectedMethodType.dropParameterTypes(0, 1)) - .asType( - expectedMethodType); // first argument type is the proxied class, but proxies - // expect Object - break; - default: - throw new IllegalStateException("unknown proxy method kind: " + methodKind); - } - return new ConstantCallSite(target); - } - - /** - * Creates a proxy factory for generating proxies for classes which are loaded by an {@link - * InstrumentationModuleClassLoader} for the provided {@link InstrumentationModule}. - * - * @param instrumentationModule the instrumentation module used to load the proxied target classes - * @return a factory for generating proxy classes - */ - public static IndyProxyFactory getProxyFactory(InstrumentationModule instrumentationModule) { - String moduleName = instrumentationModule.getClass().getName(); - return new IndyProxyFactory( - getIndyBootstrapMethod(), - (proxiedType, proxiedMethod) -> { - String methodKind; - if (proxiedMethod.isConstructor()) { - methodKind = PROXY_KIND_CONSTRUCTOR; - } else if (proxiedMethod.isMethod()) { - if (proxiedMethod.isStatic()) { - methodKind = PROXY_KIND_STATIC; - } else { - methodKind = PROXY_KIND_VIRTUAL; - } - } else { - throw new IllegalArgumentException( - "Unknown type of method: " + proxiedMethod.getName()); - } - - return asList( - JavaConstant.Simple.ofLoaded(BOOTSTRAP_KIND_PROXY), - JavaConstant.Simple.ofLoaded(moduleName), - JavaConstant.Simple.ofLoaded(proxiedType.getName()), - JavaConstant.Simple.ofLoaded(methodKind)); - }); + /** Emit invokedynamic instruction that will call the given helper class static method. */ + public static void emitIndyStaticCall( + MethodVisitor mv, + String name, + String descriptor, + Class instrumentationModule, + String helperClassDotName) { + mv.visitInvokeDynamicInsn( + name, + descriptor, + new Handle( + Opcodes.H_INVOKESTATIC, + Type.getInternalName(IndyBootstrapDispatcher.class), + "bootstrap", + "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;", + false), + instrumentationModule.getName(), + descriptor, + helperClassDotName); } } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java deleted file mode 100644 index 9cca710ccda8..000000000000 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java +++ /dev/null @@ -1,199 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.tooling.instrumentation.indy; - -import io.opentelemetry.javaagent.bootstrap.InstrumentationProxy; -import java.lang.reflect.Method; -import java.lang.reflect.Modifier; -import java.util.ArrayList; -import java.util.List; -import net.bytebuddy.ByteBuddy; -import net.bytebuddy.description.method.MethodDescription; -import net.bytebuddy.description.method.ParameterDescription; -import net.bytebuddy.description.modifier.SyntheticState; -import net.bytebuddy.description.modifier.Visibility; -import net.bytebuddy.description.type.TypeDefinition; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.dynamic.DynamicType; -import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy; -import net.bytebuddy.implementation.FieldAccessor; -import net.bytebuddy.implementation.Implementation; -import net.bytebuddy.implementation.InvokeDynamic; -import net.bytebuddy.implementation.MethodCall; -import net.bytebuddy.implementation.bytecode.StackManipulation; -import net.bytebuddy.implementation.bytecode.member.MethodInvocation; -import net.bytebuddy.implementation.bytecode.member.MethodVariableAccess; -import net.bytebuddy.utility.JavaConstant; - -/** - * Factory for generating proxies which invoke their target via {@code INVOKEDYNAMIC}. Generated - * proxy classes have the following properties: The generated proxies have the following basic - * structure: - * - *

    - *
  • it has same superclass as the proxied class - *
  • it implements all interfaces implemented by the proxied class - *
  • for every public constructor of the proxied class, it defined a matching public constructor - * which: - *
      - *
    • invokes the default constructor of the superclass - *
    • invoked the corresponding constructor of the proxied class to generate the object to - * which the proxy delegates - *
    - *
  • it "copies" every declared static and non-static public method, the implementation will - * delegate to the corresponding method in the proxied class - *
  • all annotations on the proxied class and on its methods are copied to the proxy - *
- * - *

Note that only the public methods declared by the proxied class are actually proxied. - * Inherited methods are not automatically proxied. If you want those to be proxied, you'll need to - * explicitly override them in the proxied class. - */ -public class IndyProxyFactory { - - @FunctionalInterface - public interface BootstrapArgsProvider { - - /** - * Defines the additional arguments to pass to the invokedynamic bootstrap method for a given - * proxied method. The arguments have to be storable in the constant pool. - * - * @param classBeingProxied the type for which {@link - * IndyProxyFactory#generateProxy(TypeDescription, String)} was invoked - * @param proxiedMethodOrCtor the method or constructor from the proxied class for which the - * arguments are requested - * @return the arguments to pass to the bootstrap method - */ - List getBootstrapArgsForMethod( - TypeDescription classBeingProxied, MethodDescription.InDefinedShape proxiedMethodOrCtor); - } - - private static final String DELEGATE_FIELD_NAME = "delegate"; - - // Matches the single method of IndyProxy interface - static final String PROXY_DELEGATE_NAME = "__getIndyProxyDelegate"; - - private final MethodDescription.InDefinedShape indyBootstrapMethod; - - private final BootstrapArgsProvider bootstrapArgsProvider; - - public IndyProxyFactory(Method bootstrapMethod, BootstrapArgsProvider bootstrapArgsProvider) { - this.indyBootstrapMethod = new MethodDescription.ForLoadedMethod(bootstrapMethod); - this.bootstrapArgsProvider = bootstrapArgsProvider; - } - - /** - * Generates a proxy. - * - * @param classToProxy the class for which a proxy will be generated - * @param proxyClassName the desired fully qualified name for the proxy class - * @return the generated proxy class - */ - public DynamicType.Unloaded generateProxy( - TypeDescription classToProxy, String proxyClassName) { - TypeDescription.Generic superClass = classToProxy.getSuperClass(); - List interfaces = new ArrayList<>(classToProxy.getInterfaces()); - interfaces.add(TypeDescription.ForLoadedType.of(InstrumentationProxy.class)); - DynamicType.Builder builder = - new ByteBuddy() - .subclass(superClass, ConstructorStrategy.Default.NO_CONSTRUCTORS) - .implement(interfaces) - .name(proxyClassName) - .annotateType(classToProxy.getDeclaredAnnotations()) - .defineField(DELEGATE_FIELD_NAME, Object.class, Modifier.PRIVATE | Modifier.FINAL); - - for (MethodDescription.InDefinedShape method : classToProxy.getDeclaredMethods()) { - if (method.isPublic()) { - if (method.isConstructor()) { - List bootstrapArgs = - bootstrapArgsProvider.getBootstrapArgsForMethod(classToProxy, method); - builder = createProxyConstructor(superClass, method, bootstrapArgs, builder); - } else if (method.isMethod()) { - List bootstrapArgs = - bootstrapArgsProvider.getBootstrapArgsForMethod(classToProxy, method); - builder = createProxyMethod(method, bootstrapArgs, builder); - } - } - } - - // Implement IndyProxy class and return the delegate field - builder = - builder - .defineMethod( - PROXY_DELEGATE_NAME, Object.class, Visibility.PUBLIC, SyntheticState.SYNTHETIC) - .intercept(FieldAccessor.ofField(DELEGATE_FIELD_NAME)); - - return builder.make(); - } - - private DynamicType.Builder createProxyMethod( - MethodDescription.InDefinedShape proxiedMethod, - List bootstrapArgs, - DynamicType.Builder builder) { - InvokeDynamic body = InvokeDynamic.bootstrap(indyBootstrapMethod, bootstrapArgs); - if (!proxiedMethod.isStatic()) { - body = body.withField(DELEGATE_FIELD_NAME); - } - body = body.withMethodArguments(); - int modifiers = Modifier.PUBLIC | (proxiedMethod.isStatic() ? Modifier.STATIC : 0); - return createProxyMethodOrConstructor( - proxiedMethod, - builder.defineMethod(proxiedMethod.getName(), proxiedMethod.getReturnType(), modifiers), - body); - } - - private DynamicType.Builder createProxyConstructor( - TypeDescription.Generic superClass, - MethodDescription.InDefinedShape proxiedConstructor, - List bootstrapArgs, - DynamicType.Builder builder) { - MethodDescription defaultSuperCtor = findDefaultConstructor(superClass); - - Implementation.Composable fieldAssignment = - FieldAccessor.ofField(DELEGATE_FIELD_NAME) - .setsValue( - new StackManipulation.Compound( - MethodVariableAccess.allArgumentsOf(proxiedConstructor), - MethodInvocation.invoke(indyBootstrapMethod) - .dynamic( - "ctor", // the actual method name is not allowed by the verifier - TypeDescription.ForLoadedType.of(Object.class), - proxiedConstructor.getParameters().asTypeList().asErasures(), - bootstrapArgs)), - Object.class); - Implementation.Composable ctorBody = - MethodCall.invoke(defaultSuperCtor).andThen(fieldAssignment); - return createProxyMethodOrConstructor( - proxiedConstructor, builder.defineConstructor(Modifier.PUBLIC), ctorBody); - } - - private static MethodDescription findDefaultConstructor(TypeDescription.Generic superClass) { - return superClass.getDeclaredMethods().stream() - .filter(MethodDescription::isConstructor) - .filter(constructor -> constructor.getParameters().isEmpty()) - .findFirst() - .orElseThrow( - () -> - new IllegalArgumentException( - "Superclass of provided type does not define a default constructor")); - } - - private static DynamicType.Builder createProxyMethodOrConstructor( - MethodDescription.InDefinedShape method, - DynamicType.Builder.MethodDefinition.ParameterDefinition methodDef, - Implementation methodBody) { - for (ParameterDescription param : method.getParameters()) { - methodDef = - methodDef - .withParameter(param.getType(), param.getName(), param.getModifiers()) - .annotateParameter(param.getDeclaredAnnotations()); - } - return methodDef - .throwing(method.getExceptionTypes()) - .intercept(methodBody) - .annotateMethod(method.getDeclaredAnnotations()); - } -} diff --git a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java deleted file mode 100644 index 100da5f73661..000000000000 --- a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java +++ /dev/null @@ -1,364 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.tooling.instrumentation.indy; - -import static java.util.Arrays.asList; -import static org.assertj.core.api.Assertions.assertThat; - -import io.opentelemetry.javaagent.bootstrap.InstrumentationProxy; -import io.opentelemetry.javaagent.tooling.instrumentation.indy.dummies.DummyAnnotation; -import java.lang.invoke.CallSite; -import java.lang.invoke.ConstantCallSite; -import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; -import java.lang.invoke.MethodType; -import java.lang.reflect.Constructor; -import java.lang.reflect.Method; -import java.util.List; -import java.util.concurrent.Callable; -import net.bytebuddy.description.method.MethodDescription; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.dynamic.DynamicType; -import net.bytebuddy.utility.JavaConstant; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; - -public class IndyProxyFactoryTest { - - private static IndyProxyFactory proxyFactory; - - @BeforeAll - public static void init() throws Exception { - Method bootstrap = - IndyProxyFactoryTest.class.getMethod( - "indyBootstrap", - MethodHandles.Lookup.class, - String.class, - MethodType.class, - Object[].class); - proxyFactory = new IndyProxyFactory(bootstrap, IndyProxyFactoryTest::bootstrapArgsGenerator); - } - - public static CallSite indyBootstrap( - MethodHandles.Lookup lookup, String methodName, MethodType methodType, Object... args) { - - try { - String delegateClassName = (String) args[0]; - String kind = (String) args[1]; - - Class proxiedClass = Class.forName(delegateClassName); - - MethodHandle target; - - switch (kind) { - case "static": - target = MethodHandles.publicLookup().findStatic(proxiedClass, methodName, methodType); - break; - case "constructor": - target = - MethodHandles.publicLookup() - .findConstructor(proxiedClass, methodType.changeReturnType(void.class)) - .asType(methodType); - break; - case "virtual": - target = - MethodHandles.publicLookup() - .findVirtual(proxiedClass, methodName, methodType.dropParameterTypes(0, 1)) - .asType(methodType); - break; - default: - throw new IllegalStateException("unknown kind"); - } - return new ConstantCallSite(target); - } catch (Exception e) { - throw new IllegalStateException(e); - } - } - - private static List bootstrapArgsGenerator( - TypeDescription proxiedType, MethodDescription.InDefinedShape proxiedMethod) { - String kind = "virtual"; - if (proxiedMethod.isConstructor()) { - kind = "constructor"; - } else if (proxiedMethod.isStatic()) { - kind = "static"; - } - return asList( - JavaConstant.Simple.ofLoaded(proxiedType.getName()), JavaConstant.Simple.ofLoaded(kind)); - } - - public static class StatefulObj { - - static StatefulObj lastCreatedInstance; - - int counter = 0; - - public StatefulObj() { - lastCreatedInstance = this; - } - - public void increaseCounter() { - counter++; - } - } - - @Test - void verifyDelegateInstantiation() throws Exception { - Class proxy = generateProxy(StatefulObj.class); - Constructor ctor = proxy.getConstructor(); - Method increaseCounter = proxy.getMethod("increaseCounter"); - - Object proxyA = ctor.newInstance(); - StatefulObj delegateA = StatefulObj.lastCreatedInstance; - - Object proxyB = ctor.newInstance(); - StatefulObj delegateB = StatefulObj.lastCreatedInstance; - - assertThat(delegateA).isNotNull(); - assertThat(delegateB).isNotNull(); - assertThat(delegateA).isNotSameAs(delegateB); - - increaseCounter.invoke(proxyA); - assertThat(delegateA.counter).isEqualTo(1); - assertThat(delegateB.counter).isEqualTo(0); - - increaseCounter.invoke(proxyB); - increaseCounter.invoke(proxyB); - assertThat(delegateA.counter).isEqualTo(1); - assertThat(delegateB.counter).isEqualTo(2); - } - - public static class UtilityWithPrivateCtor { - - private UtilityWithPrivateCtor() {} - - public static String utilityMethod() { - return "util"; - } - } - - @Test - void proxyClassWithoutConstructor() throws Exception { - Class proxy = generateProxy(UtilityWithPrivateCtor.class); - - // Not legal in Java code but legal in JVM bytecode - assertThat(proxy.getConstructors()).isEmpty(); - - assertThat(proxy.getMethod("utilityMethod").invoke(null)).isEqualTo("util"); - } - - @DummyAnnotation("type") - public static class AnnotationRetention { - - @DummyAnnotation("constructor") - public AnnotationRetention(@DummyAnnotation("constructor_param") String someValue) {} - - @DummyAnnotation("virtual") - public void virtualMethod(@DummyAnnotation("virtual_param") String someValue) {} - - @DummyAnnotation("static") - public static void staticMethod(@DummyAnnotation("static_param") String someValue) {} - } - - @Test - void verifyAnnotationsRetained() throws Exception { - - Class proxy = generateProxy(AnnotationRetention.class); - - assertThat(proxy.getAnnotation(DummyAnnotation.class)) - .isNotNull() - .extracting(DummyAnnotation::value) - .isEqualTo("type"); - - Constructor ctor = proxy.getConstructor(String.class); - assertThat(ctor.getAnnotation(DummyAnnotation.class)) - .isNotNull() - .extracting(DummyAnnotation::value) - .isEqualTo("constructor"); - assertThat(ctor.getParameters()[0].getAnnotation(DummyAnnotation.class)) - .isNotNull() - .extracting(DummyAnnotation::value) - .isEqualTo("constructor_param"); - - Method virtualMethod = proxy.getMethod("virtualMethod", String.class); - assertThat(virtualMethod.getAnnotation(DummyAnnotation.class)) - .isNotNull() - .extracting(DummyAnnotation::value) - .isEqualTo("virtual"); - assertThat(virtualMethod.getParameters()[0].getAnnotation(DummyAnnotation.class)) - .isNotNull() - .extracting(DummyAnnotation::value) - .isEqualTo("virtual_param"); - - Method staticMethod = proxy.getMethod("staticMethod", String.class); - assertThat(staticMethod.getAnnotation(DummyAnnotation.class)) - .isNotNull() - .extracting(DummyAnnotation::value) - .isEqualTo("static"); - assertThat(staticMethod.getParameters()[0].getAnnotation(DummyAnnotation.class)) - .isNotNull() - .extracting(DummyAnnotation::value) - .isEqualTo("static_param"); - - staticMethod.invoke(null, "blub"); - virtualMethod.invoke(ctor.newInstance("bla"), "blub"); - } - - public static class CustomSuperClass { - - int inheritedFromSuperclassCount = 0; - - protected void overrideMe() {} - - public void inheritedFromSuperclass() { - inheritedFromSuperclassCount++; - } - } - - public static interface CustomSuperInterface extends Runnable { - - default void inheritedDefault() { - if (this instanceof WithSuperTypes) { - ((WithSuperTypes) this).inheritedDefaultCount++; - } - } - } - - public static class WithSuperTypes extends CustomSuperClass - implements CustomSuperInterface, Callable { - - static WithSuperTypes lastCreatedInstance; - - public WithSuperTypes() { - lastCreatedInstance = this; - } - - int runInvocCount = 0; - int callInvocCount = 0; - int overrideMeInvocCount = 0; - - int inheritedDefaultCount = 0; - - @Override - public void run() { - runInvocCount++; - } - - @Override - public String call() throws Exception { - callInvocCount++; - return "foo"; - } - - @Override - public void overrideMe() { - overrideMeInvocCount++; - } - } - - @Test - @SuppressWarnings("unchecked") - void verifySuperTypes() throws Exception { - Object proxy = generateProxy(WithSuperTypes.class).getConstructor().newInstance(); - WithSuperTypes proxied = WithSuperTypes.lastCreatedInstance; - - ((Runnable) proxy).run(); - assertThat(proxied.runInvocCount).isEqualTo(1); - - ((Callable) proxy).call(); - assertThat(proxied.callInvocCount).isEqualTo(1); - - ((CustomSuperClass) proxy).overrideMe(); - assertThat(proxied.overrideMeInvocCount).isEqualTo(1); - - // Non-overidden, inherited methods are not proxied - ((CustomSuperClass) proxy).inheritedFromSuperclass(); - assertThat(proxied.inheritedFromSuperclassCount).isEqualTo(0); - ((CustomSuperInterface) proxy).inheritedDefault(); - assertThat(proxied.inheritedDefaultCount).isEqualTo(0); - } - - @SuppressWarnings({"unused", "MethodCanBeStatic"}) - public static class IgnoreNonPublicMethods { - - public IgnoreNonPublicMethods() {} - - protected IgnoreNonPublicMethods(int arg) {} - - IgnoreNonPublicMethods(int arg1, int arg2) {} - - private IgnoreNonPublicMethods(int arg1, int arg2, int arg3) {} - - public void publicMethod() {} - - public static void publicStaticMethod() {} - - protected void protectedMethod() {} - - protected static void protectedStaticMethod() {} - - void packageMethod() {} - - static void packageStaticMethod() {} - - private void privateMethod() {} - - private static void privateStaticMethod() {} - } - - @Test - void verifyNonPublicMembersIgnored() { - Class proxy = generateProxy(IgnoreNonPublicMethods.class); - - assertThat(proxy.getConstructors()).hasSize(1); - assertThat(proxy.getDeclaredMethods()) - .hasSize(3) - .anySatisfy(method -> assertThat(method.getName()).isEqualTo("publicMethod")) - .anySatisfy(method -> assertThat(method.getName()).isEqualTo("publicStaticMethod")) - // IndyProxy implementation visible but later hidden by reflection instrumentation - .anySatisfy( - method -> { - assertThat(method.getName()).isEqualTo(IndyProxyFactory.PROXY_DELEGATE_NAME); - assertThat(method.isSynthetic()).isTrue(); - }); - } - - @Test - void verifyProxyClass() throws Exception { - Class proxyType = generateProxy(ProxyUnwrapTest.class); - assertThat(proxyType) - .isNotInstanceOf(InstrumentationProxy.class) - .isNotInstanceOf(ProxyUnwrapTest.class); - - assertThat(InstrumentationProxy.class.isAssignableFrom(proxyType)) - .describedAs("proxy class can be cast to IndyProxy") - .isTrue(); - - Object proxyInstance = proxyType.getConstructor().newInstance(); - Object proxyDelegate = ((InstrumentationProxy) proxyInstance).__getIndyProxyDelegate(); - assertThat(proxyDelegate).isInstanceOf(ProxyUnwrapTest.class); - } - - @SuppressWarnings("all") - public static class ProxyUnwrapTest { - public ProxyUnwrapTest() {} - - public void sampleMethod() {} - } - - private static Class generateProxy(Class clazz) { - DynamicType.Unloaded unloaded = - proxyFactory.generateProxy( - TypeDescription.ForLoadedType.of(clazz), clazz.getName() + "Proxy"); - // Uncomment the following block to view the generated bytecode if needed - // try { - // unloaded.saveIn(new File("build/generated_proxies")); - // } catch (IOException e) { - // throw new RuntimeException(e); - // } - return unloaded.load(clazz.getClassLoader()).getLoaded(); - } -} diff --git a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/dummies/DummyAnnotation.java b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/dummies/DummyAnnotation.java deleted file mode 100644 index 8ce3e917040b..000000000000 --- a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/dummies/DummyAnnotation.java +++ /dev/null @@ -1,14 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.tooling.instrumentation.indy.dummies; - -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; - -@Retention(RetentionPolicy.RUNTIME) -public @interface DummyAnnotation { - String value(); -} diff --git a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperClassDefinition.java b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperClassDefinition.java index 2cca3e58dfc2..529f3fd4e545 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperClassDefinition.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperClassDefinition.java @@ -5,7 +5,6 @@ package io.opentelemetry.javaagent.tooling; -import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode; import net.bytebuddy.dynamic.DynamicType; public class HelperClassDefinition { 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 100e5814d238..5965efafd3ab 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java @@ -21,7 +21,6 @@ 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; import io.opentelemetry.javaagent.instrumentation.httpurlconnection.HttpUrlConnectionLookupSupplier; import io.opentelemetry.javaagent.instrumentation.internal.lambda.LambdaLookupSupplier; diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/injection/InjectionMode.java b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/InjectionMode.java similarity index 74% rename from javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/injection/InjectionMode.java rename to muzzle/src/main/java/io/opentelemetry/javaagent/tooling/InjectionMode.java index 2eaea5f66d6c..cdcd04e09a35 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/injection/InjectionMode.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/InjectionMode.java @@ -3,12 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.javaagent.extension.instrumentation.internal.injection; +package io.opentelemetry.javaagent.tooling; -/** - * This class is internal and is hence not for public use. Its APIs are unstable and can change at - * any time. - */ public enum InjectionMode { CLASS_ONLY(true, false), RESOURCE_ONLY(false, true), diff --git a/testing-common/integration-tests/src/main/java/indy/ProxyMe.java b/testing-common/integration-tests/src/main/java/indy/ProxyMe.java deleted file mode 100644 index 8ab7387ad30e..000000000000 --- a/testing-common/integration-tests/src/main/java/indy/ProxyMe.java +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package indy; - -import java.util.concurrent.Callable; -import library.MyProxySuperclass; - -public class ProxyMe extends MyProxySuperclass implements Callable { - - @Override - public String call() { - return "Hi from ProxyMe"; - } - - public static String staticHello() { - return "Hi from static"; - } -} diff --git a/testing-common/integration-tests/src/main/java/io/opentelemetry/javaagent/IndyInstrumentationTestModule.java b/testing-common/integration-tests/src/main/java/io/opentelemetry/javaagent/IndyInstrumentationTestModule.java index 86df2a0c09bf..383ef1b5cf0e 100644 --- a/testing-common/integration-tests/src/main/java/io/opentelemetry/javaagent/IndyInstrumentationTestModule.java +++ b/testing-common/integration-tests/src/main/java/io/opentelemetry/javaagent/IndyInstrumentationTestModule.java @@ -12,9 +12,6 @@ import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -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.asm.Advice; import net.bytebuddy.asm.Advice.AssignReturned.ToArguments.ToArgument; @@ -23,8 +20,7 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumentationModule.class) -public class IndyInstrumentationTestModule extends InstrumentationModule - implements ExperimentalInstrumentationModule { +public class IndyInstrumentationTestModule extends InstrumentationModule { public IndyInstrumentationTestModule() { super("indy-test"); @@ -40,18 +36,6 @@ public boolean isHelperClass(String className) { return className.equals(LocalHelper.class.getName()); } - @Override - public List getAdditionalHelperClassNames() { - // TODO: should not be needed as soon as we automatically add proxied classes to the muzzle root - // set - return singletonList("indy.ProxyMe"); - } - - @Override - public void injectClasses(ClassInjector injector) { - injector.proxyBuilder("indy.ProxyMe", "foo.bar.Proxy").inject(InjectionMode.CLASS_AND_RESOURCE); - } - @Override public boolean defaultEnabled() { return Boolean.getBoolean("otel.javaagent.experimental.indy"); diff --git a/testing-common/integration-tests/src/test/java/indy/IndyInstrumentationTest.java b/testing-common/integration-tests/src/test/java/indy/IndyInstrumentationTest.java index bac4da293e7c..d9d921f93e14 100644 --- a/testing-common/integration-tests/src/test/java/indy/IndyInstrumentationTest.java +++ b/testing-common/integration-tests/src/test/java/indy/IndyInstrumentationTest.java @@ -11,11 +11,6 @@ import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.javaagent.testing.common.TestAgentListenerAccess; -import java.io.InputStream; -import java.lang.reflect.Field; -import java.util.concurrent.Callable; -import library.MyProxySuperclass; -import org.apache.commons.io.IOUtils; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledIfSystemProperty; @@ -135,38 +130,4 @@ void testHelperClassLoading() { assertThat(globalHelper.getName()).endsWith("GlobalHelper"); assertThat(globalHelper.getClassLoader().getClass().getName()).endsWith("AgentClassLoader"); } - - @Test - @SuppressWarnings("unchecked") - void testProxyInjection() throws Exception { - Class proxyClass = Class.forName("foo.bar.Proxy"); - - // create an instance and invoke static & non-static methods - // this verifies that our invokedynamic bootstrapping works for constructors, static and - // non-static methods - - Object proxyInstance = proxyClass.getConstructor().newInstance(); - assertThat(proxyInstance).isInstanceOf(Callable.class); - assertThat(proxyInstance).isInstanceOf(MyProxySuperclass.class); - - String invocResult = ((Callable) proxyInstance).call(); - assertThat(invocResult).isEqualTo("Hi from ProxyMe"); - - String staticResult = (String) proxyClass.getMethod("staticHello").invoke(null); - assertThat(staticResult).isEqualTo("Hi from static"); - - Field delegateField = proxyClass.getDeclaredField("delegate"); - delegateField.setAccessible(true); - Object delegate = delegateField.get(proxyInstance); - - ClassLoader delegateCl = delegate.getClass().getClassLoader(); - assertThat(delegate.getClass().getName()).isEqualTo("indy.ProxyMe"); - assertThat(delegateCl.getClass().getName()).endsWith("InstrumentationModuleClassLoader"); - - // Ensure that the bytecode of the proxy is injected as a resource - InputStream res = - IndyInstrumentationTest.class.getClassLoader().getResourceAsStream("foo/bar/Proxy.class"); - byte[] bytecode = IOUtils.toByteArray(res); - assertThat(bytecode).startsWith(0xCA, 0xFE, 0xBA, 0xBE); - } } diff --git a/testing-common/library-for-integration-tests/src/main/java/library/MyProxySuperclass.java b/testing-common/library-for-integration-tests/src/main/java/library/MyProxySuperclass.java deleted file mode 100644 index 6c17e3d38590..000000000000 --- a/testing-common/library-for-integration-tests/src/main/java/library/MyProxySuperclass.java +++ /dev/null @@ -1,12 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package library; - -/** - * Custom superclass only living in the instrumented {@link ClassLoader} to be used for testing the - * proxying mechanism. - */ -public class MyProxySuperclass {}