From e14a9d995070914fd091c1524440b4a44c4078ee Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Thu, 29 Jul 2021 15:42:23 +0300 Subject: [PATCH 1/6] Move helper class to spring package so that loadClass can find it --- .../DispatcherServletInstrumentation.java | 5 +++-- .../SpringWebMvcInstrumentationModule.java | 5 +++++ .../WebApplicationContextInstrumentation.java | 3 ++- .../servlet/OpenTelemetryHandlerMappingFilter.java} | 11 +++++------ 4 files changed, 15 insertions(+), 9 deletions(-) rename instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/{io/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerMappingResourceNameFilter.java => org/springframework/web/servlet/OpenTelemetryHandlerMappingFilter.java} (92%) diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/DispatcherServletInstrumentation.java b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/DispatcherServletInstrumentation.java index a06a90cbda1a..112f5921441a 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/DispatcherServletInstrumentation.java +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/DispatcherServletInstrumentation.java @@ -24,6 +24,7 @@ import org.springframework.context.ApplicationContext; import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.ModelAndView; +import org.springframework.web.servlet.OpenTelemetryHandlerMappingFilter; public class DispatcherServletInstrumentation implements TypeInstrumentation { @@ -62,8 +63,8 @@ public static void afterRefresh( @Advice.Argument(0) ApplicationContext springCtx, @Advice.FieldValue("handlerMappings") List handlerMappings) { if (springCtx.containsBean("otelAutoDispatcherFilter")) { - HandlerMappingResourceNameFilter filter = - (HandlerMappingResourceNameFilter) springCtx.getBean("otelAutoDispatcherFilter"); + OpenTelemetryHandlerMappingFilter filter = + (OpenTelemetryHandlerMappingFilter) springCtx.getBean("otelAutoDispatcherFilter"); if (handlerMappings != null && filter != null) { filter.setHandlerMappings(handlerMappings); } diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcInstrumentationModule.java b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcInstrumentationModule.java index 7d60e3a5b0e8..a20951b65ee3 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcInstrumentationModule.java +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcInstrumentationModule.java @@ -18,6 +18,11 @@ public SpringWebMvcInstrumentationModule() { super("spring-webmvc", "spring-webmvc-3.1"); } + @Override + public boolean isHelperClass(String className) { + return className.startsWith("org.springframework.web.servlet.OpenTelemetryHandlerMappingFilter"); + } + @Override public List typeInstrumentations() { return asList( diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/WebApplicationContextInstrumentation.java b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/WebApplicationContextInstrumentation.java index 4657c9908e89..3b40344f446c 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/WebApplicationContextInstrumentation.java +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/WebApplicationContextInstrumentation.java @@ -19,6 +19,7 @@ import net.bytebuddy.matcher.ElementMatcher; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.web.servlet.OpenTelemetryHandlerMappingFilter; /** * This instrumentation adds the HandlerMappingResourceNameFilter definition to the spring context @@ -62,7 +63,7 @@ public static void onEnter(@Advice.Argument(0) ConfigurableListableBeanFactory b ((BeanDefinitionRegistry) beanFactory) .registerBeanDefinition( - "otelAutoDispatcherFilter", new HandlerMappingResourceNameFilter.BeanDefinition()); + "otelAutoDispatcherFilter", new OpenTelemetryHandlerMappingFilter.BeanDefinition()); } } } diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerMappingResourceNameFilter.java b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/org/springframework/web/servlet/OpenTelemetryHandlerMappingFilter.java similarity index 92% rename from instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerMappingResourceNameFilter.java rename to instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/org/springframework/web/servlet/OpenTelemetryHandlerMappingFilter.java index 98f8e499bf7a..a9f4b40c63e5 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerMappingResourceNameFilter.java +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/org/springframework/web/servlet/OpenTelemetryHandlerMappingFilter.java @@ -3,12 +3,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.javaagent.instrumentation.springwebmvc; +package org.springframework.web.servlet; import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; +import io.opentelemetry.javaagent.instrumentation.springwebmvc.SpringWebMvcServerSpanNaming; import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -22,11 +23,9 @@ import javax.servlet.http.HttpServletResponse; import org.springframework.beans.factory.support.GenericBeanDefinition; import org.springframework.core.Ordered; -import org.springframework.web.servlet.HandlerExecutionChain; -import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; -public class HandlerMappingResourceNameFilter implements Filter, Ordered { +public class OpenTelemetryHandlerMappingFilter implements Filter, Ordered { private volatile List handlerMappings; @Override @@ -126,8 +125,8 @@ public int getOrder() { public static class BeanDefinition extends GenericBeanDefinition { public BeanDefinition() { setScope(SCOPE_SINGLETON); - setBeanClass(HandlerMappingResourceNameFilter.class); - setBeanClassName(HandlerMappingResourceNameFilter.class.getName()); + setBeanClass(OpenTelemetryHandlerMappingFilter.class); + setBeanClassName(OpenTelemetryHandlerMappingFilter.class.getName()); } } } From 0462416e8fdc0603c75e8ab0a09e6e0729fea5c4 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Thu, 29 Jul 2021 22:05:29 +0300 Subject: [PATCH 2/6] spotless --- .../springwebmvc/SpringWebMvcInstrumentationModule.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcInstrumentationModule.java b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcInstrumentationModule.java index a20951b65ee3..ca975304b42f 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcInstrumentationModule.java +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcInstrumentationModule.java @@ -20,7 +20,8 @@ public SpringWebMvcInstrumentationModule() { @Override public boolean isHelperClass(String className) { - return className.startsWith("org.springframework.web.servlet.OpenTelemetryHandlerMappingFilter"); + return className.startsWith( + "org.springframework.web.servlet.OpenTelemetryHandlerMappingFilter"); } @Override From 32f2e2be5da3bfb2ff85f2557a2279da1508bf98 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 4 Aug 2021 19:29:41 +0300 Subject: [PATCH 3/6] Add tests --- .../spring-web-3.1/javaagent/build.gradle.kts | 23 +++ .../SpringWebInstrumentationModule.java | 33 ++++ .../WebApplicationContextInstrumentation.java | 32 +++- .../javaagent/build.gradle.kts | 14 +- .../SpringWebMvcInstrumentationModule.java | 5 +- .../OpenTelemetryHandlerMappingFilter.java | 9 - .../wildfly-testing/build.gradle.kts | 82 +++++++++ ...enTelemetryHandlerMappingFilterTest.groovy | 160 ++++++++++++++++++ .../com/example/hello/HelloController.java | 21 +++ .../java/com/example/hello/TestFilter.java | 41 +++++ .../src/test/resources/application.xml | 11 ++ .../src/test/resources/applicationContext.xml | 13 ++ .../src/test/resources/arquillian.xml | 15 ++ .../src/test/resources/dispatcher-servlet.xml | 10 ++ .../src/test/resources/web.xml | 47 +++++ settings.gradle.kts | 2 + 16 files changed, 486 insertions(+), 32 deletions(-) create mode 100644 instrumentation/spring/spring-web-3.1/javaagent/build.gradle.kts create mode 100644 instrumentation/spring/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/SpringWebInstrumentationModule.java rename instrumentation/spring/{spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc => spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb}/WebApplicationContextInstrumentation.java (64%) create mode 100644 instrumentation/spring/spring-webmvc-3.1/wildfly-testing/build.gradle.kts create mode 100644 instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/groovy/OpenTelemetryHandlerMappingFilterTest.groovy create mode 100644 instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/java/com/example/hello/HelloController.java create mode 100644 instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/java/com/example/hello/TestFilter.java create mode 100644 instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/application.xml create mode 100644 instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/applicationContext.xml create mode 100644 instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/arquillian.xml create mode 100644 instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/dispatcher-servlet.xml create mode 100644 instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/web.xml diff --git a/instrumentation/spring/spring-web-3.1/javaagent/build.gradle.kts b/instrumentation/spring/spring-web-3.1/javaagent/build.gradle.kts new file mode 100644 index 000000000000..89eb5ee8d718 --- /dev/null +++ b/instrumentation/spring/spring-web-3.1/javaagent/build.gradle.kts @@ -0,0 +1,23 @@ +plugins { + id("otel.javaagent-instrumentation") +} + +muzzle { + pass { + group.set("org.springframework") + module.set("spring-web") + versions.set("[3.1.0.RELEASE,]") + // these versions depend on org.springframework:spring-web which has a bad dependency on + // javax.faces:jsf-api:1.1 which was released as pom only + skip("1.2.1", "1.2.2", "1.2.3", "1.2.4") + // 3.2.1.RELEASE has transitive dependencies like spring-web as "provided" instead of "compile" + skip("3.2.1.RELEASE") + extraDependency("javax.servlet:javax.servlet-api:3.0.1") + assertInverse.set(true) + } +} + +dependencies { + compileOnly("org.springframework:spring-web:3.1.0.RELEASE") + compileOnly("javax.servlet:javax.servlet-api:3.1.0") +} diff --git a/instrumentation/spring/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/SpringWebInstrumentationModule.java b/instrumentation/spring/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/SpringWebInstrumentationModule.java new file mode 100644 index 000000000000..d0c2a8e03f03 --- /dev/null +++ b/instrumentation/spring/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/SpringWebInstrumentationModule.java @@ -0,0 +1,33 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.springweb; + +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; +import static java.util.Collections.singletonList; + +import com.google.auto.service.AutoService; +import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import java.util.List; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(InstrumentationModule.class) +public class SpringWebInstrumentationModule extends InstrumentationModule { + public SpringWebInstrumentationModule() { + super("spring-web", "spring-web-3.1"); + } + + @Override + public ElementMatcher.Junction classLoaderMatcher() { + // class added in 3.1 + return hasClassesNamed("org.springframework.web.method.HandlerMethod"); + } + + @Override + public List typeInstrumentations() { + return singletonList(new WebApplicationContextInstrumentation()); + } +} diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/WebApplicationContextInstrumentation.java b/instrumentation/spring/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/WebApplicationContextInstrumentation.java similarity index 64% rename from instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/WebApplicationContextInstrumentation.java rename to instrumentation/spring/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/WebApplicationContextInstrumentation.java index 3b40344f446c..a26b139e7901 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/WebApplicationContextInstrumentation.java +++ b/instrumentation/spring/spring-web-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springweb/WebApplicationContextInstrumentation.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.javaagent.instrumentation.springwebmvc; +package io.opentelemetry.javaagent.instrumentation.springweb; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.extendsClass; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; @@ -11,6 +11,7 @@ import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static org.springframework.beans.factory.config.BeanDefinition.SCOPE_SINGLETON; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; @@ -19,10 +20,10 @@ import net.bytebuddy.matcher.ElementMatcher; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.support.BeanDefinitionRegistry; -import org.springframework.web.servlet.OpenTelemetryHandlerMappingFilter; +import org.springframework.beans.factory.support.GenericBeanDefinition; /** - * This instrumentation adds the HandlerMappingResourceNameFilter definition to the spring context + * This instrumentation adds the OpenTelemetryHandlerMappingFilter definition to the spring context * When the context is created, the filter will be added to the beginning of the filter chain. */ public class WebApplicationContextInstrumentation implements TypeInstrumentation { @@ -60,10 +61,29 @@ public static class FilterInjectingAdvice { public static void onEnter(@Advice.Argument(0) ConfigurableListableBeanFactory beanFactory) { if (beanFactory instanceof BeanDefinitionRegistry && !beanFactory.containsBean("otelAutoDispatcherFilter")) { + try { + // Firstly check whether DispatcherServlet is present. We need to load an instrumented + // class from spring-webmvc to trigger injection that makes + // OpenTelemetryHandlerMappingFilter available. + beanFactory + .getBeanClassLoader() + .loadClass("org.springframework.web.servlet.DispatcherServlet"); - ((BeanDefinitionRegistry) beanFactory) - .registerBeanDefinition( - "otelAutoDispatcherFilter", new OpenTelemetryHandlerMappingFilter.BeanDefinition()); + // Now attempt to load our injected instrumentation class. + Class clazz = + beanFactory + .getBeanClassLoader() + .loadClass("org.springframework.web.servlet.OpenTelemetryHandlerMappingFilter"); + GenericBeanDefinition beanDefinition = new GenericBeanDefinition(); + beanDefinition.setScope(SCOPE_SINGLETON); + beanDefinition.setBeanClass(clazz); + beanDefinition.setBeanClassName(clazz.getName()); + + ((BeanDefinitionRegistry) beanFactory) + .registerBeanDefinition("otelAutoDispatcherFilter", beanDefinition); + } catch (ClassNotFoundException ignored) { + // Ignore + } } } } diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/build.gradle.kts b/instrumentation/spring/spring-webmvc-3.1/javaagent/build.gradle.kts index adb881b8b900..a66d452187e0 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/build.gradle.kts +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/build.gradle.kts @@ -15,17 +15,6 @@ muzzle { extraDependency("javax.servlet:javax.servlet-api:3.0.1") assertInverse.set(true) } - - // FIXME: webmvc depends on web, so we need a separate instrumentation for spring-web specifically. - fail { - group.set("org.springframework") - module.set("spring-web") - versions.set("[,]") - // these versions depend on org.springframework:spring-web which has a bad dependency on - // javax.faces:jsf-api:1.1 which was released as pom only - skip("1.2.1", "1.2.2", "1.2.3", "1.2.4") - extraDependency("javax.servlet:javax.servlet-api:3.0.1") - } } val versions: Map by project @@ -36,12 +25,11 @@ dependencies { // compileOnly("org.springframework:spring-webmvc:2.5.6") // compileOnly("javax.servlet:servlet-api:2.4") - testImplementation(project(":testing-common")) - // Include servlet instrumentation for verifying the tomcat requests testInstrumentation(project(":instrumentation:servlet:servlet-3.0:javaagent")) testInstrumentation(project(":instrumentation:servlet:servlet-javax-common:javaagent")) testInstrumentation(project(":instrumentation:tomcat:tomcat-7.0:javaagent")) + testInstrumentation(project(":instrumentation:spring:spring-web-3.1:javaagent")) testImplementation("javax.validation:validation-api:1.1.0.Final") testImplementation("org.hibernate:hibernate-validator:5.4.2.Final") diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcInstrumentationModule.java b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcInstrumentationModule.java index ca975304b42f..c171597d5586 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcInstrumentationModule.java +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcInstrumentationModule.java @@ -26,9 +26,6 @@ public boolean isHelperClass(String className) { @Override public List typeInstrumentations() { - return asList( - new WebApplicationContextInstrumentation(), - new DispatcherServletInstrumentation(), - new HandlerAdapterInstrumentation()); + return asList(new DispatcherServletInstrumentation(), new HandlerAdapterInstrumentation()); } } diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/org/springframework/web/servlet/OpenTelemetryHandlerMappingFilter.java b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/org/springframework/web/servlet/OpenTelemetryHandlerMappingFilter.java index a9f4b40c63e5..6fd2c4b9ea57 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/org/springframework/web/servlet/OpenTelemetryHandlerMappingFilter.java +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/org/springframework/web/servlet/OpenTelemetryHandlerMappingFilter.java @@ -21,7 +21,6 @@ import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.springframework.beans.factory.support.GenericBeanDefinition; import org.springframework.core.Ordered; import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; @@ -121,12 +120,4 @@ public int getOrder() { // Run after all HIGHEST_PRECEDENCE items return Ordered.HIGHEST_PRECEDENCE + 1; } - - public static class BeanDefinition extends GenericBeanDefinition { - public BeanDefinition() { - setScope(SCOPE_SINGLETON); - setBeanClass(OpenTelemetryHandlerMappingFilter.class); - setBeanClassName(OpenTelemetryHandlerMappingFilter.class.getName()); - } - } } diff --git a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/build.gradle.kts b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/build.gradle.kts new file mode 100644 index 000000000000..0c0b5a3596de --- /dev/null +++ b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/build.gradle.kts @@ -0,0 +1,82 @@ +plugins { + id("otel.javaagent-testing") +} + +val testServer by configurations.creating +val appLibrary by configurations.creating + +configurations.named("testCompileOnly") { + extendsFrom(appLibrary) +} + +dependencies { + appLibrary("org.springframework:spring-webmvc:3.1.0.RELEASE") + testImplementation("javax.servlet:javax.servlet-api:3.1.0") + + val arquillianVersion = "1.4.0.Final" + testImplementation("org.jboss.arquillian.junit:arquillian-junit-container:${arquillianVersion}") + testImplementation("org.jboss.arquillian.protocol:arquillian-protocol-servlet:${arquillianVersion}") + testImplementation("org.jboss.arquillian.spock:arquillian-spock-container:1.0.0.CR1") + testImplementation("org.jboss.shrinkwrap:shrinkwrap-impl-base:1.2.6") + + testRuntimeOnly("org.wildfly.arquillian:wildfly-arquillian-container-embedded:2.2.0.Final") + + testInstrumentation(project(":instrumentation:servlet:servlet-3.0:javaagent")) + testInstrumentation(project(":instrumentation:spring:spring-webmvc-3.1:javaagent")) + testInstrumentation(project(":instrumentation:spring:spring-web-3.1:javaagent")) + + // wildfly version used to run tests + testServer("org.wildfly:wildfly-dist:18.0.0.Final@zip") +} + +tasks { + // extract wildfly dist, path is used from arquillian.xml + val setupServer by registering(Copy::class) { + from(zipTree(testServer.singleFile)) + into(file("build/server/")) + } + + // logback-classic contains /META-INF/services/javax.servlet.ServletContainerInitializer + // that breaks deploy on embedded wildfly + // create a copy of logback-classic jar that does not have this file + val modifyLogbackJar by registering(Jar::class) { + destinationDirectory.set(file("$buildDir/tmp")) + archiveFileName.set("logback-classic-modified.jar") + exclude("/META-INF/services/javax.servlet.ServletContainerInitializer") + doFirst { + configurations.configureEach { + if (name.toLowerCase().endsWith("testruntimeclasspath")) { + val logbackJar = find { it.name.contains("logback-classic") } + from(zipTree(logbackJar)) + } + } + } + } + + val copyDependencies by registering(Copy::class) { + // test looks for spring jars that are bundled inside deployed application from this directory + from(appLibrary).into("$buildDir/app-libs") + } + + named("test") { + dependsOn(modifyLogbackJar) + dependsOn(setupServer) + dependsOn(copyDependencies) + + doFirst { + // --add-modules is unrecognized on jdk8, ignore it instead of failing + jvmArgs("-XX:+IgnoreUnrecognizedVMOptions") + // needed for java 11 to avoid org.jboss.modules.ModuleNotFoundException: java.se + jvmArgs("--add-modules=java.se") + // add offset to default port values + jvmArgs("-Djboss.socket.binding.port-offset=300") + + // remove logback-classic from classpath + classpath = classpath.filter { + !it.absolutePath.contains("logback-classic") + } + // add modified copy of logback-classic to classpath + classpath = classpath.plus(files("$buildDir/tmp/logback-classic-modified.jar")) + } + } +} diff --git a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/groovy/OpenTelemetryHandlerMappingFilterTest.groovy b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/groovy/OpenTelemetryHandlerMappingFilterTest.groovy new file mode 100644 index 000000000000..1d57db1767dc --- /dev/null +++ b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/groovy/OpenTelemetryHandlerMappingFilterTest.groovy @@ -0,0 +1,160 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +import static io.opentelemetry.api.trace.SpanKind.INTERNAL +import static io.opentelemetry.api.trace.SpanKind.SERVER + +import com.example.hello.HelloController +import com.example.hello.TestFilter +import io.opentelemetry.api.trace.StatusCode +import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes +import io.opentelemetry.testing.internal.armeria.client.WebClient +import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpResponse +import org.jboss.arquillian.container.test.api.Deployment +import org.jboss.arquillian.container.test.api.RunAsClient +import org.jboss.arquillian.spock.ArquillianSputnik +import org.jboss.arquillian.test.api.ArquillianResource +import org.jboss.shrinkwrap.api.Archive +import org.jboss.shrinkwrap.api.ShrinkWrap +import org.jboss.shrinkwrap.api.spec.EnterpriseArchive +import org.jboss.shrinkwrap.api.spec.WebArchive +import org.junit.runner.RunWith + +@RunWith(ArquillianSputnik) +@RunAsClient +abstract class OpenTelemetryHandlerMappingFilterTest extends AgentInstrumentationSpecification { + + static WebClient client = WebClient.of() + + @ArquillianResource + static URI url + + def getAddress(String service) { + return url.resolve(service).toString() + } + + def "test success"() { + when: + AggregatedHttpResponse response = client.get(getAddress("hello/world")).aggregate().join() + + then: + response.status().code() == 200 + response.contentUtf8() == "hello world" + + and: + assertTraces(1) { + trace(0, 2) { + span(0) { + name "/hello/{name}" + kind SERVER + hasNoParent() + } + span(1) { + name "HelloController.hello" + kind INTERNAL + childOf(span(0)) + } + } + } + } + + def "test exception"() { + when: + AggregatedHttpResponse response = client.get(getAddress("hello/exception")).aggregate().join() + + then: + response.status().code() == 500 + + and: + assertTraces(1) { + trace(0, 1) { + span(0) { + name "/hello/{name}" + kind SERVER + status StatusCode.ERROR + hasNoParent() + + event(0) { + eventName(SemanticAttributes.EXCEPTION_EVENT_NAME) + attributes { + "${SemanticAttributes.EXCEPTION_TYPE.key}" "javax.servlet.ServletException" + "${SemanticAttributes.EXCEPTION_MESSAGE.key}" "exception" + "${SemanticAttributes.EXCEPTION_STACKTRACE.key}" { it == null || it instanceof String } + } + } + } + } + } + } +} + +class LibsInEarTest extends OpenTelemetryHandlerMappingFilterTest { + @Deployment + static Archive createDeployment() { + WebArchive war = ShrinkWrap.create(WebArchive, "test.war") + .addAsWebInfResource("web.xml") + .addAsWebInfResource("dispatcher-servlet.xml") + .addAsWebInfResource("applicationContext.xml") + .addClass(HelloController) + .addClass(TestFilter) + + EnterpriseArchive ear = ShrinkWrap.create(EnterpriseArchive) + .setApplicationXML("application.xml") + .addAsModule(war) + .addAsLibraries(new File("build/app-libs").listFiles()) + + return ear + } +} + +class LibsInWarTest extends OpenTelemetryHandlerMappingFilterTest { + @Deployment + static Archive createDeployment() { + WebArchive war = ShrinkWrap.create(WebArchive, "test.war") + .addAsWebInfResource("web.xml") + .addAsWebInfResource("dispatcher-servlet.xml") + .addAsWebInfResource("applicationContext.xml") + .addClass(HelloController) + .addClass(TestFilter) + .addAsLibraries(new File("build/app-libs").listFiles()) + + EnterpriseArchive ear = ShrinkWrap.create(EnterpriseArchive) + .setApplicationXML("application.xml") + .addAsModule(war) + + return ear + } +} + +class MixedLibsTest extends OpenTelemetryHandlerMappingFilterTest { + @Deployment + static Archive createDeployment() { + WebArchive war = ShrinkWrap.create(WebArchive, "test.war") + .addAsWebInfResource("web.xml") + .addAsWebInfResource("dispatcher-servlet.xml") + .addAsWebInfResource("applicationContext.xml") + .addClass(HelloController) + .addClass(TestFilter) + .addAsLibraries(new File("build/app-libs").listFiles(new FilenameFilter() { + @Override + boolean accept(File dir, String name) { + return name.contains("spring-webmvc") + } + })) + + EnterpriseArchive ear = ShrinkWrap.create(EnterpriseArchive) + .setApplicationXML("application.xml") + .addAsModule(war) + .addAsLibraries(new File("build/app-libs").listFiles(new FilenameFilter() { + @Override + boolean accept(File dir, String name) { + return !name.contains("spring-webmvc") + } + })) + + return ear + } +} diff --git a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/java/com/example/hello/HelloController.java b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/java/com/example/hello/HelloController.java new file mode 100644 index 000000000000..390550a475f5 --- /dev/null +++ b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/java/com/example/hello/HelloController.java @@ -0,0 +1,21 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.example.hello; + +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.ResponseBody; + +@Controller +public class HelloController { + + @RequestMapping("/hello/{name}") + @ResponseBody + public String hello(@PathVariable("name") String name) { + return "hello " + name; + } +} diff --git a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/java/com/example/hello/TestFilter.java b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/java/com/example/hello/TestFilter.java new file mode 100644 index 000000000000..a611b7eec23d --- /dev/null +++ b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/java/com/example/hello/TestFilter.java @@ -0,0 +1,41 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.example.hello; + +import java.io.IOException; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import org.springframework.stereotype.Component; + +@Component("testFilter") +public class TestFilter implements Filter { + public TestFilter() {} + + @Override + public void init(FilterConfig filterConfig) {} + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) + throws IOException, ServletException { + HttpServletRequest httpServletRequest = (HttpServletRequest) request; + // To test OpenTelemetryHandlerMappingFilter we need to stop the request before it reaches + // HandlerAdapter which gives server span the same name as OpenTelemetryHandlerMappingFilter. + // Throwing an exception from servlet filter works for that. + if (httpServletRequest.getRequestURI().contains("exception")) { + throw new ServletException("exception"); + } + + chain.doFilter(request, response); + } + + @Override + public void destroy() {} +} diff --git a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/application.xml b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/application.xml new file mode 100644 index 000000000000..950a543036d4 --- /dev/null +++ b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/application.xml @@ -0,0 +1,11 @@ + + + hello-ear + + + test.war + / + + + lib + \ No newline at end of file diff --git a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/applicationContext.xml b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/applicationContext.xml new file mode 100644 index 000000000000..317003e646da --- /dev/null +++ b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/applicationContext.xml @@ -0,0 +1,13 @@ + + + + + + + \ No newline at end of file diff --git a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/arquillian.xml b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/arquillian.xml new file mode 100644 index 000000000000..02e0432fac5e --- /dev/null +++ b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/arquillian.xml @@ -0,0 +1,15 @@ + + + + + + + build/server/wildfly-18.0.0.Final + build/server/wildfly-18.0.0.Final/modules + + + \ No newline at end of file diff --git a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/dispatcher-servlet.xml b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/dispatcher-servlet.xml new file mode 100644 index 000000000000..e22637661479 --- /dev/null +++ b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/dispatcher-servlet.xml @@ -0,0 +1,10 @@ + + + + \ No newline at end of file diff --git a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/web.xml b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/web.xml new file mode 100644 index 000000000000..c25a29dddedd --- /dev/null +++ b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/web.xml @@ -0,0 +1,47 @@ + + + + + dispatcher + org.springframework.web.servlet.DispatcherServlet + 1 + + + + + otelAutoDispatcherFilter + org.springframework.web.filter.DelegatingFilterProxy + + + + testFilter + org.springframework.web.filter.DelegatingFilterProxy + + + + dispatcher + / + + + + otelAutoDispatcherFilter + dispatcher + + + + testFilter + dispatcher + + + + org.springframework.web.context.ContextLoaderListener + + \ No newline at end of file diff --git a/settings.gradle.kts b/settings.gradle.kts index f3c67abd7441..00d160d6ab97 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -301,9 +301,11 @@ include(":instrumentation:spring:spring-integration-4.1:library") include(":instrumentation:spring:spring-integration-4.1:testing") include(":instrumentation:spring:spring-rabbit-1.0:javaagent") include(":instrumentation:spring:spring-scheduling-3.1:javaagent") +include(":instrumentation:spring:spring-web-3.1:javaagent") include(":instrumentation:spring:spring-web-3.1:library") include(":instrumentation:spring:spring-webmvc-3.1:javaagent") include(":instrumentation:spring:spring-webmvc-3.1:library") +include(":instrumentation:spring:spring-webmvc-3.1:wildfly-testing") include(":instrumentation:spring:spring-webflux-5.0:javaagent") include(":instrumentation:spring:spring-webflux-5.0:library") include(":instrumentation:spring:spring-ws-2.0:javaagent") From 63fa70fc8fa6f1b3cf334282caf3c7e6125e2774 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 4 Aug 2021 19:35:28 +0300 Subject: [PATCH 4/6] Add comments --- .../test/groovy/OpenTelemetryHandlerMappingFilterTest.groovy | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/groovy/OpenTelemetryHandlerMappingFilterTest.groovy b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/groovy/OpenTelemetryHandlerMappingFilterTest.groovy index 1d57db1767dc..64b1ebc8c1bb 100644 --- a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/groovy/OpenTelemetryHandlerMappingFilterTest.groovy +++ b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/groovy/OpenTelemetryHandlerMappingFilterTest.groovy @@ -25,6 +25,8 @@ import org.junit.runner.RunWith @RunWith(ArquillianSputnik) @RunAsClient +// Test that OpenTelemetryHandlerMappingFilter injection works when spring libraries are in various +// locations inside deployment. abstract class OpenTelemetryHandlerMappingFilterTest extends AgentInstrumentationSpecification { static WebClient client = WebClient.of() @@ -91,6 +93,7 @@ abstract class OpenTelemetryHandlerMappingFilterTest extends AgentInstrumentatio } } +// spring is inside ear lib class LibsInEarTest extends OpenTelemetryHandlerMappingFilterTest { @Deployment static Archive createDeployment() { @@ -110,6 +113,7 @@ class LibsInEarTest extends OpenTelemetryHandlerMappingFilterTest { } } +// spring is inside war/WEB-INF/lib class LibsInWarTest extends OpenTelemetryHandlerMappingFilterTest { @Deployment static Archive createDeployment() { @@ -129,6 +133,7 @@ class LibsInWarTest extends OpenTelemetryHandlerMappingFilterTest { } } +// Everything except spring-webmvc is in ear/lib, spring-webmvc is in war/WEB-INF/lib class MixedLibsTest extends OpenTelemetryHandlerMappingFilterTest { @Deployment static Archive createDeployment() { From b7944e800833d2da6a5ef066c4038387a3d18b60 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 4 Aug 2021 23:35:41 +0300 Subject: [PATCH 5/6] remove unneeded dependency --- .../spring/spring-web-3.1/javaagent/build.gradle.kts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/instrumentation/spring/spring-web-3.1/javaagent/build.gradle.kts b/instrumentation/spring/spring-web-3.1/javaagent/build.gradle.kts index 89eb5ee8d718..6a33679f891c 100644 --- a/instrumentation/spring/spring-web-3.1/javaagent/build.gradle.kts +++ b/instrumentation/spring/spring-web-3.1/javaagent/build.gradle.kts @@ -7,17 +7,12 @@ muzzle { group.set("org.springframework") module.set("spring-web") versions.set("[3.1.0.RELEASE,]") - // these versions depend on org.springframework:spring-web which has a bad dependency on - // javax.faces:jsf-api:1.1 which was released as pom only + // these versions depend on javax.faces:jsf-api:1.1 which was released as pom only skip("1.2.1", "1.2.2", "1.2.3", "1.2.4") - // 3.2.1.RELEASE has transitive dependencies like spring-web as "provided" instead of "compile" - skip("3.2.1.RELEASE") - extraDependency("javax.servlet:javax.servlet-api:3.0.1") assertInverse.set(true) } } dependencies { compileOnly("org.springframework:spring-web:3.1.0.RELEASE") - compileOnly("javax.servlet:javax.servlet-api:3.1.0") } From ccb73740e7921a89cccd1920e024164f7794125c Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 4 Aug 2021 23:36:48 +0300 Subject: [PATCH 6/6] comments --- .../test/groovy/OpenTelemetryHandlerMappingFilterTest.groovy | 2 +- .../wildfly-testing/src/test/resources/web.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/groovy/OpenTelemetryHandlerMappingFilterTest.groovy b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/groovy/OpenTelemetryHandlerMappingFilterTest.groovy index 64b1ebc8c1bb..19fb019201e3 100644 --- a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/groovy/OpenTelemetryHandlerMappingFilterTest.groovy +++ b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/groovy/OpenTelemetryHandlerMappingFilterTest.groovy @@ -93,7 +93,7 @@ abstract class OpenTelemetryHandlerMappingFilterTest extends AgentInstrumentatio } } -// spring is inside ear lib +// spring is inside ear/lib class LibsInEarTest extends OpenTelemetryHandlerMappingFilterTest { @Deployment static Archive createDeployment() { diff --git a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/web.xml b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/web.xml index c25a29dddedd..d04a4197394d 100644 --- a/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/web.xml +++ b/instrumentation/spring/spring-webmvc-3.1/wildfly-testing/src/test/resources/web.xml @@ -13,7 +13,7 @@