Skip to content

Delete ClassInjector api #18112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
trask merged 4 commits into from
Apr 22, 2026
Merged

Delete ClassInjector api #18112

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dd7fb83
Delete ClassInjector api
laurit Apr 21, 2026
4b76fdc
add back
laurit Apr 21, 2026
d42e6e9
update servialver
laurit Apr 21, 2026
a745fd0
address review comments
laurit Apr 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions ...reflection/javaagent-integration-tests/src/main/java/instrumentation/TestHelperClass.java
View file
Open in desktop

This file was deleted.

21 changes: 2 additions & 19 deletions .../javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,16 @@

package instrumentation;

import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;

import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.ClassInjector;
import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode;
import java.util.List;

@AutoService(InstrumentationModule.class)
public class TestInstrumentationModule extends InstrumentationModule
implements ExperimentalInstrumentationModule {
public class TestInstrumentationModule extends InstrumentationModule {

public TestInstrumentationModule() {
super("test-instrumentation");
}
Expand All @@ -28,21 +24,8 @@ public List<TypeInstrumentation> typeInstrumentations() {
return singletonList(new TestTypeInstrumentation());
}

@Override
public List<String> getAdditionalHelperClassNames() {
// makes the class from instrumentation from the instrumented class with inlined advice
return asList("instrumentation.TestHelperClass");
}

@Override
public boolean isHelperClass(String className) {
return "instrumentation.TestSingletons".equals(className);
}

@Override
public void injectClasses(ClassInjector injector) {
injector
.proxyBuilder("instrumentation.TestHelperClass")
.inject(InjectionMode.CLASS_AND_RESOURCE);
}
}
36 changes: 1 addition & 35 deletions ...t/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@

import static org.assertj.core.api.Assertions.assertThat;

import instrumentation.TestHelperClass;
import io.opentelemetry.javaagent.bootstrap.InstrumentationProxy;
import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldAccessorMarker;
import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldInstalledMarker;
import java.io.ObjectStreamClass;
Expand Down Expand Up @@ -46,39 +44,7 @@ void testOurFieldsAndMethodsAreNotVisibleWithReflection() {
void testGeneratedSerialVersionUid() {
// expected value is computed with serialver utility that comes with jdk
assertThat(ObjectStreamClass.lookup(TestClass.class).getSerialVersionUID())
.isEqualTo(1115158932483123524L);
.isEqualTo(-1006206785953990857L);
assertThat(TestClass.class.getDeclaredFields()).isEmpty();
}

@Test
void testInjectedClassProxyUnwrap() throws Exception {
TestClass testClass = new TestClass();
Class<?> helperType = testClass.testHelperClass();
assertThat(helperType).isNotNull();

Object instance = helperType.getConstructor().newInstance();
if (InstrumentationProxy.class.isAssignableFrom(helperType)) {
// indy advice: must be an indy proxy

for (Method method : helperType.getMethods()) {
assertThat(method.getName()).isNotEqualTo("__getIndyProxyDelegate");
}

for (Class<?> interfaceType : helperType.getInterfaces()) {
assertThat(interfaceType).isNotEqualTo(InstrumentationProxy.class);
}

assertThat(instance).isInstanceOf(InstrumentationProxy.class);

Object proxyDelegate = ((InstrumentationProxy) instance).__getIndyProxyDelegate();
assertThat(proxyDelegate).isNotInstanceOf(InstrumentationProxy.class);

} else {
// inline advice: must be of the expected type
assertThat(helperType).isEqualTo(TestHelperClass.class);
assertThat(instance)
.isInstanceOf(TestHelperClass.class)
.isNotInstanceOf(InstrumentationProxy.class);
}
}
}
9 changes: 0 additions & 9 deletions ...c/test/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/TestClass.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,4 @@ public String testMethod() {
public String testMethod2() {
return "not instrumented";
}

public Class<?> testHelperClass() {
try {
return Class.forName(
"instrumentation.TestHelperClass", false, TestClass.class.getClassLoader());
} catch (ClassNotFoundException e) {
return null;
}
}
}
30 changes: 22 additions & 8 deletions .../io/opentelemetry/javaagent/instrumentation/internal/reflection/ClassInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import io.opentelemetry.javaagent.extension.instrumentation.internal.AsmApi;
import io.opentelemetry.javaagent.tooling.instrumentation.indy.IndyBootstrap;
import io.opentelemetry.javaagent.tooling.instrumentation.indy.IndyTypeTransformerImpl;
import net.bytebuddy.asm.AsmVisitorWrapper;
import net.bytebuddy.description.field.FieldDescription;
import net.bytebuddy.description.field.FieldList;
Expand Down Expand Up @@ -58,15 +60,18 @@ public ClassVisitor wrap(
MethodList<?> methods,
int writerFlags,
int readerFlags) {
return new ClassClassVisitor(classVisitor);
return new ClassClassVisitor(
classVisitor, transformer instanceof IndyTypeTransformerImpl);
}
}));
}

private static class ClassClassVisitor extends ClassVisitor {
private final boolean isIndy;

ClassClassVisitor(ClassVisitor cv) {
ClassClassVisitor(ClassVisitor cv, boolean isIndy) {
super(AsmApi.VERSION, cv);
this.isIndy = isIndy;
}

@Override
Expand All @@ -92,12 +97,21 @@ public void visitMethodInsn(
&& "java/lang/J9VMInternals".equals(owner)
&& "(Ljava/lang/Class;)[Ljava/lang/Class;".equals(descriptor))) {
super.visitVarInsn(Opcodes.ALOAD, 0);
super.visitMethodInsn(
Opcodes.INVOKESTATIC,
Type.getInternalName(ReflectionHelper.class),
"filterInterfaces",
"([Ljava/lang/Class;Ljava/lang/Class;)[Ljava/lang/Class;",
false);
if (isIndy) {
IndyBootstrap.invokeStatic(
Copy link
Copy Markdown
Contributor

@SylvainJuge SylvainJuge Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With indy, this will invoke the ReflectionHelper.filterInterfaces method using invokedynamic. If I'm understanding this correctly we have to do this because the ReflectionInstrumentationModule is loaded in its own classloader and is thus not directly accessible to the instrumented class which is in the bootstrap CL. It could be worth adding some javadoc/comments to clarify this for future reference.

Copy link
Copy Markdown
Contributor Author

@laurit laurit Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your understanding is correct. Added a comment. In boot loader we can't use exposedClassNames to make the helper class accessible. An alternative would be to use inline advice in this module.

mv,
"filterInterfaces",
"([Ljava/lang/Class;Ljava/lang/Class;)[Ljava/lang/Class;",
ReflectionInstrumentationModule.class,
ReflectionHelper.class.getName());
} else {
super.visitMethodInsn(
Opcodes.INVOKESTATIC,
Type.getInternalName(ReflectionHelper.class),
"filterInterfaces",
"([Ljava/lang/Class;Ljava/lang/Class;)[Ljava/lang/Class;",
false);
}
}
}
};
Expand Down
6 changes: 1 addition & 5 deletions ...java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

package io.opentelemetry.javaagent.instrumentation.internal.reflection;

import io.opentelemetry.javaagent.bootstrap.InstrumentationProxy;
import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldAccessorMarker;
import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldDetector;
import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldInstalledMarker;
Expand Down Expand Up @@ -70,8 +69,6 @@ public static Class<?>[] filterInterfaces(Class<?>[] interfaces, Class<?> contai
// filter out virtual field marker and accessor interfaces
if (interfaceClass == VirtualFieldInstalledMarker.class) {
continue;
} else if (interfaceClass == InstrumentationProxy.class) {
continue;
} else if (VirtualFieldAccessorMarker.class.isAssignableFrom(interfaceClass)
&& interfaceClass.isSynthetic()
&& interfaceClass.getName().contains("VirtualFieldAccessor$")) {
Expand All @@ -87,7 +84,6 @@ public static Class<?>[] filterInterfaces(Class<?>[] interfaces, Class<?> contai
}

private static boolean noInterfaceToHide(Class<?> containingClass) {
return !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass)
&& !InstrumentationProxy.class.isAssignableFrom(containingClass);
return !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass);
}
}
17 changes: 1 addition & 16 deletions ...emetry/javaagent/instrumentation/internal/reflection/ReflectionInstrumentationModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,10 @@
import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.ClassInjector;
import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode;
import java.util.List;

@AutoService(InstrumentationModule.class)
public class ReflectionInstrumentationModule extends InstrumentationModule
implements ExperimentalInstrumentationModule {
public class ReflectionInstrumentationModule extends InstrumentationModule {
public ReflectionInstrumentationModule() {
super("internal-reflection");
}
Expand All @@ -32,15 +28,4 @@ public boolean defaultEnabled() {
public List<TypeInstrumentation> typeInstrumentations() {
return asList(new ClassInstrumentation(), new ReflectionInstrumentation());
}

@Override
public void injectClasses(ClassInjector injector) {
// we do not use ByteBuddy Advice dispatching in this instrumentation
// Instead, we manually call ReflectionHelper via ASM
// Easiest solution to work with indy is to inject an indy-proxy to be invoked
injector
.proxyBuilder(
"io.opentelemetry.javaagent.instrumentation.internal.reflection.ReflectionHelper")
.inject(InjectionMode.CLASS_ONLY);
}
}
20 changes: 0 additions & 20 deletions ...nt-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxy.java
View file
Open in desktop

This file was deleted.

36 changes: 0 additions & 36 deletions ...tstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxyHelper.java
View file
Open in desktop

This file was deleted.

38 changes: 0 additions & 38 deletions ...ap/src/test/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxyHelperTest.java
View file
Open in desktop

This file was deleted.

13 changes: 0 additions & 13 deletions ...metry/javaagent/extension/instrumentation/internal/ExperimentalInstrumentationModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import static java.util.Collections.emptyMap;

import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.ClassInjector;
import java.util.List;
import java.util.Map;
import net.bytebuddy.utility.JavaModule;
Expand All @@ -20,18 +19,6 @@
*/
public interface ExperimentalInstrumentationModule {

/**
* Only functional for Modules where {@link InstrumentationModule#isIndyModule()} returns {@code
* true}.
*
* <p>Normally, helper and advice classes are loaded in a child classloader of the instrumented
* classloader. This method allows to inject classes directly into the instrumented classloader
* instead.
*
* @param injector the builder for injecting classes
*/
default void injectClasses(ClassInjector injector) {}

/**
* Returns a list of helper classes that will be defined in the class loader of the instrumented
* library.
Expand Down
Loading
Loading