Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

make servlet3 + spring webmvc + jaxrs 2.0 indy compatible #12432

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
c73f5c6
make servlet3 indy compatible
SylvainJuge Oct 10, 2024
59f857e
spring web mvc
SylvainJuge Oct 10, 2024
7a0993d
wip but still broken
SylvainJuge Oct 14, 2024
e6e4f7a
fix clasloading of injected class
SylvainJuge Oct 15, 2024
37970e8
avoid stack overflow with proxy
SylvainJuge Oct 15, 2024
cdd978c
spotless
SylvainJuge Oct 15, 2024
30f5356
spotless again
SylvainJuge Oct 15, 2024
4e6ce38
remove debugging exceptions
SylvainJuge Oct 15, 2024
dba9e7b
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Oct 15, 2024
dbbbdea
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Oct 25, 2024
b043e83
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Oct 28, 2024
99d8bff
add comment to clarify public field access
SylvainJuge Oct 28, 2024
0c91cbc
turns out proxy delegate access is not needed
SylvainJuge Oct 28, 2024
5b3b408
fix spotless
SylvainJuge Oct 28, 2024
d2714ad
public delegate needed with indy
SylvainJuge Oct 28, 2024
978ce81
add a few comments to elaborate on servlet module
SylvainJuge Oct 29, 2024
8740eff
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Nov 4, 2024
43ddd93
post review comment
SylvainJuge Nov 6, 2024
da8d9bc
remove indy changes for grails
SylvainJuge Nov 6, 2024
affe804
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Nov 6, 2024
54a9f00
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Nov 12, 2024
a7f57cf
better proxy unwrap
SylvainJuge Nov 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand All @@ -26,11 +28,9 @@ public ElementMatcher.Junction<ClassLoader> 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() {
// depends on Servlet3SnippetInjectingResponseWrapper
return "servlet";
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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() {
Expand All @@ -34,9 +36,9 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
}

@Override
public boolean isIndyModule() {
// GrailsTest fails
return false;
public String getModuleGroup() {
// depends on servlet instrumentation
return "servlet";
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down Expand Up @@ -47,6 +49,12 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
"org.springframework.security.authentication.ObservationAuthenticationManager");
}

@Override
public String getModuleGroup() {
// depends on servlet instrumentation
return "servlet";
}

@Override
public List<TypeInstrumentation> typeInstrumentations() {
return singletonList(new HttpSecurityInstrumentation());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@
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 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");
}
Expand All @@ -31,13 +32,9 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
}

@Override
public void registerHelperResources(HelperResourceBuilder helperResourceBuilder) {
laurit marked this conversation as resolved.
Show resolved Hide resolved
// 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");
public String getModuleGroup() {
// depends on OpenTelemetryHandlerMappingFilter
return "servlet";
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@
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 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");
Expand All @@ -29,13 +30,9 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
}

@Override
public void registerHelperResources(HelperResourceBuilder helperResourceBuilder) {
// 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");
public String getModuleGroup() {
// depends on OpenTelemetryHandlerMappingFilter
return "servlet";
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

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.util.List;
Expand Down Expand Up @@ -61,13 +62,14 @@ public static class HandlerMappingAdvice {
public static void afterRefresh(
@Advice.Argument(0) ApplicationContext springCtx,
@Advice.FieldValue("handlerMappings") List<HandlerMapping> handlerMappings) {
if (springCtx.containsBean("otelAutoDispatcherFilter")) {
OpenTelemetryHandlerMappingFilter filter =
(OpenTelemetryHandlerMappingFilter) springCtx.getBean("otelAutoDispatcherFilter");
if (handlerMappings != null && filter != null) {
filter.setHandlerMappings(handlerMappings);
}

if (handlerMappings == null || !springCtx.containsBean("otelAutoDispatcherFilter")) {
return;
}
Object bean = springCtx.getBean("otelAutoDispatcherFilter");
OpenTelemetryHandlerMappingFilter filter =
IndyProxy.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class);
filter.setHandlerMappings(handlerMappings);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,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.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");
Expand All @@ -28,13 +32,21 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {

@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 boolean isIndyModule() {
return false;
public void injectClasses(ClassInjector injector) {
injector
.proxyBuilder("org.springframework.web.servlet.v3_1.OpenTelemetryHandlerMappingFilter")
.inject(InjectionMode.CLASS_AND_RESOURCE);
}

@Override
public String getModuleGroup() {
return "servlet";
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

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.util.List;
Expand Down Expand Up @@ -61,13 +62,15 @@ public static class HandlerMappingAdvice {
public static void afterRefresh(
@Advice.Argument(0) ApplicationContext springCtx,
@Advice.FieldValue("handlerMappings") List<HandlerMapping> handlerMappings) {
if (springCtx.containsBean("otelAutoDispatcherFilter")) {
OpenTelemetryHandlerMappingFilter filter =
(OpenTelemetryHandlerMappingFilter) springCtx.getBean("otelAutoDispatcherFilter");
if (handlerMappings != null) {
filter.setHandlerMappings(handlerMappings);
}

if (handlerMappings == null || !springCtx.containsBean("otelAutoDispatcherFilter")) {
return;
}

Object bean = springCtx.getBean("otelAutoDispatcherFilter");
OpenTelemetryHandlerMappingFilter filter =
IndyProxy.unwrapIfNeeded(bean, OpenTelemetryHandlerMappingFilter.class);
filter.setHandlerMappings(handlerMappings);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,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.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");
Expand All @@ -28,13 +32,21 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {

@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 boolean isIndyModule() {
return false;
public void injectClasses(ClassInjector injector) {
injector
.proxyBuilder("org.springframework.web.servlet.v6_0.OpenTelemetryHandlerMappingFilter")
.inject(InjectionMode.CLASS_AND_RESOURCE);
}

@Override
public String getModuleGroup() {
return "servlet";
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -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 <T> type of the proxied object
* @return unwrapped object
* @throws IllegalArgumentException when object can't be cast or unwrapped to the desired type
*/
static <T> T unwrapIfNeeded(Object o, Class<T> 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");
}
}
Loading
Loading