From 1d8900b0d0f149e08e0924e5f70a7b96229d4c77 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 24 Oct 2025 14:19:07 +0200 Subject: [PATCH 1/6] javaagent-tooling --- .../javaagent/tooling/bytebuddy/BadAdvice.java | 5 +++-- .../javaagent/tooling/bytebuddy/TestAdvice.java | 6 ++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/javaagent-tooling/src/testExceptionHandler/java/io/opentelemetry/javaagent/tooling/bytebuddy/BadAdvice.java b/javaagent-tooling/src/testExceptionHandler/java/io/opentelemetry/javaagent/tooling/bytebuddy/BadAdvice.java index 3bc0d08f3c6b..c2e68daaf792 100644 --- a/javaagent-tooling/src/testExceptionHandler/java/io/opentelemetry/javaagent/tooling/bytebuddy/BadAdvice.java +++ b/javaagent-tooling/src/testExceptionHandler/java/io/opentelemetry/javaagent/tooling/bytebuddy/BadAdvice.java @@ -6,13 +6,14 @@ package io.opentelemetry.javaagent.tooling.bytebuddy; import net.bytebuddy.asm.Advice; +import net.bytebuddy.asm.Advice.AssignReturned; @SuppressWarnings("unused") public class BadAdvice { + @AssignReturned.ToReturned @Advice.OnMethodExit(suppress = Throwable.class) - public static void throwAnException(@Advice.Return(readOnly = false) boolean returnVal) { - returnVal = true; + public static boolean throwAnException(@Advice.Return boolean originalReturnVal) { throw new IllegalStateException("Test Exception"); } diff --git a/javaagent-tooling/src/testMissingType/java/io/opentelemetry/javaagent/tooling/bytebuddy/TestAdvice.java b/javaagent-tooling/src/testMissingType/java/io/opentelemetry/javaagent/tooling/bytebuddy/TestAdvice.java index 892f7d46efdf..fe7cc9b5d491 100644 --- a/javaagent-tooling/src/testMissingType/java/io/opentelemetry/javaagent/tooling/bytebuddy/TestAdvice.java +++ b/javaagent-tooling/src/testMissingType/java/io/opentelemetry/javaagent/tooling/bytebuddy/TestAdvice.java @@ -6,12 +6,14 @@ package io.opentelemetry.javaagent.tooling.bytebuddy; import net.bytebuddy.asm.Advice; +import net.bytebuddy.asm.Advice.AssignReturned; @SuppressWarnings("unused") public class TestAdvice { + @AssignReturned.ToReturned @Advice.OnMethodExit - public static void returnTrue(@Advice.Return(readOnly = false) boolean returnVal) { - returnVal = true; + public static boolean returnTrue() { + return true; } } From 6c4c934380732443f9ddadf6a7ee314fc9d08cee Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 24 Oct 2025 14:19:46 +0200 Subject: [PATCH 2/6] testing-common --- .../context/ContextTestInstrumentation.java | 54 ++++++++----------- .../ContextTestInstrumentationModule.java | 9 +++- ...VirtualFieldTestInstrumentationModule.java | 6 ++- .../DuplicateHelperInstrumentation.java | 6 ++- 4 files changed, 37 insertions(+), 38 deletions(-) diff --git a/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentation.java b/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentation.java index b733e5102dc5..14e26e8de4d3 100644 --- a/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentation.java +++ b/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentation.java @@ -5,15 +5,17 @@ package context; +import static context.ContextTestSingletons.CONTEXT; +import static context.ContextTestSingletons.INTEGER; +import static context.ContextTestSingletons.INTERFACE_INTEGER; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; import static net.bytebuddy.matcher.ElementMatchers.named; -import io.opentelemetry.instrumentation.api.util.VirtualField; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import library.KeyClass; -import library.KeyInterface; import net.bytebuddy.asm.Advice; +import net.bytebuddy.asm.Advice.AssignReturned; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -42,40 +44,35 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class MarkInstrumentedAdvice { + @AssignReturned.ToReturned @Advice.OnMethodExit - public static void methodExit(@Advice.Return(readOnly = false) boolean isInstrumented) { - isInstrumented = true; + public static boolean methodExit() { + return true; } } @SuppressWarnings("unused") public static class StoreAndIncrementApiUsageAdvice { + @AssignReturned.ToReturned @Advice.OnMethodExit - public static void methodExit( - @Advice.This KeyClass thiz, @Advice.Return(readOnly = false) int contextCount) { - - VirtualField virtualField = - VirtualField.find(KeyClass.class, Context.class); - - Context context = virtualField.get(thiz); + public static int methodExit(@Advice.This KeyClass thiz) { + Context context = CONTEXT.get(thiz); if (context == null) { context = new Context(); - virtualField.set(thiz, context); + CONTEXT.set(thiz, context); } - contextCount = ++context.count; + return ++context.count; } } @SuppressWarnings("unused") public static class GetApiUsageAdvice { + @AssignReturned.ToReturned @Advice.OnMethodExit - public static void methodExit( - @Advice.This KeyClass thiz, @Advice.Return(readOnly = false) int contextCount) { - VirtualField virtualField = - VirtualField.find(KeyClass.class, Context.class); - Context context = virtualField.get(thiz); - contextCount = context == null ? 0 : context.count; + public static int methodExit(@Advice.This KeyClass thiz) { + Context context = CONTEXT.get(thiz); + return context == null ? 0 : context.count; } } @@ -83,11 +80,9 @@ public static void methodExit( public static class PutApiUsageAdvice { @Advice.OnMethodExit public static void methodExit(@Advice.This KeyClass thiz, @Advice.Argument(0) int value) { - VirtualField virtualField = - VirtualField.find(KeyClass.class, Context.class); Context context = new Context(); context.count = value; - virtualField.set(thiz, context); + CONTEXT.set(thiz, context); } } @@ -95,9 +90,7 @@ public static void methodExit(@Advice.This KeyClass thiz, @Advice.Argument(0) in public static class RemoveApiUsageAdvice { @Advice.OnMethodExit public static void methodExit(@Advice.This KeyClass thiz) { - VirtualField virtualField = - VirtualField.find(KeyClass.class, Context.class); - virtualField.set(thiz, null); + CONTEXT.set(thiz, null); } } @@ -105,15 +98,10 @@ public static void methodExit(@Advice.This KeyClass thiz) { public static class UseMultipleFieldsAdvice { @Advice.OnMethodExit public static void methodExit(@Advice.This KeyClass thiz) { - VirtualField field1 = VirtualField.find(KeyClass.class, Context.class); - VirtualField field2 = VirtualField.find(KeyClass.class, Integer.class); - VirtualField interfaceField = - VirtualField.find(KeyInterface.class, Integer.class); - - Context context = field1.get(thiz); + Context context = CONTEXT.get(thiz); int count = context == null ? 0 : context.count; - field2.set(thiz, count); - interfaceField.set(thiz, count); + INTEGER.set(thiz, count); + INTERFACE_INTEGER.set(thiz, count); } } } diff --git a/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentationModule.java b/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentationModule.java index a1be8d62cc66..bdd665f3f4fa 100644 --- a/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentationModule.java +++ b/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentationModule.java @@ -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 ContextTestInstrumentationModule extends InstrumentationModule { +public class ContextTestInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public ContextTestInstrumentationModule() { super("context-test-instrumentation"); } @@ -27,4 +29,9 @@ public boolean isHelperClass(String className) { public List typeInstrumentations() { return singletonList(new ContextTestInstrumentation()); } + + @Override + public boolean isIndyReady() { + return true; + } } diff --git a/testing-common/integration-tests/src/main/java/field/VirtualFieldTestInstrumentationModule.java b/testing-common/integration-tests/src/main/java/field/VirtualFieldTestInstrumentationModule.java index 3a81438eb0e7..26980a893591 100644 --- a/testing-common/integration-tests/src/main/java/field/VirtualFieldTestInstrumentationModule.java +++ b/testing-common/integration-tests/src/main/java/field/VirtualFieldTestInstrumentationModule.java @@ -14,6 +14,7 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import java.util.List; import net.bytebuddy.asm.Advice; +import net.bytebuddy.asm.Advice.AssignReturned; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -50,10 +51,11 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class TestAdvice { + @AssignReturned.ToReturned @Advice.OnMethodExit - public static void onExit(@Advice.Return(readOnly = false) boolean result) { + public static boolean onExit() { VirtualFieldTestHelper.test(); - result = true; + return true; } } } diff --git a/testing-common/integration-tests/src/main/java/helper/DuplicateHelperInstrumentation.java b/testing-common/integration-tests/src/main/java/helper/DuplicateHelperInstrumentation.java index 8cf4db9008cd..09e4f2f8d6ce 100644 --- a/testing-common/integration-tests/src/main/java/helper/DuplicateHelperInstrumentation.java +++ b/testing-common/integration-tests/src/main/java/helper/DuplicateHelperInstrumentation.java @@ -10,6 +10,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; @@ -26,9 +27,10 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class TestAdvice { + @AssignReturned.ToReturned @Advice.OnMethodExit(suppress = Throwable.class) - public static void addSuffix(@Advice.Return(readOnly = false) String string) { - string = DuplicateHelper.addSuffix(string, " foo"); + public static String addSuffix(@Advice.Return String string) { + return DuplicateHelper.addSuffix(string, " foo"); } } } From 49b6dc207e8765fbdc833df5989592b626bafd17 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 24 Oct 2025 14:20:21 +0200 Subject: [PATCH 3/6] add missing file --- .../java/context/ContextTestSingletons.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 testing-common/integration-tests/src/main/java/context/ContextTestSingletons.java diff --git a/testing-common/integration-tests/src/main/java/context/ContextTestSingletons.java b/testing-common/integration-tests/src/main/java/context/ContextTestSingletons.java new file mode 100644 index 000000000000..9c56302adbbc --- /dev/null +++ b/testing-common/integration-tests/src/main/java/context/ContextTestSingletons.java @@ -0,0 +1,22 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package context; + +import io.opentelemetry.instrumentation.api.util.VirtualField; +import library.KeyClass; +import library.KeyInterface; + +public class ContextTestSingletons { + + public static final VirtualField CONTEXT = + VirtualField.find(KeyClass.class, Context.class); + public static final VirtualField INTEGER = + VirtualField.find(KeyClass.class, Integer.class); + public static final VirtualField INTERFACE_INTEGER = + VirtualField.find(KeyInterface.class, Integer.class); + + private ContextTestSingletons() {} +} From 2a2b04f32d4e126c40a0a9e25fbf5ec10499e1b6 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 24 Oct 2025 15:46:51 +0200 Subject: [PATCH 4/6] fix AssignReturned.ToReturned --- .../javaagent/tooling/bytebuddy/MissingTypeTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/javaagent-tooling/src/testMissingType/java/io/opentelemetry/javaagent/tooling/bytebuddy/MissingTypeTest.java b/javaagent-tooling/src/testMissingType/java/io/opentelemetry/javaagent/tooling/bytebuddy/MissingTypeTest.java index d86733e56cd8..71ddfda7cf8a 100644 --- a/javaagent-tooling/src/testMissingType/java/io/opentelemetry/javaagent/tooling/bytebuddy/MissingTypeTest.java +++ b/javaagent-tooling/src/testMissingType/java/io/opentelemetry/javaagent/tooling/bytebuddy/MissingTypeTest.java @@ -16,6 +16,7 @@ import net.bytebuddy.agent.ByteBuddyAgent; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.agent.builder.ResettableClassFileTransformer; +import net.bytebuddy.asm.Advice; import net.bytebuddy.dynamic.ClassFileLocator; import net.bytebuddy.dynamic.scaffold.MethodGraph; import net.bytebuddy.utility.JavaModule; @@ -54,7 +55,10 @@ public void onError( }) .type(named(MissingTypeTest.class.getName() + "$SomeClass")) .transform( - new AgentBuilder.Transformer.ForAdvice() + new AgentBuilder.Transformer.ForAdvice( + Advice.withCustomMapping() + // required for AssignReturned to work + .with(new Advice.AssignReturned.Factory())) .with( new AgentBuilder.LocationStrategy.Simple( ClassFileLocator.ForClassLoader.of(TestAdvice.class.getClassLoader()))) From 340c5194c8347b800f24afa4c7249c97d7bba785 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 24 Oct 2025 16:22:54 +0200 Subject: [PATCH 5/6] fix things --- .../javaagent/tooling/bytebuddy/BadAdvice.java | 4 +--- .../tooling/bytebuddy/ExceptionHandlerTest.java | 14 +++++++++++--- .../tooling/bytebuddy/MissingTypeTest.java | 7 +++---- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/javaagent-tooling/src/testExceptionHandler/java/io/opentelemetry/javaagent/tooling/bytebuddy/BadAdvice.java b/javaagent-tooling/src/testExceptionHandler/java/io/opentelemetry/javaagent/tooling/bytebuddy/BadAdvice.java index c2e68daaf792..a150caadebe2 100644 --- a/javaagent-tooling/src/testExceptionHandler/java/io/opentelemetry/javaagent/tooling/bytebuddy/BadAdvice.java +++ b/javaagent-tooling/src/testExceptionHandler/java/io/opentelemetry/javaagent/tooling/bytebuddy/BadAdvice.java @@ -6,14 +6,12 @@ package io.opentelemetry.javaagent.tooling.bytebuddy; import net.bytebuddy.asm.Advice; -import net.bytebuddy.asm.Advice.AssignReturned; @SuppressWarnings("unused") public class BadAdvice { - @AssignReturned.ToReturned @Advice.OnMethodExit(suppress = Throwable.class) - public static boolean throwAnException(@Advice.Return boolean originalReturnVal) { + public static boolean throwAnException() { throw new IllegalStateException("Test Exception"); } diff --git a/javaagent-tooling/src/testExceptionHandler/java/io/opentelemetry/javaagent/tooling/bytebuddy/ExceptionHandlerTest.java b/javaagent-tooling/src/testExceptionHandler/java/io/opentelemetry/javaagent/tooling/bytebuddy/ExceptionHandlerTest.java index 92dd4046ba69..2e0489769ade 100644 --- a/javaagent-tooling/src/testExceptionHandler/java/io/opentelemetry/javaagent/tooling/bytebuddy/ExceptionHandlerTest.java +++ b/javaagent-tooling/src/testExceptionHandler/java/io/opentelemetry/javaagent/tooling/bytebuddy/ExceptionHandlerTest.java @@ -20,6 +20,7 @@ import net.bytebuddy.agent.ByteBuddyAgent; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.agent.builder.ResettableClassFileTransformer; +import net.bytebuddy.asm.Advice; import net.bytebuddy.dynamic.ClassFileLocator; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -34,20 +35,24 @@ class ExceptionHandlerTest { @BeforeAll static void setUp() { + Advice.WithCustomMapping customMapping = + Advice.withCustomMapping() + // required for AssignReturned annotation and throwable suppression + .with(new Advice.AssignReturned.Factory().withSuppressed(Throwable.class)); AgentBuilder builder = new AgentBuilder.Default() .disableClassFormatChanges() .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION) .type(named(ExceptionHandlerTest.class.getName() + "$SomeClass")) .transform( - new AgentBuilder.Transformer.ForAdvice() + new AgentBuilder.Transformer.ForAdvice(customMapping) .with( new AgentBuilder.LocationStrategy.Simple( ClassFileLocator.ForClassLoader.of(BadAdvice.class.getClassLoader()))) .withExceptionHandler(ExceptionHandlers.defaultExceptionHandler()) .advice(isMethod().and(named("isInstrumented")), BadAdvice.class.getName())) .transform( - new AgentBuilder.Transformer.ForAdvice() + new AgentBuilder.Transformer.ForAdvice(customMapping) .with( new AgentBuilder.LocationStrategy.Simple( ClassFileLocator.ForClassLoader.of(BadAdvice.class.getClassLoader()))) @@ -80,7 +85,10 @@ void exceptionHandlerInvoked() { int initLogEvents = testHandler.getRecords().size(); // Triggers classload and instrumentation - assertThat(SomeClass.isInstrumented()).isTrue(); + assertThat(SomeClass.isInstrumented()) + .describedAs("method return value should be preserved when exception is thrown in advice") + .isFalse(); + assertThat(testHandler.getRecords()) .hasSize(initLogEvents + 1) .last() diff --git a/javaagent-tooling/src/testMissingType/java/io/opentelemetry/javaagent/tooling/bytebuddy/MissingTypeTest.java b/javaagent-tooling/src/testMissingType/java/io/opentelemetry/javaagent/tooling/bytebuddy/MissingTypeTest.java index 71ddfda7cf8a..f9cf847c69e3 100644 --- a/javaagent-tooling/src/testMissingType/java/io/opentelemetry/javaagent/tooling/bytebuddy/MissingTypeTest.java +++ b/javaagent-tooling/src/testMissingType/java/io/opentelemetry/javaagent/tooling/bytebuddy/MissingTypeTest.java @@ -34,6 +34,8 @@ class MissingTypeTest { @BeforeAll static void setUp() { + Advice.WithCustomMapping customMapping = + Advice.withCustomMapping().with(new Advice.AssignReturned.Factory()); AgentBuilder builder = new AgentBuilder.Default( new ByteBuddy().with(MethodGraph.Compiler.ForDeclaredMethods.INSTANCE)) @@ -55,10 +57,7 @@ public void onError( }) .type(named(MissingTypeTest.class.getName() + "$SomeClass")) .transform( - new AgentBuilder.Transformer.ForAdvice( - Advice.withCustomMapping() - // required for AssignReturned to work - .with(new Advice.AssignReturned.Factory())) + new AgentBuilder.Transformer.ForAdvice(customMapping) .with( new AgentBuilder.LocationStrategy.Simple( ClassFileLocator.ForClassLoader.of(TestAdvice.class.getClassLoader()))) From 723b3f71261c339899f21b8c219519ddc5cb3915 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 24 Oct 2025 18:18:52 +0200 Subject: [PATCH 6/6] fix helper class visibility --- .../main/java/context/ContextTestInstrumentationModule.java | 3 ++- .../opentelemetry/javaagent/IndyInstrumentationTestModule.java | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentationModule.java b/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentationModule.java index bdd665f3f4fa..dbb88cea989c 100644 --- a/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentationModule.java +++ b/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentationModule.java @@ -22,7 +22,8 @@ public ContextTestInstrumentationModule() { @Override public boolean isHelperClass(String className) { - return className.equals(getClass().getPackage().getName() + ".Context"); + String pkg = getClass().getPackage().getName(); + return className.equals(pkg + ".Context") || className.equals(pkg + ".ContextTestSingletons"); } @Override diff --git a/testing-common/integration-tests/src/main/java/io/opentelemetry/javaagent/IndyInstrumentationTestModule.java b/testing-common/integration-tests/src/main/java/io/opentelemetry/javaagent/IndyInstrumentationTestModule.java index c6424d8fea65..ad077f6f11cc 100644 --- a/testing-common/integration-tests/src/main/java/io/opentelemetry/javaagent/IndyInstrumentationTestModule.java +++ b/testing-common/integration-tests/src/main/java/io/opentelemetry/javaagent/IndyInstrumentationTestModule.java @@ -176,7 +176,6 @@ public static void onMethodEnter() { throw new RuntimeException("This exception should be suppressed"); } - @Advice.AssignReturned.ToReturned @Advice.OnMethodExit( suppress = Throwable.class, onThrowable = Throwable.class,