Move helper class to spring package so that loadClass can find it

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

@@ -0,0 +1,18 @@
+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 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")
+    assertInverse.set(true)
+  }
+}
+
+dependencies {
+  compileOnly("org.springframework:spring-web:3.1.0.RELEASE")
+}

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<ClassLoader> classLoaderMatcher() {
+    // class added in 3.1
+    return hasClassesNamed("org.springframework.web.method.HandlerMethod");
+  }
+
+  @Override
+  public List<TypeInstrumentation> typeInstrumentations() {
+    return singletonList(new WebApplicationContextInstrumentation());
+  }
+}

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

@@ -3,14 +3,15 @@
  * 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;
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
 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,9 +20,10 @@
 import net.bytebuddy.matcher.ElementMatcher;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
+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 {
@@ -59,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 HandlerMappingResourceNameFilter.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
+        }
       }
     }
   }

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<String, String> 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")

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<HandlerMapping> 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);
         }

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

@@ -18,11 +18,14 @@ 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<TypeInstrumentation> typeInstrumentations() {
-    return asList(
-        new WebApplicationContextInstrumentation(),
-        new DispatcherServletInstrumentation(),
-        new HandlerAdapterInstrumentation());
+    return asList(new DispatcherServletInstrumentation(), new HandlerAdapterInstrumentation());
   }
 }

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,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;
@@ -20,13 +21,10 @@
 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.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<HandlerMapping> handlerMappings;
 
   @Override
@@ -122,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(HandlerMappingResourceNameFilter.class);
-      setBeanClassName(HandlerMappingResourceNameFilter.class.getName());
-    }
-  }
 }

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>("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"))
+    }
+  }
+}