From 6eb8ae19dfd404791f8f715edec07b2dfa73f066 Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Mon, 6 Nov 2023 18:37:52 +0100 Subject: [PATCH] Allow injection of helper bytecode as resources (#9752) --- ...ringBootActuatorInstrumentationModule.java | 21 +- .../internal/injection/InjectionMode.java | 22 +- .../InstrumentationModuleInstaller.java | 34 ++- .../indy/ClassInjectorImpl.java | 26 +- .../indy/IndyModuleRegistry.java | 5 +- .../InstrumentationModuleClassLoader.java | 19 +- .../InstrumentationModuleClassLoaderTest.java | 17 +- .../javaagent/tooling/ByteArrayUrl.java | 104 +++++++ .../javaagent/tooling/BytecodeWithUrl.java | 88 +++++- .../tooling/HelperClassDefinition.java | 53 ++++ .../javaagent/tooling/HelperInjector.java | 281 +++++++++--------- .../javaagent/tooling/ByteArrayUrlTest.java | 30 ++ .../IndyInstrumentationTestModule.java | 2 +- .../java/indy/IndyInstrumentationTest.java | 8 + 14 files changed, 500 insertions(+), 210 deletions(-) create mode 100644 muzzle/src/main/java/io/opentelemetry/javaagent/tooling/ByteArrayUrl.java rename javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/ClassCopySource.java => muzzle/src/main/java/io/opentelemetry/javaagent/tooling/BytecodeWithUrl.java (56%) create mode 100644 muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperClassDefinition.java create mode 100644 muzzle/src/test/java/io/opentelemetry/javaagent/tooling/ByteArrayUrlTest.java diff --git a/instrumentation/spring/spring-boot-actuator-autoconfigure-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/actuator/v2_0/SpringBootActuatorInstrumentationModule.java b/instrumentation/spring/spring-boot-actuator-autoconfigure-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/actuator/v2_0/SpringBootActuatorInstrumentationModule.java index 04ddf6d6135d..a189da3f81e6 100644 --- a/instrumentation/spring/spring-boot-actuator-autoconfigure-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/actuator/v2_0/SpringBootActuatorInstrumentationModule.java +++ b/instrumentation/spring/spring-boot-actuator-autoconfigure-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/actuator/v2_0/SpringBootActuatorInstrumentationModule.java @@ -12,11 +12,15 @@ import io.opentelemetry.javaagent.extension.instrumentation.HelperResourceBuilder; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule; +import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.ClassInjector; +import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode; import java.util.List; import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumentationModule.class) -public class SpringBootActuatorInstrumentationModule extends InstrumentationModule { +public class SpringBootActuatorInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public SpringBootActuatorInstrumentationModule() { super( @@ -39,14 +43,19 @@ public void registerHelperResources(HelperResourceBuilder helperResourceBuilder) // this line will make OpenTelemetryMeterRegistryAutoConfiguration available to all // classloaders, so that the bean class loader (different from the instrumented class loader) // can load it - helperResourceBuilder.registerForAllClassLoaders( - "io/opentelemetry/javaagent/instrumentation/spring/actuator/v2_0/OpenTelemetryMeterRegistryAutoConfiguration.class"); + if (!isIndyModule()) { + // For indy module the proxy-bytecode will be injected as resource by injectClasses() + helperResourceBuilder.registerForAllClassLoaders( + "io/opentelemetry/javaagent/instrumentation/spring/actuator/v2_0/OpenTelemetryMeterRegistryAutoConfiguration.class"); + } } @Override - public boolean isIndyModule() { - // can not access OpenTelemetryMeterRegistryAutoConfiguration - return false; + public void injectClasses(ClassInjector injector) { + injector + .proxyBuilder( + "io.opentelemetry.javaagent.instrumentation.spring.actuator.v2_0.OpenTelemetryMeterRegistryAutoConfiguration") + .inject(InjectionMode.CLASS_AND_RESOURCE); } @Override 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 index b78759a9f65f..2eaea5f66d6c 100644 --- 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 @@ -10,9 +10,23 @@ * 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 + CLASS_ONLY(true, false), + RESOURCE_ONLY(false, true), + CLASS_AND_RESOURCE(true, true); + private final boolean injectClass; + private final boolean injectResource; + + InjectionMode(boolean injectClass, boolean injectResource) { + this.injectClass = injectClass; + this.injectResource = injectResource; + } + + public boolean shouldInjectClass() { + return injectClass; + } + + public boolean shouldInjectResource() { + return injectResource; + } } 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 ea0f03752aa8..147fd73b0b3c 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 @@ -14,6 +14,8 @@ 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.InjectionMode; +import io.opentelemetry.javaagent.tooling.HelperClassDefinition; import io.opentelemetry.javaagent.tooling.HelperInjector; import io.opentelemetry.javaagent.tooling.TransformSafeLogger; import io.opentelemetry.javaagent.tooling.Utils; @@ -31,8 +33,10 @@ import io.opentelemetry.javaagent.tooling.util.NamedMatcher; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import java.lang.instrument.Instrumentation; +import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.function.Function; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.description.annotation.AnnotationSource; import net.bytebuddy.description.type.TypeDescription; @@ -96,11 +100,13 @@ private AgentBuilder installIndyModule( return parentAgentBuilder; } - List injectedHelperClassNames = Collections.emptyList(); + List injectedHelperClassNames; if (instrumentationModule instanceof ExperimentalInstrumentationModule) { ExperimentalInstrumentationModule experimentalInstrumentationModule = (ExperimentalInstrumentationModule) instrumentationModule; injectedHelperClassNames = experimentalInstrumentationModule.injectedClassNames(); + } else { + injectedHelperClassNames = Collections.emptyList(); } IndyModuleRegistry.registerIndyModule(instrumentationModule); @@ -113,20 +119,27 @@ private AgentBuilder installIndyModule( MuzzleMatcher muzzleMatcher = new MuzzleMatcher(logger, instrumentationModule, config); + Function> helperGenerator = + cl -> { + List helpers = + new ArrayList<>(injectedClassesCollector.getClassesToInject(cl)); + for (String helperName : injectedHelperClassNames) { + helpers.add( + HelperClassDefinition.create( + helperName, + instrumentationModule.getClass().getClassLoader(), + InjectionMode.CLASS_ONLY)); + } + return helpers; + }; + AgentBuilder.Transformer helperInjector = new HelperInjector( instrumentationModule.instrumentationName(), - injectedHelperClassNames, + helperGenerator, helperResourceBuilder.getResources(), instrumentationModule.getClass().getClassLoader(), instrumentation); - AgentBuilder.Transformer indyHelperInjector = - new HelperInjector( - instrumentationModule.instrumentationName(), - injectedClassesCollector.getClassesToInject(), - Collections.emptyList(), - instrumentationModule.getClass().getClassLoader(), - instrumentation); VirtualFieldImplementationInstaller contextProvider = virtualFieldInstallerFactory.create(instrumentationModule); @@ -137,8 +150,7 @@ private AgentBuilder installIndyModule( setTypeMatcher(agentBuilder, instrumentationModule, typeInstrumentation) .and(muzzleMatcher) .transform(new PatchByteCodeVersionTransformer()) - .transform(helperInjector) - .transform(indyHelperInjector); + .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 index b3d5e9f047d3..bc365b17eec4 100644 --- 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 @@ -9,28 +9,33 @@ 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 io.opentelemetry.javaagent.tooling.HelperClassDefinition; +import java.util.ArrayList; +import java.util.List; import java.util.function.Function; +import java.util.stream.Collectors; 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 Map> classesToInject; + private final List> classesToInject; private final IndyProxyFactory proxyFactory; public ClassInjectorImpl(InstrumentationModule module) { instrumentationModule = module; - classesToInject = new HashMap<>(); + classesToInject = new ArrayList<>(); proxyFactory = IndyBootstrap.getProxyFactory(module); } - public Map> getClassesToInject() { - return classesToInject; + public List getClassesToInject(ClassLoader instrumentedCl) { + return classesToInject.stream() + .map(generator -> generator.apply(instrumentedCl)) + .collect(Collectors.toList()); } @Override @@ -50,15 +55,12 @@ private class ProxyBuilder implements ProxyInjectionBuilder { @Override public void inject(InjectionMode mode) { - if (mode != InjectionMode.CLASS_ONLY) { - throw new UnsupportedOperationException("Not yet implemented"); - } - classesToInject.put( - proxyClassName, + classesToInject.add( cl -> { TypePool typePool = IndyModuleTypePool.get(cl, instrumentationModule); TypeDescription proxiedType = typePool.describe(classToProxy).resolve(); - return proxyFactory.generateProxy(proxiedType, proxyClassName).getBytes(); + 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/IndyModuleRegistry.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyModuleRegistry.java index fa98cbd30ebb..b2c56763cfa4 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 @@ -10,6 +10,7 @@ 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.tooling.BytecodeWithUrl; import io.opentelemetry.javaagent.tooling.muzzle.InstrumentationModuleMuzzle; import java.lang.ref.WeakReference; import java.util.HashSet; @@ -90,11 +91,11 @@ static InstrumentationModuleClassLoader createInstrumentationModuleClassloader( } ClassLoader agentOrExtensionCl = module.getClass().getClassLoader(); - Map injectedClasses = + Map injectedClasses = toInject.stream() .collect( Collectors.toMap( - name -> name, name -> ClassCopySource.create(name, agentOrExtensionCl))); + name -> name, name -> BytecodeWithUrl.create(name, agentOrExtensionCl))); return new InstrumentationModuleClassLoader( instrumentedClassloader, agentOrExtensionCl, injectedClasses); 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 eba2d240aa1a..4cf5859fd5f2 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 @@ -5,6 +5,7 @@ package io.opentelemetry.javaagent.tooling.instrumentation.indy; +import io.opentelemetry.javaagent.tooling.BytecodeWithUrl; import java.io.IOException; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; @@ -43,13 +44,13 @@ public class InstrumentationModuleClassLoader extends ClassLoader { ClassLoader.registerAsParallelCapable(); } - private static final Map ALWAYS_INJECTED_CLASSES = + private static final Map ALWAYS_INJECTED_CLASSES = Collections.singletonMap( - LookupExposer.class.getName(), ClassCopySource.create(LookupExposer.class).cached()); + LookupExposer.class.getName(), BytecodeWithUrl.create(LookupExposer.class).cached()); private static final ProtectionDomain PROTECTION_DOMAIN = getProtectionDomain(); private static final MethodHandle FIND_PACKAGE_METHOD = getFindPackageMethod(); - private final Map additionalInjectedClasses; + private final Map additionalInjectedClasses; private final ClassLoader agentOrExtensionCl; private volatile MethodHandles.Lookup cachedLookup; @@ -59,14 +60,14 @@ public class InstrumentationModuleClassLoader extends ClassLoader { public InstrumentationModuleClassLoader( ClassLoader instrumentedCl, ClassLoader agentOrExtensionCl, - Map injectedClasses) { + Map injectedClasses) { this(instrumentedCl, agentOrExtensionCl, injectedClasses, false); } InstrumentationModuleClassLoader( ClassLoader instrumentedCl, ClassLoader agentOrExtensionCl, - Map injectedClasses, + Map injectedClasses, boolean delegateAllToAgent) { // agent/extension-classloader is "main"-parent, but class lookup is overridden super(agentOrExtensionCl); @@ -105,7 +106,7 @@ protected Class loadClass(String name, boolean resolve) throws ClassNotFoundE // This CL is self-first: Injected class are loaded BEFORE a parent lookup if (result == null) { - ClassCopySource injected = getInjectedClass(name); + BytecodeWithUrl injected = getInjectedClass(name); if (injected != null) { byte[] bytecode = bytecodeOverride.get(name) != null @@ -158,7 +159,7 @@ public URL getResource(String resourceName) { return super.getResource(resourceName); } // for classes use the same precedence as in loadClass - ClassCopySource injected = getInjectedClass(className); + BytecodeWithUrl injected = getInjectedClass(className); if (injected != null) { return injected.getUrl(); } @@ -196,8 +197,8 @@ private static String resourceToClassName(String resourceName) { } @Nullable - private ClassCopySource getInjectedClass(String name) { - ClassCopySource alwaysInjected = ALWAYS_INJECTED_CLASSES.get(name); + private BytecodeWithUrl getInjectedClass(String name) { + BytecodeWithUrl alwaysInjected = ALWAYS_INJECTED_CLASSES.get(name); if (alwaysInjected != null) { return alwaysInjected; } diff --git a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/InstrumentationModuleClassLoaderTest.java b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/InstrumentationModuleClassLoaderTest.java index e6449f28fb2d..7fbfd45e2826 100644 --- a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/InstrumentationModuleClassLoaderTest.java +++ b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/InstrumentationModuleClassLoaderTest.java @@ -8,6 +8,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import io.opentelemetry.javaagent.tooling.BytecodeWithUrl; import io.opentelemetry.javaagent.tooling.instrumentation.indy.dummies.Bar; import io.opentelemetry.javaagent.tooling.instrumentation.indy.dummies.Foo; import java.io.IOException; @@ -36,9 +37,9 @@ class InstrumentationModuleClassLoaderTest { @Test void checkLookup() throws Throwable { - Map toInject = new HashMap<>(); - toInject.put(Foo.class.getName(), ClassCopySource.create(Foo.class)); - toInject.put(Bar.class.getName(), ClassCopySource.create(Bar.class)); + Map toInject = new HashMap<>(); + toInject.put(Foo.class.getName(), BytecodeWithUrl.create(Foo.class)); + toInject.put(Bar.class.getName(), BytecodeWithUrl.create(Bar.class)); ClassLoader dummyParent = new URLClassLoader(new URL[] {}, null); @@ -72,9 +73,9 @@ private static void lookupAndInvokeFoo(InstrumentationModuleClassLoader classLoa @Test void checkInjectedClassesHavePackage() throws Throwable { - Map toInject = new HashMap<>(); - toInject.put(A.class.getName(), ClassCopySource.create(A.class)); - toInject.put(B.class.getName(), ClassCopySource.create(B.class)); + Map toInject = new HashMap<>(); + toInject.put(A.class.getName(), BytecodeWithUrl.create(A.class)); + toInject.put(B.class.getName(), BytecodeWithUrl.create(B.class)); String packageName = A.class.getName().substring(0, A.class.getName().lastIndexOf('.')); ClassLoader dummyParent = new URLClassLoader(new URL[] {}, null); @@ -116,8 +117,8 @@ void checkClassLookupPrecedence(@TempDir Path tempDir) throws Exception { URLClassLoader moduleSourceCl = new URLClassLoader(new URL[] {moduleJar.toUri().toURL()}, null); try { - Map toInject = new HashMap<>(); - toInject.put(C.class.getName(), ClassCopySource.create(C.class.getName(), moduleSourceCl)); + Map toInject = new HashMap<>(); + toInject.put(C.class.getName(), BytecodeWithUrl.create(C.class.getName(), moduleSourceCl)); InstrumentationModuleClassLoader moduleCl = new InstrumentationModuleClassLoader(appCl, agentCl, toInject, true); diff --git a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/ByteArrayUrl.java b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/ByteArrayUrl.java new file mode 100644 index 000000000000..ccbce0f16a6f --- /dev/null +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/ByteArrayUrl.java @@ -0,0 +1,104 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling; + +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.io.UnsupportedEncodingException; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.URLConnection; +import java.net.URLEncoder; +import java.net.URLStreamHandler; +import java.nio.charset.StandardCharsets; +import java.security.Permission; +import java.security.PrivilegedAction; + +public class ByteArrayUrl { + + private ByteArrayUrl() {} + + private static final String URL_SCHEMA = "x-otel-binary"; + + @SuppressWarnings("removal") + public static URL create(String contentName, byte[] data) { + if (System.getSecurityManager() != null) { + return java.security.AccessController.doPrivileged( + (PrivilegedAction) + () -> { + return doCreate(contentName, data); + }); + } else { + return doCreate(contentName, data); + } + } + + private static URL doCreate(String contentName, byte[] data) { + try { + String file = URLEncoder.encode(contentName, StandardCharsets.UTF_8.toString()); + return new URL(URL_SCHEMA, null, -1, file, new ByteArrayUrlStreamHandler(data)); + } catch (MalformedURLException e) { + throw new IllegalArgumentException("Failed to generate URL for the provided arguments", e); + } catch (UnsupportedEncodingException e) { + throw new IllegalStateException(e); + } + } + + /** + * This class is based on ByteBuddy {@link + * net.bytebuddy.dynamic.loading.ByteArrayClassLoader.PersistenceHandler}. + */ + private static class ByteArrayUrlStreamHandler extends URLStreamHandler { + + /** The binary representation of a type's class file. */ + private final byte[] binaryRepresentation; + + /** + * Creates a new byte array URL stream handler. + * + * @param binaryRepresentation The binary representation of a type's class file. + */ + private ByteArrayUrlStreamHandler(byte[] binaryRepresentation) { + this.binaryRepresentation = binaryRepresentation; + } + + @Override + protected URLConnection openConnection(URL url) { + return new ByteArrayUrlConnection(url); + } + + private class ByteArrayUrlConnection extends URLConnection { + + private final InputStream inputStream; + + protected ByteArrayUrlConnection(URL url) { + super(url); + inputStream = new ByteArrayInputStream(binaryRepresentation); + } + + @Override + public void connect() { + connected = true; + } + + @Override + public InputStream getInputStream() { + connect(); // Mimics the semantics of an actual URL connection. + return inputStream; + } + + @Override + public Permission getPermission() { + return null; + } + + @Override + public long getContentLengthLong() { + return binaryRepresentation.length; + } + } + } +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/ClassCopySource.java b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/BytecodeWithUrl.java similarity index 56% rename from javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/ClassCopySource.java rename to muzzle/src/main/java/io/opentelemetry/javaagent/tooling/BytecodeWithUrl.java index ad599e48d1e9..82edcc6cc256 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/ClassCopySource.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/BytecodeWithUrl.java @@ -3,21 +3,24 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.javaagent.tooling.instrumentation.indy; +package io.opentelemetry.javaagent.tooling; import java.io.IOException; import java.io.InputStream; import java.net.URL; +import net.bytebuddy.dynamic.DynamicType; import net.bytebuddy.utility.StreamDrainer; /** * Provides the bytecode and the original resource URL for loaded and not-yet loaded classes. The * implementation is based on {@link net.bytebuddy.dynamic.ClassFileLocator.ForClassLoader}, with - * the difference that it preserves the original classfile resource URL. + * the difference that it preserves the original classfile resource URL. In addition {@link + * BytecodeWithUrl} created can be generated for runtime generated types, this class will take care + * of runtime generating URLs for those. */ -abstract class ClassCopySource { +public abstract class BytecodeWithUrl { - private ClassCopySource() {} + private BytecodeWithUrl() {} /** * Provides a URL pointing to the specific classfile. @@ -35,16 +38,16 @@ private ClassCopySource() {} public abstract byte[] getBytecode(); /** - * Creates a cached copy of this {@link ClassCopySource}. The cached copy eagerly loads the + * Creates a cached copy of this {@link BytecodeWithUrl}. The cached copy eagerly loads the * bytecode, so that {@link #getBytecode()} is guaranteed to not cause any IO. This comes at the * cost of a higher heap consumption, as the bytecode is kept in memory. * * @return an ClassFileSource implementing the described caching behaviour. */ - public abstract ClassCopySource cached(); + public abstract BytecodeWithUrl cached(); /** - * Creates a {@link ClassCopySource} for the class with the provided fully qualified name. The + * Creates a {@link BytecodeWithUrl} for the class with the provided fully qualified name. The * .class file for the provided classname must be available as a resource in the provided * classloader. The class is guaranteed to not be loaded during this process. * @@ -52,7 +55,7 @@ private ClassCopySource() {} * @param classLoader the classloader * @return the ClassCopySource which can be used to copy the provided class to other classloaders. */ - public static ClassCopySource create(String className, ClassLoader classLoader) { + public static BytecodeWithUrl create(String className, ClassLoader classLoader) { if (classLoader == null) { throw new IllegalArgumentException( "Copying classes from the bootstrap classloader is not supported!"); @@ -67,11 +70,34 @@ public static ClassCopySource create(String className, ClassLoader classLoader) * @param loadedClass the class to copy * @return the ClassCopySource which can be used to copy the provided class to other classloaders. */ - public static ClassCopySource create(Class loadedClass) { + public static BytecodeWithUrl create(Class loadedClass) { return create(loadedClass.getName(), loadedClass.getClassLoader()); } - private static class Lazy extends ClassCopySource { + /** + * Creates a {@link BytecodeWithUrl} for a runtime-generated type. It will also provide an + * artificially generated {@link URL} pointing to the in-memory bytecode. + * + * @param className the name of the class represented by the provided bytecode + * @param bytecode the bytecode of the class + * @return the {@link BytecodeWithUrl} referring to this dynamically generated class + */ + public static BytecodeWithUrl create(String className, byte[] bytecode) { + return new ForDynamicType(className, bytecode); + } + + /** + * Invokes {@link #create(String, byte[])} for the provided dynamic type. + * + * @param dynamicType the type to generate the {@link BytecodeWithUrl} + * @return the {@link BytecodeWithUrl} referring to this dynamically generated type + */ + public static BytecodeWithUrl create(DynamicType.Unloaded dynamicType) { + String className = dynamicType.getTypeDescription().getName(); + return new ForDynamicType(className, dynamicType.getBytes()); + } + + private static class Lazy extends BytecodeWithUrl { private final ClassLoader classLoader; private final String resourceName; @@ -101,18 +127,18 @@ public byte[] getBytecode() { } @Override - public ClassCopySource cached() { + public BytecodeWithUrl cached() { return new Cached(this); } } - private static class Cached extends ClassCopySource { + private static class Cached extends BytecodeWithUrl { private final URL classFileUrl; private final byte[] cachedByteCode; - private Cached(ClassCopySource.Lazy from) { + private Cached(BytecodeWithUrl.Lazy from) { classFileUrl = from.getUrl(); cachedByteCode = from.getBytecode(); } @@ -128,8 +154,42 @@ public byte[] getBytecode() { } @Override - public ClassCopySource cached() { + public BytecodeWithUrl cached() { return this; } } + + private static class ForDynamicType extends BytecodeWithUrl { + + private final byte[] byteCode; + private final String className; + private volatile URL generatedUrl; + + private ForDynamicType(String className, byte[] byteCode) { + this.byteCode = byteCode; + this.className = className; + } + + @Override + public URL getUrl() { + if (generatedUrl == null) { + synchronized (this) { + if (generatedUrl == null) { + generatedUrl = ByteArrayUrl.create(className, byteCode); + } + } + } + return generatedUrl; + } + + @Override + public byte[] getBytecode() { + return byteCode; + } + + @Override + public BytecodeWithUrl cached() { + return this; // this type already holds the bytecode in-memory + } + } } diff --git a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperClassDefinition.java b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperClassDefinition.java new file mode 100644 index 000000000000..703033b6f085 --- /dev/null +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperClassDefinition.java @@ -0,0 +1,53 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling; + +import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode; +import net.bytebuddy.dynamic.DynamicType; + +public class HelperClassDefinition { + + private final String className; + private final BytecodeWithUrl bytecode; + private final InjectionMode injectionMode; + + private HelperClassDefinition( + String className, BytecodeWithUrl bytecode, InjectionMode injectionMode) { + this.className = className; + this.bytecode = bytecode; + this.injectionMode = injectionMode; + } + + public static HelperClassDefinition create( + String className, BytecodeWithUrl bytecode, InjectionMode injectionMode) { + return new HelperClassDefinition(className, bytecode, injectionMode); + } + + public static HelperClassDefinition create( + DynamicType.Unloaded type, InjectionMode injectionMode) { + String name = type.getTypeDescription().getName(); + BytecodeWithUrl code = BytecodeWithUrl.create(type); + return create(name, code, injectionMode); + } + + public static HelperClassDefinition create( + String className, ClassLoader copyFrom, InjectionMode injectionMode) { + BytecodeWithUrl code = BytecodeWithUrl.create(className, copyFrom); + return create(className, code, injectionMode); + } + + public String getClassName() { + return className; + } + + public BytecodeWithUrl getBytecode() { + return bytecode; + } + + public InjectionMode getInjectionMode() { + return injectionMode; + } +} 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 12ebb113ff5a..b3ae20e43e10 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java @@ -12,6 +12,7 @@ import io.opentelemetry.instrumentation.api.internal.cache.Cache; import io.opentelemetry.javaagent.bootstrap.HelperResources; import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper; +import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode; import io.opentelemetry.javaagent.tooling.muzzle.HelperResource; import java.io.File; import java.io.IOException; @@ -23,19 +24,16 @@ import java.security.SecureClassLoader; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.LinkedHashMap; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; import java.util.function.Supplier; +import java.util.stream.Collectors; import javax.annotation.Nullable; import net.bytebuddy.agent.builder.AgentBuilder.Transformer; import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.dynamic.ClassFileLocator; import net.bytebuddy.dynamic.DynamicType; import net.bytebuddy.dynamic.loading.ClassInjector; import net.bytebuddy.utility.JavaModule; @@ -91,14 +89,12 @@ Class inject(ClassLoader classLoader, String className) { private final String requestingName; - private final Set helperClassNames; + private final Function> helperClassesGenerator; private final List helperResources; @Nullable private final ClassLoader helpersSource; @Nullable private final Instrumentation instrumentation; - private final Map> dynamicTypeMap = new LinkedHashMap<>(); private final Cache injectedClassLoaders = Cache.weak(); - private final Cache resourcesInjectedClassLoaders = Cache.weak(); /** * Construct HelperInjector. @@ -119,7 +115,15 @@ public HelperInjector( Instrumentation instrumentation) { this.requestingName = requestingName; - this.helperClassNames = new LinkedHashSet<>(helperClassNames); + List helpers = + helperClassNames.stream() + .map( + className -> + HelperClassDefinition.create( + className, helpersSource, InjectionMode.CLASS_ONLY)) + .collect(Collectors.toList()); + + this.helperClassesGenerator = (cl) -> helpers; this.helperResources = helperResources; this.helpersSource = helpersSource; this.instrumentation = instrumentation; @@ -127,15 +131,13 @@ public HelperInjector( public HelperInjector( String requestingName, - Map> helperMap, + Function> helperClassesGenerators, List helperResources, ClassLoader helpersSource, Instrumentation instrumentation) { this.requestingName = requestingName; - this.helperClassNames = helperMap.keySet(); - this.dynamicTypeMap.putAll(helperMap); - + this.helperClassesGenerator = helperClassesGenerators; this.helperResources = helperResources; this.helpersSource = helpersSource; this.instrumentation = instrumentation; @@ -145,49 +147,20 @@ public static HelperInjector forDynamicTypes( String requestingName, Collection> helpers, Instrumentation instrumentation) { - Map> bytes = new HashMap<>(helpers.size()); - for (DynamicType.Unloaded helper : helpers) { - bytes.put(helper.getTypeDescription().getName(), cl -> helper.getBytes()); - } + + List helperDefinitions = + helpers.stream() + .map(helperType -> HelperClassDefinition.create(helperType, InjectionMode.CLASS_ONLY)) + .collect(Collectors.toList()); + return new HelperInjector( - requestingName, bytes, Collections.emptyList(), null, instrumentation); + requestingName, cl -> helperDefinitions, Collections.emptyList(), null, instrumentation); } public static void setHelperInjectorListener(HelperInjectorListener listener) { helperInjectorListener = listener; } - private Map> getHelperMap(ClassLoader targetClassloader) { - Map> result = new LinkedHashMap<>(); - if (dynamicTypeMap.isEmpty()) { - for (String helperClassName : helperClassNames) { - result.put( - helperClassName, - () -> { - try (ClassFileLocator locator = ClassFileLocator.ForClassLoader.of(helpersSource)) { - return locator.locate(helperClassName).resolve(); - } catch (IOException exception) { - if (logger.isLoggable(SEVERE)) { - logger.log( - SEVERE, "Failed to read {0}", new Object[] {helperClassName}, exception); - } - throw new IllegalStateException("Failed to read " + helperClassName, exception); - } - }); - } - } else { - 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 @CanIgnoreReturnValue public DynamicType.Builder transform( @@ -196,114 +169,136 @@ public DynamicType.Builder transform( ClassLoader classLoader, JavaModule javaModule, ProtectionDomain protectionDomain) { - if (!helperClassNames.isEmpty()) { - injectHelperClasses(typeDescription, classLoader); - } - - if (classLoader != null && helpersSource != null && !helperResources.isEmpty()) { - injectHelperResources(classLoader); - } - - return builder; - } - - private void injectHelperResources(ClassLoader classLoader) { - resourcesInjectedClassLoaders.computeIfAbsent( - classLoader, + injectedClassLoaders.computeIfAbsent( + maskNullClassLoader(classLoader), cl -> { - for (HelperResource helperResource : helperResources) { - List resources; - try { - resources = - Collections.list(helpersSource.getResources(helperResource.getAgentPath())); - } catch (IOException e) { - logger.log( - SEVERE, - "Unexpected exception occurred when loading resources {}; skipping", - new Object[] {helperResource.getAgentPath()}, - e); - continue; - } - if (resources.isEmpty()) { - logger.log( - FINE, - "Helper resources {0} requested but not found.", - helperResource.getAgentPath()); - continue; - } - - if (helperResource.allClassLoaders()) { - logger.log( - FINE, - "Injecting resources onto all classloaders: {0}", - helperResource.getApplicationPath()); - HelperResources.registerForAllClassLoaders( - helperResource.getApplicationPath(), resources); - } else { - if (logger.isLoggable(FINE)) { - logger.log( - FINE, - "Injecting resources onto class loader {0} -> {1}", - new Object[] {classLoader, helperResource.getApplicationPath()}); - } - HelperResources.register(classLoader, helperResource.getApplicationPath(), resources); - } + List helpers = helperClassesGenerator.apply(cl); + + LinkedHashMap> classesToInject = + helpers.stream() + .filter(helper -> helper.getInjectionMode().shouldInjectClass()) + .collect( + Collectors.toMap( + HelperClassDefinition::getClassName, + helper -> () -> helper.getBytecode().getBytecode(), + (a, b) -> { + throw new IllegalStateException( + "Duplicate classnames for helper class detected!"); + }, + LinkedHashMap::new)); + + Map classResourcesToInject = + helpers.stream() + .filter(helper -> helper.getInjectionMode().shouldInjectResource()) + .collect( + Collectors.toMap( + helper -> helper.getClassName().replace('.', '/') + ".class", + helper -> helper.getBytecode().getUrl())); + + injectHelperClasses(typeDescription, cl, classesToInject); + if (!isBootClassLoader(cl)) { + injectHelperResources(cl, classResourcesToInject); } - return true; }); + return builder; } - private void injectHelperClasses(TypeDescription typeDescription, ClassLoader classLoader) { - classLoader = maskNullClassLoader(classLoader); + private void injectHelperResources( + ClassLoader classLoader, Map additionalResources) { + for (HelperResource helperResource : helperResources) { + List resources; + try { + resources = Collections.list(helpersSource.getResources(helperResource.getAgentPath())); + } catch (IOException e) { + logger.log( + SEVERE, + "Unexpected exception occurred when loading resources {}; skipping", + new Object[] {helperResource.getAgentPath()}, + e); + continue; + } + if (resources.isEmpty()) { + logger.log( + FINE, "Helper resources {0} requested but not found.", helperResource.getAgentPath()); + continue; + } + + if (helperResource.allClassLoaders()) { + logger.log( + FINE, + "Injecting resources onto all classloaders: {0}", + helperResource.getApplicationPath()); + HelperResources.registerForAllClassLoaders(helperResource.getApplicationPath(), resources); + } else { + injectResourceToClassloader(classLoader, helperResource.getApplicationPath(), resources); + } + } + additionalResources.forEach( + (path, url) -> + injectResourceToClassloader(classLoader, path, Collections.singletonList(url))); + } + + private static void injectResourceToClassloader( + ClassLoader classLoader, String path, List resources) { + if (logger.isLoggable(FINE)) { + logger.log( + FINE, + "Injecting resources onto class loader {0} -> {1}", + new Object[] {classLoader, path}); + } + HelperResources.register(classLoader, path, resources); + } + + @SuppressWarnings("NonApiType") + private void injectHelperClasses( + TypeDescription typeDescription, + ClassLoader classLoader, + LinkedHashMap> classnameToBytes) { + if (classnameToBytes.isEmpty()) { + return; + } if (classLoader == BOOTSTRAP_CLASSLOADER_PLACEHOLDER && instrumentation == null) { logger.log( SEVERE, "Cannot inject helpers into the bootstrap class loader without an instance of Instrumentation. Programmer error!"); return; } + try { + if (logger.isLoggable(FINE)) { + logger.log( + FINE, + "Injecting classes onto class loader {0} -> {1}", + new Object[] {classLoader, classnameToBytes.keySet()}); + } - injectedClassLoaders.computeIfAbsent( - classLoader, - cl -> { - try { - if (logger.isLoggable(FINE)) { - logger.log( - FINE, - "Injecting classes onto class loader {0} -> {1}", - new Object[] {cl, helperClassNames}); - } - - Map> classnameToBytes = getHelperMap(cl); - Map map = - helperInjectors.computeIfAbsent(cl, (unused) -> new ConcurrentHashMap<>()); - for (Map.Entry> entry : classnameToBytes.entrySet()) { - // for boot loader we use a placeholder injector, we only need these classes to be - // in the injected classes map to later tell which of the classes are injected - HelperClassInjector injector = - isBootClassLoader(cl) - ? BOOT_CLASS_INJECTOR - : new HelperClassInjector(entry.getValue()); - map.put(entry.getKey(), injector); - } - - // For boot loader we define the classes immediately. For other loaders we load them - // from the loadClass method of the class loader. - if (isBootClassLoader(cl)) { - injectBootstrapClassLoader(classnameToBytes); - } - } catch (Exception e) { - if (logger.isLoggable(SEVERE)) { - logger.log( - SEVERE, - "Error preparing helpers while processing {0} for {1}. Failed to inject helper classes into instance {2}", - new Object[] {typeDescription, requestingName, cl}, - e); - } - throw new IllegalStateException(e); - } - return true; - }); + Map map = + helperInjectors.computeIfAbsent(classLoader, (unused) -> new ConcurrentHashMap<>()); + for (Map.Entry> entry : classnameToBytes.entrySet()) { + // for boot loader we use a placeholder injector, we only need these classes to be + // in the injected classes map to later tell which of the classes are injected + HelperClassInjector injector = + isBootClassLoader(classLoader) + ? BOOT_CLASS_INJECTOR + : new HelperClassInjector(entry.getValue()); + map.put(entry.getKey(), injector); + } + + // For boot loader we define the classes immediately. For other loaders we load them + // from the loadClass method of the class loader. + if (isBootClassLoader(classLoader)) { + injectBootstrapClassLoader(classnameToBytes); + } + } catch (Exception e) { + if (logger.isLoggable(SEVERE)) { + logger.log( + SEVERE, + "Error preparing helpers while processing {0} for {1}. Failed to inject helper classes into instance {2}", + new Object[] {typeDescription, requestingName, classLoader}, + e); + } + throw new IllegalStateException(e); + } } private static Map resolve(Map> classes) { diff --git a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/ByteArrayUrlTest.java b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/ByteArrayUrlTest.java new file mode 100644 index 000000000000..7b56a2c8a3ab --- /dev/null +++ b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/ByteArrayUrlTest.java @@ -0,0 +1,30 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +import java.net.URL; +import java.net.URLConnection; +import org.junit.jupiter.api.Test; + +public class ByteArrayUrlTest { + + @Test + public void testUrlCreation() throws Exception { + byte[] content = new byte[] {1, 2, 3, 4}; + + URL url = ByteArrayUrl.create("my.data$foo", content); + + assertThat(url).hasPath("my.data%24foo").hasProtocol("x-otel-binary"); + + URLConnection connection = url.openConnection(); + assertThat(connection.getContentLengthLong()).isEqualTo(4); + assertThat(connection.getContentLength()).isEqualTo(4); + + assertThat(connection.getInputStream()).hasBinaryContent(content); + } +} 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 88848fb7a23d..82a0dc29fbd5 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 @@ -55,7 +55,7 @@ public List getAdditionalHelperClassNames() { @Override public void injectClasses(ClassInjector injector) { - injector.proxyBuilder("indy.ProxyMe", "foo.bar.Proxy").inject(InjectionMode.CLASS_ONLY); + injector.proxyBuilder("indy.ProxyMe", "foo.bar.Proxy").inject(InjectionMode.CLASS_AND_RESOURCE); } public static class Instrumentation implements TypeInstrumentation { 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 b4668d8d1e6c..4973641b331e 100644 --- a/testing-common/integration-tests/src/test/java/indy/IndyInstrumentationTest.java +++ b/testing-common/integration-tests/src/test/java/indy/IndyInstrumentationTest.java @@ -11,12 +11,14 @@ 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.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.testcontainers.shaded.org.apache.commons.io.IOUtils; @SuppressWarnings({"unused", "MethodCanBeStatic"}) public class IndyInstrumentationTest { @@ -148,5 +150,11 @@ void testProxyInjection() throws Exception { 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); } }