From c73f5c602e879d173edd517e10b87fa3364d1068 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 10 Oct 2024 16:55:50 +0200 Subject: [PATCH 01/35] make servlet3 indy compatible --- .../servlet/v3_0/Servlet3InstrumentationModule.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java index 46cc5596a9a8..94935ade7b7a 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java @@ -33,12 +33,6 @@ public ElementMatcher.Junction classLoaderMatcher() { return hasClassesNamed("javax.servlet.ServletRegistration"); } - @Override - public boolean isIndyModule() { - // GrailsTest fails - return false; - } - @Override public List typeInstrumentations() { return asList( From 59f857e3be7448965160b79fbb8574918f76b0ce Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 10 Oct 2024 17:44:42 +0200 Subject: [PATCH 02/35] spring web mvc --- .../grails/GrailsInstrumentationModule.java | 9 ++++++++- .../servlet/v3_0/Servlet3InstrumentationModule.java | 9 ++++++++- .../webmvc/v3_1/SpringWebMvcInstrumentationModule.java | 8 +++++--- .../webmvc/v6_0/SpringWebMvcInstrumentationModule.java | 8 +++++--- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/GrailsInstrumentationModule.java b/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/GrailsInstrumentationModule.java index caca5eff09f7..17027e45070d 100644 --- a/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/GrailsInstrumentationModule.java +++ b/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/GrailsInstrumentationModule.java @@ -10,14 +10,21 @@ 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 GrailsInstrumentationModule extends InstrumentationModule { +public class GrailsInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public GrailsInstrumentationModule() { super("grails", "grails-3.0"); } + @Override + public String getModuleGroup() { + return "servlet"; + } + @Override public List typeInstrumentations() { return asList( diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java index 94935ade7b7a..875d7d6903a0 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java @@ -11,6 +11,7 @@ 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.instrumentation.servlet.common.async.AsyncContextInstrumentation; import io.opentelemetry.javaagent.instrumentation.servlet.common.async.AsyncContextStartInstrumentation; import io.opentelemetry.javaagent.instrumentation.servlet.common.async.AsyncStartInstrumentation; @@ -21,7 +22,8 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumentationModule.class) -public class Servlet3InstrumentationModule extends InstrumentationModule { +public class Servlet3InstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { private static final String BASE_PACKAGE = "javax.servlet"; public Servlet3InstrumentationModule() { @@ -33,6 +35,11 @@ public ElementMatcher.Junction classLoaderMatcher() { return hasClassesNamed("javax.servlet.ServletRegistration"); } + @Override + public String getModuleGroup() { + return "servlet"; + } + @Override public List typeInstrumentations() { return asList( diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java index 4fcececf341c..01f74e3e4b28 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java @@ -11,11 +11,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 java.util.List; import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumentationModule.class) -public class SpringWebMvcInstrumentationModule extends InstrumentationModule { +public class SpringWebMvcInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public SpringWebMvcInstrumentationModule() { super("spring-webmvc", "spring-webmvc-3.1"); @@ -33,8 +35,8 @@ public boolean isHelperClass(String className) { } @Override - public boolean isIndyModule() { - return false; + public String getModuleGroup() { + return "servlet"; } @Override diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java index a3d7cef051c4..6a32079546ab 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java @@ -11,11 +11,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 java.util.List; import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumentationModule.class) -public class SpringWebMvcInstrumentationModule extends InstrumentationModule { +public class SpringWebMvcInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public SpringWebMvcInstrumentationModule() { super("spring-webmvc", "spring-webmvc-6.0"); @@ -33,8 +35,8 @@ public boolean isHelperClass(String className) { } @Override - public boolean isIndyModule() { - return false; + public String getModuleGroup() { + return "servlet"; } @Override From 7a0993dc9a8851f67a5738b14b300b5a41b2c868 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Mon, 14 Oct 2024 15:38:26 +0200 Subject: [PATCH 03/35] wip but still broken --- ...ityConfigServletInstrumentationModule.java | 9 ++++++++- .../v3_1/SpringWebInstrumentationModule.java | 19 ++++++++++++++----- .../WebApplicationContextInstrumentation.java | 3 ++- .../v6_0/SpringWebInstrumentationModule.java | 19 ++++++++++++++----- .../WebApplicationContextInstrumentation.java | 2 +- .../DispatcherServletInstrumentation.java | 17 +++++++++++++---- .../DispatcherServletInstrumentation.java | 17 +++++++++++++---- .../indy/IndyProxyFactory.java | 2 +- 8 files changed, 66 insertions(+), 22 deletions(-) diff --git a/instrumentation/spring/spring-security-config-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/security/config/v6_0/servlet/SpringSecurityConfigServletInstrumentationModule.java b/instrumentation/spring/spring-security-config-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/security/config/v6_0/servlet/SpringSecurityConfigServletInstrumentationModule.java index 926bd6d94103..1e97cee24815 100644 --- a/instrumentation/spring/spring-security-config-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/security/config/v6_0/servlet/SpringSecurityConfigServletInstrumentationModule.java +++ b/instrumentation/spring/spring-security-config-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/security/config/v6_0/servlet/SpringSecurityConfigServletInstrumentationModule.java @@ -12,13 +12,15 @@ import io.opentelemetry.javaagent.bootstrap.internal.AgentCommonConfig; 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; import net.bytebuddy.matcher.ElementMatcher; /** Instrumentation module for servlet-based applications that use spring-security-config. */ @AutoService(InstrumentationModule.class) -public class SpringSecurityConfigServletInstrumentationModule extends InstrumentationModule { +public class SpringSecurityConfigServletInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public SpringSecurityConfigServletInstrumentationModule() { super("spring-security-config-servlet", "spring-security-config-servlet-6.0"); } @@ -47,6 +49,11 @@ public ElementMatcher.Junction classLoaderMatcher() { "org.springframework.security.authentication.ObservationAuthenticationManager"); } + @Override + public String getModuleGroup() { + return "servlet"; + } + @Override public List typeInstrumentations() { return singletonList(new HttpSecurityInstrumentation()); diff --git a/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java b/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java index 53e67dcd6e34..1efde99c95f1 100644 --- a/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java +++ b/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java @@ -10,14 +10,17 @@ import static net.bytebuddy.matcher.ElementMatchers.not; import com.google.auto.service.AutoService; -import io.opentelemetry.javaagent.extension.instrumentation.HelperResourceBuilder; 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; import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumentationModule.class) -public class SpringWebInstrumentationModule extends InstrumentationModule { +public class SpringWebInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public SpringWebInstrumentationModule() { super("spring-web", "spring-web-3.1"); } @@ -31,13 +34,19 @@ public ElementMatcher.Junction classLoaderMatcher() { } @Override - public void registerHelperResources(HelperResourceBuilder helperResourceBuilder) { + public void injectClasses(ClassInjector injector) { // make the filter class file loadable by ClassPathResource - in some cases (e.g. spring-guice, // see https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/7428) Spring // might want to read the class file metadata; this line will make the filter class file visible // to the bean class loader - helperResourceBuilder.register( - "org/springframework/web/servlet/v3_1/OpenTelemetryHandlerMappingFilter.class"); + injector + .proxyBuilder("org.springframework.web.servlet.v3_1.OpenTelemetryHandlerMappingFilter") + .inject(InjectionMode.CLASS_AND_RESOURCE); + } + + @Override + public String getModuleGroup() { + return "servlet"; } @Override diff --git a/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/WebApplicationContextInstrumentation.java b/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/WebApplicationContextInstrumentation.java index fb9cbd7c3bbe..50b0e62c090b 100644 --- a/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/WebApplicationContextInstrumentation.java +++ b/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/WebApplicationContextInstrumentation.java @@ -84,7 +84,8 @@ public static void onEnter(@Advice.Argument(0) ConfigurableListableBeanFactory b ((BeanDefinitionRegistry) beanFactory) .registerBeanDefinition("otelAutoDispatcherFilter", beanDefinition); } catch (ClassNotFoundException ignored) { - // Ignore + ignored.printStackTrace(System.out); + throw new IllegalStateException(ignored); // TODO: for debugging } } } diff --git a/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java b/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java index f246e678c3b3..420461c388b5 100644 --- a/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java +++ b/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java @@ -9,14 +9,17 @@ import static java.util.Collections.singletonList; import com.google.auto.service.AutoService; -import io.opentelemetry.javaagent.extension.instrumentation.HelperResourceBuilder; 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; import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumentationModule.class) -public class SpringWebInstrumentationModule extends InstrumentationModule { +public class SpringWebInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public SpringWebInstrumentationModule() { super("spring-web", "spring-web-6.0"); @@ -29,13 +32,19 @@ public ElementMatcher.Junction classLoaderMatcher() { } @Override - public void registerHelperResources(HelperResourceBuilder helperResourceBuilder) { + public void injectClasses(ClassInjector injector) { // make the filter class file loadable by ClassPathResource - in some cases (e.g. spring-guice, // see https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/7428) Spring // might want to read the class file metadata; this line will make the filter class file visible // to the bean class loader - helperResourceBuilder.register( - "org/springframework/web/servlet/v6_0/OpenTelemetryHandlerMappingFilter.class"); + injector + .proxyBuilder("org.springframework.web.servlet.v6_0.OpenTelemetryHandlerMappingFilter") + .inject(InjectionMode.CLASS_AND_RESOURCE); + } + + @Override + public String getModuleGroup() { + return "servlet"; } @Override diff --git a/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/WebApplicationContextInstrumentation.java b/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/WebApplicationContextInstrumentation.java index 3ecad4e9d748..a0d70f9d1b3b 100644 --- a/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/WebApplicationContextInstrumentation.java +++ b/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/WebApplicationContextInstrumentation.java @@ -84,7 +84,7 @@ public static void onEnter(@Advice.Argument(0) ConfigurableListableBeanFactory b ((BeanDefinitionRegistry) beanFactory) .registerBeanDefinition("otelAutoDispatcherFilter", beanDefinition); } catch (ClassNotFoundException ignored) { - // Ignore + throw new IllegalStateException(ignored); // TODO : for debugging } } } diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java index 0ecea21b5e21..852e45abaa54 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java @@ -17,6 +17,7 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import java.lang.reflect.Field; import java.util.List; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -61,11 +62,19 @@ public static class HandlerMappingAdvice { public static void afterRefresh( @Advice.Argument(0) ApplicationContext springCtx, @Advice.FieldValue("handlerMappings") List handlerMappings) { + if (springCtx.containsBean("otelAutoDispatcherFilter")) { - OpenTelemetryHandlerMappingFilter filter = - (OpenTelemetryHandlerMappingFilter) springCtx.getBean("otelAutoDispatcherFilter"); - if (handlerMappings != null && filter != null) { - filter.setHandlerMappings(handlerMappings); + Object bean = springCtx.getBean("otelAutoDispatcherFilter"); + try { + // the bean is a proxy of OpenTelemetryHandlerMappingFilter, so we need to access the + // proxied object, trying to use reflection to invoke methods triggers stack overflows + Field delegate = bean.getClass().getField("delegate"); + Object delegateObject = delegate.get(bean); + OpenTelemetryHandlerMappingFilter mappingFilter = + (OpenTelemetryHandlerMappingFilter) delegateObject; + mappingFilter.setHandlerMappings(handlerMappings); + } catch (IllegalAccessException | NoSuchFieldException e) { + throw new IllegalStateException(e); } } } diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java index 60d663663bea..de1110647845 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java @@ -17,6 +17,7 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import java.lang.reflect.Field; import java.util.List; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -61,11 +62,19 @@ public static class HandlerMappingAdvice { public static void afterRefresh( @Advice.Argument(0) ApplicationContext springCtx, @Advice.FieldValue("handlerMappings") List handlerMappings) { + if (springCtx.containsBean("otelAutoDispatcherFilter")) { - OpenTelemetryHandlerMappingFilter filter = - (OpenTelemetryHandlerMappingFilter) springCtx.getBean("otelAutoDispatcherFilter"); - if (handlerMappings != null) { - filter.setHandlerMappings(handlerMappings); + Object bean = springCtx.getBean("otelAutoDispatcherFilter"); + try { + // the bean is a proxy of OpenTelemetryHandlerMappingFilter, so we need to access the + // proxied object, trying to use reflection to invoke methods triggers stack overflows + Field delegate = bean.getClass().getField("delegate"); + Object delegateObject = delegate.get(bean); + OpenTelemetryHandlerMappingFilter mappingFilter = + (OpenTelemetryHandlerMappingFilter) delegateObject; + mappingFilter.setHandlerMappings(handlerMappings); + } catch (IllegalAccessException | NoSuchFieldException e) { + throw new IllegalStateException(e); } } } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java index 0a1b774ba60f..532ae89ee381 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java @@ -93,7 +93,7 @@ public DynamicType.Unloaded generateProxy( .implement(classToProxy.getInterfaces()) .name(proxyClassName) .annotateType(classToProxy.getDeclaredAnnotations()) - .defineField(DELEGATE_FIELD_NAME, Object.class, Modifier.PRIVATE | Modifier.FINAL); + .defineField(DELEGATE_FIELD_NAME, Object.class, Modifier.PUBLIC | Modifier.FINAL); for (MethodDescription.InDefinedShape method : classToProxy.getDeclaredMethods()) { if (method.isPublic()) { From e6e4f7ab113d2133c2622a42d04201139086d5c9 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 15 Oct 2024 11:11:13 +0200 Subject: [PATCH 04/35] fix clasloading of injected class --- .../v3_1/SpringWebInstrumentationModule.java | 13 ------------- .../v3_1/WebApplicationContextInstrumentation.java | 3 +++ .../web/v6_0/SpringWebInstrumentationModule.java | 13 ------------- .../v6_0/WebApplicationContextInstrumentation.java | 2 ++ .../v3_1/SpringWebMvcInstrumentationModule.java | 9 ++++++--- .../v6_0/SpringWebMvcInstrumentationModule.java | 9 ++++++--- 6 files changed, 17 insertions(+), 32 deletions(-) diff --git a/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java b/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java index 1efde99c95f1..6ba9f8d0efd7 100644 --- a/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java +++ b/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java @@ -13,8 +13,6 @@ 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; import net.bytebuddy.matcher.ElementMatcher; @@ -33,17 +31,6 @@ public ElementMatcher.Junction classLoaderMatcher() { .and(not(hasClassesNamed("org.springframework.web.ErrorResponse"))); } - @Override - public void injectClasses(ClassInjector injector) { - // make the filter class file loadable by ClassPathResource - in some cases (e.g. spring-guice, - // see https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/7428) Spring - // might want to read the class file metadata; this line will make the filter class file visible - // to the bean class loader - injector - .proxyBuilder("org.springframework.web.servlet.v3_1.OpenTelemetryHandlerMappingFilter") - .inject(InjectionMode.CLASS_AND_RESOURCE); - } - @Override public String getModuleGroup() { return "servlet"; diff --git a/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/WebApplicationContextInstrumentation.java b/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/WebApplicationContextInstrumentation.java index 50b0e62c090b..544b10ee8476 100644 --- a/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/WebApplicationContextInstrumentation.java +++ b/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/WebApplicationContextInstrumentation.java @@ -61,6 +61,9 @@ public static class FilterInjectingAdvice { public static void onEnter(@Advice.Argument(0) ConfigurableListableBeanFactory beanFactory) { if (beanFactory instanceof BeanDefinitionRegistry && !beanFactory.containsBean("otelAutoDispatcherFilter")) { + + // Explicitly loading classes allows to catch any class-loading issue or deal with cases + // where the class is not visible. try { // Firstly check whether DispatcherServlet is present. We need to load an instrumented // class from spring-webmvc to trigger injection that makes diff --git a/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java b/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java index 420461c388b5..d91d9cd5827e 100644 --- a/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java +++ b/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java @@ -12,8 +12,6 @@ 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; import net.bytebuddy.matcher.ElementMatcher; @@ -31,17 +29,6 @@ public ElementMatcher.Junction classLoaderMatcher() { return hasClassesNamed("org.springframework.web.ErrorResponse"); } - @Override - public void injectClasses(ClassInjector injector) { - // make the filter class file loadable by ClassPathResource - in some cases (e.g. spring-guice, - // see https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/7428) Spring - // might want to read the class file metadata; this line will make the filter class file visible - // to the bean class loader - injector - .proxyBuilder("org.springframework.web.servlet.v6_0.OpenTelemetryHandlerMappingFilter") - .inject(InjectionMode.CLASS_AND_RESOURCE); - } - @Override public String getModuleGroup() { return "servlet"; diff --git a/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/WebApplicationContextInstrumentation.java b/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/WebApplicationContextInstrumentation.java index a0d70f9d1b3b..077bfd5abf58 100644 --- a/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/WebApplicationContextInstrumentation.java +++ b/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/WebApplicationContextInstrumentation.java @@ -61,6 +61,8 @@ public static class FilterInjectingAdvice { public static void onEnter(@Advice.Argument(0) ConfigurableListableBeanFactory beanFactory) { if (beanFactory instanceof BeanDefinitionRegistry && !beanFactory.containsBean("otelAutoDispatcherFilter")) { + // Explicitly loading classes allows to catch any class-loading issue or deal with cases + // where the class is not visible. try { // Firstly check whether DispatcherServlet is present. We need to load an instrumented // class from spring-webmvc to trigger injection that makes diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java index 01f74e3e4b28..b6f2d3b23450 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java @@ -12,6 +12,8 @@ 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; import net.bytebuddy.matcher.ElementMatcher; @@ -29,9 +31,10 @@ public ElementMatcher.Junction classLoaderMatcher() { } @Override - public boolean isHelperClass(String className) { - return className.startsWith( - "org.springframework.web.servlet.v3_1.OpenTelemetryHandlerMappingFilter"); + public void injectClasses(ClassInjector injector) { + injector + .proxyBuilder("org.springframework.web.servlet.v3_1.OpenTelemetryHandlerMappingFilter") + .inject(InjectionMode.CLASS_AND_RESOURCE); } @Override diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java index 6a32079546ab..ca98ff8e099c 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java @@ -12,6 +12,8 @@ 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; import net.bytebuddy.matcher.ElementMatcher; @@ -29,9 +31,10 @@ public ElementMatcher.Junction classLoaderMatcher() { } @Override - public boolean isHelperClass(String className) { - return className.startsWith( - "org.springframework.web.servlet.v6_0.OpenTelemetryHandlerMappingFilter"); + public void injectClasses(ClassInjector injector) { + injector + .proxyBuilder("org.springframework.web.servlet.v6_0.OpenTelemetryHandlerMappingFilter") + .inject(InjectionMode.CLASS_AND_RESOURCE); } @Override From 37970e843f651306959dd8662f949b84ab2b531b Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 15 Oct 2024 11:38:11 +0200 Subject: [PATCH 05/35] avoid stack overflow with proxy --- .../webmvc/v3_1/SpringWebMvcInstrumentationModule.java | 6 ++++++ .../webmvc/v6_0/SpringWebMvcInstrumentationModule.java | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java index b6f2d3b23450..5f6c989efbfc 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java @@ -30,6 +30,12 @@ public ElementMatcher.Junction classLoaderMatcher() { return hasClassesNamed("javax.servlet.Filter"); } + @Override + public boolean isHelperClass(String className) { + // filter on prefix due to inner classes + return className.startsWith("org.springframework.web.servlet.v3_1.OpenTelemetryHandlerMappingFilter"); + } + @Override public void injectClasses(ClassInjector injector) { injector diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java index ca98ff8e099c..a12fea9dddb8 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java @@ -30,6 +30,12 @@ public ElementMatcher.Junction classLoaderMatcher() { return hasClassesNamed("jakarta.servlet.Filter"); } + @Override + public boolean isHelperClass(String className) { + // filter on prefix due to inner classes + return className.startsWith("org.springframework.web.servlet.v6_0.OpenTelemetryHandlerMappingFilter"); + } + @Override public void injectClasses(ClassInjector injector) { injector From cdd978c2fb880bdb0c35afd57d2b022a51f1ebe1 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 15 Oct 2024 13:54:11 +0200 Subject: [PATCH 06/35] spotless --- .../jaxrs/v2_0/JerseyInstrumentationModule.java | 11 +++++------ .../v3_1/SpringWebMvcInstrumentationModule.java | 3 ++- .../v6_0/SpringWebMvcInstrumentationModule.java | 3 ++- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java index 1e2a1e296bc0..98cade1d288c 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java @@ -11,11 +11,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 java.util.List; import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumentationModule.class) -public class JerseyInstrumentationModule extends InstrumentationModule { +public class JerseyInstrumentationModule extends InstrumentationModule implements + ExperimentalInstrumentationModule { public JerseyInstrumentationModule() { super("jaxrs", "jaxrs-2.0", "jersey", "jersey-2.0"); } @@ -26,11 +28,8 @@ public ElementMatcher.Junction classLoaderMatcher() { } @Override - public boolean isIndyModule() { - // net.bytebuddy.pool.TypePool$Resolution$NoSuchTypeException: Cannot resolve type description - // for - // io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet.Servlet3SnippetInjectingResponseWrapper - return false; + public String getModuleGroup() { + return "servlet"; } @Override diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java index 5f6c989efbfc..0a11d2e8d109 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/SpringWebMvcInstrumentationModule.java @@ -33,7 +33,8 @@ public ElementMatcher.Junction classLoaderMatcher() { @Override public boolean isHelperClass(String className) { // filter on prefix due to inner classes - return className.startsWith("org.springframework.web.servlet.v3_1.OpenTelemetryHandlerMappingFilter"); + return className.startsWith( + "org.springframework.web.servlet.v3_1.OpenTelemetryHandlerMappingFilter"); } @Override diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java index a12fea9dddb8..d48a2d697f35 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/SpringWebMvcInstrumentationModule.java @@ -33,7 +33,8 @@ public ElementMatcher.Junction classLoaderMatcher() { @Override public boolean isHelperClass(String className) { // filter on prefix due to inner classes - return className.startsWith("org.springframework.web.servlet.v6_0.OpenTelemetryHandlerMappingFilter"); + return className.startsWith( + "org.springframework.web.servlet.v6_0.OpenTelemetryHandlerMappingFilter"); } @Override From 30f535674d1eff39ea2e39cf02a6c02a7750d545 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 15 Oct 2024 14:04:54 +0200 Subject: [PATCH 07/35] spotless again --- .../jaxrs/v2_0/JerseyInstrumentationModule.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java index 98cade1d288c..18e9a7ea4294 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java @@ -16,8 +16,8 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumentationModule.class) -public class JerseyInstrumentationModule extends InstrumentationModule implements - ExperimentalInstrumentationModule { +public class JerseyInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public JerseyInstrumentationModule() { super("jaxrs", "jaxrs-2.0", "jersey", "jersey-2.0"); } From 4e6ce38865a932b81a8afe044beff8b38ae0e308 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 15 Oct 2024 14:11:44 +0200 Subject: [PATCH 08/35] remove debugging exceptions --- .../springweb/v3_1/WebApplicationContextInstrumentation.java | 3 +-- .../spring/web/v6_0/WebApplicationContextInstrumentation.java | 2 +- .../spring/webmvc/v3_1/DispatcherServletInstrumentation.java | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/WebApplicationContextInstrumentation.java b/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/WebApplicationContextInstrumentation.java index 544b10ee8476..fa15d4c912a9 100644 --- a/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/WebApplicationContextInstrumentation.java +++ b/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/WebApplicationContextInstrumentation.java @@ -87,8 +87,7 @@ public static void onEnter(@Advice.Argument(0) ConfigurableListableBeanFactory b ((BeanDefinitionRegistry) beanFactory) .registerBeanDefinition("otelAutoDispatcherFilter", beanDefinition); } catch (ClassNotFoundException ignored) { - ignored.printStackTrace(System.out); - throw new IllegalStateException(ignored); // TODO: for debugging + // Ignore } } } diff --git a/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/WebApplicationContextInstrumentation.java b/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/WebApplicationContextInstrumentation.java index 077bfd5abf58..69fbc36e1e70 100644 --- a/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/WebApplicationContextInstrumentation.java +++ b/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/WebApplicationContextInstrumentation.java @@ -86,7 +86,7 @@ public static void onEnter(@Advice.Argument(0) ConfigurableListableBeanFactory b ((BeanDefinitionRegistry) beanFactory) .registerBeanDefinition("otelAutoDispatcherFilter", beanDefinition); } catch (ClassNotFoundException ignored) { - throw new IllegalStateException(ignored); // TODO : for debugging + // Ignore } } } diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java index 852e45abaa54..696aeeda1207 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java @@ -62,7 +62,6 @@ public static class HandlerMappingAdvice { public static void afterRefresh( @Advice.Argument(0) ApplicationContext springCtx, @Advice.FieldValue("handlerMappings") List handlerMappings) { - if (springCtx.containsBean("otelAutoDispatcherFilter")) { Object bean = springCtx.getBean("otelAutoDispatcherFilter"); try { From 99d8bff51f8df4c20147c88b23e32383217231eb Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Mon, 28 Oct 2024 15:17:52 +0100 Subject: [PATCH 09/35] add comment to clarify public field access --- .../javaagent/tooling/instrumentation/indy/IndyProxyFactory.java | 1 + 1 file changed, 1 insertion(+) diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java index 532ae89ee381..56cd8fb62a74 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java @@ -93,6 +93,7 @@ public DynamicType.Unloaded generateProxy( .implement(classToProxy.getInterfaces()) .name(proxyClassName) .annotateType(classToProxy.getDeclaredAnnotations()) + // field must be public to enable resolving the proxy target using introspection .defineField(DELEGATE_FIELD_NAME, Object.class, Modifier.PUBLIC | Modifier.FINAL); for (MethodDescription.InDefinedShape method : classToProxy.getDeclaredMethods()) { From 0c91cbcc178669bee7c8cce2d131f6f8a21cb174 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Mon, 28 Oct 2024 15:49:11 +0100 Subject: [PATCH 10/35] turns out proxy delegate access is not needed --- .../DispatcherServletInstrumentation.java | 20 ++++++------------ .../DispatcherServletInstrumentation.java | 21 +++++++------------ .../indy/IndyProxyFactory.java | 3 +-- 3 files changed, 14 insertions(+), 30 deletions(-) diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java index 696aeeda1207..4e0835e00163 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java @@ -17,7 +17,6 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import java.lang.reflect.Field; import java.util.List; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -62,20 +61,13 @@ public static class HandlerMappingAdvice { public static void afterRefresh( @Advice.Argument(0) ApplicationContext springCtx, @Advice.FieldValue("handlerMappings") List handlerMappings) { - if (springCtx.containsBean("otelAutoDispatcherFilter")) { - Object bean = springCtx.getBean("otelAutoDispatcherFilter"); - try { - // the bean is a proxy of OpenTelemetryHandlerMappingFilter, so we need to access the - // proxied object, trying to use reflection to invoke methods triggers stack overflows - Field delegate = bean.getClass().getField("delegate"); - Object delegateObject = delegate.get(bean); - OpenTelemetryHandlerMappingFilter mappingFilter = - (OpenTelemetryHandlerMappingFilter) delegateObject; - mappingFilter.setHandlerMappings(handlerMappings); - } catch (IllegalAccessException | NoSuchFieldException e) { - throw new IllegalStateException(e); - } + + if (handlerMappings == null || !springCtx.containsBean("otelAutoDispatcherFilter")) { + return; } + OpenTelemetryHandlerMappingFilter filter = + springCtx.getBean("otelAutoDispatcherFilter", OpenTelemetryHandlerMappingFilter.class); + filter.setHandlerMappings(handlerMappings); } } diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java index de1110647845..27f291c8a318 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java @@ -17,7 +17,6 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import java.lang.reflect.Field; import java.util.List; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -63,21 +62,15 @@ public static void afterRefresh( @Advice.Argument(0) ApplicationContext springCtx, @Advice.FieldValue("handlerMappings") List handlerMappings) { - if (springCtx.containsBean("otelAutoDispatcherFilter")) { - Object bean = springCtx.getBean("otelAutoDispatcherFilter"); - try { - // the bean is a proxy of OpenTelemetryHandlerMappingFilter, so we need to access the - // proxied object, trying to use reflection to invoke methods triggers stack overflows - Field delegate = bean.getClass().getField("delegate"); - Object delegateObject = delegate.get(bean); - OpenTelemetryHandlerMappingFilter mappingFilter = - (OpenTelemetryHandlerMappingFilter) delegateObject; - mappingFilter.setHandlerMappings(handlerMappings); - } catch (IllegalAccessException | NoSuchFieldException e) { - throw new IllegalStateException(e); - } + if (handlerMappings == null || !springCtx.containsBean("otelAutoDispatcherFilter")) { + return; } + + OpenTelemetryHandlerMappingFilter filter = springCtx.getBean("otelAutoDispatcherFilter", + OpenTelemetryHandlerMappingFilter.class); + filter.setHandlerMappings(handlerMappings); } + } @SuppressWarnings("unused") diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java index 56cd8fb62a74..0a1b774ba60f 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java @@ -93,8 +93,7 @@ public DynamicType.Unloaded generateProxy( .implement(classToProxy.getInterfaces()) .name(proxyClassName) .annotateType(classToProxy.getDeclaredAnnotations()) - // field must be public to enable resolving the proxy target using introspection - .defineField(DELEGATE_FIELD_NAME, Object.class, Modifier.PUBLIC | Modifier.FINAL); + .defineField(DELEGATE_FIELD_NAME, Object.class, Modifier.PRIVATE | Modifier.FINAL); for (MethodDescription.InDefinedShape method : classToProxy.getDeclaredMethods()) { if (method.isPublic()) { From 5b3b408731693c12f5beab3d4e2109e1574f0535 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Mon, 28 Oct 2024 15:58:53 +0100 Subject: [PATCH 11/35] fix spotless --- .../spring/webmvc/v6_0/DispatcherServletInstrumentation.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java index 27f291c8a318..811c670d9526 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java @@ -66,11 +66,10 @@ public static void afterRefresh( return; } - OpenTelemetryHandlerMappingFilter filter = springCtx.getBean("otelAutoDispatcherFilter", - OpenTelemetryHandlerMappingFilter.class); + OpenTelemetryHandlerMappingFilter filter = + springCtx.getBean("otelAutoDispatcherFilter", OpenTelemetryHandlerMappingFilter.class); filter.setHandlerMappings(handlerMappings); } - } @SuppressWarnings("unused") From d2714ad1a4df0697ed3018d8bcb761d2ea85e100 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Mon, 28 Oct 2024 19:10:37 +0100 Subject: [PATCH 12/35] public delegate needed with indy --- .../v3_1/DispatcherServletInstrumentation.java | 17 +++++++++++++++-- .../v6_0/DispatcherServletInstrumentation.java | 17 +++++++++++++++-- .../instrumentation/indy/IndyProxyFactory.java | 3 ++- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java index 4e0835e00163..4f00f5de3197 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java @@ -17,6 +17,7 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import java.lang.reflect.Field; import java.util.List; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -65,8 +66,20 @@ public static void afterRefresh( if (handlerMappings == null || !springCtx.containsBean("otelAutoDispatcherFilter")) { return; } - OpenTelemetryHandlerMappingFilter filter = - springCtx.getBean("otelAutoDispatcherFilter", OpenTelemetryHandlerMappingFilter.class); + Object bean = springCtx.getBean("otelAutoDispatcherFilter"); + OpenTelemetryHandlerMappingFilter filter; + if (bean instanceof OpenTelemetryHandlerMappingFilter) { + // inline advice: no proxy class is used + filter = (OpenTelemetryHandlerMappingFilter) bean; + } else { + // non-inlined advice: proxy class is used + try { + Field delegate = bean.getClass().getField("delegate"); + filter = (OpenTelemetryHandlerMappingFilter) delegate.get(bean); + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new IllegalStateException(e); + } + } filter.setHandlerMappings(handlerMappings); } } diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java index 811c670d9526..add1816dac3f 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java @@ -17,6 +17,7 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import java.lang.reflect.Field; import java.util.List; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -66,8 +67,20 @@ public static void afterRefresh( return; } - OpenTelemetryHandlerMappingFilter filter = - springCtx.getBean("otelAutoDispatcherFilter", OpenTelemetryHandlerMappingFilter.class); + Object bean = springCtx.getBean("otelAutoDispatcherFilter"); + OpenTelemetryHandlerMappingFilter filter; + if (bean instanceof OpenTelemetryHandlerMappingFilter) { + // inline advice: no proxy class is used + filter = (OpenTelemetryHandlerMappingFilter) bean; + } else { + // non-inlined advice: proxy class is used + try { + Field delegate = bean.getClass().getField("delegate"); + filter = (OpenTelemetryHandlerMappingFilter) delegate.get(bean); + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new IllegalStateException(e); + } + } filter.setHandlerMappings(handlerMappings); } } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java index 0a1b774ba60f..56cd8fb62a74 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java @@ -93,7 +93,8 @@ public DynamicType.Unloaded generateProxy( .implement(classToProxy.getInterfaces()) .name(proxyClassName) .annotateType(classToProxy.getDeclaredAnnotations()) - .defineField(DELEGATE_FIELD_NAME, Object.class, Modifier.PRIVATE | Modifier.FINAL); + // field must be public to enable resolving the proxy target using introspection + .defineField(DELEGATE_FIELD_NAME, Object.class, Modifier.PUBLIC | Modifier.FINAL); for (MethodDescription.InDefinedShape method : classToProxy.getDeclaredMethods()) { if (method.isPublic()) { From 978ce819e781227456345da883ce452c6e75983b Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 29 Oct 2024 10:23:22 +0100 Subject: [PATCH 13/35] add a few comments to elaborate on servlet module --- .../instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java | 1 + .../servlet/v3_0/Servlet3InstrumentationModule.java | 1 + .../SpringSecurityConfigServletInstrumentationModule.java | 1 + .../springweb/v3_1/SpringWebInstrumentationModule.java | 1 + .../spring/web/v6_0/SpringWebInstrumentationModule.java | 1 + 5 files changed, 5 insertions(+) diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java index 18e9a7ea4294..8ab480006922 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyInstrumentationModule.java @@ -29,6 +29,7 @@ public ElementMatcher.Junction classLoaderMatcher() { @Override public String getModuleGroup() { + // depends on Servlet3SnippetInjectingResponseWrapper return "servlet"; } diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java index 875d7d6903a0..f7a45a270f8b 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java @@ -37,6 +37,7 @@ public ElementMatcher.Junction classLoaderMatcher() { @Override public String getModuleGroup() { + // depends on servlet instrumentation return "servlet"; } diff --git a/instrumentation/spring/spring-security-config-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/security/config/v6_0/servlet/SpringSecurityConfigServletInstrumentationModule.java b/instrumentation/spring/spring-security-config-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/security/config/v6_0/servlet/SpringSecurityConfigServletInstrumentationModule.java index 1e97cee24815..85c3e0bc00d0 100644 --- a/instrumentation/spring/spring-security-config-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/security/config/v6_0/servlet/SpringSecurityConfigServletInstrumentationModule.java +++ b/instrumentation/spring/spring-security-config-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/security/config/v6_0/servlet/SpringSecurityConfigServletInstrumentationModule.java @@ -51,6 +51,7 @@ public ElementMatcher.Junction classLoaderMatcher() { @Override public String getModuleGroup() { + // depends on servlet instrumentation return "servlet"; } diff --git a/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java b/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java index 6ba9f8d0efd7..bafcb24794cf 100644 --- a/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java +++ b/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java @@ -33,6 +33,7 @@ public ElementMatcher.Junction classLoaderMatcher() { @Override public String getModuleGroup() { + // depends on OpenTelemetryHandlerMappingFilter return "servlet"; } diff --git a/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java b/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java index d91d9cd5827e..756c9a1f23e6 100644 --- a/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java +++ b/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java @@ -31,6 +31,7 @@ public ElementMatcher.Junction classLoaderMatcher() { @Override public String getModuleGroup() { + // depends on OpenTelemetryHandlerMappingFilter return "servlet"; } From 43ddd93c676645beb5c2285003616b6a1a323f09 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 6 Nov 2024 13:36:32 +0100 Subject: [PATCH 14/35] post review comment --- .../spring/webmvc/v3_1/DispatcherServletInstrumentation.java | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java index 4f00f5de3197..1b61fcf2fa96 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java @@ -69,6 +69,7 @@ public static void afterRefresh( Object bean = springCtx.getBean("otelAutoDispatcherFilter"); OpenTelemetryHandlerMappingFilter filter; if (bean instanceof OpenTelemetryHandlerMappingFilter) { + // TODO: remove this branch once advices are not inlined anymore as it's no longer relevant // inline advice: no proxy class is used filter = (OpenTelemetryHandlerMappingFilter) bean; } else { From da8d9bc4c8f4155bdf64c4144eeb9078b2dd6b4b Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 6 Nov 2024 13:47:48 +0100 Subject: [PATCH 15/35] remove indy changes for grails --- .../grails/GrailsInstrumentationModule.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/GrailsInstrumentationModule.java b/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/GrailsInstrumentationModule.java index 17027e45070d..caca5eff09f7 100644 --- a/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/GrailsInstrumentationModule.java +++ b/instrumentation/grails-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grails/GrailsInstrumentationModule.java @@ -10,21 +10,14 @@ 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 GrailsInstrumentationModule extends InstrumentationModule - implements ExperimentalInstrumentationModule { +public class GrailsInstrumentationModule extends InstrumentationModule { public GrailsInstrumentationModule() { super("grails", "grails-3.0"); } - @Override - public String getModuleGroup() { - return "servlet"; - } - @Override public List typeInstrumentations() { return asList( From a7f57cfef3ab6febf6e9fcca1aca4378612c22f7 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 13 Nov 2024 19:06:47 +0100 Subject: [PATCH 16/35] better proxy unwrap --- .../DispatcherServletInstrumentation.java | 18 ++----- .../DispatcherServletInstrumentation.java | 17 ++---- .../javaagent/bootstrap/IndyProxy.java | 52 +++++++++++++++++++ .../indy/IndyProxyFactory.java | 7 ++- .../indy/IndyProxyFactoryTest.java | 23 ++++++++ 5 files changed, 87 insertions(+), 30 deletions(-) create mode 100644 javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java index 1b61fcf2fa96..42bb70d9723f 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java @@ -15,9 +15,9 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; +import io.opentelemetry.javaagent.bootstrap.IndyProxy; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import java.lang.reflect.Field; import java.util.List; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -67,20 +67,8 @@ public static void afterRefresh( return; } Object bean = springCtx.getBean("otelAutoDispatcherFilter"); - OpenTelemetryHandlerMappingFilter filter; - if (bean instanceof OpenTelemetryHandlerMappingFilter) { - // TODO: remove this branch once advices are not inlined anymore as it's no longer relevant - // inline advice: no proxy class is used - filter = (OpenTelemetryHandlerMappingFilter) bean; - } else { - // non-inlined advice: proxy class is used - try { - Field delegate = bean.getClass().getField("delegate"); - filter = (OpenTelemetryHandlerMappingFilter) delegate.get(bean); - } catch (NoSuchFieldException | IllegalAccessException e) { - throw new IllegalStateException(e); - } - } + OpenTelemetryHandlerMappingFilter filter = + IndyProxy.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); filter.setHandlerMappings(handlerMappings); } } diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java index add1816dac3f..1436fce6f492 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java @@ -15,9 +15,9 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; +import io.opentelemetry.javaagent.bootstrap.IndyProxy; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import java.lang.reflect.Field; import java.util.List; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -68,19 +68,8 @@ public static void afterRefresh( } Object bean = springCtx.getBean("otelAutoDispatcherFilter"); - OpenTelemetryHandlerMappingFilter filter; - if (bean instanceof OpenTelemetryHandlerMappingFilter) { - // inline advice: no proxy class is used - filter = (OpenTelemetryHandlerMappingFilter) bean; - } else { - // non-inlined advice: proxy class is used - try { - Field delegate = bean.getClass().getField("delegate"); - filter = (OpenTelemetryHandlerMappingFilter) delegate.get(bean); - } catch (NoSuchFieldException | IllegalAccessException e) { - throw new IllegalStateException(e); - } - } + OpenTelemetryHandlerMappingFilter filter = + IndyProxy.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); filter.setHandlerMappings(handlerMappings); } } diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java new file mode 100644 index 000000000000..588be08eceba --- /dev/null +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java @@ -0,0 +1,52 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.bootstrap; + +import java.lang.reflect.Field; + +/** Interface added to proxy classes injected for indy instrumentation */ +@SuppressWarnings("InterfaceWithOnlyStatics") // TODO: remove this once we have a method +public interface IndyProxy { + + /** + * Unwraps the proxied object + * + * @return unwrapped object + */ + default Object unwrap() { + try { + // current implementation based on introspection + public delegate field + Field delegate = this.getClass().getField("delegate"); + return delegate.get(this); + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new IllegalStateException(e); + } + } + + /** + * Transparently unwraps an object that might have been proxied for indy instrumentation. When + * unwrapping is not needed, for example for inlined advices, the original object is returned + * effectively making this equivalent to a type cast. + * + * @param o object that might need unwrapping + * @param type expected unwrapped object type + * @param type of the proxied object + * @return unwrapped object + * @throws IllegalArgumentException when object can't be cast or unwrapped to the desired type + */ + static T unwrapIfNeeded(Object o, Class type) { + if (type.isAssignableFrom(o.getClass())) { + return type.cast(o); + } + if (o instanceof IndyProxy) { + Object unwrapped = ((IndyProxy) o).unwrap(); + if (type.isAssignableFrom(unwrapped.getClass())) { + return type.cast(unwrapped); + } + } + throw new IllegalArgumentException("unexpected object unwrap"); + } +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java index 56cd8fb62a74..20f16ed7c453 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java @@ -5,12 +5,15 @@ package io.opentelemetry.javaagent.tooling.instrumentation.indy; +import io.opentelemetry.javaagent.bootstrap.IndyProxy; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.util.ArrayList; import java.util.List; import net.bytebuddy.ByteBuddy; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.method.ParameterDescription; +import net.bytebuddy.description.type.TypeDefinition; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.dynamic.DynamicType; import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy; @@ -87,10 +90,12 @@ public IndyProxyFactory(Method bootstrapMethod, BootstrapArgsProvider bootstrapA public DynamicType.Unloaded generateProxy( TypeDescription classToProxy, String proxyClassName) { TypeDescription.Generic superClass = classToProxy.getSuperClass(); + List interfaces = new ArrayList<>(classToProxy.getInterfaces()); + interfaces.add(TypeDescription.ForLoadedType.of(IndyProxy.class)); DynamicType.Builder builder = new ByteBuddy() .subclass(superClass, ConstructorStrategy.Default.NO_CONSTRUCTORS) - .implement(classToProxy.getInterfaces()) + .implement(interfaces) .name(proxyClassName) .annotateType(classToProxy.getDeclaredAnnotations()) // field must be public to enable resolving the proxy target using introspection diff --git a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java index c5d1c287013b..047254e71ecd 100644 --- a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java +++ b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java @@ -7,6 +7,7 @@ import static org.assertj.core.api.Assertions.assertThat; +import io.opentelemetry.javaagent.bootstrap.IndyProxy; import io.opentelemetry.javaagent.tooling.instrumentation.indy.dummies.DummyAnnotation; import java.lang.invoke.CallSite; import java.lang.invoke.ConstantCallSite; @@ -319,6 +320,28 @@ void verifyNonPublicMembersIgnored() throws Exception { .anySatisfy(method -> assertThat(method.getName()).isEqualTo("publicStaticMethod")); } + @Test + void verifyUnwrap() throws Exception { + Class proxy = generateProxy(WrappingTest.class); + Object proxyInstance = proxy.getConstructor().newInstance(); + assertThat(proxyInstance) + .describedAs("proxies should implement IndyProxy") + .isNotInstanceOf(WrappingTest.class) + .isInstanceOf(IndyProxy.class); + + WrappingTest unwrapped = IndyProxy.unwrapIfNeeded(proxyInstance, WrappingTest.class); + assertThat(unwrapped).isInstanceOf(WrappingTest.class).isNotInstanceOf(IndyProxy.class); + + assertThat(IndyProxy.unwrapIfNeeded(unwrapped, WrappingTest.class)) + .describedAs("unwrap an already unwrapped is a no-op") + .isSameAs(unwrapped); + } + + public static class WrappingTest { + + public WrappingTest() {} + } + private static Class generateProxy(Class clazz) { DynamicType.Unloaded unloaded = proxyFactory.generateProxy( From 33c2778da4e40147c1f5bc43ad354b45199d9f81 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 13 Nov 2024 19:09:51 +0100 Subject: [PATCH 17/35] add todo to remove reflection and public field --- .../java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java | 1 + 1 file changed, 1 insertion(+) diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java index 588be08eceba..3ed67762096b 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java @@ -19,6 +19,7 @@ public interface IndyProxy { default Object unwrap() { try { // current implementation based on introspection + public delegate field + // TODO: replace this implementation with proper implementation on the proxy class Field delegate = this.getClass().getField("delegate"); return delegate.get(this); } catch (NoSuchFieldException | IllegalAccessException e) { From 41630472f679013b7b8915ec07a2bbe02892e162 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 14 Nov 2024 13:46:48 +0100 Subject: [PATCH 18/35] revert previous impl --- .../DispatcherServletInstrumentation.java | 18 +++++-- .../DispatcherServletInstrumentation.java | 18 +++++-- .../javaagent/bootstrap/IndyProxy.java | 53 ------------------- .../indy/IndyProxyFactory.java | 7 +-- .../indy/IndyProxyFactoryTest.java | 23 -------- 5 files changed, 31 insertions(+), 88 deletions(-) delete mode 100644 javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java index 42bb70d9723f..1b61fcf2fa96 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java @@ -15,9 +15,9 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; -import io.opentelemetry.javaagent.bootstrap.IndyProxy; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import java.lang.reflect.Field; import java.util.List; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -67,8 +67,20 @@ public static void afterRefresh( return; } Object bean = springCtx.getBean("otelAutoDispatcherFilter"); - OpenTelemetryHandlerMappingFilter filter = - IndyProxy.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); + OpenTelemetryHandlerMappingFilter filter; + if (bean instanceof OpenTelemetryHandlerMappingFilter) { + // TODO: remove this branch once advices are not inlined anymore as it's no longer relevant + // inline advice: no proxy class is used + filter = (OpenTelemetryHandlerMappingFilter) bean; + } else { + // non-inlined advice: proxy class is used + try { + Field delegate = bean.getClass().getField("delegate"); + filter = (OpenTelemetryHandlerMappingFilter) delegate.get(bean); + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new IllegalStateException(e); + } + } filter.setHandlerMappings(handlerMappings); } } diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java index 1436fce6f492..689efb9c09e5 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java @@ -15,9 +15,9 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; -import io.opentelemetry.javaagent.bootstrap.IndyProxy; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import java.lang.reflect.Field; import java.util.List; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -68,8 +68,20 @@ public static void afterRefresh( } Object bean = springCtx.getBean("otelAutoDispatcherFilter"); - OpenTelemetryHandlerMappingFilter filter = - IndyProxy.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); + OpenTelemetryHandlerMappingFilter filter; + if (bean instanceof OpenTelemetryHandlerMappingFilter) { + // TODO: remove this branch once advices are not inlined anymore as it's no longer relevant + // inline advice: no proxy class is used + filter = (OpenTelemetryHandlerMappingFilter) bean; + } else { + // non-inlined advice: proxy class is used + try { + Field delegate = bean.getClass().getField("delegate"); + filter = (OpenTelemetryHandlerMappingFilter) delegate.get(bean); + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new IllegalStateException(e); + } + } filter.setHandlerMappings(handlerMappings); } } diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java deleted file mode 100644 index 3ed67762096b..000000000000 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.bootstrap; - -import java.lang.reflect.Field; - -/** Interface added to proxy classes injected for indy instrumentation */ -@SuppressWarnings("InterfaceWithOnlyStatics") // TODO: remove this once we have a method -public interface IndyProxy { - - /** - * Unwraps the proxied object - * - * @return unwrapped object - */ - default Object unwrap() { - try { - // current implementation based on introspection + public delegate field - // TODO: replace this implementation with proper implementation on the proxy class - Field delegate = this.getClass().getField("delegate"); - return delegate.get(this); - } catch (NoSuchFieldException | IllegalAccessException e) { - throw new IllegalStateException(e); - } - } - - /** - * Transparently unwraps an object that might have been proxied for indy instrumentation. When - * unwrapping is not needed, for example for inlined advices, the original object is returned - * effectively making this equivalent to a type cast. - * - * @param o object that might need unwrapping - * @param type expected unwrapped object type - * @param type of the proxied object - * @return unwrapped object - * @throws IllegalArgumentException when object can't be cast or unwrapped to the desired type - */ - static T unwrapIfNeeded(Object o, Class type) { - if (type.isAssignableFrom(o.getClass())) { - return type.cast(o); - } - if (o instanceof IndyProxy) { - Object unwrapped = ((IndyProxy) o).unwrap(); - if (type.isAssignableFrom(unwrapped.getClass())) { - return type.cast(unwrapped); - } - } - throw new IllegalArgumentException("unexpected object unwrap"); - } -} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java index 20f16ed7c453..56cd8fb62a74 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java @@ -5,15 +5,12 @@ package io.opentelemetry.javaagent.tooling.instrumentation.indy; -import io.opentelemetry.javaagent.bootstrap.IndyProxy; import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.util.ArrayList; import java.util.List; import net.bytebuddy.ByteBuddy; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.method.ParameterDescription; -import net.bytebuddy.description.type.TypeDefinition; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.dynamic.DynamicType; import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy; @@ -90,12 +87,10 @@ public IndyProxyFactory(Method bootstrapMethod, BootstrapArgsProvider bootstrapA public DynamicType.Unloaded generateProxy( TypeDescription classToProxy, String proxyClassName) { TypeDescription.Generic superClass = classToProxy.getSuperClass(); - List interfaces = new ArrayList<>(classToProxy.getInterfaces()); - interfaces.add(TypeDescription.ForLoadedType.of(IndyProxy.class)); DynamicType.Builder builder = new ByteBuddy() .subclass(superClass, ConstructorStrategy.Default.NO_CONSTRUCTORS) - .implement(interfaces) + .implement(classToProxy.getInterfaces()) .name(proxyClassName) .annotateType(classToProxy.getDeclaredAnnotations()) // field must be public to enable resolving the proxy target using introspection diff --git a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java index 047254e71ecd..c5d1c287013b 100644 --- a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java +++ b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java @@ -7,7 +7,6 @@ import static org.assertj.core.api.Assertions.assertThat; -import io.opentelemetry.javaagent.bootstrap.IndyProxy; import io.opentelemetry.javaagent.tooling.instrumentation.indy.dummies.DummyAnnotation; import java.lang.invoke.CallSite; import java.lang.invoke.ConstantCallSite; @@ -320,28 +319,6 @@ void verifyNonPublicMembersIgnored() throws Exception { .anySatisfy(method -> assertThat(method.getName()).isEqualTo("publicStaticMethod")); } - @Test - void verifyUnwrap() throws Exception { - Class proxy = generateProxy(WrappingTest.class); - Object proxyInstance = proxy.getConstructor().newInstance(); - assertThat(proxyInstance) - .describedAs("proxies should implement IndyProxy") - .isNotInstanceOf(WrappingTest.class) - .isInstanceOf(IndyProxy.class); - - WrappingTest unwrapped = IndyProxy.unwrapIfNeeded(proxyInstance, WrappingTest.class); - assertThat(unwrapped).isInstanceOf(WrappingTest.class).isNotInstanceOf(IndyProxy.class); - - assertThat(IndyProxy.unwrapIfNeeded(unwrapped, WrappingTest.class)) - .describedAs("unwrap an already unwrapped is a no-op") - .isSameAs(unwrapped); - } - - public static class WrappingTest { - - public WrappingTest() {} - } - private static Class generateProxy(Class clazz) { DynamicType.Unloaded unloaded = proxyFactory.generateProxy( From f330abb298431db5a1d3ee96b9b5a0673ee6083d Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:06:58 +0100 Subject: [PATCH 19/35] another attempt with helper class --- .../DispatcherServletInstrumentation.java | 10 ++---- .../DispatcherServletInstrumentation.java | 10 ++---- .../javaagent/bootstrap/IndyProxyHelper.java | 32 +++++++++++++++++++ 3 files changed, 36 insertions(+), 16 deletions(-) create mode 100644 javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java index 1b61fcf2fa96..80dcab75ae6f 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java @@ -15,9 +15,9 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; +import io.opentelemetry.javaagent.bootstrap.IndyProxyHelper; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import java.lang.reflect.Field; import java.util.List; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -73,13 +73,7 @@ public static void afterRefresh( // inline advice: no proxy class is used filter = (OpenTelemetryHandlerMappingFilter) bean; } else { - // non-inlined advice: proxy class is used - try { - Field delegate = bean.getClass().getField("delegate"); - filter = (OpenTelemetryHandlerMappingFilter) delegate.get(bean); - } catch (NoSuchFieldException | IllegalAccessException e) { - throw new IllegalStateException(e); - } + filter = IndyProxyHelper.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); } filter.setHandlerMappings(handlerMappings); } diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java index 689efb9c09e5..1b977dfc4bf5 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java @@ -15,9 +15,9 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; +import io.opentelemetry.javaagent.bootstrap.IndyProxyHelper; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import java.lang.reflect.Field; import java.util.List; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -74,13 +74,7 @@ public static void afterRefresh( // inline advice: no proxy class is used filter = (OpenTelemetryHandlerMappingFilter) bean; } else { - // non-inlined advice: proxy class is used - try { - Field delegate = bean.getClass().getField("delegate"); - filter = (OpenTelemetryHandlerMappingFilter) delegate.get(bean); - } catch (NoSuchFieldException | IllegalAccessException e) { - throw new IllegalStateException(e); - } + filter = IndyProxyHelper.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); } filter.setHandlerMappings(handlerMappings); } diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java new file mode 100644 index 000000000000..edb135f38165 --- /dev/null +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java @@ -0,0 +1,32 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.bootstrap; + +import java.lang.reflect.Field; + +public class IndyProxyHelper { + + private IndyProxyHelper() { + } + + public static T unwrapIfNeeded(Object o, Class type) { + if (type.isAssignableFrom(o.getClass())) { + return type.cast(o); + } + // public delegate field on indy proxy + try { + Field delegateField = o.getClass().getField("delegate"); + Object delegate = delegateField.get(o); + if (type.isAssignableFrom(delegate.getClass())) { + return type.cast(delegate); + } + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new IllegalStateException(e); + } + + return null; + } +} From abc58f840fc32f3ecfa4af995667a5399ba9917b Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:19:52 +0100 Subject: [PATCH 20/35] code reformat --- .../io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java index edb135f38165..9ecd909cc4ae 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java @@ -9,8 +9,7 @@ public class IndyProxyHelper { - private IndyProxyHelper() { - } + private IndyProxyHelper() {} public static T unwrapIfNeeded(Object o, Class type) { if (type.isAssignableFrom(o.getClass())) { From 6d9198bd6614ff690fbc2da5b2dd14b3b281ff87 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:22:27 +0100 Subject: [PATCH 21/35] further simplify caller code --- .../webmvc/v3_1/DispatcherServletInstrumentation.java | 11 ++--------- .../webmvc/v6_0/DispatcherServletInstrumentation.java | 10 ++-------- .../javaagent/bootstrap/IndyProxyHelper.java | 2 +- 3 files changed, 5 insertions(+), 18 deletions(-) diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java index 80dcab75ae6f..1a736c71ad6d 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java @@ -67,15 +67,8 @@ public static void afterRefresh( return; } Object bean = springCtx.getBean("otelAutoDispatcherFilter"); - OpenTelemetryHandlerMappingFilter filter; - if (bean instanceof OpenTelemetryHandlerMappingFilter) { - // TODO: remove this branch once advices are not inlined anymore as it's no longer relevant - // inline advice: no proxy class is used - filter = (OpenTelemetryHandlerMappingFilter) bean; - } else { - filter = IndyProxyHelper.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); - } - filter.setHandlerMappings(handlerMappings); + OpenTelemetryHandlerMappingFilter filter = + IndyProxyHelper.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); } } diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java index 1b977dfc4bf5..94057be8987c 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java @@ -68,14 +68,8 @@ public static void afterRefresh( } Object bean = springCtx.getBean("otelAutoDispatcherFilter"); - OpenTelemetryHandlerMappingFilter filter; - if (bean instanceof OpenTelemetryHandlerMappingFilter) { - // TODO: remove this branch once advices are not inlined anymore as it's no longer relevant - // inline advice: no proxy class is used - filter = (OpenTelemetryHandlerMappingFilter) bean; - } else { - filter = IndyProxyHelper.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); - } + OpenTelemetryHandlerMappingFilter filter = + IndyProxyHelper.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); filter.setHandlerMappings(handlerMappings); } } diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java index 9ecd909cc4ae..360e0e4945b7 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java @@ -26,6 +26,6 @@ public static T unwrapIfNeeded(Object o, Class type) { throw new IllegalStateException(e); } - return null; + throw new IllegalArgumentException("unexpected object type"); } } From bceeb00e69b135f93af006309d9163b8166dd204 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:45:26 +0100 Subject: [PATCH 22/35] fix pebkc --- .../spring/webmvc/v3_1/DispatcherServletInstrumentation.java | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java index 1a736c71ad6d..1b04adc3e07f 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java @@ -69,6 +69,7 @@ public static void afterRefresh( Object bean = springCtx.getBean("otelAutoDispatcherFilter"); OpenTelemetryHandlerMappingFilter filter = IndyProxyHelper.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); + filter.setHandlerMappings(handlerMappings); } } From 8708bf791f0d09885a9ec21b53ca54320ef3b199 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 14 Nov 2024 17:15:16 +0100 Subject: [PATCH 23/35] do not rely on public delegate field --- .../internal/reflection/ReflectionHelper.java | 15 ++++++++-- .../javaagent/bootstrap/IndyProxy.java | 14 +++++++++ .../javaagent/bootstrap/IndyProxyHelper.java | 11 ++----- .../indy/IndyProxyFactory.java | 20 +++++++++++-- .../indy/IndyProxyFactoryTest.java | 30 +++++++++++++++++-- 5 files changed, 74 insertions(+), 16 deletions(-) create mode 100644 javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java 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 241c731f1469..3706b16e4129 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 @@ -5,6 +5,7 @@ package io.opentelemetry.javaagent.instrumentation.internal.reflection; +import io.opentelemetry.javaagent.bootstrap.IndyProxy; import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker; import io.opentelemetry.javaagent.bootstrap.VirtualFieldDetector; import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker; @@ -38,8 +39,9 @@ public static Field[] filterFields(Class containingClass, Field[] fields) { public static Method[] filterMethods(Class containingClass, Method[] methods) { if (methods.length == 0 - || !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass)) { - // nothing to filter when class does not have any added virtual fields + || !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass) + || !IndyProxy.class.isAssignableFrom(containingClass)) { + // nothing to filter when class does not have any added virtual fields or is not a proxy return methods; } List result = new ArrayList<>(methods.length); @@ -50,6 +52,9 @@ public static Method[] filterMethods(Class containingClass, Method[] methods) || method.getName().startsWith("__set__opentelemetryVirtualField$"))) { continue; } + if (method.getName().equals("__getIndyProxyDelegate")) { + continue; + } result.add(method); } return result.toArray(new Method[0]); @@ -58,8 +63,10 @@ public static Method[] filterMethods(Class containingClass, Method[] methods) @SuppressWarnings("unused") public static Class[] filterInterfaces(Class[] interfaces, Class containingClass) { if (interfaces.length == 0 - || !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass)) { + || !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass) + || !IndyProxy.class.isAssignableFrom(containingClass)) { // nothing to filter when class does not have any added virtual fields + System.out.println("no interface to filter" + containingClass); return interfaces; } List> result = new ArrayList<>(interfaces.length); @@ -74,6 +81,8 @@ public static Class[] filterInterfaces(Class[] interfaces, Class contai && interfaceClass.getName().contains("VirtualFieldAccessor$")) { virtualFieldClassNames.add(interfaceClass.getName()); continue; + } else if (IndyProxy.class.isAssignableFrom(interfaceClass)) { + continue; } result.add(interfaceClass); } diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java new file mode 100644 index 000000000000..eb32b0095db2 --- /dev/null +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java @@ -0,0 +1,14 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.bootstrap; + +public interface IndyProxy { + + // Method name does not fit common naming practices on purpose + // Also, when modifying this make sure to also update string references. + @SuppressWarnings("all") + Object __getIndyProxyDelegate(); +} diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java index 360e0e4945b7..4e1de5372433 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java @@ -5,8 +5,6 @@ package io.opentelemetry.javaagent.bootstrap; -import java.lang.reflect.Field; - public class IndyProxyHelper { private IndyProxyHelper() {} @@ -15,15 +13,12 @@ public static T unwrapIfNeeded(Object o, Class type) { if (type.isAssignableFrom(o.getClass())) { return type.cast(o); } - // public delegate field on indy proxy - try { - Field delegateField = o.getClass().getField("delegate"); - Object delegate = delegateField.get(o); + + if (o instanceof IndyProxy) { + Object delegate = ((IndyProxy) o).__getIndyProxyDelegate(); if (type.isAssignableFrom(delegate.getClass())) { return type.cast(delegate); } - } catch (NoSuchFieldException | IllegalAccessException e) { - throw new IllegalStateException(e); } throw new IllegalArgumentException("unexpected object type"); diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java index 56cd8fb62a74..d092eed5b1f6 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java @@ -5,12 +5,15 @@ package io.opentelemetry.javaagent.tooling.instrumentation.indy; +import io.opentelemetry.javaagent.bootstrap.IndyProxy; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.util.ArrayList; import java.util.List; import net.bytebuddy.ByteBuddy; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.method.ParameterDescription; +import net.bytebuddy.description.type.TypeDefinition; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.dynamic.DynamicType; import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy; @@ -68,6 +71,9 @@ List getBootstrapArgsForMethod( private static final String DELEGATE_FIELD_NAME = "delegate"; + // Matches the single method of IndyProxy interface + private static final String PROXY_DELEGATE_NAME = "__getIndyProxyDelegate"; + private final MethodDescription.InDefinedShape indyBootstrapMethod; private final BootstrapArgsProvider bootstrapArgsProvider; @@ -87,14 +93,15 @@ public IndyProxyFactory(Method bootstrapMethod, BootstrapArgsProvider bootstrapA public DynamicType.Unloaded generateProxy( TypeDescription classToProxy, String proxyClassName) { TypeDescription.Generic superClass = classToProxy.getSuperClass(); + List interfaces = new ArrayList<>(classToProxy.getInterfaces()); + interfaces.add(TypeDescription.ForLoadedType.of(IndyProxy.class)); DynamicType.Builder builder = new ByteBuddy() .subclass(superClass, ConstructorStrategy.Default.NO_CONSTRUCTORS) - .implement(classToProxy.getInterfaces()) + .implement(interfaces) .name(proxyClassName) .annotateType(classToProxy.getDeclaredAnnotations()) - // field must be public to enable resolving the proxy target using introspection - .defineField(DELEGATE_FIELD_NAME, Object.class, Modifier.PUBLIC | Modifier.FINAL); + .defineField(DELEGATE_FIELD_NAME, Object.class, Modifier.PRIVATE | Modifier.FINAL); for (MethodDescription.InDefinedShape method : classToProxy.getDeclaredMethods()) { if (method.isPublic()) { @@ -109,6 +116,13 @@ public DynamicType.Unloaded generateProxy( } } } + + // Implement IndyProxy class and return the delegate field + builder = + builder + .defineMethod(PROXY_DELEGATE_NAME, Object.class, Modifier.PUBLIC) + .intercept(FieldAccessor.ofField(DELEGATE_FIELD_NAME)); + return builder.make(); } diff --git a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java index c5d1c287013b..3d3c57eed4c4 100644 --- a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java +++ b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java @@ -7,6 +7,7 @@ import static org.assertj.core.api.Assertions.assertThat; +import io.opentelemetry.javaagent.bootstrap.IndyProxy; import io.opentelemetry.javaagent.tooling.instrumentation.indy.dummies.DummyAnnotation; import java.lang.invoke.CallSite; import java.lang.invoke.ConstantCallSite; @@ -319,12 +320,37 @@ void verifyNonPublicMembersIgnored() throws Exception { .anySatisfy(method -> assertThat(method.getName()).isEqualTo("publicStaticMethod")); } + @Test + void verifyProxyClass() throws Exception { + Class proxyType = generateProxy(ProxyUnwrapTest.class); + assertThat(proxyType).isNotInstanceOf(IndyProxy.class).isNotInstanceOf(ProxyUnwrapTest.class); + + assertThat(IndyProxy.class.isAssignableFrom(proxyType)) + .describedAs("proxy class can be cast to IndyProxy") + .isTrue(); + + Object proxyInstance = proxyType.getConstructor().newInstance(); + Object proxyDelegate = ((IndyProxy) proxyInstance).__getIndyProxyDelegate(); + assertThat(proxyDelegate).isInstanceOf(ProxyUnwrapTest.class); + } + + @SuppressWarnings("all") + public static class ProxyUnwrapTest { + public ProxyUnwrapTest() {} + + public void sampleMethod() {} + } + private static Class generateProxy(Class clazz) { DynamicType.Unloaded unloaded = proxyFactory.generateProxy( TypeDescription.ForLoadedType.of(clazz), clazz.getName() + "Proxy"); - // Uncomment the following line to view the generated bytecode if needed - // unloaded.saveIn(new File("generated_proxies")); + // Uncomment the following block to view the generated bytecode if needed + // try { + // unloaded.saveIn(new File("build/generated_proxies")); + // } catch (IOException e) { + // throw new RuntimeException(e); + // } return unloaded.load(clazz.getClassLoader()).getLoaded(); } } From 59552ab9c27ccc20de33f1e3b4cd943aabe58cf7 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 14 Nov 2024 17:28:05 +0100 Subject: [PATCH 24/35] pebkc printf debug --- .../instrumentation/internal/reflection/ReflectionHelper.java | 1 - 1 file changed, 1 deletion(-) 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 3706b16e4129..849cdea92601 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 @@ -66,7 +66,6 @@ public static Class[] filterInterfaces(Class[] interfaces, Class contai || !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass) || !IndyProxy.class.isAssignableFrom(containingClass)) { // nothing to filter when class does not have any added virtual fields - System.out.println("no interface to filter" + containingClass); return interfaces; } List> result = new ArrayList<>(interfaces.length); From 56a52b1fb0bb0ed4da08a46924005dc2252f5fa1 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 14 Nov 2024 18:38:16 +0100 Subject: [PATCH 25/35] fix test on proxy class --- .../tooling/instrumentation/indy/IndyProxyFactory.java | 2 +- .../tooling/instrumentation/indy/IndyProxyFactoryTest.java | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java index d092eed5b1f6..504b8d5f7b11 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java @@ -72,7 +72,7 @@ List getBootstrapArgsForMethod( private static final String DELEGATE_FIELD_NAME = "delegate"; // Matches the single method of IndyProxy interface - private static final String PROXY_DELEGATE_NAME = "__getIndyProxyDelegate"; + static final String PROXY_DELEGATE_NAME = "__getIndyProxyDelegate"; private final MethodDescription.InDefinedShape indyBootstrapMethod; diff --git a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java index 3d3c57eed4c4..3b5b526c6ee2 100644 --- a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java +++ b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java @@ -315,9 +315,11 @@ void verifyNonPublicMembersIgnored() throws Exception { assertThat(proxy.getConstructors()).hasSize(1); assertThat(proxy.getDeclaredMethods()) - .hasSize(2) + .hasSize(3) .anySatisfy(method -> assertThat(method.getName()).isEqualTo("publicMethod")) - .anySatisfy(method -> assertThat(method.getName()).isEqualTo("publicStaticMethod")); + .anySatisfy(method -> assertThat(method.getName()).isEqualTo("publicStaticMethod")) + .anySatisfy(method -> assertThat(method.getName()).isEqualTo( + IndyProxyFactory.PROXY_DELEGATE_NAME)); } @Test From c468d478b892e45e951598d0aeee22f6218c85a5 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 19 Nov 2024 09:39:15 +0100 Subject: [PATCH 26/35] hide IndyProxy interface from reflection --- .../internal/reflection/ReflectionHelper.java | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) 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 849cdea92601..78a9fb78598a 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 @@ -38,22 +38,20 @@ public static Field[] filterFields(Class containingClass, Field[] fields) { } public static Method[] filterMethods(Class containingClass, Method[] methods) { - if (methods.length == 0 - || !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass) - || !IndyProxy.class.isAssignableFrom(containingClass)) { - // nothing to filter when class does not have any added virtual fields or is not a proxy + // nothing to filter when class does not have any added virtual fields or is not a proxy + if (methods.length == 0 || noInterfaceToHide(containingClass)) { return methods; } List result = new ArrayList<>(methods.length); for (Method method : methods) { - // virtual field accessor methods are marked as synthetic - if (method.isSynthetic() - && (method.getName().startsWith("__get__opentelemetryVirtualField$") - || method.getName().startsWith("__set__opentelemetryVirtualField$"))) { - continue; - } - if (method.getName().equals("__getIndyProxyDelegate")) { - continue; + // virtual field accessor or proxy methods are marked as synthetic + if (method.isSynthetic()) { + String name = method.getName(); + if ((name.startsWith("__get__opentelemetryVirtualField$") + || name.startsWith("__set__opentelemetryVirtualField$") + || name.equals("__getIndyProxyDelegate"))) { + continue; + } } result.add(method); } @@ -62,10 +60,8 @@ public static Method[] filterMethods(Class containingClass, Method[] methods) @SuppressWarnings("unused") public static Class[] filterInterfaces(Class[] interfaces, Class containingClass) { - if (interfaces.length == 0 - || !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass) - || !IndyProxy.class.isAssignableFrom(containingClass)) { - // nothing to filter when class does not have any added virtual fields + // nothing to filter when class does not have any added virtual fields or is not a proxy + if (interfaces.length == 0 || noInterfaceToHide(containingClass)) { return interfaces; } List> result = new ArrayList<>(interfaces.length); @@ -75,21 +71,24 @@ public static Class[] filterInterfaces(Class[] interfaces, Class contai // filter out virtual field marker and accessor interfaces if (interfaceClass == VirtualFieldInstalledMarker.class) { continue; + } else if (interfaceClass == IndyProxy.class) { + continue; } else if (VirtualFieldAccessorMarker.class.isAssignableFrom(interfaceClass) && interfaceClass.isSynthetic() && interfaceClass.getName().contains("VirtualFieldAccessor$")) { virtualFieldClassNames.add(interfaceClass.getName()); continue; - } else if (IndyProxy.class.isAssignableFrom(interfaceClass)) { - continue; } result.add(interfaceClass); } - if (!virtualFieldClassNames.isEmpty()) { VirtualFieldDetector.markVirtualFields(containingClass, virtualFieldClassNames); } - return result.toArray(new Class[0]); } + + private static boolean noInterfaceToHide(Class containingClass) { + return !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass) + && !IndyProxy.class.isAssignableFrom(containingClass); + } } From 20a95bce6bd806e31c184826d9746c68e58846d2 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 19 Nov 2024 09:41:15 +0100 Subject: [PATCH 27/35] indy proxy method impl should be synthetic --- .../tooling/instrumentation/indy/IndyProxyFactory.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java index 504b8d5f7b11..bcd9e166d311 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java @@ -13,6 +13,8 @@ import net.bytebuddy.ByteBuddy; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.method.ParameterDescription; +import net.bytebuddy.description.modifier.SyntheticState; +import net.bytebuddy.description.modifier.Visibility; import net.bytebuddy.description.type.TypeDefinition; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.dynamic.DynamicType; @@ -120,7 +122,8 @@ public DynamicType.Unloaded generateProxy( // Implement IndyProxy class and return the delegate field builder = builder - .defineMethod(PROXY_DELEGATE_NAME, Object.class, Modifier.PUBLIC) + .defineMethod( + PROXY_DELEGATE_NAME, Object.class, Visibility.PUBLIC, SyntheticState.SYNTHETIC) .intercept(FieldAccessor.ofField(DELEGATE_FIELD_NAME)); return builder.make(); From 56f1aa93f1140457d0c0e12f5c354327e0e9d792 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 19 Nov 2024 09:57:22 +0100 Subject: [PATCH 28/35] update proxy factory test --- .../instrumentation/indy/IndyProxyFactoryTest.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java index 3b5b526c6ee2..c8d09654063f 100644 --- a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java +++ b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java @@ -310,7 +310,7 @@ private static void privateStaticMethod() {} } @Test - void verifyNonPublicMembersIgnored() throws Exception { + void verifyNonPublicMembersIgnored() { Class proxy = generateProxy(IgnoreNonPublicMethods.class); assertThat(proxy.getConstructors()).hasSize(1); @@ -318,8 +318,12 @@ void verifyNonPublicMembersIgnored() throws Exception { .hasSize(3) .anySatisfy(method -> assertThat(method.getName()).isEqualTo("publicMethod")) .anySatisfy(method -> assertThat(method.getName()).isEqualTo("publicStaticMethod")) - .anySatisfy(method -> assertThat(method.getName()).isEqualTo( - IndyProxyFactory.PROXY_DELEGATE_NAME)); + // IndyProxy implementation visible but later hidden by reflection instrumentation + .anySatisfy( + method -> { + assertThat(method.getName()).isEqualTo(IndyProxyFactory.PROXY_DELEGATE_NAME); + assertThat(method.isSynthetic()).isTrue(); + }); } @Test From c13b72e13184ec54aa376added2419b81fd192cf Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 19 Nov 2024 16:37:01 +0100 Subject: [PATCH 29/35] add a bit of javadoc --- .../opentelemetry/javaagent/bootstrap/IndyProxy.java | 8 ++++++++ .../javaagent/bootstrap/IndyProxyHelper.java | 10 ++++++++++ 2 files changed, 18 insertions(+) diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java index eb32b0095db2..c14b33329c20 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java @@ -5,8 +5,16 @@ package io.opentelemetry.javaagent.bootstrap; +/** + * Interface added to indy proxies to allow unwrapping the proxy object + */ public interface IndyProxy { + /** + * Unwraps the proxy delegate instance + * + * @return wrapped object instance + */ // Method name does not fit common naming practices on purpose // Also, when modifying this make sure to also update string references. @SuppressWarnings("all") diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java index 4e1de5372433..bdfb6398391f 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java @@ -9,6 +9,16 @@ public class IndyProxyHelper { private IndyProxyHelper() {} + /** + * Unwraps and casts an indy proxy, or just casts if it's not an indy proxy. + * + * @param type of object to return + * @param o object to unwrap + * @param type expected object type + * @return unwrapped proxy instance or the original object (if not a proxy) cast to the expected type + * @throws IllegalArgumentException if the provided object the proxied object can't be cast to the expected type + * + */ public static T unwrapIfNeeded(Object o, Class type) { if (type.isAssignableFrom(o.getClass())) { return type.cast(o); From 1806c8254cefb71d586a42e5b6b05cf7ae2ad27b Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 19 Nov 2024 17:02:19 +0100 Subject: [PATCH 30/35] test IndyProxyHelper --- .../javaagent/bootstrap/IndyProxy.java | 4 +- .../javaagent/bootstrap/IndyProxyHelper.java | 14 +++---- .../bootstrap/IndyProxyHelperTest.java | 40 +++++++++++++++++++ 3 files changed, 48 insertions(+), 10 deletions(-) create mode 100644 javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelperTest.java diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java index c14b33329c20..2e664fc428e5 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java @@ -5,9 +5,7 @@ package io.opentelemetry.javaagent.bootstrap; -/** - * Interface added to indy proxies to allow unwrapping the proxy object - */ +/** Interface added to indy proxies to allow unwrapping the proxy object */ public interface IndyProxy { /** diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java index bdfb6398391f..b7174a614ed0 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java @@ -15,21 +15,21 @@ private IndyProxyHelper() {} * @param type of object to return * @param o object to unwrap * @param type expected object type - * @return unwrapped proxy instance or the original object (if not a proxy) cast to the expected type - * @throws IllegalArgumentException if the provided object the proxied object can't be cast to the expected type - * + * @return unwrapped proxy instance or the original object (if not a proxy) cast to the expected + * type + * @throws IllegalArgumentException if the provided object the proxied object can't be cast to the + * expected type */ public static T unwrapIfNeeded(Object o, Class type) { - if (type.isAssignableFrom(o.getClass())) { - return type.cast(o); - } - if (o instanceof IndyProxy) { Object delegate = ((IndyProxy) o).__getIndyProxyDelegate(); if (type.isAssignableFrom(delegate.getClass())) { return type.cast(delegate); } } + if (type.isAssignableFrom(o.getClass())) { + return type.cast(o); + } throw new IllegalArgumentException("unexpected object type"); } diff --git a/javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelperTest.java b/javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelperTest.java new file mode 100644 index 000000000000..6686abbbbc92 --- /dev/null +++ b/javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelperTest.java @@ -0,0 +1,40 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.bootstrap; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; + +class IndyProxyHelperTest { + + @Test + void wrongType() { + assertThrows( + IllegalArgumentException.class, + () -> IndyProxyHelper.unwrapIfNeeded("", Integer.class)); + assertThrows( + IllegalArgumentException.class, + () -> IndyProxyHelper.unwrapIfNeeded(proxy(""), Integer.class)); + } + + @Test + void unwrap() { + + // no wrapping + Number number = IndyProxyHelper.unwrapIfNeeded(42, Number.class); + assertThat(number).isEqualTo(42); + + // unwrap needed + String string = IndyProxyHelper.unwrapIfNeeded(proxy("hello"), String.class); + assertThat(string).isEqualTo("hello"); + } + + private static IndyProxy proxy(Object delegate) { + return () -> delegate; + } +} From 44977f28ab8bd9af984c407dee05e2372e378137 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 20 Nov 2024 14:02:54 +0100 Subject: [PATCH 31/35] spotless --- .../opentelemetry/javaagent/bootstrap/IndyProxyHelperTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelperTest.java b/javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelperTest.java index 6686abbbbc92..f476d9c01b22 100644 --- a/javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelperTest.java +++ b/javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelperTest.java @@ -15,8 +15,7 @@ class IndyProxyHelperTest { @Test void wrongType() { assertThrows( - IllegalArgumentException.class, - () -> IndyProxyHelper.unwrapIfNeeded("", Integer.class)); + IllegalArgumentException.class, () -> IndyProxyHelper.unwrapIfNeeded("", Integer.class)); assertThrows( IllegalArgumentException.class, () -> IndyProxyHelper.unwrapIfNeeded(proxy(""), Integer.class)); From 99cc98ac3ee154e18017212f8cd4cf52f047f14e Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 20 Nov 2024 17:04:22 +0100 Subject: [PATCH 32/35] add test for reflection method filtering --- .../java/instrumentation/TestHelperClass.java | 15 ++++++++ .../TestInstrumentationModule.java | 20 ++++++++++- .../src/test/java/ReflectionTest.java | 34 ++++++++++++++++++- .../src/test/java/TestClass.java | 9 +++++ 4 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestHelperClass.java diff --git a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestHelperClass.java b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestHelperClass.java new file mode 100644 index 000000000000..fa310587f74f --- /dev/null +++ b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestHelperClass.java @@ -0,0 +1,15 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package instrumentation; + +/** + * Class that will be injected in target classloader with inline instrumentation and proxied with + * indy instrumentation + */ +public class TestHelperClass { + + public TestHelperClass() {} +} diff --git a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule.java b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule.java index d9e406633db8..67a74f26e7ad 100644 --- a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule.java +++ b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule.java @@ -10,10 +10,15 @@ 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.Arrays; import java.util.List; @AutoService(InstrumentationModule.class) -public class TestInstrumentationModule extends InstrumentationModule { +public class TestInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public TestInstrumentationModule() { super("test-instrumentation"); } @@ -22,4 +27,17 @@ public TestInstrumentationModule() { public List typeInstrumentations() { return singletonList(new TestTypeInstrumentation()); } + + @Override + public List getAdditionalHelperClassNames() { + // makes the class from instrumentation from the instrumented class with inlined advice + return Arrays.asList("instrumentation.TestHelperClass"); + } + + @Override + public void injectClasses(ClassInjector injector) { + injector + .proxyBuilder("instrumentation.TestHelperClass") + .inject(InjectionMode.CLASS_AND_RESOURCE); + } } 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 5d142fbd4d0c..f75f892d7879 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 @@ -5,6 +5,8 @@ import static org.assertj.core.api.Assertions.assertThat; +import instrumentation.TestHelperClass; +import io.opentelemetry.javaagent.bootstrap.IndyProxy; import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker; import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker; import java.io.ObjectStreamClass; @@ -42,7 +44,37 @@ void testOurFieldsAndMethodsAreNotVisibleWithReflection() { void testGeneratedSerialVersionUid() { // expected value is computed with serialver utility that comes with jdk assertThat(ObjectStreamClass.lookup(TestClass.class).getSerialVersionUID()) - .isEqualTo(-1508684692096503670L); + .isEqualTo(-4292813100633930936L); assertThat(TestClass.class.getDeclaredFields().length).isEqualTo(0); } + + @Test + void testInjectedClassProxyUnwrap() throws Exception { + TestClass testClass = new TestClass(); + Class helperType = testClass.testHelperClass(); + assertThat(helperType) + .describedAs("unable to resolve injected class from instrumented class") + .isNotNull(); + + Object instance = helperType.getConstructor().newInstance(); + if (IndyProxy.class.isAssignableFrom(helperType)) { + // indy advice: must be an indy proxy + + for (Method method : helperType.getMethods()) { + assertThat(method.getName()) + .describedAs("proxy method must be hidden from reflection") + .isNotEqualTo("__getIndyProxyDelegate"); + } + + assertThat(instance).isInstanceOf(IndyProxy.class); + + Object proxyDelegate = ((IndyProxy) instance).__getIndyProxyDelegate(); + assertThat(proxyDelegate).isNotInstanceOf(IndyProxy.class); + + } else { + // inline advice: must be of the expected type + assertThat(helperType).isEqualTo(TestHelperClass.class); + assertThat(instance).isInstanceOf(TestHelperClass.class).isNotInstanceOf(IndyProxy.class); + } + } } diff --git a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/TestClass.java b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/TestClass.java index 20d1fef8e12e..96f304b6b3a2 100644 --- a/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/TestClass.java +++ b/instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/TestClass.java @@ -17,4 +17,13 @@ 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; + } + } } From a43b667aeba9271e8f77eb113679d46d8c29c060 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 20 Nov 2024 17:13:26 +0100 Subject: [PATCH 33/35] also test interface is hidden from reflection --- .../src/test/java/ReflectionTest.java | 6 ++++++ 1 file changed, 6 insertions(+) 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 f75f892d7879..4b5e4ff45f06 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 @@ -66,6 +66,12 @@ void testInjectedClassProxyUnwrap() throws Exception { .isNotEqualTo("__getIndyProxyDelegate"); } + for (Class interfaceType : helperType.getInterfaces()) { + assertThat(interfaceType) + .describedAs("indy proxy interface must be hidden from reflection") + .isNotEqualTo(IndyProxy.class); + } + assertThat(instance).isInstanceOf(IndyProxy.class); Object proxyDelegate = ((IndyProxy) instance).__getIndyProxyDelegate(); From a5806c600bb35e202a191543631d32bf334b421f Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 27 Nov 2024 13:49:54 +0100 Subject: [PATCH 34/35] keep registerHelperResources for inlined --- .../v3_1/SpringWebInstrumentationModule.java | 16 ++++++++++++++++ .../web/v6_0/SpringWebInstrumentationModule.java | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java b/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java index bafcb24794cf..f257f527177a 100644 --- a/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java +++ b/instrumentation/spring/spring-web/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/v3_1/SpringWebInstrumentationModule.java @@ -10,6 +10,7 @@ import static net.bytebuddy.matcher.ElementMatchers.not; import com.google.auto.service.AutoService; +import io.opentelemetry.javaagent.extension.instrumentation.HelperResourceBuilder; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule; @@ -31,6 +32,21 @@ public ElementMatcher.Junction classLoaderMatcher() { .and(not(hasClassesNamed("org.springframework.web.ErrorResponse"))); } + @Override + public void registerHelperResources(HelperResourceBuilder helperResourceBuilder) { + if (!isIndyModule()) { + // make the filter class file loadable by ClassPathResource - in some cases (e.g. + // spring-guice, + // see https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/7428) + // Spring + // might want to read the class file metadata; this line will make the filter class file + // visible + // to the bean class loader + helperResourceBuilder.register( + "org/springframework/web/servlet/v3_1/OpenTelemetryHandlerMappingFilter.class"); + } + } + @Override public String getModuleGroup() { // depends on OpenTelemetryHandlerMappingFilter diff --git a/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java b/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java index 756c9a1f23e6..6a66e70cd866 100644 --- a/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java +++ b/instrumentation/spring/spring-web/spring-web-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/web/v6_0/SpringWebInstrumentationModule.java @@ -9,6 +9,7 @@ import static java.util.Collections.singletonList; import com.google.auto.service.AutoService; +import io.opentelemetry.javaagent.extension.instrumentation.HelperResourceBuilder; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule; @@ -29,6 +30,21 @@ public ElementMatcher.Junction classLoaderMatcher() { return hasClassesNamed("org.springframework.web.ErrorResponse"); } + @Override + public void registerHelperResources(HelperResourceBuilder helperResourceBuilder) { + if (!isIndyModule()) { + // make the filter class file loadable by ClassPathResource - in some cases (e.g. + // spring-guice, + // see https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/7428) + // Spring + // might want to read the class file metadata; this line will make the filter class file + // visible + // to the bean class loader + helperResourceBuilder.register( + "org/springframework/web/servlet/v6_0/OpenTelemetryHandlerMappingFilter.class"); + } + } + @Override public String getModuleGroup() { // depends on OpenTelemetryHandlerMappingFilter From 9def1a6001be1e8d08d982105c3c0366ebe81417 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 27 Nov 2024 14:10:22 +0100 Subject: [PATCH 35/35] IndyProxy > InstrumentationProxy --- .../src/test/java/ReflectionTest.java | 16 +++++++++------- .../internal/reflection/ReflectionHelper.java | 6 +++--- .../v3_1/DispatcherServletInstrumentation.java | 4 ++-- .../v6_0/DispatcherServletInstrumentation.java | 4 ++-- ...{IndyProxy.java => InstrumentationProxy.java} | 2 +- ...lper.java => InstrumentationProxyHelper.java} | 8 ++++---- ....java => InstrumentationProxyHelperTest.java} | 13 +++++++------ .../instrumentation/indy/IndyProxyFactory.java | 4 ++-- .../indy/IndyProxyFactoryTest.java | 10 ++++++---- 9 files changed, 36 insertions(+), 31 deletions(-) rename javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/{IndyProxy.java => InstrumentationProxy.java} (92%) rename javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/{IndyProxyHelper.java => InstrumentationProxyHelper.java} (81%) rename javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/{IndyProxyHelperTest.java => InstrumentationProxyHelperTest.java} (55%) 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 4b5e4ff45f06..7293136a21bb 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 @@ -6,7 +6,7 @@ import static org.assertj.core.api.Assertions.assertThat; import instrumentation.TestHelperClass; -import io.opentelemetry.javaagent.bootstrap.IndyProxy; +import io.opentelemetry.javaagent.bootstrap.InstrumentationProxy; import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker; import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker; import java.io.ObjectStreamClass; @@ -57,7 +57,7 @@ void testInjectedClassProxyUnwrap() throws Exception { .isNotNull(); Object instance = helperType.getConstructor().newInstance(); - if (IndyProxy.class.isAssignableFrom(helperType)) { + if (InstrumentationProxy.class.isAssignableFrom(helperType)) { // indy advice: must be an indy proxy for (Method method : helperType.getMethods()) { @@ -69,18 +69,20 @@ void testInjectedClassProxyUnwrap() throws Exception { for (Class interfaceType : helperType.getInterfaces()) { assertThat(interfaceType) .describedAs("indy proxy interface must be hidden from reflection") - .isNotEqualTo(IndyProxy.class); + .isNotEqualTo(InstrumentationProxy.class); } - assertThat(instance).isInstanceOf(IndyProxy.class); + assertThat(instance).isInstanceOf(InstrumentationProxy.class); - Object proxyDelegate = ((IndyProxy) instance).__getIndyProxyDelegate(); - assertThat(proxyDelegate).isNotInstanceOf(IndyProxy.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(IndyProxy.class); + assertThat(instance) + .isInstanceOf(TestHelperClass.class) + .isNotInstanceOf(InstrumentationProxy.class); } } } 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 78a9fb78598a..e38e67de2a71 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 @@ -5,7 +5,7 @@ package io.opentelemetry.javaagent.instrumentation.internal.reflection; -import io.opentelemetry.javaagent.bootstrap.IndyProxy; +import io.opentelemetry.javaagent.bootstrap.InstrumentationProxy; import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker; import io.opentelemetry.javaagent.bootstrap.VirtualFieldDetector; import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker; @@ -71,7 +71,7 @@ public static Class[] filterInterfaces(Class[] interfaces, Class contai // filter out virtual field marker and accessor interfaces if (interfaceClass == VirtualFieldInstalledMarker.class) { continue; - } else if (interfaceClass == IndyProxy.class) { + } else if (interfaceClass == InstrumentationProxy.class) { continue; } else if (VirtualFieldAccessorMarker.class.isAssignableFrom(interfaceClass) && interfaceClass.isSynthetic() @@ -89,6 +89,6 @@ public static Class[] filterInterfaces(Class[] interfaces, Class contai private static boolean noInterfaceToHide(Class containingClass) { return !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass) - && !IndyProxy.class.isAssignableFrom(containingClass); + && !InstrumentationProxy.class.isAssignableFrom(containingClass); } } diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java index 1b04adc3e07f..2b35744d63ed 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v3_1/DispatcherServletInstrumentation.java @@ -15,7 +15,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; -import io.opentelemetry.javaagent.bootstrap.IndyProxyHelper; +import io.opentelemetry.javaagent.bootstrap.InstrumentationProxyHelper; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import java.util.List; @@ -68,7 +68,7 @@ public static void afterRefresh( } Object bean = springCtx.getBean("otelAutoDispatcherFilter"); OpenTelemetryHandlerMappingFilter filter = - IndyProxyHelper.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); + InstrumentationProxyHelper.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); filter.setHandlerMappings(handlerMappings); } } diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java index 94057be8987c..826a1a174803 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webmvc/v6_0/DispatcherServletInstrumentation.java @@ -15,7 +15,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; -import io.opentelemetry.javaagent.bootstrap.IndyProxyHelper; +import io.opentelemetry.javaagent.bootstrap.InstrumentationProxyHelper; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import java.util.List; @@ -69,7 +69,7 @@ public static void afterRefresh( Object bean = springCtx.getBean("otelAutoDispatcherFilter"); OpenTelemetryHandlerMappingFilter filter = - IndyProxyHelper.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); + InstrumentationProxyHelper.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class); filter.setHandlerMappings(handlerMappings); } } diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxy.java similarity index 92% rename from javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java rename to javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxy.java index 2e664fc428e5..b92f0994dd09 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxy.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxy.java @@ -6,7 +6,7 @@ package io.opentelemetry.javaagent.bootstrap; /** Interface added to indy proxies to allow unwrapping the proxy object */ -public interface IndyProxy { +public interface InstrumentationProxy { /** * Unwraps the proxy delegate instance diff --git a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxyHelper.java similarity index 81% rename from javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java rename to javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxyHelper.java index b7174a614ed0..2a369ab9a224 100644 --- a/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelper.java +++ b/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxyHelper.java @@ -5,9 +5,9 @@ package io.opentelemetry.javaagent.bootstrap; -public class IndyProxyHelper { +public class InstrumentationProxyHelper { - private IndyProxyHelper() {} + private InstrumentationProxyHelper() {} /** * Unwraps and casts an indy proxy, or just casts if it's not an indy proxy. @@ -21,8 +21,8 @@ private IndyProxyHelper() {} * expected type */ public static T unwrapIfNeeded(Object o, Class type) { - if (o instanceof IndyProxy) { - Object delegate = ((IndyProxy) o).__getIndyProxyDelegate(); + if (o instanceof InstrumentationProxy) { + Object delegate = ((InstrumentationProxy) o).__getIndyProxyDelegate(); if (type.isAssignableFrom(delegate.getClass())) { return type.cast(delegate); } diff --git a/javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelperTest.java b/javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxyHelperTest.java similarity index 55% rename from javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelperTest.java rename to javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxyHelperTest.java index f476d9c01b22..996e414ea857 100644 --- a/javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/IndyProxyHelperTest.java +++ b/javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/InstrumentationProxyHelperTest.java @@ -10,30 +10,31 @@ import org.junit.jupiter.api.Test; -class IndyProxyHelperTest { +class InstrumentationProxyHelperTest { @Test void wrongType() { assertThrows( - IllegalArgumentException.class, () -> IndyProxyHelper.unwrapIfNeeded("", Integer.class)); + IllegalArgumentException.class, + () -> InstrumentationProxyHelper.unwrapIfNeeded("", Integer.class)); assertThrows( IllegalArgumentException.class, - () -> IndyProxyHelper.unwrapIfNeeded(proxy(""), Integer.class)); + () -> InstrumentationProxyHelper.unwrapIfNeeded(proxy(""), Integer.class)); } @Test void unwrap() { // no wrapping - Number number = IndyProxyHelper.unwrapIfNeeded(42, Number.class); + Number number = InstrumentationProxyHelper.unwrapIfNeeded(42, Number.class); assertThat(number).isEqualTo(42); // unwrap needed - String string = IndyProxyHelper.unwrapIfNeeded(proxy("hello"), String.class); + String string = InstrumentationProxyHelper.unwrapIfNeeded(proxy("hello"), String.class); assertThat(string).isEqualTo("hello"); } - private static IndyProxy proxy(Object delegate) { + private static InstrumentationProxy proxy(Object delegate) { return () -> delegate; } } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java index bcd9e166d311..9cca710ccda8 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactory.java @@ -5,7 +5,7 @@ package io.opentelemetry.javaagent.tooling.instrumentation.indy; -import io.opentelemetry.javaagent.bootstrap.IndyProxy; +import io.opentelemetry.javaagent.bootstrap.InstrumentationProxy; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.ArrayList; @@ -96,7 +96,7 @@ public DynamicType.Unloaded generateProxy( TypeDescription classToProxy, String proxyClassName) { TypeDescription.Generic superClass = classToProxy.getSuperClass(); List interfaces = new ArrayList<>(classToProxy.getInterfaces()); - interfaces.add(TypeDescription.ForLoadedType.of(IndyProxy.class)); + interfaces.add(TypeDescription.ForLoadedType.of(InstrumentationProxy.class)); DynamicType.Builder builder = new ByteBuddy() .subclass(superClass, ConstructorStrategy.Default.NO_CONSTRUCTORS) diff --git a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java index c8d09654063f..9bd9ecb78917 100644 --- a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java +++ b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyProxyFactoryTest.java @@ -7,7 +7,7 @@ import static org.assertj.core.api.Assertions.assertThat; -import io.opentelemetry.javaagent.bootstrap.IndyProxy; +import io.opentelemetry.javaagent.bootstrap.InstrumentationProxy; import io.opentelemetry.javaagent.tooling.instrumentation.indy.dummies.DummyAnnotation; import java.lang.invoke.CallSite; import java.lang.invoke.ConstantCallSite; @@ -329,14 +329,16 @@ void verifyNonPublicMembersIgnored() { @Test void verifyProxyClass() throws Exception { Class proxyType = generateProxy(ProxyUnwrapTest.class); - assertThat(proxyType).isNotInstanceOf(IndyProxy.class).isNotInstanceOf(ProxyUnwrapTest.class); + assertThat(proxyType) + .isNotInstanceOf(InstrumentationProxy.class) + .isNotInstanceOf(ProxyUnwrapTest.class); - assertThat(IndyProxy.class.isAssignableFrom(proxyType)) + assertThat(InstrumentationProxy.class.isAssignableFrom(proxyType)) .describedAs("proxy class can be cast to IndyProxy") .isTrue(); Object proxyInstance = proxyType.getConstructor().newInstance(); - Object proxyDelegate = ((IndyProxy) proxyInstance).__getIndyProxyDelegate(); + Object proxyDelegate = ((InstrumentationProxy) proxyInstance).__getIndyProxyDelegate(); assertThat(proxyDelegate).isInstanceOf(ProxyUnwrapTest.class); }