Set http.route in spring-autoconfigure webmvc instrumentation (open-t…

…elemetry#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

@@ -69,10 +69,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

@@ -69,9 +69,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());
  }
 }