Set http.route in spring-autoconfigure webmvc instrumentation (#6414)

* Set http.route in spring-autoconfigure webmvc instrumentation

* Bump spring-webmvc library instrumentation version to 5.3

* nit: protected -> private

* Remove duplicated test (already covered by HttpSpanStatusExtractorTest)

* Move the README to the correct module

* fix link

* fix more links

* liiiiiiinks

* fix tests

* remove not needed weakref

docs/standalone-library-instrumentation.md

@@ -30,7 +30,7 @@ that can be used if you prefer that over using the Java agent:
 * [RxJava 2.0](../instrumentation/rxjava/rxjava-2.0/library)
 * [RxJava 3.0](../instrumentation/rxjava/rxjava-3.0/library)
 * [Spring RestTemplate](../instrumentation/spring/spring-web-3.1/library)
-* [Spring Web MVC](../instrumentation/spring/spring-webmvc-3.1/library)
+* [Spring Web MVC](../instrumentation/spring/spring-webmvc-5.3/library)
 * [Spring WebFlux Client](../instrumentation/spring/spring-webflux-5.0/library)
 * [Vibur DBCP](../instrumentation/vibur-dbcp-11.0/library)
 

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java

@@ -62,10 +62,11 @@ public void onEnd(
         attributes, SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH, requestContentLength(request));
 
     if (response != null) {
-      Integer statusCode = getter.statusCode(request, response);
+      Integer statusCode = getter.statusCode(request, response, error);
       if (statusCode != null && statusCode > 0) {
         internalSet(attributes, SemanticAttributes.HTTP_STATUS_CODE, (long) statusCode);
       }
+
       internalSet(
           attributes,
           SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH,

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesGetter.java

@@ -64,9 +64,21 @@ default Long requestContentLengthUncompressed(REQUEST request, @Nullable RESPONS
    * <p>This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, only when
    * {@code response} is non-{@code null}.
    */
+  // TODO: deprecate this method and use the new one everywhere
   @Nullable
   Integer statusCode(REQUEST request, RESPONSE response);
 
+  /**
+   * Extracts the {@code http.status_code} span attribute.
+   *
+   * <p>This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, only when
+   * {@code response} is non-{@code null}.
+   */
+  @Nullable
+  default Integer statusCode(REQUEST request, RESPONSE response, @Nullable Throwable error) {
+    return statusCode(request, response);
+  }
+
   /**
    * Extracts the {@code http.response_content_length} span attribute.
    *

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java

@@ -54,9 +54,10 @@ public void extract(
       SpanStatusBuilder spanStatusBuilder,
       REQUEST request,
       @Nullable RESPONSE response,
-      Throwable error) {
+      @Nullable Throwable error) {
+
     if (response != null) {
-      Integer statusCode = getter.statusCode(request, response);
+      Integer statusCode = getter.statusCode(request, response, error);
       if (statusCode != null) {
         StatusCode statusCodeObj = statusConverter.statusFromHttpStatus(statusCode);
         if (statusCodeObj == StatusCode.ERROR) {

instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractorTest.java

@@ -6,6 +6,8 @@
 package io.opentelemetry.instrumentation.api.instrumenter.http;
 
 import static org.mockito.ArgumentMatchers.anyMap;
+import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.ArgumentMatchers.same;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
@@ -33,7 +35,7 @@ class HttpSpanStatusExtractorTest {
   @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 500, 501, 600, 601})
   void hasServerStatus(int statusCode) {
     StatusCode expectedStatusCode = HttpStatusConverter.SERVER.statusFromHttpStatus(statusCode);
-    when(serverGetter.statusCode(anyMap(), anyMap())).thenReturn(statusCode);
+    when(serverGetter.statusCode(anyMap(), anyMap(), isNull())).thenReturn(statusCode);
 
     HttpSpanStatusExtractor.create(serverGetter)
         .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null);
@@ -49,7 +51,7 @@ void hasServerStatus(int statusCode) {
   @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601})
   void hasClientStatus(int statusCode) {
     StatusCode expectedStatusCode = HttpStatusConverter.CLIENT.statusFromHttpStatus(statusCode);
-    when(clientGetter.statusCode(anyMap(), anyMap())).thenReturn(statusCode);
+    when(clientGetter.statusCode(anyMap(), anyMap(), isNull())).thenReturn(statusCode);
 
     HttpSpanStatusExtractor.create(clientGetter)
         .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null);
@@ -64,48 +66,32 @@ void hasClientStatus(int statusCode) {
   @ParameterizedTest
   @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601})
   void hasServerStatusAndException(int statusCode) {
-    StatusCode expectedStatusCode = HttpStatusConverter.SERVER.statusFromHttpStatus(statusCode);
-    when(serverGetter.statusCode(anyMap(), anyMap())).thenReturn(statusCode);
+    Throwable error = new IllegalStateException("test");
+    when(serverGetter.statusCode(anyMap(), anyMap(), same(error))).thenReturn(statusCode);
 
-    // Presence of exception has no effect.
+    // Presence of exception overshadows the HTTP status
     HttpSpanStatusExtractor.create(serverGetter)
-        .extract(
-            spanStatusBuilder,
-            Collections.emptyMap(),
-            Collections.emptyMap(),
-            new IllegalStateException("test"));
+        .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), error);
 
-    if (expectedStatusCode == StatusCode.ERROR) {
-      verify(spanStatusBuilder).setStatus(expectedStatusCode);
-    } else {
-      verify(spanStatusBuilder).setStatus(StatusCode.ERROR);
-    }
+    verify(spanStatusBuilder).setStatus(StatusCode.ERROR);
   }
 
   @ParameterizedTest
   @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601})
   void hasClientStatusAndException(int statusCode) {
-    StatusCode expectedStatusCode = HttpStatusConverter.CLIENT.statusFromHttpStatus(statusCode);
-    when(clientGetter.statusCode(anyMap(), anyMap())).thenReturn(statusCode);
+    Throwable error = new IllegalStateException("test");
+    when(clientGetter.statusCode(anyMap(), anyMap(), same(error))).thenReturn(statusCode);
 
-    // Presence of exception has no effect.
+    // Presence of exception overshadows the HTTP status
     HttpSpanStatusExtractor.create(clientGetter)
-        .extract(
-            spanStatusBuilder,
-            Collections.emptyMap(),
-            Collections.emptyMap(),
-            new IllegalStateException("test"));
+        .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), error);
 
-    if (expectedStatusCode == StatusCode.ERROR) {
-      verify(spanStatusBuilder).setStatus(expectedStatusCode);
-    } else {
-      verify(spanStatusBuilder).setStatus(StatusCode.ERROR);
-    }
+    verify(spanStatusBuilder).setStatus(StatusCode.ERROR);
   }
 
   @Test
   void hasNoServerStatus_fallsBackToDefault_unset() {
-    when(serverGetter.statusCode(anyMap(), anyMap())).thenReturn(null);
+    when(serverGetter.statusCode(anyMap(), anyMap(), isNull())).thenReturn(null);
 
     HttpSpanStatusExtractor.create(serverGetter)
         .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null);
@@ -115,7 +101,7 @@ void hasNoServerStatus_fallsBackToDefault_unset() {
 
   @Test
   void hasNoClientStatus_fallsBackToDefault_unset() {
-    when(clientGetter.statusCode(anyMap(), anyMap())).thenReturn(null);
+    when(clientGetter.statusCode(anyMap(), anyMap(), isNull())).thenReturn(null);
 
     HttpSpanStatusExtractor.create(clientGetter)
         .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null);
@@ -125,27 +111,23 @@ void hasNoClientStatus_fallsBackToDefault_unset() {
 
   @Test
   void hasNoServerStatus_fallsBackToDefault_error() {
-    when(serverGetter.statusCode(anyMap(), anyMap())).thenReturn(null);
+    Throwable error = new IllegalStateException("test");
+    when(serverGetter.statusCode(anyMap(), anyMap(), same(error))).thenReturn(null);
 
     HttpSpanStatusExtractor.create(serverGetter)
-        .extract(
-            spanStatusBuilder,
-            Collections.emptyMap(),
-            Collections.emptyMap(),
-            new IllegalStateException("test"));
+        .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), error);
+
     verify(spanStatusBuilder).setStatus(StatusCode.ERROR);
   }
 
   @Test
   void hasNoClientStatus_fallsBackToDefault_error() {
-    when(clientGetter.statusCode(anyMap(), anyMap())).thenReturn(null);
+    IllegalStateException error = new IllegalStateException("test");
+    when(clientGetter.statusCode(anyMap(), anyMap(), same(error))).thenReturn(null);
 
     HttpSpanStatusExtractor.create(clientGetter)
-        .extract(
-            spanStatusBuilder,
-            Collections.emptyMap(),
-            Collections.emptyMap(),
-            new IllegalStateException("test"));
+        .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), error);
+
     verify(spanStatusBuilder).setStatus(StatusCode.ERROR);
   }
 }

instrumentation/spring/spring-boot-autoconfigure/README.md

@@ -153,7 +153,12 @@ Provides auto-configuration for the OpenTelemetry RestTemplate trace interceptor
 
 #### Spring Web MVC Auto Configuration
 
-This feature auto-configures instrumentation for spring-webmvc controllers by adding a [WebMvcTracingFilter](../spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/WebMvcTracingFilter.java) bean to the application context. This request filter implements the [OncePerRequestFilter](https://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/web/filter/OncePerRequestFilter.html) interface to capture OpenTelemetry server spans and propagate distribute tracing context if provided in the request. [Spring Web MVC - Server Span](#spring-web-mvc---server-span) show cases a sample span generated by the WebMvcTracingFilter. Check out [opentelemetry-spring-webmvc-3.1](../spring-webmvc-3.1/) to learn more about the OpenTelemetry WebMvcTracingFilter.
+This feature autoconfigures instrumentation for Spring WebMVC controllers by adding
+a [telemetry producing servlet `Filter`](../spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/WebMvcTelemetryProducingFilter.java)
+bean to the application context. This filter decorates the request execution with an OpenTelemetry
+server span, propagating the incoming tracing context if received in the HTTP request. Check
+out [`opentelemetry-spring-webmvc-5.3` instrumentation library](../spring-webmvc-5.3/library) to
+learn more about the OpenTelemetry Spring WebMVC instrumentation.
 
 #### Spring WebFlux Auto Configuration
 

instrumentation/spring/spring-boot-autoconfigure/build.gradle.kts

@@ -17,7 +17,7 @@ dependencies {
   implementation(project(":instrumentation:kafka:kafka-clients:kafka-clients-2.6:library"))
   implementation(project(":instrumentation:spring:spring-kafka-2.7:library"))
   implementation(project(":instrumentation:spring:spring-web-3.1:library"))
-  implementation(project(":instrumentation:spring:spring-webmvc-3.1:library"))
+  implementation(project(":instrumentation:spring:spring-webmvc-5.3:library"))
   implementation(project(":instrumentation:spring:spring-webflux-5.0:library"))
   implementation("io.opentelemetry:opentelemetry-micrometer1-shim") {
     // just get the instrumentation, without micrometer itself

instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/webmvc/WebMvcFilterAutoConfiguration.java

@@ -6,24 +6,27 @@
 package io.opentelemetry.instrumentation.spring.autoconfigure.webmvc;
 
 import io.opentelemetry.api.OpenTelemetry;
-import io.opentelemetry.instrumentation.spring.webmvc.SpringWebMvcTelemetry;
+import io.opentelemetry.instrumentation.spring.webmvc.v5_3.SpringWebMvcTelemetry;
 import javax.servlet.Filter;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.web.filter.OncePerRequestFilter;
+import org.springframework.web.servlet.DispatcherServlet;
 
 /** Configures {@link SpringWebMvcTelemetry} for tracing. */
 @Configuration
 @EnableConfigurationProperties(WebMvcProperties.class)
 @ConditionalOnProperty(prefix = "otel.springboot.web", name = "enabled", matchIfMissing = true)
-@ConditionalOnClass(OncePerRequestFilter.class)
+@ConditionalOnClass({OncePerRequestFilter.class, DispatcherServlet.class})
+@ConditionalOnBean(OpenTelemetry.class)
 public class WebMvcFilterAutoConfiguration {
 
   @Bean
-  public Filter otelWebMvcTracingFilter(OpenTelemetry openTelemetry) {
-    return SpringWebMvcTelemetry.create(openTelemetry).newServletFilter();
+  public Filter otelWebMvcInstrumentationFilter(OpenTelemetry openTelemetry) {
+    return SpringWebMvcTelemetry.create(openTelemetry).createServletFilter();
   }
 }

instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/webmvc/WebMvcFilterAutoConfigurationTest.java

@@ -29,22 +29,26 @@ void webEnabled() {
         .withPropertyValues("otel.springboot.web.enabled=true")
         .run(
             context ->
-                assertThat(context.getBean("otelWebMvcTracingFilter", Filter.class)).isNotNull());
+                assertThat(context.getBean("otelWebMvcInstrumentationFilter", Filter.class))
+                    .isNotNull());
   }
 
   @Test
   @DisplayName("when web is DISABLED should NOT initialize WebMvcTracingFilter bean")
   void disabledProperty() {
     this.contextRunner
         .withPropertyValues("otel.springboot.web.enabled=false")
-        .run(context -> assertThat(context.containsBean("otelWebMvcTracingFilter")).isFalse());
+        .run(
+            context ->
+                assertThat(context.containsBean("otelWebMvcInstrumentationFilter")).isFalse());
   }
 
   @Test
   @DisplayName("when web property is MISSING should initialize WebMvcTracingFilter bean")
   void noProperty() {
     this.contextRunner.run(
         context ->
-            assertThat(context.getBean("otelWebMvcTracingFilter", Filter.class)).isNotNull());
+            assertThat(context.getBean("otelWebMvcInstrumentationFilter", Filter.class))
+                .isNotNull());
   }
 }