diff --git a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/contextdata/v2_17/Log4j2InstrumentationModule.java b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/contextdata/v2_17/Log4j2InstrumentationModule.java index f68ded00e4f8..238122e7fd7c 100644 --- a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/contextdata/v2_17/Log4j2InstrumentationModule.java +++ b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/contextdata/v2_17/Log4j2InstrumentationModule.java @@ -14,12 +14,16 @@ 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.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumentationModule.class) -public class Log4j2InstrumentationModule extends InstrumentationModule { +public class Log4j2InstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public Log4j2InstrumentationModule() { super("log4j-context-data", "log4j-context-data-2.17"); } @@ -31,8 +35,11 @@ public void registerHelperResources(HelperResourceBuilder helperResourceBuilder) } @Override - public boolean isIndyModule() { - return false; + public void injectClasses(ClassInjector injector) { + injector + .proxyBuilder( + "io.opentelemetry.instrumentation.log4j.contextdata.v2_17.OpenTelemetryContextDataProvider") + .inject(InjectionMode.CLASS_ONLY); } @Override 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 new file mode 100644 index 000000000000..bfd0a620166e --- /dev/null +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/ExperimentalInstrumentationModule.java @@ -0,0 +1,28 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.extension.instrumentation.internal; + +import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; +import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.ClassInjector; + +/** + * This class is internal and is hence not for public use. Its APIs are unstable and can change at + * any time. + */ +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) {} +} 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 new file mode 100644 index 000000000000..9a11cbd6c689 --- /dev/null +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/injection/ClassInjector.java @@ -0,0 +1,39 @@ +/* + * 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/InjectionMode.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/injection/InjectionMode.java new file mode 100644 index 000000000000..b78759a9f65f --- /dev/null +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/injection/InjectionMode.java @@ -0,0 +1,18 @@ +/* + * 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 enum InjectionMode { + CLASS_ONLY + // TODO: implement the modes RESOURCE_ONLY and CLASS_AND_RESOURCE + // This will require a custom URL implementation for byte arrays, similar to how bytebuddy's + // ByteArrayClassLoader does it + +} 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 new file mode 100644 index 000000000000..23d5237aabda --- /dev/null +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/injection/ProxyInjectionBuilder.java @@ -0,0 +1,15 @@ +/* + * 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 8a866a43b771..23c293d1c142 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 @@ -13,6 +13,7 @@ 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.tooling.HelperInjector; import io.opentelemetry.javaagent.tooling.TransformSafeLogger; import io.opentelemetry.javaagent.tooling.Utils; @@ -20,6 +21,7 @@ import io.opentelemetry.javaagent.tooling.config.AgentConfig; 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.IndyModuleRegistry; import io.opentelemetry.javaagent.tooling.instrumentation.indy.IndyTypeTransformerImpl; import io.opentelemetry.javaagent.tooling.instrumentation.indy.PatchByteCodeVersionTransformer; @@ -78,8 +80,25 @@ private AgentBuilder installIndyModule( IndyModuleRegistry.registerIndyModule(instrumentationModule); + HelperResourceBuilderImpl helperResourceBuilder = new HelperResourceBuilderImpl(); + instrumentationModule.registerHelperResources(helperResourceBuilder); + + ClassInjectorImpl injectedClassesCollector = new ClassInjectorImpl(instrumentationModule); + if (instrumentationModule instanceof ExperimentalInstrumentationModule) { + ((ExperimentalInstrumentationModule) instrumentationModule) + .injectClasses(injectedClassesCollector); + } + + AgentBuilder.Transformer helperInjector = + new HelperInjector( + instrumentationModule.instrumentationName(), + injectedClassesCollector.getClassesToInject(), + helperResourceBuilder.getResources(), + instrumentationModule.getClass().getClassLoader(), + instrumentation); + // TODO (Jonas): Adapt MuzzleMatcher to use the same type lookup strategy as the - // InstrumentationModuleClassLoader + // InstrumentationModuleClassLoader (see IndyModuleTypePool) // MuzzleMatcher muzzleMatcher = new MuzzleMatcher(logger, instrumentationModule, config); VirtualFieldImplementationInstaller contextProvider = virtualFieldInstallerFactory.create(instrumentationModule); @@ -88,7 +107,8 @@ private AgentBuilder installIndyModule( for (TypeInstrumentation typeInstrumentation : instrumentationModule.typeInstrumentations()) { AgentBuilder.Identified.Extendable extendableAgentBuilder = setTypeMatcher(agentBuilder, instrumentationModule, typeInstrumentation) - .transform(new PatchByteCodeVersionTransformer()); + .transform(new PatchByteCodeVersionTransformer()) + .transform(helperInjector); // TODO (Jonas): we are not calling // contextProvider.rewriteVirtualFieldsCalls(extendableAgentBuilder) anymore 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 new file mode 100644 index 000000000000..b3d5e9f047d3 --- /dev/null +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/ClassInjectorImpl.java @@ -0,0 +1,65 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling.instrumentation.indy; + +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 java.util.HashMap; +import java.util.Map; +import java.util.function.Function; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.pool.TypePool; + +public class ClassInjectorImpl implements ClassInjector { + + private final InstrumentationModule instrumentationModule; + + private final Map> classesToInject; + + private final IndyProxyFactory proxyFactory; + + public ClassInjectorImpl(InstrumentationModule module) { + instrumentationModule = module; + classesToInject = new HashMap<>(); + proxyFactory = IndyBootstrap.getProxyFactory(module); + } + + public Map> getClassesToInject() { + return classesToInject; + } + + @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) { + if (mode != InjectionMode.CLASS_ONLY) { + throw new UnsupportedOperationException("Not yet implemented"); + } + classesToInject.put( + proxyClassName, + cl -> { + TypePool typePool = IndyModuleTypePool.get(cl, instrumentationModule); + TypeDescription proxiedType = typePool.describe(classToProxy).resolve(); + return proxyFactory.generateProxy(proxiedType, proxyClassName).getBytes(); + }); + } + } +} 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 bea30381559b..4574caa6ed8e 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 @@ -63,6 +63,13 @@ 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 = @@ -118,6 +125,38 @@ private static ConstantCallSite internalBootstrap( String adviceMethodName, 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]); + 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); + } + } catch (Exception e) { + logger.log(Level.SEVERE, e.getMessage(), e); + return null; + } + } + + private static ConstantCallSite bootstrapAdvice( + MethodHandles.Lookup lookup, + String adviceMethodName, + MethodType adviceMethodType, + String moduleClassName, + String adviceClassName) + throws NoSuchMethodException, IllegalAccessException, ClassNotFoundException { CallDepth callDepth = CallDepth.forClass(IndyBootstrap.class); try { if (callDepth.getAndIncrement() > 0) { @@ -130,9 +169,6 @@ private static ConstantCallSite internalBootstrap( new Throwable()); return null; } - // See the getAdviceBootstrapArguments method for where these arguments come from - String moduleClassName = (String) args[0]; - String adviceClassName = (String) args[1]; InstrumentationModuleClassLoader instrumentationClassloader = IndyModuleRegistry.getInstrumentationClassloader( @@ -146,9 +182,6 @@ private static ConstantCallSite internalBootstrap( .getLookup() .findStatic(adviceClass, adviceMethodName, adviceMethodType); return new ConstantCallSite(methodHandle); - } catch (Exception e) { - logger.log(Level.SEVERE, e.getMessage(), e); - return null; } finally { callDepth.decrementAndGet(); } @@ -160,7 +193,85 @@ static Advice.BootstrapArgumentResolver.Factory getAdviceBootstrapArguments( return (adviceMethod, exit) -> (instrumentedType, instrumentedMethod) -> Arrays.asList( + JavaConstant.Simple.ofLoaded(BOOTSTRAP_KIND_ADVICE), JavaConstant.Simple.ofLoaded(moduleName), 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 Arrays.asList( + JavaConstant.Simple.ofLoaded(BOOTSTRAP_KIND_PROXY), + JavaConstant.Simple.ofLoaded(moduleName), + JavaConstant.Simple.ofLoaded(proxiedType.getName()), + JavaConstant.Simple.ofLoaded(methodKind)); + }); + } } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyModuleRegistry.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyModuleRegistry.java index b6099ee12050..dfe421224c23 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyModuleRegistry.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyModuleRegistry.java @@ -77,7 +77,7 @@ private static ClassLoader maskNullClassLoader(ClassLoader classLoader) { return classLoader == null ? BOOT_LOADER : classLoader; } - private static InstrumentationModuleClassLoader createInstrumentationModuleClassloader( + static InstrumentationModuleClassLoader createInstrumentationModuleClassloader( InstrumentationModule module, ClassLoader instrumentedClassloader) { Set toInject = new HashSet<>(InstrumentationModuleMuzzle.getHelperClassNames(module)); diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyModuleTypePool.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyModuleTypePool.java new file mode 100644 index 000000000000..7cf63528a870 --- /dev/null +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyModuleTypePool.java @@ -0,0 +1,34 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling.instrumentation.indy; + +import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; +import io.opentelemetry.javaagent.tooling.muzzle.AgentTooling; +import net.bytebuddy.pool.TypePool; + +public class IndyModuleTypePool { + + private IndyModuleTypePool() {} + + /** + * Provides a {@link TypePool} which has the same lookup rules for {@link + * net.bytebuddy.description.type.TypeDescription}s as {@link InstrumentationModuleClassLoader} + * have for classes. + * + * @param instrumentedCl the classloader being instrumented (e.g. for which the {@link + * InstrumentationModuleClassLoader} is being created). + * @param module the {@link InstrumentationModule} performing the instrumentation + * @return the type pool, must not be cached! + */ + public static TypePool get(ClassLoader instrumentedCl, InstrumentationModule module) { + // TODO: this implementation doesn't allow caching the returned pool and its types + // This could be improved by implementing a custom TypePool instead, which delegates to parent + // TypePools and mirrors the delegation model of the InstrumentationModuleClassLoader + InstrumentationModuleClassLoader dummyCl = + IndyModuleRegistry.createInstrumentationModuleClassloader(module, instrumentedCl); + return TypePool.Default.of(AgentTooling.locationStrategy().classFileLocator(dummyCl)); + } +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/InstrumentationModuleClassLoader.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/InstrumentationModuleClassLoader.java index c790f34b1252..30641430464f 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/InstrumentationModuleClassLoader.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/InstrumentationModuleClassLoader.java @@ -51,9 +51,10 @@ public class InstrumentationModuleClassLoader extends ClassLoader { private final Map additionalInjectedClasses; private final ClassLoader agentOrExtensionCl; - private final ClassLoader instrumentedCl; private volatile MethodHandles.Lookup cachedLookup; + private final ClassLoader instrumentedCl; + public InstrumentationModuleClassLoader( ClassLoader instrumentedCl, ClassLoader agentOrExtensionCl, 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 4b4e50b4c90e..2b2d0e4b9475 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java @@ -30,6 +30,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; import java.util.function.Supplier; import javax.annotation.Nullable; import net.bytebuddy.agent.builder.AgentBuilder.Transformer; @@ -94,7 +95,7 @@ Class inject(ClassLoader classLoader, String className) { private final List helperResources; @Nullable private final ClassLoader helpersSource; @Nullable private final Instrumentation instrumentation; - private final Map> dynamicTypeMap = new LinkedHashMap<>(); + private final Map> dynamicTypeMap = new LinkedHashMap<>(); private final Cache injectedClassLoaders = Cache.weak(); private final Cache resourcesInjectedClassLoaders = Cache.weak(); @@ -124,17 +125,19 @@ public HelperInjector( this.instrumentation = instrumentation; } - private HelperInjector( + public HelperInjector( String requestingName, - Map> helperMap, + Map> helperMap, + List helperResources, + ClassLoader helpersSource, Instrumentation instrumentation) { this.requestingName = requestingName; this.helperClassNames = helperMap.keySet(); this.dynamicTypeMap.putAll(helperMap); - this.helperResources = Collections.emptyList(); - this.helpersSource = null; + this.helperResources = helperResources; + this.helpersSource = helpersSource; this.instrumentation = instrumentation; } @@ -142,20 +145,21 @@ public static HelperInjector forDynamicTypes( String requestingName, Collection> helpers, Instrumentation instrumentation) { - Map> bytes = new HashMap<>(helpers.size()); + Map> bytes = new HashMap<>(helpers.size()); for (DynamicType.Unloaded helper : helpers) { - bytes.put(helper.getTypeDescription().getName(), helper::getBytes); + bytes.put(helper.getTypeDescription().getName(), cl -> helper.getBytes()); } - return new HelperInjector(requestingName, bytes, instrumentation); + return new HelperInjector( + requestingName, bytes, Collections.emptyList(), null, instrumentation); } public static void setHelperInjectorListener(HelperInjectorListener listener) { helperInjectorListener = listener; } - private Map> getHelperMap() { + private Map> getHelperMap(ClassLoader targetClassloader) { + Map> result = new LinkedHashMap<>(); if (dynamicTypeMap.isEmpty()) { - Map> result = new LinkedHashMap<>(); for (String helperClassName : helperClassNames) { result.put( @@ -172,11 +176,17 @@ private Map> getHelperMap() { } }); } - - return result; } else { - return dynamicTypeMap; + dynamicTypeMap.forEach( + (name, bytecodeGenerator) -> { + // Eagerly compute bytecode to not risk accidentally holding onto targetClassloader for + // too long + byte[] bytecode = bytecodeGenerator.apply(targetClassloader); + result.put(name, () -> bytecode); + }); } + + return result; } @Override @@ -265,7 +275,7 @@ private void injectHelperClasses(TypeDescription typeDescription, ClassLoader cl new Object[] {cl, helperClassNames}); } - Map> classnameToBytes = getHelperMap(); + Map> classnameToBytes = getHelperMap(cl); Map map = helperInjectors.computeIfAbsent(cl, (unused) -> new ConcurrentHashMap<>()); for (Map.Entry> entry : classnameToBytes.entrySet()) { diff --git a/testing-common/integration-tests/src/main/java/indy/IndyInstrumentationTestModule.java b/testing-common/integration-tests/src/main/java/indy/IndyInstrumentationTestModule.java index 3b1a831d931c..12089bcde530 100644 --- a/testing-common/integration-tests/src/main/java/indy/IndyInstrumentationTestModule.java +++ b/testing-common/integration-tests/src/main/java/indy/IndyInstrumentationTestModule.java @@ -12,6 +12,9 @@ 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.Collections; import java.util.List; import net.bytebuddy.asm.Advice; @@ -21,7 +24,8 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumentationModule.class) -public class IndyInstrumentationTestModule extends InstrumentationModule { +public class IndyInstrumentationTestModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public IndyInstrumentationTestModule() { super("indy-test"); @@ -42,6 +46,18 @@ 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 Collections.singletonList("indy.ProxyMe"); + } + + @Override + public void injectClasses(ClassInjector injector) { + injector.proxyBuilder("indy.ProxyMe", "foo.bar.Proxy").inject(InjectionMode.CLASS_ONLY); + } + public static class Instrumentation implements TypeInstrumentation { @Override diff --git a/testing-common/integration-tests/src/main/java/indy/ProxyMe.java b/testing-common/integration-tests/src/main/java/indy/ProxyMe.java new file mode 100644 index 000000000000..8ab7387ad30e --- /dev/null +++ b/testing-common/integration-tests/src/main/java/indy/ProxyMe.java @@ -0,0 +1,21 @@ +/* + * 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/test/java/indy/IndyInstrumentationTest.java b/testing-common/integration-tests/src/test/java/indy/IndyInstrumentationTest.java index 10a1ebd80976..b4668d8d1e6c 100644 --- a/testing-common/integration-tests/src/test/java/indy/IndyInstrumentationTest.java +++ b/testing-common/integration-tests/src/test/java/indy/IndyInstrumentationTest.java @@ -11,6 +11,9 @@ import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.javaagent.testing.common.TestAgentListenerAccess; +import java.lang.reflect.Field; +import java.util.concurrent.Callable; +import library.MyProxySuperclass; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -118,4 +121,32 @@ 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"); + } } 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 new file mode 100644 index 000000000000..6c17e3d38590 --- /dev/null +++ b/testing-common/library-for-integration-tests/src/main/java/library/MyProxySuperclass.java @@ -0,0 +1,12 @@ +/* + * 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 {}