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/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..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")) + 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 5520b9b8dc3a..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") } @@ -5,6 +7,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 +24,32 @@ dependencies { testImplementation("org.eclipse.platform:org.eclipse.osgi:3.13.200") 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 { + 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/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..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 @@ -57,10 +57,37 @@ public void transform(TypeTransformer transformer) { @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 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) public static Class onEnter( - @Advice.This ClassLoader classLoader, @Advice.Argument(0) String name) { - return InjectedClassHelper.loadHelperClass(classLoader, name); + @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; } @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..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,6 +5,7 @@ package io.opentelemetry.javaagent.bootstrap; +import java.security.ProtectionDomain; import java.util.function.BiFunction; import java.util.function.BiPredicate; import java.util.function.Function; @@ -37,21 +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 Class loadHelperClass(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 HelperClassInfo { + byte[] getClassBytes(); + + ProtectionDomain getProtectionDomain(); } } 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/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..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 @@ -5,7 +5,8 @@ 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 @@ -25,6 +26,37 @@ 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) + } + + @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) + } + } + def "helpers injected to non-delegating classloader"() { setup: URL[] helpersSourceUrls = new URL[1] @@ -33,7 +65,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 +74,6 @@ class HelperInjectionTest extends Specification { when: injector.transform(null, null, emptyLoader.get(), null, null) - HelperInjector.loadHelperClass(emptyLoader.get(), helperClassName) emptyLoader.get().loadClass(helperClassName) then: isClassLoaded(helperClassName, emptyLoader.get()) diff --git a/javaagent/build.gradle.kts b/javaagent/build.gradle.kts index fb3ebe8cf449..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")) + 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)) + } } } 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..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,6 +12,9 @@ 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.HelperClassInfo; +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; @@ -19,11 +22,11 @@ import java.lang.instrument.Instrumentation; 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; @@ -64,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. @@ -76,16 +79,9 @@ public String toString() { } }; - private static final HelperClassInjector BOOT_CLASS_INJECTOR = - new HelperClassInjector(null) { - @Override - Class inject(ClassLoader classLoader, String className) { - 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; @@ -274,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); } @@ -311,16 +305,54 @@ private static Map resolve(Map> classes return result; } - private Map> injectBootstrapClassLoader(Map> inject) - throws IOException { + private static final String virtualFieldPackage = + VirtualFieldAccessorMarker.class.getPackage().getName() + "."; + + 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()) { + // 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(); + 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 +360,16 @@ private Map> injectBootstrapClassLoader(Map 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); + return helperMap.containsKey(className); } - public static Class 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 helperClassInjector.inject(classLoader, className); + + return new HelperClassInfo() { + @Override + public byte[] getClassBytes() { + return helperClass.bytes.get(); + } + + @Override + public ProtectionDomain getProtectionDomain() { + return PROTECTION_DOMAIN; + } + }; } - 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) { - // 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(); - } } } 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/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") 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; }