Skip to content

make internal indy-ready #14646

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
merged 12 commits into from
Sep 24, 2025
Merged

make internal indy-ready #14646

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c775072
internal-class-loader
SylvainJuge Sep 12, 2025
72edf42
internal-eclipse-osgi-3.6
SylvainJuge Sep 12, 2025
a8cd4f7
internal-reflection
SylvainJuge Sep 12, 2025
6ac9fb4
internal-url-class-loader
SylvainJuge Sep 12, 2025
fd0069c
revert changes for integration tests
SylvainJuge Sep 12, 2025
7b443bd
keep changes to the advice migration
SylvainJuge Sep 12, 2025
4748664
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Sep 12, 2025
31d3ac6
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Sep 18, 2025
8835bc3
add comment forced inline instrumentation
SylvainJuge Sep 22, 2025
7c21b1c
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Sep 22, 2025
a8bcf29
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Sep 24, 2025
182cb53
post-review changes
SylvainJuge Sep 24, 2025
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
9 changes: 6 additions & 3 deletions ...lemetry/javaagent/instrumentation/internal/classloader/BootDelegationInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
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.matcher.ElementMatcher;
Expand Down Expand Up @@ -99,13 +100,15 @@ public static Class<?> onEnter(@Advice.Argument(0) String name) {
return null;
}

@AssignReturned.ToReturned
Copy link
Contributor

Choose a reason for hiding this comment

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

Both the BootDelegationInstrumentation and LoadInjectedClassInstrumentation enforce inlining, even if indy is enabled. This is done in the transform method above. We do this, to avoid recursion problems with the indy bootstrap.

For this reason they should not be converted to the @AssignReturned notation.

Interesting that your change doesn't cause a test failure though, I would have expected the @AssignReturned annotations to not work, but apparently they do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I remember, @AssignReturned usage is completely independent of inlining the code or not, so this is more about code style and consistency here.

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void onExit(
@Advice.Return(readOnly = false) Class<?> result,
@Advice.Enter Class<?> resultFromBootstrapLoader) {
public static Class<?> onExit(
@Advice.Return Class<?> originalResult, @Advice.Enter Class<?> resultFromBootstrapLoader) {
Class<?> result = originalResult;
if (resultFromBootstrapLoader != null) {
result = resultFromBootstrapLoader;
}
return result;
}
}
}
7 changes: 7 additions & 0 deletions ...etry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,11 @@ public List<TypeInstrumentation> typeInstrumentations() {
new ResourceInjectionInstrumentation(),
new DefineClassInstrumentation());
}

@Override
public boolean isIndyReady() {
// This module uses inlined advices to prevent recursion issues with invokedynamic, which is
// forced by using 'applyInlineAdvice' in 'transform' method of instrumentations.
return true;
}
}
9 changes: 6 additions & 3 deletions ...etry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
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.matcher.ElementMatcher;
Expand Down Expand Up @@ -63,16 +64,18 @@ public static Class<?> onEnter(
if (helperClass != null) {
return helperClass;
}

return null;
}

@AssignReturned.ToReturned
@Advice.OnMethodExit(onThrowable = Throwable.class)
public static void onExit(
@Advice.Return(readOnly = false) Class<?> result, @Advice.Enter Class<?> loadedClass) {
public static Class<?> onExit(
@Advice.Return Class<?> originalResult, @Advice.Enter Class<?> loadedClass) {
Class<?> result = originalResult;
if (loadedClass != null) {
result = loadedClass;
}
return result;
}
}
}
9 changes: 6 additions & 3 deletions .../io/opentelemetry/javaagent/instrumentation/internal/osgi/EclipseOsgiInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.asm.Advice.AssignReturned;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;

Expand Down Expand Up @@ -52,13 +53,15 @@ public static boolean onEnter(@Advice.Argument(0) String packageName) {
return InClassLoaderMatcher.get() && !packageName.startsWith("io.opentelemetry.");
}

@AssignReturned.ToReturned
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void onExit(
@Advice.Return(readOnly = false) boolean result,
@Advice.Enter boolean inClassLoaderMatcher) {
public static boolean onExit(
@Advice.Return boolean originalResult, @Advice.Enter boolean inClassLoaderMatcher) {
boolean result = originalResult;
if (inClassLoaderMatcher) {
result = false;
}
return result;
}
}
}
9 changes: 8 additions & 1 deletion ...entelemetry/javaagent/instrumentation/internal/osgi/EclipseOsgiInstrumentationModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@
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.sdk.autoconfigure.spi.ConfigProperties;
import java.util.List;

@AutoService(InstrumentationModule.class)
public class EclipseOsgiInstrumentationModule extends InstrumentationModule {
public class EclipseOsgiInstrumentationModule extends InstrumentationModule
implements ExperimentalInstrumentationModule {
public EclipseOsgiInstrumentationModule() {
super("internal-eclipse-osgi", "internal-eclipse-osgi-3.6");
}
Expand All @@ -29,4 +31,9 @@ public boolean defaultEnabled(ConfigProperties config) {
public List<TypeInstrumentation> typeInstrumentations() {
return singletonList(new EclipseOsgiInstrumentation());
}

@Override
public boolean isIndyReady() {
return true;
}
}
13 changes: 7 additions & 6 deletions ...on/javaagent-integration-tests/src/main/java/instrumentation/TestTypeInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.asm.Advice.AssignReturned;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;

Expand All @@ -32,21 +33,21 @@ public void transform(TypeTransformer transformer) {
@SuppressWarnings("unused")
public static class TestAdvice {

@AssignReturned.ToReturned
@Advice.OnMethodExit
public static void methodExit(
@Advice.This Runnable test, @Advice.Return(readOnly = false) String result) {
public static String methodExit(@Advice.This Runnable test) {
VirtualField.find(Runnable.class, String.class).set(test, "instrumented");
result = "instrumented";
return "instrumented";
}
}

@SuppressWarnings("unused")
public static class Test2Advice {

@AssignReturned.ToReturned
@Advice.OnMethodExit
public static void methodExit(
@Advice.This Runnable test, @Advice.Return(readOnly = false) String result) {
result = VirtualField.find(Runnable.class, String.class).get(test);
public static String methodExit(@Advice.This Runnable test) {
return VirtualField.find(Runnable.class, String.class).get(test);
}
}
}
9 changes: 8 additions & 1 deletion .../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 @@ -10,10 +10,12 @@
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 java.util.List;

@AutoService(InstrumentationModule.class)
public class TestInstrumentationModule extends InstrumentationModule {
public class TestInstrumentationModule extends InstrumentationModule
implements ExperimentalInstrumentationModule {
public TestInstrumentationModule() {
super("test-instrumentation");
}
Expand All @@ -22,4 +24,9 @@ public TestInstrumentationModule() {
public List<TypeInstrumentation> typeInstrumentations() {
return singletonList(new TestTypeInstrumentation());
}

@Override
public boolean isIndyReady() {
return true;
}
}
6 changes: 4 additions & 2 deletions ...er/javaagent-integration-tests/src/main/java/instrumentation/TestTypeInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.asm.Advice.AssignReturned;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;

Expand All @@ -34,9 +35,10 @@ public void transform(TypeTransformer transformer) {
@SuppressWarnings("unused")
public static class GetHostNameAdvice {

@AssignReturned.ToReturned
@Advice.OnMethodExit
public static void methodExit(@Advice.Return(readOnly = false) String hostName) {
hostName = "not-the-host-name";
public static String methodExit() {
return "not-the-host-name";
}
}
}