From c9f31da9b74a090898c76eada0569bf71866cf05 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 1 Oct 2025 17:27:56 +0300 Subject: [PATCH 01/13] Reduce unsafe usages --- .../executors/ThreadPoolExecutorTest.java | 2 +- .../LoadInjectedClassInstrumentation.java | 10 +- .../lambda/LambdaInstrumentationTest.java | 2 +- .../src/test/java/ReflectionTest.java | 4 +- .../internal/reflection/ReflectionHelper.java | 6 +- .../bootstrap/InjectedClassHelper.java | 11 +- .../VirtualFieldAccessorMarker.java | 2 +- .../{ => field}/VirtualFieldDetector.java | 7 +- .../VirtualFieldInstalledMarker.java | 2 +- .../javaagent/tooling/AgentInstaller.java | 17 ++ .../FieldAccessorInterfacesGenerator.java | 2 +- ...mplementationClassFileLocatorProvider.java | 2 +- .../FieldBackedImplementationInstaller.java | 2 +- .../field/GeneratedVirtualFieldNames.java | 5 +- .../tooling/field/RealFieldInjector.java | 2 +- .../javaagent/tooling/HelperInjector.java | 170 ++++++++++++++---- .../muzzle/AgentCachingPoolStrategy.java | 2 +- .../FieldBackedImplementationTest.java | 2 +- .../context/FieldInjectionDisabledTest.java | 2 +- 19 files changed, 194 insertions(+), 58 deletions(-) rename javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/{ => field}/VirtualFieldAccessorMarker.java (79%) rename javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/{ => field}/VirtualFieldDetector.java (92%) rename javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/{ => field}/VirtualFieldInstalledMarker.java (83%) diff --git a/instrumentation/executors/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/executors/ThreadPoolExecutorTest.java b/instrumentation/executors/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/executors/ThreadPoolExecutorTest.java index 010399cab3ce..009173b3a5de 100644 --- a/instrumentation/executors/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/executors/ThreadPoolExecutorTest.java +++ b/instrumentation/executors/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/executors/ThreadPoolExecutorTest.java @@ -10,7 +10,7 @@ import io.opentelemetry.api.baggage.Baggage; import io.opentelemetry.context.Scope; -import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldInstalledMarker; import java.util.concurrent.CountDownLatch; import java.util.concurrent.FutureTask; import java.util.concurrent.LinkedBlockingQueue; diff --git a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java index a7049805c491..08b87bb257d7 100644 --- a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java +++ b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java @@ -17,8 +17,10 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper; +import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper.HelperClassLoader; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import java.lang.invoke.MethodHandles; import net.bytebuddy.asm.Advice; import net.bytebuddy.asm.Advice.AssignReturned; import net.bytebuddy.description.method.MethodDescription; @@ -59,8 +61,12 @@ public static class LoadClassAdvice { @Advice.OnMethodEnter(skipOn = Advice.OnNonDefaultValue.class) public static Class onEnter( - @Advice.This ClassLoader classLoader, @Advice.Argument(0) String name) { - return InjectedClassHelper.loadHelperClass(classLoader, name); + @Advice.This ClassLoader classLoader, @Advice.Argument(0) String name) throws Throwable { + HelperClassLoader helperClassLoader = + InjectedClassHelper.getHelperClassLoader(classLoader, name); + return helperClassLoader != null + ? helperClassLoader.loadHelperClass(MethodHandles.lookup()) + : null; } @AssignReturned.ToReturned diff --git a/instrumentation/internal/internal-lambda/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaInstrumentationTest.java b/instrumentation/internal/internal-lambda/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaInstrumentationTest.java index 01bbb39b6921..2ecf6d9049fc 100644 --- a/instrumentation/internal/internal-lambda/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaInstrumentationTest.java +++ b/instrumentation/internal/internal-lambda/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaInstrumentationTest.java @@ -7,7 +7,7 @@ import static org.assertj.core.api.Assertions.assertThat; -import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldInstalledMarker; import org.junit.jupiter.api.Test; class LambdaInstrumentationTest { diff --git a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/ReflectionTest.java b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/ReflectionTest.java index 7293136a21bb..1e026529ce4b 100644 --- a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/ReflectionTest.java +++ b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/ReflectionTest.java @@ -7,8 +7,8 @@ import instrumentation.TestHelperClass; import io.opentelemetry.javaagent.bootstrap.InstrumentationProxy; -import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker; -import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldAccessorMarker; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldInstalledMarker; import java.io.ObjectStreamClass; import java.io.Serializable; import java.lang.reflect.Field; 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 e38e67de2a71..b649f00f1f94 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 @@ -6,9 +6,9 @@ package io.opentelemetry.javaagent.instrumentation.internal.reflection; import io.opentelemetry.javaagent.bootstrap.InstrumentationProxy; -import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker; -import io.opentelemetry.javaagent.bootstrap.VirtualFieldDetector; -import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldAccessorMarker; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldDetector; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldInstalledMarker; import java.lang.reflect.Field; import java.lang.reflect.Method; import java.util.ArrayList; diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InjectedClassHelper.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InjectedClassHelper.java index b3d68690b183..f64c7209ab89 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InjectedClassHelper.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InjectedClassHelper.java @@ -5,6 +5,7 @@ package io.opentelemetry.javaagent.bootstrap; +import java.lang.invoke.MethodHandles; import java.util.function.BiFunction; import java.util.function.BiPredicate; import java.util.function.Function; @@ -37,10 +38,10 @@ public static boolean isHelperClass(ClassLoader classLoader, String className) { return helperClassDetector.test(classLoader, className); } - private static volatile BiFunction> helperClassLoader; + private static volatile BiFunction helperClassLoader; public static void internalSetHelperClassLoader( - BiFunction> helperClassLoader) { + BiFunction helperClassLoader) { if (InjectedClassHelper.helperClassLoader != null) { // Only possible by misuse of this API, just ignore. return; @@ -48,10 +49,14 @@ public static void internalSetHelperClassLoader( InjectedClassHelper.helperClassLoader = helperClassLoader; } - public static Class loadHelperClass(ClassLoader classLoader, String className) { + public static HelperClassLoader getHelperClassLoader(ClassLoader classLoader, String className) { if (helperClassLoader == null) { return null; } return helperClassLoader.apply(classLoader, className); } + + public interface HelperClassLoader { + Class loadHelperClass(MethodHandles.Lookup lookup) throws Throwable; + } } diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/VirtualFieldAccessorMarker.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/field/VirtualFieldAccessorMarker.java similarity index 79% rename from javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/VirtualFieldAccessorMarker.java rename to javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/field/VirtualFieldAccessorMarker.java index 2107acdef3d5..df2a5e16bb49 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/VirtualFieldAccessorMarker.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/field/VirtualFieldAccessorMarker.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.javaagent.bootstrap; +package io.opentelemetry.javaagent.bootstrap.field; /** A marker interface implemented by virtual field accessor classes. */ public interface VirtualFieldAccessorMarker {} diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/VirtualFieldDetector.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/field/VirtualFieldDetector.java similarity index 92% rename from javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/VirtualFieldDetector.java rename to javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/field/VirtualFieldDetector.java index 0a1664bdf034..a08da8d9088c 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/VirtualFieldDetector.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/field/VirtualFieldDetector.java @@ -3,9 +3,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.javaagent.bootstrap; +package io.opentelemetry.javaagent.bootstrap.field; import io.opentelemetry.instrumentation.api.internal.cache.Cache; +import java.lang.invoke.MethodHandles; import java.util.Collection; /** Helper class for detecting whether given class has virtual fields. */ @@ -48,4 +49,8 @@ public static boolean hasVirtualField(Class clazz, String virtualFieldInterfa public static void markVirtualFields(Class clazz, Collection virtualFieldClassName) { classesWithVirtualFields.put(clazz, virtualFieldClassName); } + + public static MethodHandles.Lookup lookup() { + return MethodHandles.lookup(); + } } diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/VirtualFieldInstalledMarker.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/field/VirtualFieldInstalledMarker.java similarity index 83% rename from javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/VirtualFieldInstalledMarker.java rename to javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/field/VirtualFieldInstalledMarker.java index d92c7e763dda..403a39a7f413 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/VirtualFieldInstalledMarker.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/field/VirtualFieldInstalledMarker.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.javaagent.bootstrap; +package io.opentelemetry.javaagent.bootstrap.field; /** * A marker interface that signifies that virtual fields have been installed into the class that diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java index 948d0352844e..5d5ff93bf8c4 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java @@ -131,6 +131,16 @@ private static void installBytebuddyAgent( // https://bugs.openjdk.org/browse/JDK-8164165 ThreadLocalRandom.current(); + // AgentBuilder.Default constructor triggers sun.misc.Unsafe::objectFieldOffset called warning + // AgentBuilder$Default. + // -> NexusAccessor. + // -> ClassInjector$UsingReflection. + // -> ClassInjector$UsingUnsafe. + // -> WARNING: sun.misc.Unsafe::objectFieldOffset called by + // net.bytebuddy.dynamic.loading.ClassInjector$UsingUnsafe$Dispatcher$CreationAction + // we don't use byte-buddy nexus so we disable it and later restore the original value for the + // system property + String originalNexusDisabled = System.setProperty("net.bytebuddy.nexus.disabled", "true"); AgentBuilder agentBuilder = new AgentBuilder.Default( // default method graph compiler inspects the class hierarchy, we don't need it, so @@ -147,6 +157,13 @@ private static void installBytebuddyAgent( .with(AgentTooling.poolStrategy()) .with(AgentTooling.transformListener()) .with(AgentTooling.locationStrategy()); + // restore the original value for the nexus disabled property + if (originalNexusDisabled != null) { + System.setProperty("net.bytebuddy.nexus.disabled", originalNexusDisabled); + } else { + System.clearProperty("net.bytebuddy.nexus.disabled"); + } + if (JavaModule.isSupported()) { agentBuilder = agentBuilder.with(new ExposeAgentBootstrapListener(inst)); } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldAccessorInterfacesGenerator.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldAccessorInterfacesGenerator.java index c25d50a631aa..c90a7819e993 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldAccessorInterfacesGenerator.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldAccessorInterfacesGenerator.java @@ -9,7 +9,7 @@ import static io.opentelemetry.javaagent.tooling.field.GeneratedVirtualFieldNames.getRealGetterName; import static io.opentelemetry.javaagent.tooling.field.GeneratedVirtualFieldNames.getRealSetterName; -import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldAccessorMarker; import io.opentelemetry.javaagent.tooling.muzzle.VirtualFieldMappings; import java.util.HashMap; import java.util.Map; diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldBackedImplementationClassFileLocatorProvider.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldBackedImplementationClassFileLocatorProvider.java index f857d0376fa0..bd66613c46f7 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldBackedImplementationClassFileLocatorProvider.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldBackedImplementationClassFileLocatorProvider.java @@ -6,7 +6,7 @@ package io.opentelemetry.javaagent.tooling.field; import com.google.auto.service.AutoService; -import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldAccessorMarker; import io.opentelemetry.javaagent.tooling.muzzle.ClassFileLocatorProvider; import net.bytebuddy.ByteBuddy; import net.bytebuddy.description.modifier.SyntheticState; diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldBackedImplementationInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldBackedImplementationInstaller.java index bce86a829911..47ffafcec926 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldBackedImplementationInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldBackedImplementationInstaller.java @@ -15,7 +15,7 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.opentelemetry.javaagent.bootstrap.InstrumentationHolder; -import io.opentelemetry.javaagent.bootstrap.VirtualFieldDetector; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldDetector; import io.opentelemetry.javaagent.tooling.HelperInjector; import io.opentelemetry.javaagent.tooling.TransformSafeLogger; import io.opentelemetry.javaagent.tooling.instrumentation.InstrumentationModuleInstaller; diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/GeneratedVirtualFieldNames.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/GeneratedVirtualFieldNames.java index 7c3ac6ffb807..1c186a2e0c8f 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/GeneratedVirtualFieldNames.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/GeneratedVirtualFieldNames.java @@ -5,6 +5,8 @@ package io.opentelemetry.javaagent.tooling.field; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldAccessorMarker; + final class GeneratedVirtualFieldNames { /** @@ -13,7 +15,8 @@ final class GeneratedVirtualFieldNames { * 'isolating' (or 'module') classloaders like jboss and osgi see injected classes. This works * because we instrument those classloaders to load everything inside bootstrap packages. */ - static final String DYNAMIC_CLASSES_PACKAGE = "io.opentelemetry.javaagent.bootstrap.field."; + static final String DYNAMIC_CLASSES_PACKAGE = + VirtualFieldAccessorMarker.class.getPackage().getName() + "."; private GeneratedVirtualFieldNames() {} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/RealFieldInjector.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/RealFieldInjector.java index f58d4201efb4..b94a8f42dfaf 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/RealFieldInjector.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/RealFieldInjector.java @@ -10,7 +10,7 @@ import static io.opentelemetry.javaagent.tooling.field.GeneratedVirtualFieldNames.getRealSetterName; import com.google.errorprone.annotations.CanIgnoreReturnValue; -import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldInstalledMarker; import io.opentelemetry.javaagent.extension.instrumentation.internal.AsmApi; import io.opentelemetry.javaagent.tooling.Utils; import java.util.Arrays; 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 381d66a6ec06..b55dd9835922 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java @@ -12,18 +12,24 @@ import io.opentelemetry.instrumentation.api.internal.cache.Cache; import io.opentelemetry.javaagent.bootstrap.HelperResources; import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper; +import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper.HelperClassLoader; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldAccessorMarker; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldDetector; import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode; import io.opentelemetry.javaagent.tooling.muzzle.HelperResource; import java.io.File; import java.io.IOException; import java.lang.instrument.Instrumentation; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; import java.net.URL; import java.nio.file.Files; -import java.security.PrivilegedAction; import java.security.ProtectionDomain; import java.security.SecureClassLoader; import java.util.Collection; import java.util.Collections; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -79,7 +85,8 @@ public String toString() { private static final HelperClassInjector BOOT_CLASS_INJECTOR = new HelperClassInjector(null) { @Override - Class inject(ClassLoader classLoader, String className) { + Class inject( + ClassLoader classLoader, String className, ClassLoaderAccess classLoaderAccess) { throw new UnsupportedOperationException("should not be called"); } }; @@ -311,16 +318,53 @@ private static Map resolve(Map> classes return result; } - private Map> injectBootstrapClassLoader(Map> inject) - throws IOException { + private static final String virtualFieldPackage = + VirtualFieldAccessorMarker.class.getPackage().getName() + "."; + // because all generated virtual field classes are in the same package we can use lookup to define + // them + private static final ClassInjector.UsingLookup virtualFieldInjector = + ClassInjector.UsingLookup.of(VirtualFieldDetector.lookup()); + + private void injectBootstrapClassLoader(Map> inject) throws IOException { Map classnameToBytes = resolve(inject); if (helperInjectorListener != null) { helperInjectorListener.onInjection(classnameToBytes); } - if (ClassInjector.UsingUnsafe.isAvailable()) { - return ClassInjector.UsingUnsafe.ofBootLoader().injectRaw(classnameToBytes); + if (ClassInjector.UsingLookup.isAvailable()) { + for (Iterator> iterator = classnameToBytes.entrySet().iterator(); + iterator.hasNext(); ) { + Map.Entry entry = iterator.next(); + String className = entry.getKey(); + + if (className.startsWith(virtualFieldPackage)) { + iterator.remove(); + try { + virtualFieldInjector.injectRaw(Collections.singletonMap(className, entry.getValue())); + } catch (LinkageError error) { + // Unlike the ClassInjector.UsingUnsafe.ofBootLoader() ClassInjector.UsingLookup doesn't + // check whether the class got loaded when there is an exception defining it. + // We attempt to define some classes multiple times and fail with LinkageError duplicate + // class definition on the second attempt. We recover from this by checking whether the + // class is loaded and if it is, we ignore the error. + try { + Class.forName(className, false, null); + } catch (ClassNotFoundException unused) { + // throw the original error + throw error; + } + } + } + } + } + + // TODO by default, we use unsafe to define rest of the classes into boot loader + // can be disabled with -Dnet.bytebuddy.safe=true + // use -Dsun.misc.unsafe.memory.access=debug to check where unsafe is used + if (ClassInjector.UsingUnsafe.isAvailable() && !classnameToBytes.isEmpty()) { + ClassInjector.UsingUnsafe.ofBootLoader().injectRaw(classnameToBytes); + return; } // Mar 2020: Since we're proactively cleaning up tempDirs, we cannot share dirs per thread. @@ -328,14 +372,16 @@ private Map> injectBootstrapClassLoader(Map loadHelperClass(ClassLoader classLoader, String className) { + private static final ClassValue classLoaderAccessClassValue = + new ClassValue() { + @Override + protected ClassLoaderAccess computeValue(Class type) { + return new ClassLoaderAccess(); + } + }; + + private static class ClassLoaderAccess { + volatile MethodHandle findLoadedClass; + volatile MethodHandle defineClass; + + void initialize(MethodHandles.Lookup lookup) throws Exception { + if (findLoadedClass == null) { + findLoadedClass = + lookup.findVirtual( + ClassLoader.class, + "findLoadedClass", + MethodType.methodType(Class.class, String.class)); + } + if (defineClass == null) { + defineClass = + lookup.findVirtual( + ClassLoader.class, + "defineClass", + MethodType.methodType( + Class.class, + String.class, + byte[].class, + int.class, + int.class, + ProtectionDomain.class)); + } + } + + Class loadClass(ClassLoader cLassLoader, String className, byte[] classBytes) + throws Throwable { + // first check whether the class is already defined + Class clazz = (Class) findLoadedClass.invoke(cLassLoader, className); + if (clazz != null) { + return clazz; + } + + // we don't attempt to synchronize class definitions but let them race and recover from + // duplicate class definitions by checking whether the class got defined in the catch block + // we don't define packages for injected helper classes + try { + return (Class) + defineClass.invoke( + cLassLoader, + className, + classBytes, + 0, + classBytes.length, + HelperInjector.PROTECTION_DOMAIN); + } catch (Throwable throwable) { + clazz = (Class) findLoadedClass.invoke(cLassLoader, className); + if (clazz != null) { + return clazz; + } + throw throwable; + } + } + } + + static HelperClassLoader loadHelperClass(ClassLoader classLoader, String className) { if (classLoader == null) { throw new IllegalStateException("boot loader not supported"); } @@ -390,7 +501,12 @@ public static Class loadHelperClass(ClassLoader classLoader, String className if (helperClassInjector == null) { return null; } - return helperClassInjector.inject(classLoader, className); + + return lookup -> { + ClassLoaderAccess classLoaderAccess = classLoaderAccessClassValue.get(classLoader.getClass()); + classLoaderAccess.initialize(lookup); + return helperClassInjector.inject(classLoader, className, classLoaderAccess); + }; } private static class HelperClassInjector { @@ -400,25 +516,9 @@ private static class HelperClassInjector { this.bytes = bytes; } - Class inject(ClassLoader classLoader, String className) { - // if security manager is present byte buddy calls - // checkPermission(new ReflectPermission("suppressAccessChecks")) so we must call class - // injection with AccessController.doPrivileged when security manager is enabled - Map> result = - execute( - () -> - new ClassInjector.UsingReflection(classLoader, PROTECTION_DOMAIN) - .injectRaw(Collections.singletonMap(className, bytes.get()))); - return result.get(className); - } - } - - @SuppressWarnings("removal") // AccessController is deprecated for removal - private static T execute(PrivilegedAction action) { - if (System.getSecurityManager() != null) { - return java.security.AccessController.doPrivileged(action); - } else { - return action.run(); + Class inject(ClassLoader classLoader, String className, ClassLoaderAccess classLoaderAccess) + throws Throwable { + return classLoaderAccess.loadClass(classLoader, className, bytes.get()); } } } diff --git a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/AgentCachingPoolStrategy.java b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/AgentCachingPoolStrategy.java index e931af800f76..2ca3a20ca4a6 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/AgentCachingPoolStrategy.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/AgentCachingPoolStrategy.java @@ -8,7 +8,7 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.opentelemetry.instrumentation.api.internal.cache.Cache; import io.opentelemetry.javaagent.bootstrap.InstrumentationHolder; -import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker; +import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldAccessorMarker; import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig; import java.lang.instrument.Instrumentation; import java.lang.ref.WeakReference; diff --git a/testing-common/integration-tests/src/test/java/context/FieldBackedImplementationTest.java b/testing-common/integration-tests/src/test/java/context/FieldBackedImplementationTest.java index 1cd4ce7e8861..c128337d8093 100644 --- a/testing-common/integration-tests/src/test/java/context/FieldBackedImplementationTest.java +++ b/testing-common/integration-tests/src/test/java/context/FieldBackedImplementationTest.java @@ -79,7 +79,7 @@ void structureModified(Class keyClass) throws Exception { boolean hasAccessorInterface = false; boolean accessorInterfaceIsSynthetic = false; for (Class iface : keyClass.getInterfaces()) { - if ("io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker" + if ("io.opentelemetry.javaagent.bootstrap.field.VirtualFieldInstalledMarker" .equals(iface.getName())) { hasMarkerInterface = true; } diff --git a/testing-common/integration-tests/src/test/java/context/FieldInjectionDisabledTest.java b/testing-common/integration-tests/src/test/java/context/FieldInjectionDisabledTest.java index 5b6db62b6e17..50417dfe2d61 100644 --- a/testing-common/integration-tests/src/test/java/context/FieldInjectionDisabledTest.java +++ b/testing-common/integration-tests/src/test/java/context/FieldInjectionDisabledTest.java @@ -47,7 +47,7 @@ void structuralModificationDisabled() { boolean hasMarkerInterface = false; boolean hasAccessorInterface = false; for (Class inter : keyClass.getInterfaces()) { - if ("io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker" + if ("io.opentelemetry.javaagent.bootstrap.field.VirtualFieldInstalledMarker" .equals(inter.getName())) { hasMarkerInterface = true; } From 569b517d685528659b2e11d05110aece306165d9 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 1 Oct 2025 17:54:22 +0300 Subject: [PATCH 02/13] fix typo --- .../opentelemetry/javaagent/tooling/HelperInjector.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 b55dd9835922..b7b6c17c09f4 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java @@ -459,10 +459,10 @@ void initialize(MethodHandles.Lookup lookup) throws Exception { } } - Class loadClass(ClassLoader cLassLoader, String className, byte[] classBytes) + Class loadClass(ClassLoader classLoader, String className, byte[] classBytes) throws Throwable { // first check whether the class is already defined - Class clazz = (Class) findLoadedClass.invoke(cLassLoader, className); + Class clazz = (Class) findLoadedClass.invoke(classLoader, className); if (clazz != null) { return clazz; } @@ -473,14 +473,14 @@ Class loadClass(ClassLoader cLassLoader, String className, byte[] classBytes) try { return (Class) defineClass.invoke( - cLassLoader, + classLoader, className, classBytes, 0, classBytes.length, HelperInjector.PROTECTION_DOMAIN); } catch (Throwable throwable) { - clazz = (Class) findLoadedClass.invoke(cLassLoader, className); + clazz = (Class) findLoadedClass.invoke(classLoader, className); if (clazz != null) { return clazz; } From 6006123f2acd66536f1eb7c67e6dfd3efc02f218 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 1 Oct 2025 18:00:35 +0300 Subject: [PATCH 03/13] fix java8 --- .../opentelemetry/javaagent/tooling/HelperInjector.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) 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 b7b6c17c09f4..7326c0aad0e6 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java @@ -320,10 +320,6 @@ private static Map resolve(Map> classes private static final String virtualFieldPackage = VirtualFieldAccessorMarker.class.getPackage().getName() + "."; - // because all generated virtual field classes are in the same package we can use lookup to define - // them - private static final ClassInjector.UsingLookup virtualFieldInjector = - ClassInjector.UsingLookup.of(VirtualFieldDetector.lookup()); private void injectBootstrapClassLoader(Map> inject) throws IOException { @@ -333,6 +329,11 @@ private void injectBootstrapClassLoader(Map> inject) th } if (ClassInjector.UsingLookup.isAvailable()) { + // because all generated virtual field classes are in the same package we can use lookup to + // define them + ClassInjector virtualFieldInjector = + ClassInjector.UsingLookup.of(VirtualFieldDetector.lookup()); + for (Iterator> iterator = classnameToBytes.entrySet().iterator(); iterator.hasNext(); ) { Map.Entry entry = iterator.next(); From ce39147ff044b05168a3622403f52dc42c96b9bc Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 1 Oct 2025 18:30:35 +0300 Subject: [PATCH 04/13] add comment --- .../javaagent/test/HelperInjectionTest.groovy | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/test/HelperInjectionTest.groovy b/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/test/HelperInjectionTest.groovy index 7f2882806cd5..02a4680199e5 100644 --- a/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/test/HelperInjectionTest.groovy +++ b/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/test/HelperInjectionTest.groovy @@ -5,7 +5,7 @@ package io.opentelemetry.javaagent.test - +import groovy.transform.CompileStatic import io.opentelemetry.javaagent.tooling.AgentInstaller import io.opentelemetry.javaagent.tooling.HelperInjector import io.opentelemetry.javaagent.tooling.Utils @@ -16,6 +16,7 @@ import net.bytebuddy.dynamic.ClassFileLocator import net.bytebuddy.dynamic.loading.ClassInjector import spock.lang.Specification +import java.lang.invoke.MethodHandles import java.lang.ref.WeakReference import java.time.Duration import java.util.concurrent.atomic.AtomicReference @@ -25,6 +26,17 @@ import static io.opentelemetry.instrumentation.test.utils.GcUtils.awaitGc class HelperInjectionTest extends Specification { + @CompileStatic + class EmptyLoader extends URLClassLoader { + EmptyLoader() { + super(new URL[0], (ClassLoader) null) + } + + def lookup() { + MethodHandles.lookup() + } + } + def "helpers injected to non-delegating classloader"() { setup: URL[] helpersSourceUrls = new URL[1] @@ -33,7 +45,7 @@ class HelperInjectionTest extends Specification { String helperClassName = HelperInjectionTest.getPackage().getName() + '.HelperClass' HelperInjector injector = new HelperInjector("test", [helperClassName], [], helpersSourceLoader, null) - AtomicReference emptyLoader = new AtomicReference<>(new URLClassLoader(new URL[0], (ClassLoader) null)) + AtomicReference emptyLoader = new AtomicReference<>(new EmptyLoader()) when: emptyLoader.get().loadClass(helperClassName) @@ -42,7 +54,7 @@ class HelperInjectionTest extends Specification { when: injector.transform(null, null, emptyLoader.get(), null, null) - HelperInjector.loadHelperClass(emptyLoader.get(), helperClassName) + HelperInjector.loadHelperClass(emptyLoader.get(), helperClassName).loadHelperClass(emptyLoader.get().lookup()) emptyLoader.get().loadClass(helperClassName) then: isClassLoaded(helperClassName, emptyLoader.get()) From cf03cc86ebe0aada5fd1a30d9faa378c09a8131d Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 1 Oct 2025 18:30:44 +0300 Subject: [PATCH 05/13] fix test --- .../java/io/opentelemetry/javaagent/tooling/HelperInjector.java | 1 + 1 file changed, 1 insertion(+) 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 7326c0aad0e6..7c4087cb462d 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java @@ -490,6 +490,7 @@ Class loadClass(ClassLoader classLoader, String className, byte[] classBytes) } } + // visible for testing static HelperClassLoader loadHelperClass(ClassLoader classLoader, String className) { if (classLoader == null) { throw new IllegalStateException("boot loader not supported"); From 8bd3f803c4d76548f6602fb234eb0b1feddb1970 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 1 Oct 2025 19:04:19 +0300 Subject: [PATCH 06/13] fix jdk8 --- .../LoadInjectedClassInstrumentation.java | 26 ++++++++++++++++++- .../javaagent/tooling/HelperInjector.java | 25 ++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java index 08b87bb257d7..63f69a66ad76 100644 --- a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java +++ b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java @@ -52,8 +52,13 @@ public void transform(TypeTransformer transformer) { .and(takesArgument(1, boolean.class)))) .and(isPublic().or(isProtected())) .and(not(isStatic())); + boolean useLookup = Double.parseDouble(System.getProperty("java.specification.version")) >= 11; // Inline instrumentation to prevent problems with invokedynamic-recursion - applyInlineAdvice(transformer, methodMatcher, this.getClass().getName() + "$LoadClassAdvice"); + applyInlineAdvice( + transformer, + methodMatcher, + this.getClass().getName() + + (useLookup ? "$LoadClassAdvice" : "$LoadClassWithoutLookupAdvice")); } @SuppressWarnings("unused") @@ -76,4 +81,23 @@ public static Class onExit( return loadedClass != null ? loadedClass : originalResult; } } + + @SuppressWarnings("unused") + public static class LoadClassWithoutLookupAdvice { + + @Advice.OnMethodEnter(skipOn = Advice.OnNonDefaultValue.class) + public static Class onEnter( + @Advice.This ClassLoader classLoader, @Advice.Argument(0) String name) throws Throwable { + HelperClassLoader helperClassLoader = + InjectedClassHelper.getHelperClassLoader(classLoader, name); + return helperClassLoader != null ? helperClassLoader.loadHelperClass(null) : null; + } + + @AssignReturned.ToReturned + @Advice.OnMethodExit(onThrowable = Throwable.class) + public static Class onExit( + @Advice.Return Class originalResult, @Advice.Enter Class loadedClass) { + return loadedClass != null ? loadedClass : originalResult; + } + } } 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 7c4087cb462d..f68503bac8c3 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java @@ -25,6 +25,7 @@ import java.lang.invoke.MethodType; import java.net.URL; import java.nio.file.Files; +import java.security.PrivilegedAction; import java.security.ProtectionDomain; import java.security.SecureClassLoader; import java.util.Collection; @@ -505,6 +506,9 @@ static HelperClassLoader loadHelperClass(ClassLoader classLoader, String classNa } return lookup -> { + if (lookup == null) { + return helperClassInjector.inject(classLoader, className); + } ClassLoaderAccess classLoaderAccess = classLoaderAccessClassValue.get(classLoader.getClass()); classLoaderAccess.initialize(lookup); return helperClassInjector.inject(classLoader, className, classLoaderAccess); @@ -522,5 +526,26 @@ Class inject(ClassLoader classLoader, String className, ClassLoaderAccess cla throws Throwable { return classLoaderAccess.loadClass(classLoader, className, bytes.get()); } + + Class inject(ClassLoader classLoader, String className) { + // if security manager is present byte buddy calls + // checkPermission(new ReflectPermission("suppressAccessChecks")) so we must call class + // injection with AccessController.doPrivileged when security manager is enabled + Map> result = + execute( + () -> + new ClassInjector.UsingReflection(classLoader, PROTECTION_DOMAIN) + .injectRaw(Collections.singletonMap(className, bytes.get()))); + return result.get(className); + } + } + + @SuppressWarnings("removal") // AccessController is deprecated for removal + private static T execute(PrivilegedAction action) { + if (System.getSecurityManager() != null) { + return java.security.AccessController.doPrivileged(action); + } else { + return action.run(); + } } } From 3e640a66ec13308d8b8d5b8d92bc7be82f1eee1c Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Thu, 2 Oct 2025 08:59:13 +0300 Subject: [PATCH 07/13] add comments --- .../internal/classloader/LoadInjectedClassInstrumentation.java | 3 +++ .../io/opentelemetry/javaagent/tooling/HelperInjector.java | 2 ++ 2 files changed, 5 insertions(+) diff --git a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java index 63f69a66ad76..936c7a7330c3 100644 --- a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java +++ b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java @@ -90,6 +90,9 @@ public static Class onEnter( @Advice.This ClassLoader classLoader, @Advice.Argument(0) String name) throws Throwable { HelperClassLoader helperClassLoader = InjectedClassHelper.getHelperClassLoader(classLoader, name); + // on jdk8 we can't use MethodHandles.lookup() because it fails when called from + // java.lang.ClassLoader with java.lang.IllegalArgumentException: illegal lookupClass: class + // java.lang.ClassLoader return helperClassLoader != null ? helperClassLoader.loadHelperClass(null) : null; } 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 f68503bac8c3..d79b8f538dc4 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java @@ -506,6 +506,8 @@ static HelperClassLoader loadHelperClass(ClassLoader classLoader, String classNa } return lookup -> { + // if lookup is not specified fall back to using ClassInjector.UsingReflection + // this is used on jdk8 if (lookup == null) { return helperClassInjector.inject(classLoader, className); } From e745dd30ecca0f50dcf6041ac26c74db8692bc58 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 3 Oct 2025 13:39:20 +0300 Subject: [PATCH 08/13] rewrite loading injected classes instrumentation in asm --- .../ClassLoaderInstrumentationModule.java | 3 +- .../LoadInjectedClassInstrumentation.java | 274 +++++++++++++----- .../bootstrap/InjectedClassHelper.java | 24 +- .../javaagent/test/HelperInjectionTest.groovy | 27 +- .../javaagent/tooling/HelperInjector.java | 160 ++-------- 5 files changed, 272 insertions(+), 216 deletions(-) diff --git a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java index ffbda5b61949..d656d851298f 100644 --- a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java +++ b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java @@ -35,8 +35,9 @@ public List injectedClassNames() { @Override public List typeInstrumentations() { return asList( - new BootDelegationInstrumentation(), + // added first so it would get applied after other instrumentations new LoadInjectedClassInstrumentation(), + new BootDelegationInstrumentation(), new ResourceInjectionInstrumentation(), new DefineClassInstrumentation()); } diff --git a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java index 936c7a7330c3..20dc42a9ab64 100644 --- a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java +++ b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java @@ -6,26 +6,27 @@ package io.opentelemetry.javaagent.instrumentation.internal.classloader; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.extendsClass; -import static io.opentelemetry.javaagent.instrumentation.internal.classloader.AdviceUtil.applyInlineAdvice; -import static net.bytebuddy.matcher.ElementMatchers.isMethod; -import static net.bytebuddy.matcher.ElementMatchers.isProtected; -import static net.bytebuddy.matcher.ElementMatchers.isPublic; -import static net.bytebuddy.matcher.ElementMatchers.isStatic; import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.not; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; -import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper; -import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper.HelperClassLoader; +import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper.HelperClassInfo; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import java.lang.invoke.MethodHandles; -import net.bytebuddy.asm.Advice; -import net.bytebuddy.asm.Advice.AssignReturned; -import net.bytebuddy.description.method.MethodDescription; +import io.opentelemetry.javaagent.extension.instrumentation.internal.AsmApi; +import net.bytebuddy.asm.AsmVisitorWrapper; +import net.bytebuddy.description.field.FieldDescription; +import net.bytebuddy.description.field.FieldList; +import net.bytebuddy.description.method.MethodList; import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.implementation.Implementation; import net.bytebuddy.matcher.ElementMatcher; +import net.bytebuddy.pool.TypePool; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.Label; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; /** * This instrumentation inserts loading of our injected helper classes at the start of {@code @@ -40,67 +41,206 @@ public ElementMatcher typeMatcher() { @Override public void transform(TypeTransformer transformer) { - ElementMatcher.Junction methodMatcher = - isMethod() - .and(named("loadClass")) - .and( - takesArguments(1) - .and(takesArgument(0, String.class)) - .or( - takesArguments(2) - .and(takesArgument(0, String.class)) - .and(takesArgument(1, boolean.class)))) - .and(isPublic().or(isProtected())) - .and(not(isStatic())); - boolean useLookup = Double.parseDouble(System.getProperty("java.specification.version")) >= 11; - // Inline instrumentation to prevent problems with invokedynamic-recursion - applyInlineAdvice( - transformer, - methodMatcher, - this.getClass().getName() - + (useLookup ? "$LoadClassAdvice" : "$LoadClassWithoutLookupAdvice")); + transformer.applyTransformer( + (builder, typeDescription, classLoader, module, protectionDomain) -> + builder.visit( + new AsmVisitorWrapper() { + @Override + public int mergeWriter(int flags) { + return flags | ClassWriter.COMPUTE_MAXS; + } + + @Override + public int mergeReader(int flags) { + return flags; + } + + @Override + public ClassVisitor wrap( + TypeDescription instrumentedType, + ClassVisitor classVisitor, + Implementation.Context implementationContext, + TypePool typePool, + FieldList fields, + MethodList methods, + int writerFlags, + int readerFlags) { + return new ClassLoaderClassVisitor(classVisitor); + } + })); } - @SuppressWarnings("unused") - public static class LoadClassAdvice { - - @Advice.OnMethodEnter(skipOn = Advice.OnNonDefaultValue.class) - public static Class onEnter( - @Advice.This ClassLoader classLoader, @Advice.Argument(0) String name) throws Throwable { - HelperClassLoader helperClassLoader = - InjectedClassHelper.getHelperClassLoader(classLoader, name); - return helperClassLoader != null - ? helperClassLoader.loadHelperClass(MethodHandles.lookup()) - : null; - } + private static class ClassLoaderClassVisitor extends ClassVisitor implements Opcodes { + private String internalClassName; - @AssignReturned.ToReturned - @Advice.OnMethodExit(onThrowable = Throwable.class) - public static Class onExit( - @Advice.Return Class originalResult, @Advice.Enter Class loadedClass) { - return loadedClass != null ? loadedClass : originalResult; + ClassLoaderClassVisitor(ClassVisitor classVisitor) { + super(AsmApi.VERSION, classVisitor); } - } - @SuppressWarnings("unused") - public static class LoadClassWithoutLookupAdvice { - - @Advice.OnMethodEnter(skipOn = Advice.OnNonDefaultValue.class) - public static Class onEnter( - @Advice.This ClassLoader classLoader, @Advice.Argument(0) String name) throws Throwable { - HelperClassLoader helperClassLoader = - InjectedClassHelper.getHelperClassLoader(classLoader, name); - // on jdk8 we can't use MethodHandles.lookup() because it fails when called from - // java.lang.ClassLoader with java.lang.IllegalArgumentException: illegal lookupClass: class - // java.lang.ClassLoader - return helperClassLoader != null ? helperClassLoader.loadHelperClass(null) : null; + @Override + public void visit( + int version, + int access, + String name, + String signature, + String superName, + String[] interfaces) { + super.visit(version, access, name, signature, superName, interfaces); + internalClassName = name; } - @AssignReturned.ToReturned - @Advice.OnMethodExit(onThrowable = Throwable.class) - public static Class onExit( - @Advice.Return Class originalResult, @Advice.Enter Class loadedClass) { - return loadedClass != null ? loadedClass : originalResult; + @Override + public MethodVisitor visitMethod( + int access, String name, String descriptor, String signature, String[] exceptions) { + MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions); + if ("loadClass".equals(name) + && ("(Ljava/lang/String;)Ljava/lang/Class;".equals(descriptor) + || "(Ljava/lang/String;Z)Ljava/lang/Class;".equals(descriptor))) { + + int argumentCount = Type.getArgumentTypes(descriptor).length; + return new MethodVisitor(api, mv) { + @Override + public void visitCode() { + super.visitCode(); + + // inserts the following at the start of the loadClass method: + /* + InjectedClassHelper.HelperClassInfo helperClassInfo = InjectedClassHelper.getHelperClassInfo(this, name); + if (helperClassInfo != null) { + Class clazz = findLoadedClass(name); + if (clazz != null) { + return clazz; + } + try { + byte[] bytes = helperClassInfo.getClassBytes(); + return defineClass(name, bytes, 0, bytes.length, helperClassInfo.getProtectionDomain()); + } catch (LinkageError error) { + clazz = findLoadedClass(name); + if (clazz != null) { + return clazz; + } + throw error; + } + } + */ + + Label startTry = new Label(); + Label endTry = new Label(); + Label handler = new Label(); + mv.visitTryCatchBlock(startTry, endTry, handler, "java/lang/LinkageError"); + // InjectedClassHelper.HelperClassInfo helperClassInfo = + // InjectedClassHelper.getHelperClassInfo(this, name); + mv.visitVarInsn(ALOAD, 0); + mv.visitVarInsn(ALOAD, 1); + mv.visitMethodInsn( + INVOKESTATIC, + Type.getInternalName(InjectedClassHelper.class), + "getHelperClassInfo", + "(Ljava/lang/ClassLoader;Ljava/lang/String;)" + + Type.getDescriptor(HelperClassInfo.class), + false); + mv.visitVarInsn(ASTORE, argumentCount + 1); // store helperClassInfo + mv.visitVarInsn(ALOAD, argumentCount + 1); + Label notHelperClass = new Label(); + mv.visitJumpInsn(IFNULL, notHelperClass); + + // getHelperClassInfo returned non-null + // Class clazz = findLoadedClass(name); + mv.visitVarInsn(ALOAD, 0); + mv.visitVarInsn(ALOAD, 1); + mv.visitMethodInsn( + INVOKEVIRTUAL, + internalClassName, + "findLoadedClass", + "(Ljava/lang/String;)Ljava/lang/Class;", + false); + mv.visitVarInsn(ASTORE, argumentCount + 2); // store clazz + mv.visitVarInsn(ALOAD, argumentCount + 2); + mv.visitJumpInsn(IFNULL, startTry); + + // findLoadedClass returned non-null + // return clazz + mv.visitVarInsn(ALOAD, argumentCount + 2); + mv.visitInsn(ARETURN); + + mv.visitLabel(startTry); + mv.visitFrame( + Opcodes.F_APPEND, + 2, + new Object[] {Type.getInternalName(HelperClassInfo.class), "java/lang/Class"}, + 0, + null); + // byte[] bytes = helperClassInfo.getClassBytes(); + mv.visitVarInsn(ALOAD, argumentCount + 1); + mv.visitMethodInsn( + INVOKEINTERFACE, + Type.getInternalName(HelperClassInfo.class), + "getClassBytes", + "()[B", + true); + mv.visitVarInsn(ASTORE, argumentCount + 3); // store bytes + + // return defineClass(name, bytes, 0, bytes.length, + // helperClassInfo.getProtectionDomain()); + mv.visitVarInsn(ALOAD, 0); + mv.visitVarInsn(ALOAD, 1); + mv.visitVarInsn(ALOAD, argumentCount + 3); + mv.visitInsn(ICONST_0); + mv.visitVarInsn(ALOAD, argumentCount + 3); + mv.visitInsn(ARRAYLENGTH); + mv.visitVarInsn(ALOAD, argumentCount + 1); + mv.visitMethodInsn( + INVOKEINTERFACE, + Type.getInternalName(HelperClassInfo.class), + "getProtectionDomain", + "()Ljava/security/ProtectionDomain;", + true); + mv.visitMethodInsn( + INVOKEVIRTUAL, + internalClassName, + "defineClass", + "(Ljava/lang/String;[BIILjava/security/ProtectionDomain;)Ljava/lang/Class;", + false); + mv.visitLabel(endTry); + mv.visitInsn(ARETURN); + + mv.visitLabel(handler); + mv.visitFrame(Opcodes.F_SAME1, 0, null, 1, new Object[] {"java/lang/LinkageError"}); + mv.visitVarInsn(ASTORE, argumentCount + 3); // store LinkageError + // clazz = findLoadedClass(name); + mv.visitVarInsn(ALOAD, 0); + mv.visitVarInsn(ALOAD, 1); + mv.visitMethodInsn( + INVOKEVIRTUAL, + internalClassName, + "findLoadedClass", + "(Ljava/lang/String;)Ljava/lang/Class;", + false); + mv.visitVarInsn(ASTORE, argumentCount + 2); // score clazz + mv.visitVarInsn(ALOAD, argumentCount + 2); + Label throwError = new Label(); + mv.visitJumpInsn(IFNULL, throwError); + // return clazz + mv.visitVarInsn(ALOAD, argumentCount + 2); + mv.visitInsn(ARETURN); + mv.visitLabel(throwError); + mv.visitFrame(Opcodes.F_APPEND, 1, new Object[] {"java/lang/LinkageError"}, 0, null); + // throw error + mv.visitVarInsn(ALOAD, argumentCount + 3); + mv.visitInsn(ATHROW); + + mv.visitLabel(notHelperClass); + mv.visitFrame(Opcodes.F_CHOP, 3, null, 0, null); + } + + @Override + public void visitMaxs(int maxStack, int maxLocals) { + // minimally we have argumentCount parameters + this + 3 locals added by us + super.visitMaxs(maxStack, Math.max(maxLocals, argumentCount + 1 + 3)); + } + }; + } + return mv; } } } diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InjectedClassHelper.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InjectedClassHelper.java index f64c7209ab89..7b110d2991d5 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InjectedClassHelper.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InjectedClassHelper.java @@ -5,7 +5,7 @@ package io.opentelemetry.javaagent.bootstrap; -import java.lang.invoke.MethodHandles; +import java.security.ProtectionDomain; import java.util.function.BiFunction; import java.util.function.BiPredicate; import java.util.function.Function; @@ -38,25 +38,27 @@ public static boolean isHelperClass(ClassLoader classLoader, String className) { return helperClassDetector.test(classLoader, className); } - private static volatile BiFunction helperClassLoader; + private static volatile BiFunction helperClassInfo; - public static void internalSetHelperClassLoader( - BiFunction helperClassLoader) { - if (InjectedClassHelper.helperClassLoader != null) { + public static void internalSetHelperClassInfo( + BiFunction helperClassInfo) { + if (InjectedClassHelper.helperClassInfo != null) { // Only possible by misuse of this API, just ignore. return; } - InjectedClassHelper.helperClassLoader = helperClassLoader; + InjectedClassHelper.helperClassInfo = helperClassInfo; } - public static HelperClassLoader getHelperClassLoader(ClassLoader classLoader, String className) { - if (helperClassLoader == null) { + public static HelperClassInfo getHelperClassInfo(ClassLoader classLoader, String className) { + if (helperClassInfo == null) { return null; } - return helperClassLoader.apply(classLoader, className); + return helperClassInfo.apply(classLoader, className); } - public interface HelperClassLoader { - Class loadHelperClass(MethodHandles.Lookup lookup) throws Throwable; + public interface HelperClassInfo { + byte[] getClassBytes(); + + ProtectionDomain getProtectionDomain(); } } diff --git a/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/test/HelperInjectionTest.groovy b/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/test/HelperInjectionTest.groovy index 02a4680199e5..15e9fd92c066 100644 --- a/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/test/HelperInjectionTest.groovy +++ b/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/test/HelperInjectionTest.groovy @@ -6,6 +6,7 @@ package io.opentelemetry.javaagent.test import groovy.transform.CompileStatic +import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper import io.opentelemetry.javaagent.tooling.AgentInstaller import io.opentelemetry.javaagent.tooling.HelperInjector import io.opentelemetry.javaagent.tooling.Utils @@ -16,7 +17,6 @@ import net.bytebuddy.dynamic.ClassFileLocator import net.bytebuddy.dynamic.loading.ClassInjector import spock.lang.Specification -import java.lang.invoke.MethodHandles import java.lang.ref.WeakReference import java.time.Duration import java.util.concurrent.atomic.AtomicReference @@ -32,8 +32,28 @@ class HelperInjectionTest extends Specification { super(new URL[0], (ClassLoader) null) } - def lookup() { - MethodHandles.lookup() + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + // the same code as added by LoadInjectedClassInstrumentation + InjectedClassHelper.HelperClassInfo helperClassInfo = InjectedClassHelper.getHelperClassInfo(this, name) + if (helperClassInfo != null) { + Class clazz = findLoadedClass(name) + if (clazz != null) { + return clazz + } + try { + byte[] bytes = helperClassInfo.getClassBytes() + return defineClass(name, bytes, 0, bytes.length, helperClassInfo.getProtectionDomain()) + } catch (LinkageError error) { + clazz = findLoadedClass(name) + if (clazz != null) { + return clazz + } + throw error + } + } + + return super.loadClass(name, resolve) } } @@ -54,7 +74,6 @@ class HelperInjectionTest extends Specification { when: injector.transform(null, null, emptyLoader.get(), null, null) - HelperInjector.loadHelperClass(emptyLoader.get(), helperClassName).loadHelperClass(emptyLoader.get().lookup()) emptyLoader.get().loadClass(helperClassName) then: isClassLoaded(helperClassName, emptyLoader.get()) 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 d79b8f538dc4..62575564c875 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java @@ -12,7 +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.bootstrap.InjectedClassHelper.HelperClassLoader; +import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper.HelperClassInfo; import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldAccessorMarker; import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldDetector; import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode; @@ -20,12 +20,8 @@ import java.io.File; import java.io.IOException; import java.lang.instrument.Instrumentation; -import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; -import java.lang.invoke.MethodType; import java.net.URL; import java.nio.file.Files; -import java.security.PrivilegedAction; import java.security.ProtectionDomain; import java.security.SecureClassLoader; import java.util.Collection; @@ -71,7 +67,7 @@ public class HelperInjector implements Transformer { static { InjectedClassHelper.internalSetHelperClassDetector(HelperInjector::isInjectedClass); - InjectedClassHelper.internalSetHelperClassLoader(HelperInjector::loadHelperClass); + InjectedClassHelper.internalSetHelperClassInfo(HelperInjector::getHelperClassInfo); } // Need this because we can't put null into the injectedClassLoaders map. @@ -83,17 +79,9 @@ public String toString() { } }; - private static final HelperClassInjector BOOT_CLASS_INJECTOR = - new HelperClassInjector(null) { - @Override - Class inject( - ClassLoader classLoader, String className, ClassLoaderAccess classLoaderAccess) { - throw new UnsupportedOperationException("should not be called"); - } - }; + private static final HelperClass BOOT_CLASS = new HelperClass(null); - private static final Cache> helperInjectors = - Cache.weak(); + private static final Cache> helperClasses = Cache.weak(); private final String requestingName; @@ -282,15 +270,13 @@ private void injectHelperClasses( new Object[] {classLoader, classnameToBytes.keySet()}); } - Map map = - helperInjectors.computeIfAbsent(classLoader, (unused) -> new ConcurrentHashMap<>()); + Map map = + helperClasses.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()); + HelperClass injector = + isBootClassLoader(classLoader) ? BOOT_CLASS : new HelperClass(entry.getValue()); map.put(entry.getKey(), injector); } @@ -418,136 +404,44 @@ public static boolean isInjectedClass(Class clazz) { } public static boolean isInjectedClass(ClassLoader classLoader, String className) { - Map injectorMap = - helperInjectors.get(maskNullClassLoader(classLoader)); - if (injectorMap == null) { + Map helperMap = helperClasses.get(maskNullClassLoader(classLoader)); + if (helperMap == null) { return false; } - return injectorMap.containsKey(className); - } - - private static final ClassValue classLoaderAccessClassValue = - new ClassValue() { - @Override - protected ClassLoaderAccess computeValue(Class type) { - return new ClassLoaderAccess(); - } - }; - - private static class ClassLoaderAccess { - volatile MethodHandle findLoadedClass; - volatile MethodHandle defineClass; - - void initialize(MethodHandles.Lookup lookup) throws Exception { - if (findLoadedClass == null) { - findLoadedClass = - lookup.findVirtual( - ClassLoader.class, - "findLoadedClass", - MethodType.methodType(Class.class, String.class)); - } - if (defineClass == null) { - defineClass = - lookup.findVirtual( - ClassLoader.class, - "defineClass", - MethodType.methodType( - Class.class, - String.class, - byte[].class, - int.class, - int.class, - ProtectionDomain.class)); - } - } - - Class loadClass(ClassLoader classLoader, String className, byte[] classBytes) - throws Throwable { - // first check whether the class is already defined - Class clazz = (Class) findLoadedClass.invoke(classLoader, className); - if (clazz != null) { - return clazz; - } - - // we don't attempt to synchronize class definitions but let them race and recover from - // duplicate class definitions by checking whether the class got defined in the catch block - // we don't define packages for injected helper classes - try { - return (Class) - defineClass.invoke( - classLoader, - className, - classBytes, - 0, - classBytes.length, - HelperInjector.PROTECTION_DOMAIN); - } catch (Throwable throwable) { - clazz = (Class) findLoadedClass.invoke(classLoader, className); - if (clazz != null) { - return clazz; - } - throw throwable; - } - } + return helperMap.containsKey(className); } - // visible for testing - static HelperClassLoader loadHelperClass(ClassLoader classLoader, String className) { + private static HelperClassInfo getHelperClassInfo(ClassLoader classLoader, String className) { if (classLoader == null) { throw new IllegalStateException("boot loader not supported"); } - Map injectorMap = helperInjectors.get(classLoader); - if (injectorMap == null) { + Map helperMap = helperClasses.get(classLoader); + if (helperMap == null) { return null; } - HelperClassInjector helperClassInjector = injectorMap.get(className); - if (helperClassInjector == null) { + HelperClass helperClass = helperMap.get(className); + if (helperClass == null) { return null; } - return lookup -> { - // if lookup is not specified fall back to using ClassInjector.UsingReflection - // this is used on jdk8 - if (lookup == null) { - return helperClassInjector.inject(classLoader, className); + return new HelperClassInfo() { + @Override + public byte[] getClassBytes() { + return helperClass.bytes.get(); + } + + @Override + public ProtectionDomain getProtectionDomain() { + return PROTECTION_DOMAIN; } - ClassLoaderAccess classLoaderAccess = classLoaderAccessClassValue.get(classLoader.getClass()); - classLoaderAccess.initialize(lookup); - return helperClassInjector.inject(classLoader, className, classLoaderAccess); }; } - private static class HelperClassInjector { + private static class HelperClass { private final Supplier bytes; - HelperClassInjector(Supplier bytes) { + HelperClass(Supplier bytes) { this.bytes = bytes; } - - Class inject(ClassLoader classLoader, String className, ClassLoaderAccess classLoaderAccess) - throws Throwable { - return classLoaderAccess.loadClass(classLoader, className, bytes.get()); - } - - Class inject(ClassLoader classLoader, String className) { - // if security manager is present byte buddy calls - // checkPermission(new ReflectPermission("suppressAccessChecks")) so we must call class - // injection with AccessController.doPrivileged when security manager is enabled - Map> result = - execute( - () -> - new ClassInjector.UsingReflection(classLoader, PROTECTION_DOMAIN) - .injectRaw(Collections.singletonMap(className, bytes.get()))); - return result.get(className); - } - } - - @SuppressWarnings("removal") // AccessController is deprecated for removal - private static T execute(PrivilegedAction action) { - if (System.getSecurityManager() != null) { - return java.security.AccessController.doPrivileged(action); - } else { - return action.run(); - } } } From c8fac05dd7b36feea3f48d23ab411cf42de96252 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 3 Oct 2025 14:38:04 +0300 Subject: [PATCH 09/13] don't add frames to old classes --- .../LoadInjectedClassInstrumentation.java | 287 +++++++++--------- 1 file changed, 151 insertions(+), 136 deletions(-) diff --git a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java index 20dc42a9ab64..529461f21ea2 100644 --- a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java +++ b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java @@ -13,6 +13,7 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.extension.instrumentation.internal.AsmApi; +import net.bytebuddy.ClassFileVersion; import net.bytebuddy.asm.AsmVisitorWrapper; import net.bytebuddy.description.field.FieldDescription; import net.bytebuddy.description.field.FieldList; @@ -72,6 +73,7 @@ public ClassVisitor wrap( private static class ClassLoaderClassVisitor extends ClassVisitor implements Opcodes { private String internalClassName; + private boolean frames; ClassLoaderClassVisitor(ClassVisitor classVisitor) { super(AsmApi.VERSION, classVisitor); @@ -87,160 +89,173 @@ public void visit( String[] interfaces) { super.visit(version, access, name, signature, superName, interfaces); internalClassName = name; + frames = ClassFileVersion.ofMinorMajor(version).isAtLeast(ClassFileVersion.JAVA_V6); } @Override public MethodVisitor visitMethod( int access, String name, String descriptor, String signature, String[] exceptions) { MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions); - if ("loadClass".equals(name) - && ("(Ljava/lang/String;)Ljava/lang/Class;".equals(descriptor) - || "(Ljava/lang/String;Z)Ljava/lang/Class;".equals(descriptor))) { - - int argumentCount = Type.getArgumentTypes(descriptor).length; - return new MethodVisitor(api, mv) { - @Override - public void visitCode() { - super.visitCode(); - - // inserts the following at the start of the loadClass method: - /* - InjectedClassHelper.HelperClassInfo helperClassInfo = InjectedClassHelper.getHelperClassInfo(this, name); - if (helperClassInfo != null) { - Class clazz = findLoadedClass(name); - if (clazz != null) { - return clazz; - } - try { - byte[] bytes = helperClassInfo.getClassBytes(); - return defineClass(name, bytes, 0, bytes.length, helperClassInfo.getProtectionDomain()); - } catch (LinkageError error) { - clazz = findLoadedClass(name); - if (clazz != null) { - return clazz; - } - throw error; - } - } - */ - - Label startTry = new Label(); - Label endTry = new Label(); - Label handler = new Label(); - mv.visitTryCatchBlock(startTry, endTry, handler, "java/lang/LinkageError"); - // InjectedClassHelper.HelperClassInfo helperClassInfo = - // InjectedClassHelper.getHelperClassInfo(this, name); - mv.visitVarInsn(ALOAD, 0); - mv.visitVarInsn(ALOAD, 1); - mv.visitMethodInsn( - INVOKESTATIC, - Type.getInternalName(InjectedClassHelper.class), - "getHelperClassInfo", - "(Ljava/lang/ClassLoader;Ljava/lang/String;)" - + Type.getDescriptor(HelperClassInfo.class), - false); - mv.visitVarInsn(ASTORE, argumentCount + 1); // store helperClassInfo - mv.visitVarInsn(ALOAD, argumentCount + 1); - Label notHelperClass = new Label(); - mv.visitJumpInsn(IFNULL, notHelperClass); - - // getHelperClassInfo returned non-null - // Class clazz = findLoadedClass(name); - mv.visitVarInsn(ALOAD, 0); - mv.visitVarInsn(ALOAD, 1); - mv.visitMethodInsn( - INVOKEVIRTUAL, - internalClassName, - "findLoadedClass", - "(Ljava/lang/String;)Ljava/lang/Class;", - false); - mv.visitVarInsn(ASTORE, argumentCount + 2); // store clazz - mv.visitVarInsn(ALOAD, argumentCount + 2); - mv.visitJumpInsn(IFNULL, startTry); - - // findLoadedClass returned non-null - // return clazz - mv.visitVarInsn(ALOAD, argumentCount + 2); - mv.visitInsn(ARETURN); - - mv.visitLabel(startTry); + boolean loadClassMethod = + "loadClass".equals(name) + && ("(Ljava/lang/String;)Ljava/lang/Class;".equals(descriptor) + || "(Ljava/lang/String;Z)Ljava/lang/Class;".equals(descriptor)); + if (!loadClassMethod) { + return mv; + } + + int argumentCount = Type.getArgumentTypes(descriptor).length; + return new MethodVisitor(api, mv) { + @Override + public void visitCode() { + super.visitCode(); + + // inserts the following at the start of the loadClass method: + /* + InjectedClassHelper.HelperClassInfo helperClassInfo = InjectedClassHelper.getHelperClassInfo(this, name); + if (helperClassInfo != null) { + Class clazz = findLoadedClass(name); + if (clazz != null) { + return clazz; + } + try { + byte[] bytes = helperClassInfo.getClassBytes(); + return defineClass(name, bytes, 0, bytes.length, helperClassInfo.getProtectionDomain()); + } catch (LinkageError error) { + clazz = findLoadedClass(name); + if (clazz != null) { + return clazz; + } + throw error; + } + } + */ + + Label startTry = new Label(); + Label endTry = new Label(); + Label handler = new Label(); + mv.visitTryCatchBlock(startTry, endTry, handler, "java/lang/LinkageError"); + // InjectedClassHelper.HelperClassInfo helperClassInfo = InjectedClassHelper + // .getHelperClassInfo(this, name); + mv.visitVarInsn(ALOAD, 0); + mv.visitVarInsn(ALOAD, 1); + mv.visitMethodInsn( + INVOKESTATIC, + Type.getInternalName(InjectedClassHelper.class), + "getHelperClassInfo", + "(Ljava/lang/ClassLoader;Ljava/lang/String;)" + + Type.getDescriptor(HelperClassInfo.class), + false); + mv.visitVarInsn(ASTORE, argumentCount + 1); // store helperClassInfo + mv.visitVarInsn(ALOAD, argumentCount + 1); + Label notHelperClass = new Label(); + mv.visitJumpInsn(IFNULL, notHelperClass); + + // getHelperClassInfo returned non-null + // Class clazz = findLoadedClass(name); + mv.visitVarInsn(ALOAD, 0); + mv.visitVarInsn(ALOAD, 1); + mv.visitMethodInsn( + INVOKEVIRTUAL, + internalClassName, + "findLoadedClass", + "(Ljava/lang/String;)Ljava/lang/Class;", + false); + mv.visitVarInsn(ASTORE, argumentCount + 2); // store clazz + mv.visitVarInsn(ALOAD, argumentCount + 2); + mv.visitJumpInsn(IFNULL, startTry); + + // findLoadedClass returned non-null + // return clazz + mv.visitVarInsn(ALOAD, argumentCount + 2); + mv.visitInsn(ARETURN); + + mv.visitLabel(startTry); + if (frames) { mv.visitFrame( Opcodes.F_APPEND, 2, new Object[] {Type.getInternalName(HelperClassInfo.class), "java/lang/Class"}, 0, null); - // byte[] bytes = helperClassInfo.getClassBytes(); - mv.visitVarInsn(ALOAD, argumentCount + 1); - mv.visitMethodInsn( - INVOKEINTERFACE, - Type.getInternalName(HelperClassInfo.class), - "getClassBytes", - "()[B", - true); - mv.visitVarInsn(ASTORE, argumentCount + 3); // store bytes - - // return defineClass(name, bytes, 0, bytes.length, - // helperClassInfo.getProtectionDomain()); - mv.visitVarInsn(ALOAD, 0); - mv.visitVarInsn(ALOAD, 1); - mv.visitVarInsn(ALOAD, argumentCount + 3); - mv.visitInsn(ICONST_0); - mv.visitVarInsn(ALOAD, argumentCount + 3); - mv.visitInsn(ARRAYLENGTH); - mv.visitVarInsn(ALOAD, argumentCount + 1); - mv.visitMethodInsn( - INVOKEINTERFACE, - Type.getInternalName(HelperClassInfo.class), - "getProtectionDomain", - "()Ljava/security/ProtectionDomain;", - true); - mv.visitMethodInsn( - INVOKEVIRTUAL, - internalClassName, - "defineClass", - "(Ljava/lang/String;[BIILjava/security/ProtectionDomain;)Ljava/lang/Class;", - false); - mv.visitLabel(endTry); - mv.visitInsn(ARETURN); - - mv.visitLabel(handler); + } + // byte[] bytes = helperClassInfo.getClassBytes(); + mv.visitVarInsn(ALOAD, argumentCount + 1); + mv.visitMethodInsn( + INVOKEINTERFACE, + Type.getInternalName(HelperClassInfo.class), + "getClassBytes", + "()[B", + true); + mv.visitVarInsn(ASTORE, argumentCount + 3); // store bytes + + // return defineClass(name, bytes, 0, bytes.length, + // helperClassInfo.getProtectionDomain()); + mv.visitVarInsn(ALOAD, 0); + mv.visitVarInsn(ALOAD, 1); + mv.visitVarInsn(ALOAD, argumentCount + 3); + mv.visitInsn(ICONST_0); + mv.visitVarInsn(ALOAD, argumentCount + 3); + mv.visitInsn(ARRAYLENGTH); + mv.visitVarInsn(ALOAD, argumentCount + 1); + mv.visitMethodInsn( + INVOKEINTERFACE, + Type.getInternalName(HelperClassInfo.class), + "getProtectionDomain", + "()Ljava/security/ProtectionDomain;", + true); + mv.visitMethodInsn( + INVOKEVIRTUAL, + internalClassName, + "defineClass", + "(Ljava/lang/String;[BIILjava/security/ProtectionDomain;)Ljava/lang/Class;", + false); + mv.visitLabel(endTry); + mv.visitInsn(ARETURN); + + mv.visitLabel(handler); + if (frames) { mv.visitFrame(Opcodes.F_SAME1, 0, null, 1, new Object[] {"java/lang/LinkageError"}); - mv.visitVarInsn(ASTORE, argumentCount + 3); // store LinkageError - // clazz = findLoadedClass(name); - mv.visitVarInsn(ALOAD, 0); - mv.visitVarInsn(ALOAD, 1); - mv.visitMethodInsn( - INVOKEVIRTUAL, - internalClassName, - "findLoadedClass", - "(Ljava/lang/String;)Ljava/lang/Class;", - false); - mv.visitVarInsn(ASTORE, argumentCount + 2); // score clazz - mv.visitVarInsn(ALOAD, argumentCount + 2); - Label throwError = new Label(); - mv.visitJumpInsn(IFNULL, throwError); - // return clazz - mv.visitVarInsn(ALOAD, argumentCount + 2); - mv.visitInsn(ARETURN); - mv.visitLabel(throwError); + } + mv.visitVarInsn(ASTORE, argumentCount + 3); // store LinkageError + // clazz = findLoadedClass(name); + mv.visitVarInsn(ALOAD, 0); + mv.visitVarInsn(ALOAD, 1); + mv.visitMethodInsn( + INVOKEVIRTUAL, + internalClassName, + "findLoadedClass", + "(Ljava/lang/String;)Ljava/lang/Class;", + false); + mv.visitVarInsn(ASTORE, argumentCount + 2); // score clazz + mv.visitVarInsn(ALOAD, argumentCount + 2); + Label throwError = new Label(); + mv.visitJumpInsn(IFNULL, throwError); + // return clazz + mv.visitVarInsn(ALOAD, argumentCount + 2); + mv.visitInsn(ARETURN); + mv.visitLabel(throwError); + if (frames) { mv.visitFrame(Opcodes.F_APPEND, 1, new Object[] {"java/lang/LinkageError"}, 0, null); - // throw error - mv.visitVarInsn(ALOAD, argumentCount + 3); - mv.visitInsn(ATHROW); + } + // throw error + mv.visitVarInsn(ALOAD, argumentCount + 3); + mv.visitInsn(ATHROW); - mv.visitLabel(notHelperClass); + mv.visitLabel(notHelperClass); + if (frames) { mv.visitFrame(Opcodes.F_CHOP, 3, null, 0, null); + // ensure there aren't two frames at the same location + mv.visitInsn(NOP); } + } - @Override - public void visitMaxs(int maxStack, int maxLocals) { - // minimally we have argumentCount parameters + this + 3 locals added by us - super.visitMaxs(maxStack, Math.max(maxLocals, argumentCount + 1 + 3)); - } - }; - } - return mv; + @Override + public void visitMaxs(int maxStack, int maxLocals) { + // minimally we have argumentCount parameters + this + 3 locals added by us + super.visitMaxs(maxStack, Math.max(maxLocals, argumentCount + 1 + 3)); + } + }; } } } From a611bf761a9472fac7dd9fa20d634adaff9ce904 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 3 Oct 2025 16:15:22 +0300 Subject: [PATCH 10/13] try anoter approach --- .../compile-stub/build.gradle.kts | 3 + .../classloader/stub/ClassLoader.java | 21 ++ .../build.gradle.kts | 2 +- .../javaagent/build.gradle.kts | 7 + .../ClassLoaderInstrumentationModule.java | 3 +- .../LoadInjectedClassInstrumentation.java | 287 ++++-------------- javaagent/build.gradle.kts | 2 +- settings.gradle.kts | 1 + 8 files changed, 98 insertions(+), 228 deletions(-) create mode 100644 instrumentation/internal/internal-class-loader/compile-stub/build.gradle.kts create mode 100644 instrumentation/internal/internal-class-loader/compile-stub/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/stub/ClassLoader.java diff --git a/instrumentation/internal/internal-class-loader/compile-stub/build.gradle.kts b/instrumentation/internal/internal-class-loader/compile-stub/build.gradle.kts new file mode 100644 index 000000000000..0a3932d24293 --- /dev/null +++ b/instrumentation/internal/internal-class-loader/compile-stub/build.gradle.kts @@ -0,0 +1,3 @@ +plugins { + id("otel.java-conventions") +} diff --git a/instrumentation/internal/internal-class-loader/compile-stub/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/stub/ClassLoader.java b/instrumentation/internal/internal-class-loader/compile-stub/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/stub/ClassLoader.java new file mode 100644 index 000000000000..1148799fab80 --- /dev/null +++ b/instrumentation/internal/internal-class-loader/compile-stub/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/stub/ClassLoader.java @@ -0,0 +1,21 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.internal.classloader.stub; + +import java.security.ProtectionDomain; + +/** + * A placeholder for java.lang.ClassLoader to allow compilation of advice classes that invoke + * protected methods of ClassLoader (like defineClass and findLoadedClass). During the build we'll + * use shadow plugin to replace reference to this class with the real java.lang.ClassLoader. + */ +@SuppressWarnings("JavaLangClash") +public abstract class ClassLoader { + public abstract Class findLoadedClass(String name); + + public abstract Class defineClass( + String name, byte[] b, int off, int len, ProtectionDomain protectionDomain); +} diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/build.gradle.kts b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/build.gradle.kts index 33fc4e2c6dbd..a71640842b62 100644 --- a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/build.gradle.kts +++ b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/build.gradle.kts @@ -6,5 +6,5 @@ dependencies { compileOnly("org.apache.commons:commons-lang3:3.12.0") testImplementation("org.apache.commons:commons-lang3:3.12.0") - testInstrumentation(project(":instrumentation:internal:internal-class-loader:javaagent")) + testInstrumentation(project(":instrumentation:internal:internal-class-loader:javaagent", configuration = "shadow")) } diff --git a/instrumentation/internal/internal-class-loader/javaagent/build.gradle.kts b/instrumentation/internal/internal-class-loader/javaagent/build.gradle.kts index 5520b9b8dc3a..5c2c97511926 100644 --- a/instrumentation/internal/internal-class-loader/javaagent/build.gradle.kts +++ b/instrumentation/internal/internal-class-loader/javaagent/build.gradle.kts @@ -5,6 +5,7 @@ plugins { dependencies { compileOnly(project(":javaagent-bootstrap")) compileOnly(project(":javaagent-tooling")) + compileOnly(project(":instrumentation:internal:internal-class-loader:compile-stub")) testImplementation(project(":javaagent-bootstrap")) @@ -21,3 +22,9 @@ dependencies { testImplementation("org.eclipse.platform:org.eclipse.osgi:3.13.200") testImplementation("org.apache.felix:org.apache.felix.framework:6.0.2") } + +tasks { + shadowJar { + relocate("io.opentelemetry.javaagent.instrumentation.internal.classloader.stub", "java.lang") + } +} diff --git a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java index d656d851298f..ffbda5b61949 100644 --- a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java +++ b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java @@ -35,9 +35,8 @@ public List injectedClassNames() { @Override public List typeInstrumentations() { return asList( - // added first so it would get applied after other instrumentations - new LoadInjectedClassInstrumentation(), new BootDelegationInstrumentation(), + new LoadInjectedClassInstrumentation(), new ResourceInjectionInstrumentation(), new DefineClassInstrumentation()); } diff --git a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java index 529461f21ea2..4cc4058f200a 100644 --- a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java +++ b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java @@ -6,28 +6,24 @@ package io.opentelemetry.javaagent.instrumentation.internal.classloader; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.extendsClass; +import static io.opentelemetry.javaagent.instrumentation.internal.classloader.AdviceUtil.applyInlineAdvice; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isProtected; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.isStatic; import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper; -import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper.HelperClassInfo; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import io.opentelemetry.javaagent.extension.instrumentation.internal.AsmApi; -import net.bytebuddy.ClassFileVersion; -import net.bytebuddy.asm.AsmVisitorWrapper; -import net.bytebuddy.description.field.FieldDescription; -import net.bytebuddy.description.field.FieldList; -import net.bytebuddy.description.method.MethodList; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.asm.Advice.AssignReturned; +import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.implementation.Implementation; import net.bytebuddy.matcher.ElementMatcher; -import net.bytebuddy.pool.TypePool; -import org.objectweb.asm.ClassVisitor; -import org.objectweb.asm.ClassWriter; -import org.objectweb.asm.Label; -import org.objectweb.asm.MethodVisitor; -import org.objectweb.asm.Opcodes; -import org.objectweb.asm.Type; /** * This instrumentation inserts loading of our injected helper classes at the start of {@code @@ -42,220 +38,63 @@ public ElementMatcher typeMatcher() { @Override public void transform(TypeTransformer transformer) { - transformer.applyTransformer( - (builder, typeDescription, classLoader, module, protectionDomain) -> - builder.visit( - new AsmVisitorWrapper() { - @Override - public int mergeWriter(int flags) { - return flags | ClassWriter.COMPUTE_MAXS; - } - - @Override - public int mergeReader(int flags) { - return flags; - } - - @Override - public ClassVisitor wrap( - TypeDescription instrumentedType, - ClassVisitor classVisitor, - Implementation.Context implementationContext, - TypePool typePool, - FieldList fields, - MethodList methods, - int writerFlags, - int readerFlags) { - return new ClassLoaderClassVisitor(classVisitor); - } - })); + ElementMatcher.Junction methodMatcher = + isMethod() + .and(named("loadClass")) + .and( + takesArguments(1) + .and(takesArgument(0, String.class)) + .or( + takesArguments(2) + .and(takesArgument(0, String.class)) + .and(takesArgument(1, boolean.class)))) + .and(isPublic().or(isProtected())) + .and(not(isStatic())); + // Inline instrumentation to prevent problems with invokedynamic-recursion + applyInlineAdvice(transformer, methodMatcher, this.getClass().getName() + "$LoadClassAdvice"); } - private static class ClassLoaderClassVisitor extends ClassVisitor implements Opcodes { - private String internalClassName; - private boolean frames; - - ClassLoaderClassVisitor(ClassVisitor classVisitor) { - super(AsmApi.VERSION, classVisitor); - } - - @Override - public void visit( - int version, - int access, - String name, - String signature, - String superName, - String[] interfaces) { - super.visit(version, access, name, signature, superName, interfaces); - internalClassName = name; - frames = ClassFileVersion.ofMinorMajor(version).isAtLeast(ClassFileVersion.JAVA_V6); - } - - @Override - public MethodVisitor visitMethod( - int access, String name, String descriptor, String signature, String[] exceptions) { - MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions); - boolean loadClassMethod = - "loadClass".equals(name) - && ("(Ljava/lang/String;)Ljava/lang/Class;".equals(descriptor) - || "(Ljava/lang/String;Z)Ljava/lang/Class;".equals(descriptor)); - if (!loadClassMethod) { - return mv; - } - - int argumentCount = Type.getArgumentTypes(descriptor).length; - return new MethodVisitor(api, mv) { - @Override - public void visitCode() { - super.visitCode(); - - // inserts the following at the start of the loadClass method: - /* - InjectedClassHelper.HelperClassInfo helperClassInfo = InjectedClassHelper.getHelperClassInfo(this, name); - if (helperClassInfo != null) { - Class clazz = findLoadedClass(name); - if (clazz != null) { - return clazz; - } - try { - byte[] bytes = helperClassInfo.getClassBytes(); - return defineClass(name, bytes, 0, bytes.length, helperClassInfo.getProtectionDomain()); - } catch (LinkageError error) { - clazz = findLoadedClass(name); - if (clazz != null) { - return clazz; - } - throw error; - } - } - */ - - Label startTry = new Label(); - Label endTry = new Label(); - Label handler = new Label(); - mv.visitTryCatchBlock(startTry, endTry, handler, "java/lang/LinkageError"); - // InjectedClassHelper.HelperClassInfo helperClassInfo = InjectedClassHelper - // .getHelperClassInfo(this, name); - mv.visitVarInsn(ALOAD, 0); - mv.visitVarInsn(ALOAD, 1); - mv.visitMethodInsn( - INVOKESTATIC, - Type.getInternalName(InjectedClassHelper.class), - "getHelperClassInfo", - "(Ljava/lang/ClassLoader;Ljava/lang/String;)" - + Type.getDescriptor(HelperClassInfo.class), - false); - mv.visitVarInsn(ASTORE, argumentCount + 1); // store helperClassInfo - mv.visitVarInsn(ALOAD, argumentCount + 1); - Label notHelperClass = new Label(); - mv.visitJumpInsn(IFNULL, notHelperClass); - - // getHelperClassInfo returned non-null - // Class clazz = findLoadedClass(name); - mv.visitVarInsn(ALOAD, 0); - mv.visitVarInsn(ALOAD, 1); - mv.visitMethodInsn( - INVOKEVIRTUAL, - internalClassName, - "findLoadedClass", - "(Ljava/lang/String;)Ljava/lang/Class;", - false); - mv.visitVarInsn(ASTORE, argumentCount + 2); // store clazz - mv.visitVarInsn(ALOAD, argumentCount + 2); - mv.visitJumpInsn(IFNULL, startTry); - - // findLoadedClass returned non-null - // return clazz - mv.visitVarInsn(ALOAD, argumentCount + 2); - mv.visitInsn(ARETURN); - - mv.visitLabel(startTry); - if (frames) { - mv.visitFrame( - Opcodes.F_APPEND, - 2, - new Object[] {Type.getInternalName(HelperClassInfo.class), "java/lang/Class"}, - 0, - null); - } - // byte[] bytes = helperClassInfo.getClassBytes(); - mv.visitVarInsn(ALOAD, argumentCount + 1); - mv.visitMethodInsn( - INVOKEINTERFACE, - Type.getInternalName(HelperClassInfo.class), - "getClassBytes", - "()[B", - true); - mv.visitVarInsn(ASTORE, argumentCount + 3); // store bytes - - // return defineClass(name, bytes, 0, bytes.length, - // helperClassInfo.getProtectionDomain()); - mv.visitVarInsn(ALOAD, 0); - mv.visitVarInsn(ALOAD, 1); - mv.visitVarInsn(ALOAD, argumentCount + 3); - mv.visitInsn(ICONST_0); - mv.visitVarInsn(ALOAD, argumentCount + 3); - mv.visitInsn(ARRAYLENGTH); - mv.visitVarInsn(ALOAD, argumentCount + 1); - mv.visitMethodInsn( - INVOKEINTERFACE, - Type.getInternalName(HelperClassInfo.class), - "getProtectionDomain", - "()Ljava/security/ProtectionDomain;", - true); - mv.visitMethodInsn( - INVOKEVIRTUAL, - internalClassName, - "defineClass", - "(Ljava/lang/String;[BIILjava/security/ProtectionDomain;)Ljava/lang/Class;", - false); - mv.visitLabel(endTry); - mv.visitInsn(ARETURN); - - mv.visitLabel(handler); - if (frames) { - mv.visitFrame(Opcodes.F_SAME1, 0, null, 1, new Object[] {"java/lang/LinkageError"}); - } - mv.visitVarInsn(ASTORE, argumentCount + 3); // store LinkageError - // clazz = findLoadedClass(name); - mv.visitVarInsn(ALOAD, 0); - mv.visitVarInsn(ALOAD, 1); - mv.visitMethodInsn( - INVOKEVIRTUAL, - internalClassName, - "findLoadedClass", - "(Ljava/lang/String;)Ljava/lang/Class;", - false); - mv.visitVarInsn(ASTORE, argumentCount + 2); // score clazz - mv.visitVarInsn(ALOAD, argumentCount + 2); - Label throwError = new Label(); - mv.visitJumpInsn(IFNULL, throwError); - // return clazz - mv.visitVarInsn(ALOAD, argumentCount + 2); - mv.visitInsn(ARETURN); - mv.visitLabel(throwError); - if (frames) { - mv.visitFrame(Opcodes.F_APPEND, 1, new Object[] {"java/lang/LinkageError"}, 0, null); - } - // throw error - mv.visitVarInsn(ALOAD, argumentCount + 3); - mv.visitInsn(ATHROW); - - mv.visitLabel(notHelperClass); - if (frames) { - mv.visitFrame(Opcodes.F_CHOP, 3, null, 0, null); - // ensure there aren't two frames at the same location - mv.visitInsn(NOP); + @SuppressWarnings("unused") + public static class LoadClassAdvice { + + // Class loader stub is shaded back to the real class loader class. It is used to call protected + // method from the advice that the complier won't let us call directly. During runtime it is + // fine since this code is inlined into subclasses of ClassLoader that can call protected + // methods. + @Advice.OnMethodEnter(skipOn = Advice.OnNonDefaultValue.class) + public static Class onEnter( + @Advice.This java.lang.ClassLoader classLoader, + @Advice.This + io.opentelemetry.javaagent.instrumentation.internal.classloader.stub.ClassLoader + classLoaderStub, + @Advice.Argument(0) String name) { + InjectedClassHelper.HelperClassInfo helperClassInfo = + InjectedClassHelper.getHelperClassInfo(classLoader, name); + if (helperClassInfo != null) { + Class clazz = classLoaderStub.findLoadedClass(name); + if (clazz != null) { + return clazz; + } + try { + byte[] bytes = helperClassInfo.getClassBytes(); + return classLoaderStub.defineClass( + name, bytes, 0, bytes.length, helperClassInfo.getProtectionDomain()); + } catch (LinkageError error) { + clazz = classLoaderStub.findLoadedClass(name); + if (clazz != null) { + return clazz; } + throw error; } + } + return null; + } - @Override - public void visitMaxs(int maxStack, int maxLocals) { - // minimally we have argumentCount parameters + this + 3 locals added by us - super.visitMaxs(maxStack, Math.max(maxLocals, argumentCount + 1 + 3)); - } - }; + @AssignReturned.ToReturned + @Advice.OnMethodExit(onThrowable = Throwable.class) + public static Class onExit( + @Advice.Return Class originalResult, @Advice.Enter Class loadedClass) { + return loadedClass != null ? loadedClass : originalResult; } } } diff --git a/javaagent/build.gradle.kts b/javaagent/build.gradle.kts index fb3ebe8cf449..04da29ca77d4 100644 --- a/javaagent/build.gradle.kts +++ b/javaagent/build.gradle.kts @@ -94,7 +94,7 @@ dependencies { baseJavaagentLibs(project(":instrumentation:opentelemetry-instrumentation-annotations-1.16:javaagent")) baseJavaagentLibs(project(":instrumentation:executors:javaagent")) baseJavaagentLibs(project(":instrumentation:internal:internal-application-logger:javaagent")) - baseJavaagentLibs(project(":instrumentation:internal:internal-class-loader:javaagent")) + baseJavaagentLibs(project(":instrumentation:internal:internal-class-loader:javaagent", configuration = "shadow")) baseJavaagentLibs(project(":instrumentation:internal:internal-eclipse-osgi-3.6:javaagent")) baseJavaagentLibs(project(":instrumentation:internal:internal-lambda:javaagent")) baseJavaagentLibs(project(":instrumentation:internal:internal-reflection:javaagent")) diff --git a/settings.gradle.kts b/settings.gradle.kts index 5fa2d77f50a7..7a460bc32b7c 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -264,6 +264,7 @@ include(":instrumentation:hystrix-1.4:javaagent") include(":instrumentation:influxdb-2.4:javaagent") include(":instrumentation:internal:internal-application-logger:bootstrap") include(":instrumentation:internal:internal-application-logger:javaagent") +include(":instrumentation:internal:internal-class-loader:compile-stub") include(":instrumentation:internal:internal-class-loader:javaagent") include(":instrumentation:internal:internal-class-loader:javaagent-integration-tests") include(":instrumentation:internal:internal-eclipse-osgi-3.6:javaagent") From ceb08b64aa52451be6bbd75c6b83e77a84e4fd65 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 3 Oct 2025 16:16:33 +0300 Subject: [PATCH 11/13] spelling --- .../internal/classloader/LoadInjectedClassInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java index 4cc4058f200a..be0718702b3b 100644 --- a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java +++ b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java @@ -58,7 +58,7 @@ public void transform(TypeTransformer transformer) { public static class LoadClassAdvice { // Class loader stub is shaded back to the real class loader class. It is used to call protected - // method from the advice that the complier won't let us call directly. During runtime it is + // method from the advice that the compiler won't let us call directly. During runtime it is // fine since this code is inlined into subclasses of ClassLoader that can call protected // methods. @Advice.OnMethodEnter(skipOn = Advice.OnNonDefaultValue.class) From bd7c14e2eeef078cafb05fb6c2ba810e7ac9cd3b Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 3 Oct 2025 17:27:57 +0300 Subject: [PATCH 12/13] hopefully fix shading --- .../build.gradle.kts | 2 +- .../javaagent/build.gradle.kts | 27 ++++++++++++++++++- javaagent/build.gradle.kts | 8 ++++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/build.gradle.kts b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/build.gradle.kts index a71640842b62..1a94bc8faf0d 100644 --- a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/build.gradle.kts +++ b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/build.gradle.kts @@ -6,5 +6,5 @@ dependencies { compileOnly("org.apache.commons:commons-lang3:3.12.0") testImplementation("org.apache.commons:commons-lang3:3.12.0") - testInstrumentation(project(":instrumentation:internal:internal-class-loader:javaagent", configuration = "shadow")) + testInstrumentation(project(":instrumentation:internal:internal-class-loader:javaagent", configuration = "shaded")) } diff --git a/instrumentation/internal/internal-class-loader/javaagent/build.gradle.kts b/instrumentation/internal/internal-class-loader/javaagent/build.gradle.kts index 5c2c97511926..bb56f554cf60 100644 --- a/instrumentation/internal/internal-class-loader/javaagent/build.gradle.kts +++ b/instrumentation/internal/internal-class-loader/javaagent/build.gradle.kts @@ -1,3 +1,5 @@ +import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar + plugins { id("otel.javaagent-instrumentation") } @@ -23,8 +25,31 @@ dependencies { testImplementation("org.apache.felix:org.apache.felix.framework:6.0.2") } +val shadedJar by tasks.registering(ShadowJar::class) { + from(zipTree(tasks.jar.get().archiveFile)) + archiveClassifier.set("shaded") +} + tasks { - shadowJar { + withType(ShadowJar::class) { relocate("io.opentelemetry.javaagent.instrumentation.internal.classloader.stub", "java.lang") } + + assemble { + dependsOn(shadedJar) + } +} + +// Create a consumable configuration for the shaded jar. We can't use the "shadow" configuration +// because that is taken by the agent-testing.jar +configurations { + consumable("shaded") { + attributes { + attribute(LibraryElements.LIBRARY_ELEMENTS_ATTRIBUTE, objects.named("shaded")) + } + } +} + +artifacts { + add("shaded", shadedJar) } diff --git a/javaagent/build.gradle.kts b/javaagent/build.gradle.kts index 04da29ca77d4..f7631c36673a 100644 --- a/javaagent/build.gradle.kts +++ b/javaagent/build.gradle.kts @@ -94,7 +94,7 @@ dependencies { baseJavaagentLibs(project(":instrumentation:opentelemetry-instrumentation-annotations-1.16:javaagent")) baseJavaagentLibs(project(":instrumentation:executors:javaagent")) baseJavaagentLibs(project(":instrumentation:internal:internal-application-logger:javaagent")) - baseJavaagentLibs(project(":instrumentation:internal:internal-class-loader:javaagent", configuration = "shadow")) + baseJavaagentLibs(project(":instrumentation:internal:internal-class-loader:javaagent", configuration = "shaded")) baseJavaagentLibs(project(":instrumentation:internal:internal-eclipse-osgi-3.6:javaagent")) baseJavaagentLibs(project(":instrumentation:internal:internal-lambda:javaagent")) baseJavaagentLibs(project(":instrumentation:internal:internal-reflection:javaagent")) @@ -126,7 +126,11 @@ project(":instrumentation").subprojects { plugins.withId("otel.javaagent-instrumentation") { javaagentDependencies.run { - add(javaagentLibs.name, project(subProj.path)) + // exclude :instrumentation:internal:internal-class-loader:javaagent we added the shaded + // configuration from it to baseJavaagentLibs + if (!subProj.path.contains("internal-class-loader")) { + add(javaagentLibs.name, project(subProj.path)) + } } } From 324c20bdfd1a78913c0219b231130884121d28bf Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Mon, 20 Oct 2025 17:23:49 +0300 Subject: [PATCH 13/13] address review comment --- .../javaagent/tooling/AgentInstaller.java | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java index 5d5ff93bf8c4..e45fae651526 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java @@ -131,18 +131,8 @@ private static void installBytebuddyAgent( // https://bugs.openjdk.org/browse/JDK-8164165 ThreadLocalRandom.current(); - // AgentBuilder.Default constructor triggers sun.misc.Unsafe::objectFieldOffset called warning - // AgentBuilder$Default. - // -> NexusAccessor. - // -> ClassInjector$UsingReflection. - // -> ClassInjector$UsingUnsafe. - // -> WARNING: sun.misc.Unsafe::objectFieldOffset called by - // net.bytebuddy.dynamic.loading.ClassInjector$UsingUnsafe$Dispatcher$CreationAction - // we don't use byte-buddy nexus so we disable it and later restore the original value for the - // system property - String originalNexusDisabled = System.setProperty("net.bytebuddy.nexus.disabled", "true"); AgentBuilder agentBuilder = - new AgentBuilder.Default( + newAgentBuilder( // default method graph compiler inspects the class hierarchy, we don't need it, so // we use a simpler and faster strategy instead new ByteBuddy() @@ -157,12 +147,6 @@ private static void installBytebuddyAgent( .with(AgentTooling.poolStrategy()) .with(AgentTooling.transformListener()) .with(AgentTooling.locationStrategy()); - // restore the original value for the nexus disabled property - if (originalNexusDisabled != null) { - System.setProperty("net.bytebuddy.nexus.disabled", originalNexusDisabled); - } else { - System.clearProperty("net.bytebuddy.nexus.disabled"); - } if (JavaModule.isSupported()) { agentBuilder = agentBuilder.with(new ExposeAgentBootstrapListener(inst)); @@ -242,6 +226,29 @@ private static void installBytebuddyAgent( runAfterAgentListeners(agentListeners, autoConfiguredSdk, sdkConfig); } + private static AgentBuilder newAgentBuilder(ByteBuddy byteBuddy) { + // AgentBuilder.Default constructor triggers sun.misc.Unsafe::objectFieldOffset called warning + // AgentBuilder$Default. + // -> NexusAccessor. + // -> ClassInjector$UsingReflection. + // -> ClassInjector$UsingUnsafe. + // -> WARNING: sun.misc.Unsafe::objectFieldOffset called by + // net.bytebuddy.dynamic.loading.ClassInjector$UsingUnsafe$Dispatcher$CreationAction + // we don't use byte-buddy nexus so we disable it and later restore the original value for the + // system property + String originalNexusDisabled = System.setProperty("net.bytebuddy.nexus.disabled", "true"); + try { + return new AgentBuilder.Default(byteBuddy); + } finally { + // restore the original value for the nexus disabled property + if (originalNexusDisabled != null) { + System.setProperty("net.bytebuddy.nexus.disabled", originalNexusDisabled); + } else { + System.clearProperty("net.bytebuddy.nexus.disabled"); + } + } + } + private static void installEarlyInstrumentation( AgentBuilder agentBuilder, Instrumentation instrumentation) { // We are only going to install the virtual fields here. Installing virtual field changes class