Skip to content

Commit 38c8f89

Browse files
lauritlaurit
authored
Move helper class to spring package so that loadClass can find it (#3718)
* Move helper class to spring package so that loadClass can find it * spotless * Add tests * Add comments * remove unneeded dependency * comments
1 parent 8d90462 commit 38c8f89

File tree

17 files changed

+498
-37
lines changed

17 files changed

+498
-37
lines changed

‎instrumentation/spring/spring-web-3.1/javaagent/build.gradle.kts

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
plugins {
2+
id("otel.javaagent-instrumentation")
3+
}
4+
5+
muzzle {
6+
pass {
7+
group.set("org.springframework")
8+
module.set("spring-web")
9+
versions.set("[3.1.0.RELEASE,]")
10+
// these versions depend on javax.faces:jsf-api:1.1 which was released as pom only
11+
skip("1.2.1", "1.2.2", "1.2.3", "1.2.4")
12+
assertInverse.set(true)
13+
}
14+
}
15+
16+
dependencies {
17+
compileOnly("org.springframework:spring-web:3.1.0.RELEASE")
18+
}

‎instrumentation/spring/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/SpringWebInstrumentationModule.java

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.javaagent.instrumentation.springweb;
7+
8+
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
9+
import static java.util.Collections.singletonList;
10+
11+
import com.google.auto.service.AutoService;
12+
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
13+
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
14+
import java.util.List;
15+
import net.bytebuddy.matcher.ElementMatcher;
16+
17+
@AutoService(InstrumentationModule.class)
18+
public class SpringWebInstrumentationModule extends InstrumentationModule {
19+
public SpringWebInstrumentationModule() {
20+
super("spring-web", "spring-web-3.1");
21+
}
22+
23+
@Override
24+
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
25+
// class added in 3.1
26+
return hasClassesNamed("org.springframework.web.method.HandlerMethod");
27+
}
28+
29+
@Override
30+
public List<TypeInstrumentation> typeInstrumentations() {
31+
return singletonList(new WebApplicationContextInstrumentation());
32+
}
33+
}

‎instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/WebApplicationContextInstrumentation.java ‎instrumentation/spring/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/WebApplicationContextInstrumentation.java

+26-5
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,15 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
package io.opentelemetry.javaagent.instrumentation.springwebmvc;
6+
package io.opentelemetry.javaagent.instrumentation.springweb;
77

88
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.extendsClass;
99
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
1010
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
1111
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
1212
import static net.bytebuddy.matcher.ElementMatchers.named;
1313
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
14+
import static org.springframework.beans.factory.config.BeanDefinition.SCOPE_SINGLETON;
1415

1516
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
1617
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
@@ -19,9 +20,10 @@
1920
import net.bytebuddy.matcher.ElementMatcher;
2021
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
2122
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
23+
import org.springframework.beans.factory.support.GenericBeanDefinition;
2224

2325
/**
24-
* This instrumentation adds the HandlerMappingResourceNameFilter definition to the spring context
26+
* This instrumentation adds the OpenTelemetryHandlerMappingFilter definition to the spring context
2527
* When the context is created, the filter will be added to the beginning of the filter chain.
2628
*/
2729
public class WebApplicationContextInstrumentation implements TypeInstrumentation {
@@ -59,10 +61,29 @@ public static class FilterInjectingAdvice {
5961
public static void onEnter(@Advice.Argument(0) ConfigurableListableBeanFactory beanFactory) {
6062
if (beanFactory instanceof BeanDefinitionRegistry
6163
&& !beanFactory.containsBean("otelAutoDispatcherFilter")) {
64+
try {
65+
// Firstly check whether DispatcherServlet is present. We need to load an instrumented
66+
// class from spring-webmvc to trigger injection that makes
67+
// OpenTelemetryHandlerMappingFilter available.
68+
beanFactory
69+
.getBeanClassLoader()
70+
.loadClass("org.springframework.web.servlet.DispatcherServlet");
6271

63-
((BeanDefinitionRegistry) beanFactory)
64-
.registerBeanDefinition(
65-
"otelAutoDispatcherFilter", new HandlerMappingResourceNameFilter.BeanDefinition());
72+
// Now attempt to load our injected instrumentation class.
73+
Class<?> clazz =
74+
beanFactory
75+
.getBeanClassLoader()
76+
.loadClass("org.springframework.web.servlet.OpenTelemetryHandlerMappingFilter");
77+
GenericBeanDefinition beanDefinition = new GenericBeanDefinition();
78+
beanDefinition.setScope(SCOPE_SINGLETON);
79+
beanDefinition.setBeanClass(clazz);
80+
beanDefinition.setBeanClassName(clazz.getName());
81+
82+
((BeanDefinitionRegistry) beanFactory)
83+
.registerBeanDefinition("otelAutoDispatcherFilter", beanDefinition);
84+
} catch (ClassNotFoundException ignored) {
85+
// Ignore
86+
}
6687
}
6788
}
6889
}

‎instrumentation/spring/spring-webmvc-3.1/javaagent/build.gradle.kts

+1-13
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,6 @@ muzzle {
1515
extraDependency("javax.servlet:javax.servlet-api:3.0.1")
1616
assertInverse.set(true)
1717
}
18-
19-
// FIXME: webmvc depends on web, so we need a separate instrumentation for spring-web specifically.
20-
fail {
21-
group.set("org.springframework")
22-
module.set("spring-web")
23-
versions.set("[,]")
24-
// these versions depend on org.springframework:spring-web which has a bad dependency on
25-
// javax.faces:jsf-api:1.1 which was released as pom only
26-
skip("1.2.1", "1.2.2", "1.2.3", "1.2.4")
27-
extraDependency("javax.servlet:javax.servlet-api:3.0.1")
28-
}
2918
}
3019

3120
val versions: Map<String, String> by project
@@ -36,12 +25,11 @@ dependencies {
3625
// compileOnly("org.springframework:spring-webmvc:2.5.6")
3726
// compileOnly("javax.servlet:servlet-api:2.4")
3827

39-
testImplementation(project(":testing-common"))
40-
4128
// Include servlet instrumentation for verifying the tomcat requests
4229
testInstrumentation(project(":instrumentation:servlet:servlet-3.0:javaagent"))
4330
testInstrumentation(project(":instrumentation:servlet:servlet-javax-common:javaagent"))
4431
testInstrumentation(project(":instrumentation:tomcat:tomcat-7.0:javaagent"))
32+
testInstrumentation(project(":instrumentation:spring:spring-web-3.1:javaagent"))
4533

4634
testImplementation("javax.validation:validation-api:1.1.0.Final")
4735
testImplementation("org.hibernate:hibernate-validator:5.4.2.Final")

‎instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/DispatcherServletInstrumentation.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.springframework.context.ApplicationContext;
2525
import org.springframework.web.servlet.HandlerMapping;
2626
import org.springframework.web.servlet.ModelAndView;
27+
import org.springframework.web.servlet.OpenTelemetryHandlerMappingFilter;
2728

2829
public class DispatcherServletInstrumentation implements TypeInstrumentation {
2930

@@ -62,8 +63,8 @@ public static void afterRefresh(
6263
@Advice.Argument(0) ApplicationContext springCtx,
6364
@Advice.FieldValue("handlerMappings") List<HandlerMapping> handlerMappings) {
6465
if (springCtx.containsBean("otelAutoDispatcherFilter")) {
65-
HandlerMappingResourceNameFilter filter =
66-
(HandlerMappingResourceNameFilter) springCtx.getBean("otelAutoDispatcherFilter");
66+
OpenTelemetryHandlerMappingFilter filter =
67+
(OpenTelemetryHandlerMappingFilter) springCtx.getBean("otelAutoDispatcherFilter");
6768
if (handlerMappings != null && filter != null) {
6869
filter.setHandlerMappings(handlerMappings);
6970
}

‎instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcInstrumentationModule.java

+7-4
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,14 @@ public SpringWebMvcInstrumentationModule() {
1818
super("spring-webmvc", "spring-webmvc-3.1");
1919
}
2020

21+
@Override
22+
public boolean isHelperClass(String className) {
23+
return className.startsWith(
24+
"org.springframework.web.servlet.OpenTelemetryHandlerMappingFilter");
25+
}
26+
2127
@Override
2228
public List<TypeInstrumentation> typeInstrumentations() {
23-
return asList(
24-
new WebApplicationContextInstrumentation(),
25-
new DispatcherServletInstrumentation(),
26-
new HandlerAdapterInstrumentation());
29+
return asList(new DispatcherServletInstrumentation(), new HandlerAdapterInstrumentation());
2730
}
2831
}

‎instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerMappingResourceNameFilter.java ‎instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/org/springframework/web/servlet/OpenTelemetryHandlerMappingFilter.java

+3-13
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
package io.opentelemetry.javaagent.instrumentation.springwebmvc;
6+
package org.springframework.web.servlet;
77

88
import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER;
99

1010
import io.opentelemetry.context.Context;
1111
import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming;
12+
import io.opentelemetry.javaagent.instrumentation.springwebmvc.SpringWebMvcServerSpanNaming;
1213
import java.io.IOException;
1314
import java.util.ArrayList;
1415
import java.util.List;
@@ -20,13 +21,10 @@
2021
import javax.servlet.ServletResponse;
2122
import javax.servlet.http.HttpServletRequest;
2223
import javax.servlet.http.HttpServletResponse;
23-
import org.springframework.beans.factory.support.GenericBeanDefinition;
2424
import org.springframework.core.Ordered;
25-
import org.springframework.web.servlet.HandlerExecutionChain;
26-
import org.springframework.web.servlet.HandlerMapping;
2725
import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping;
2826

29-
public class HandlerMappingResourceNameFilter implements Filter, Ordered {
27+
public class OpenTelemetryHandlerMappingFilter implements Filter, Ordered {
3028
private volatile List<HandlerMapping> handlerMappings;
3129

3230
@Override
@@ -122,12 +120,4 @@ public int getOrder() {
122120
// Run after all HIGHEST_PRECEDENCE items
123121
return Ordered.HIGHEST_PRECEDENCE + 1;
124122
}
125-
126-
public static class BeanDefinition extends GenericBeanDefinition {
127-
public BeanDefinition() {
128-
setScope(SCOPE_SINGLETON);
129-
setBeanClass(HandlerMappingResourceNameFilter.class);
130-
setBeanClassName(HandlerMappingResourceNameFilter.class.getName());
131-
}
132-
}
133123
}

‎instrumentation/spring/spring-webmvc-3.1/wildfly-testing/build.gradle.kts

+82
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
plugins {
2+
id("otel.javaagent-testing")
3+
}
4+
5+
val testServer by configurations.creating
6+
val appLibrary by configurations.creating
7+
8+
configurations.named("testCompileOnly") {
9+
extendsFrom(appLibrary)
10+
}
11+
12+
dependencies {
13+
appLibrary("org.springframework:spring-webmvc:3.1.0.RELEASE")
14+
testImplementation("javax.servlet:javax.servlet-api:3.1.0")
15+
16+
val arquillianVersion = "1.4.0.Final"
17+
testImplementation("org.jboss.arquillian.junit:arquillian-junit-container:${arquillianVersion}")
18+
testImplementation("org.jboss.arquillian.protocol:arquillian-protocol-servlet:${arquillianVersion}")
19+
testImplementation("org.jboss.arquillian.spock:arquillian-spock-container:1.0.0.CR1")
20+
testImplementation("org.jboss.shrinkwrap:shrinkwrap-impl-base:1.2.6")
21+
22+
testRuntimeOnly("org.wildfly.arquillian:wildfly-arquillian-container-embedded:2.2.0.Final")
23+
24+
testInstrumentation(project(":instrumentation:servlet:servlet-3.0:javaagent"))
25+
testInstrumentation(project(":instrumentation:spring:spring-webmvc-3.1:javaagent"))
26+
testInstrumentation(project(":instrumentation:spring:spring-web-3.1:javaagent"))
27+
28+
// wildfly version used to run tests
29+
testServer("org.wildfly:wildfly-dist:18.0.0.Final@zip")
30+
}
31+
32+
tasks {
33+
// extract wildfly dist, path is used from arquillian.xml
34+
val setupServer by registering(Copy::class) {
35+
from(zipTree(testServer.singleFile))
36+
into(file("build/server/"))
37+
}
38+
39+
// logback-classic contains /META-INF/services/javax.servlet.ServletContainerInitializer
40+
// that breaks deploy on embedded wildfly
41+
// create a copy of logback-classic jar that does not have this file
42+
val modifyLogbackJar by registering(Jar::class) {
43+
destinationDirectory.set(file("$buildDir/tmp"))
44+
archiveFileName.set("logback-classic-modified.jar")
45+
exclude("/META-INF/services/javax.servlet.ServletContainerInitializer")
46+
doFirst {
47+
configurations.configureEach {
48+
if (name.toLowerCase().endsWith("testruntimeclasspath")) {
49+
val logbackJar = find { it.name.contains("logback-classic") }
50+
from(zipTree(logbackJar))
51+
}
52+
}
53+
}
54+
}
55+
56+
val copyDependencies by registering(Copy::class) {
57+
// test looks for spring jars that are bundled inside deployed application from this directory
58+
from(appLibrary).into("$buildDir/app-libs")
59+
}
60+
61+
named<Test>("test") {
62+
dependsOn(modifyLogbackJar)
63+
dependsOn(setupServer)
64+
dependsOn(copyDependencies)
65+
66+
doFirst {
67+
// --add-modules is unrecognized on jdk8, ignore it instead of failing
68+
jvmArgs("-XX:+IgnoreUnrecognizedVMOptions")
69+
// needed for java 11 to avoid org.jboss.modules.ModuleNotFoundException: java.se
70+
jvmArgs("--add-modules=java.se")
71+
// add offset to default port values
72+
jvmArgs("-Djboss.socket.binding.port-offset=300")
73+
74+
// remove logback-classic from classpath
75+
classpath = classpath.filter {
76+
!it.absolutePath.contains("logback-classic")
77+
}
78+
// add modified copy of logback-classic to classpath
79+
classpath = classpath.plus(files("$buildDir/tmp/logback-classic-modified.jar"))
80+
}
81+
}
82+
}

0 commit comments

Comments
 (0)