From 5740099b4354b6a2807c575ebce1ab813ecd0885 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Tue, 9 Aug 2022 18:36:28 +0200 Subject: [PATCH] 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 | 2 +- .../http/HttpCommonAttributesExtractor.java | 3 +- .../http/HttpCommonAttributesGetter.java | 12 ++ .../http/HttpSpanStatusExtractor.java | 5 +- .../HttpClientSpanStatusExtractorTest.java | 91 ----------- .../http/HttpSpanStatusExtractorTest.java | 66 +++----- .../spring-boot-autoconfigure/README.md | 7 +- .../build.gradle.kts | 2 +- .../webmvc/WebMvcFilterAutoConfiguration.java | 11 +- .../WebMvcFilterAutoConfigurationTest.java | 10 +- .../spring-webmvc-3.1/library/README.md | 93 ----------- .../library/build.gradle.kts | 8 - .../spring/webmvc/StatusCodeExtractor.java | 46 ------ .../spring-webmvc-5.3/library/README.md | 93 +++++++++++ .../library/build.gradle.kts | 17 ++ .../spring/webmvc/v5_3/HttpRouteSupport.java | 153 ++++++++++++++++++ .../v5_3}/JavaxHttpServletRequestGetter.java | 2 +- .../SpringWebMvcHttpAttributesGetter.java | 36 ++++- .../SpringWebMvcNetAttributesGetter.java | 7 +- .../webmvc/v5_3}/SpringWebMvcTelemetry.java | 6 +- .../v5_3}/SpringWebMvcTelemetryBuilder.java | 5 +- .../v5_3/WebMvcTelemetryProducingFilter.java} | 33 +++- .../webmvc/v5_3/TestWebSpringBootApp.java | 130 +++++++++++++++ .../webmvc/v5_3/WebMvcHttpServerTest.java | 48 ++++++ settings.gradle.kts | 2 +- 25 files changed, 574 insertions(+), 314 deletions(-) delete mode 100644 instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientSpanStatusExtractorTest.java delete mode 100644 instrumentation/spring/spring-webmvc-3.1/library/README.md delete mode 100644 instrumentation/spring/spring-webmvc-3.1/library/build.gradle.kts delete mode 100644 instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/StatusCodeExtractor.java create mode 100644 instrumentation/spring/spring-webmvc-5.3/library/README.md create mode 100644 instrumentation/spring/spring-webmvc-5.3/library/build.gradle.kts create mode 100644 instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/HttpRouteSupport.java rename instrumentation/spring/{spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc => spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3}/JavaxHttpServletRequestGetter.java (90%) rename instrumentation/spring/{spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc => spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3}/SpringWebMvcHttpAttributesGetter.java (66%) rename instrumentation/spring/{spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc => spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3}/SpringWebMvcNetAttributesGetter.java (80%) rename instrumentation/spring/{spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc => spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3}/SpringWebMvcTelemetry.java (88%) rename instrumentation/spring/{spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc => spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3}/SpringWebMvcTelemetryBuilder.java (94%) rename instrumentation/spring/{spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/WebMvcTracingFilter.java => spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/WebMvcTelemetryProducingFilter.java} (55%) create mode 100644 instrumentation/spring/spring-webmvc-5.3/library/src/test/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/TestWebSpringBootApp.java create mode 100644 instrumentation/spring/spring-webmvc-5.3/library/src/test/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/WebMvcHttpServerTest.java diff --git a/docs/standalone-library-instrumentation.md b/docs/standalone-library-instrumentation.md index 8372a4e67bbb..0db1709f9395 100644 --- a/docs/standalone-library-instrumentation.md +++ b/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) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java index e2060be4d531..7aac2dbf80ac 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java +++ b/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, diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesGetter.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesGetter.java index 05045ce998f1..3870148a014b 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesGetter.java +++ b/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 *

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. + * + *

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. * diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java index 373efa10fe07..1ef9c60598ff 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java +++ b/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) { diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientSpanStatusExtractorTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientSpanStatusExtractorTest.java deleted file mode 100644 index 3fe7fa16c127..000000000000 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientSpanStatusExtractorTest.java +++ /dev/null @@ -1,91 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.instrumenter.http; - -import static org.mockito.ArgumentMatchers.anyMap; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.when; - -import io.opentelemetry.api.trace.StatusCode; -import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusBuilder; -import java.util.Collections; -import java.util.Map; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; - -@ExtendWith(MockitoExtension.class) -class HttpClientSpanStatusExtractorTest { - - @Mock private HttpClientAttributesGetter, Map> getter; - - @Mock private SpanStatusBuilder spanStatusBuilder; - - @ParameterizedTest - @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601}) - void hasStatus(int statusCode) { - StatusCode expectedStatusCode = HttpStatusConverter.CLIENT.statusFromHttpStatus(statusCode); - when(getter.statusCode(anyMap(), anyMap())).thenReturn(statusCode); - - HttpSpanStatusExtractor.create(getter) - .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null); - - if (expectedStatusCode != StatusCode.UNSET) { - verify(spanStatusBuilder).setStatus(expectedStatusCode); - } else { - verifyNoInteractions(spanStatusBuilder); - } - } - - @ParameterizedTest - @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601}) - void hasStatusAndException(int statusCode) { - StatusCode expectedStatusCode = HttpStatusConverter.CLIENT.statusFromHttpStatus(statusCode); - when(getter.statusCode(anyMap(), anyMap())).thenReturn(statusCode); - - // Presence of exception has no effect. - HttpSpanStatusExtractor.create(getter) - .extract( - spanStatusBuilder, - Collections.emptyMap(), - Collections.emptyMap(), - new IllegalStateException("test")); - - if (expectedStatusCode != StatusCode.UNSET) { - verify(spanStatusBuilder).setStatus(expectedStatusCode); - } else { - verify(spanStatusBuilder).setStatus(StatusCode.ERROR); - } - } - - @Test - void hasNoStatus_fallsBackToDefault_unset() { - when(getter.statusCode(anyMap(), anyMap())).thenReturn(null); - - HttpSpanStatusExtractor.create(getter) - .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null); - - verifyNoInteractions(spanStatusBuilder); - } - - @Test - void hasNoStatus_fallsBackToDefault_error() { - when(getter.statusCode(anyMap(), anyMap())).thenReturn(null); - - HttpSpanStatusExtractor.create(getter) - .extract( - spanStatusBuilder, - Collections.emptyMap(), - Collections.emptyMap(), - new IllegalStateException("test")); - - verify(spanStatusBuilder).setStatus(StatusCode.ERROR); - } -} diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractorTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractorTest.java index 484a456eb68a..5ee05e8ab9ea 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractorTest.java +++ b/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); } } diff --git a/instrumentation/spring/spring-boot-autoconfigure/README.md b/instrumentation/spring/spring-boot-autoconfigure/README.md index 281996e11924..e9efdd1bca6b 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/README.md +++ b/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 diff --git a/instrumentation/spring/spring-boot-autoconfigure/build.gradle.kts b/instrumentation/spring/spring-boot-autoconfigure/build.gradle.kts index 93907fe638b7..ef0aee4a0c4d 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/build.gradle.kts +++ b/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 diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/webmvc/WebMvcFilterAutoConfiguration.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/webmvc/WebMvcFilterAutoConfiguration.java index c73cdc2a7f6a..8ffdf2895a67 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/webmvc/WebMvcFilterAutoConfiguration.java +++ b/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(); } } diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/webmvc/WebMvcFilterAutoConfigurationTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/webmvc/WebMvcFilterAutoConfigurationTest.java index 666c452c59a2..302ae77c9ca8 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/webmvc/WebMvcFilterAutoConfigurationTest.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/webmvc/WebMvcFilterAutoConfigurationTest.java @@ -29,7 +29,8 @@ 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 @@ -37,7 +38,9 @@ void webEnabled() { 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 @@ -45,6 +48,7 @@ void disabledProperty() { void noProperty() { this.contextRunner.run( context -> - assertThat(context.getBean("otelWebMvcTracingFilter", Filter.class)).isNotNull()); + assertThat(context.getBean("otelWebMvcInstrumentationFilter", Filter.class)) + .isNotNull()); } } diff --git a/instrumentation/spring/spring-webmvc-3.1/library/README.md b/instrumentation/spring/spring-webmvc-3.1/library/README.md deleted file mode 100644 index 022dd124d88b..000000000000 --- a/instrumentation/spring/spring-webmvc-3.1/library/README.md +++ /dev/null @@ -1,93 +0,0 @@ -# Manual Instrumentation for Spring Web MVC - -Provides OpenTelemetry tracing for spring-webmvc RestControllers by leveraging spring-webmvc [filters](https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/filter). - -## Quickstart - -### Add these dependencies to your project. - -Replace `SPRING_VERSION` with the version of spring you're using. - - `Minimum version: 3.1` - -Replace `OPENTELEMETRY_VERSION` with the latest stable [release](https://mvnrepository.com/artifact/io.opentelemetry). - - `Minimum version: 1.7.0` - -For Maven add to your `pom.xml`: - -```xml - - - - io.opentelemetry.instrumentation - opentelemetry-spring-webmvc-3.1 - OPENTELEMETRY_VERSION - - - - - - io.opentelemetry - opentelemetry-exporter-logging - OPENTELEMETRY_VERSION - - - - - - org.springframework - spring-webmvc - SPRING_VERSION - - - -``` - -For Gradle add to your dependencies: - -```groovy - -// opentelemetry instrumentation -implementation("io.opentelemetry.instrumentation:opentelemetry-spring-webmvc-3.1:OPENTELEMETRY_VERSION") - -// opentelemetry exporter -// replace this default exporter with your opentelemetry exporter (ex. otlp/zipkin/jaeger/..) -implementation("io.opentelemetry:opentelemetry-exporter-logging:OPENTELEMETRY_VERSION") - -// required to instrument spring-webmvc -// this artifact should already be present in your application -implementation("org.springframework:spring-webmvc:SPRING_VERSION") -``` - -### Features - -#### SpringWebMvcTracing - -`SpringWebMvcTracing` adds OpenTelemetry server spans to requests processed by request dispatch, on any spring servlet container. An example is shown below: - -##### Usage in Spring Boot - -Spring Boot allows servlet `Filter`s to be registered as beans: - -```java -import io.opentelemetry.instrumentation.spring.webmvc.SpringWebMvcTracing; -import io.opentelemetry.api.trace.Tracer; - -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.web.client.RestTemplate; - -@Configuration -public class WebMvcTracingFilterConfig { - - @Bean - public Filter webMvcTracingFilter(OpenTelemetry openTelemetry) { - return SpringWebMvcTracing.create(openTelemetry).newServletFilter(); - } -} -``` - -### Starter Guide - -Check out [OpenTelemetry Manual Instrumentation](https://opentelemetry.io/docs/instrumentation/java/manual/) to learn more about -using the OpenTelemetry API to instrument your code. diff --git a/instrumentation/spring/spring-webmvc-3.1/library/build.gradle.kts b/instrumentation/spring/spring-webmvc-3.1/library/build.gradle.kts deleted file mode 100644 index 0d5ec74e0136..000000000000 --- a/instrumentation/spring/spring-webmvc-3.1/library/build.gradle.kts +++ /dev/null @@ -1,8 +0,0 @@ -plugins { - id("otel.library-instrumentation") -} - -dependencies { - compileOnly("org.springframework:spring-webmvc:3.1.0.RELEASE") - compileOnly("javax.servlet:javax.servlet-api:3.1.0") -} diff --git a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/StatusCodeExtractor.java b/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/StatusCodeExtractor.java deleted file mode 100644 index 1401b40246d9..000000000000 --- a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/StatusCodeExtractor.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.spring.webmvc; - -import io.opentelemetry.api.common.AttributesBuilder; -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; -import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; -import javax.annotation.Nullable; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -final class StatusCodeExtractor - implements AttributesExtractor { - - @Override - public void onStart( - AttributesBuilder attributes, Context parentContext, HttpServletRequest httpServletRequest) {} - - @Override - public void onEnd( - AttributesBuilder attributes, - Context context, - HttpServletRequest httpServletRequest, - @Nullable HttpServletResponse response, - @Nullable Throwable error) { - if (response != null) { - long statusCode; - // if response is not committed and there is a throwable set status to 500 / - // INTERNAL_SERVER_ERROR, due to servlet spec - // https://javaee.github.io/servlet-spec/downloads/servlet-4.0/servlet-4_0_FINAL.pdf: - // "If a servlet generates an error that is not handled by the error page mechanism as - // described above, the container must ensure to send a response with status 500." - if (!response.isCommitted() && error != null) { - statusCode = 500; - } else { - statusCode = response.getStatus(); - } - - attributes.put(SemanticAttributes.HTTP_STATUS_CODE, statusCode); - } - } -} diff --git a/instrumentation/spring/spring-webmvc-5.3/library/README.md b/instrumentation/spring/spring-webmvc-5.3/library/README.md new file mode 100644 index 000000000000..a1149e9c6515 --- /dev/null +++ b/instrumentation/spring/spring-webmvc-5.3/library/README.md @@ -0,0 +1,93 @@ +# Library Instrumentation for Spring Web MVC + +Provides OpenTelemetry instrumentation for Spring WebMVC controllers. + +## Quickstart + +### Add these dependencies to your project. + +Replace `SPRING_VERSION` with the version of spring you're using. + - `Minimum version: 5.3` + +Replace `OPENTELEMETRY_VERSION` with the latest stable [release](https://mvnrepository.com/artifact/io.opentelemetry). + - `Minimum version: 1.17.0` + +For Maven add the following to your `pom.xml`: + +```xml + + + + io.opentelemetry.instrumentation + opentelemetry-spring-webmvc-5.3 + OPENTELEMETRY_VERSION + + + + + + io.opentelemetry + opentelemetry-exporter-logging + OPENTELEMETRY_VERSION + + + + + + org.springframework + spring-webmvc + SPRING_VERSION + + + +``` + +For Gradle add the following to your dependencies: + +```groovy + +// OpenTelemetry instrumentation +implementation("io.opentelemetry.instrumentation:opentelemetry-spring-webmvc-5.3:OPENTELEMETRY_VERSION") + +// OpenTelemetry exporter +// replace this default exporter with your OpenTelemetry exporter (ex. otlp/zipkin/jaeger/..) +implementation("io.opentelemetry:opentelemetry-exporter-logging:OPENTELEMETRY_VERSION") + +// required to instrument Spring WebMVC +// this artifact should already be present in your application +implementation("org.springframework:spring-webmvc:SPRING_VERSION") +``` + +### Features + +#### `SpringWebMvcTelemetry` + +`SpringWebMvcTelemetry` enables creating OpenTelemetry server spans around HTTP requests processed +by the Spring servlet container. + +##### Usage in Spring Boot + +Spring Boot allows servlet `Filter`s to be registered as beans: + +```java +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.instrumentation.spring.webmvc.v5_3.SpringWebMvcTelemetry; +import javax.servlet.Filter; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class SpringWebMvcTelemetryConfiguration { + + @Bean + public Filter telemetryFilter(OpenTelemetry openTelemetry) { + return SpringWebMvcTelemetry.create(openTelemetry).createServletFilter(); + } +} +``` + +### Starter Guide + +Check +out [OpenTelemetry Manual Instrumentation](https://opentelemetry.io/docs/instrumentation/java/manual/) +to learn more about using the OpenTelemetry API to instrument your code. diff --git a/instrumentation/spring/spring-webmvc-5.3/library/build.gradle.kts b/instrumentation/spring/spring-webmvc-5.3/library/build.gradle.kts new file mode 100644 index 000000000000..e8af381e4d6b --- /dev/null +++ b/instrumentation/spring/spring-webmvc-5.3/library/build.gradle.kts @@ -0,0 +1,17 @@ +plugins { + id("otel.library-instrumentation") +} + +val versions: Map by project +val springBootVersion = versions["org.springframework.boot"] + +dependencies { + compileOnly("org.springframework:spring-webmvc:5.3.0") + compileOnly("javax.servlet:javax.servlet-api:4.0.1") + + testImplementation(project(":testing-common")) + testImplementation("org.springframework.boot:spring-boot-starter-web:$springBootVersion") + testImplementation("org.springframework.boot:spring-boot-starter-test:$springBootVersion") { + exclude("org.junit.vintage", "junit-vintage-engine") + } +} diff --git a/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/HttpRouteSupport.java b/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/HttpRouteSupport.java new file mode 100644 index 000000000000..cafac0b13abb --- /dev/null +++ b/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/HttpRouteSupport.java @@ -0,0 +1,153 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.webmvc.v5_3; + +import static java.util.Objects.requireNonNull; +import static org.springframework.web.util.ServletRequestPathUtils.PATH_ATTRIBUTE; + +import io.opentelemetry.context.Context; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; +import javax.annotation.Nullable; +import javax.servlet.FilterConfig; +import javax.servlet.http.HttpServletRequest; +import org.springframework.context.ApplicationListener; +import org.springframework.context.event.ContextRefreshedEvent; +import org.springframework.web.context.ConfigurableWebApplicationContext; +import org.springframework.web.context.WebApplicationContext; +import org.springframework.web.context.support.WebApplicationContextUtils; +import org.springframework.web.servlet.DispatcherServlet; +import org.springframework.web.servlet.HandlerExecutionChain; +import org.springframework.web.servlet.HandlerMapping; +import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; +import org.springframework.web.util.ServletRequestPathUtils; + +final class HttpRouteSupport { + + private final AtomicBoolean contextRefreshTriggered = new AtomicBoolean(); + @Nullable private volatile DispatcherServlet dispatcherServlet; + @Nullable private volatile List handlerMappings; + private volatile boolean parseRequestPath; + + void onFilterInit(FilterConfig filterConfig) { + WebApplicationContext context = + WebApplicationContextUtils.getWebApplicationContext(filterConfig.getServletContext()); + if (!(context instanceof ConfigurableWebApplicationContext)) { + return; + } + + DispatcherServlet servlet = context.getBeanProvider(DispatcherServlet.class).getIfAvailable(); + if (servlet != null) { + dispatcherServlet = servlet; + + ((ConfigurableWebApplicationContext) context) + .addApplicationListener(new WebContextRefreshListener()); + } + } + + // we can't retrieve the handler mappings from the DispatcherServlet in the onRefresh listener, + // because it loads them just after the application context refreshed event is processed + // to work around this, we're setting a boolean flag that'll cause this filter to load the + // mappings the next time it attempts to set the http.route + final class WebContextRefreshListener implements ApplicationListener { + + @Override + public void onApplicationEvent(ContextRefreshedEvent event) { + contextRefreshTriggered.set(true); + } + } + + boolean hasMappings() { + if (contextRefreshTriggered.compareAndSet(true, false)) { + // reload the handler mappings only if the web app context was recently refreshed + Optional.ofNullable(dispatcherServlet) + .map(DispatcherServlet::getHandlerMappings) + .ifPresent(this::setHandlerMappings); + } + return handlerMappings != null; + } + + private void setHandlerMappings(List mappings) { + List handlerMappings = new ArrayList<>(); + for (HandlerMapping mapping : mappings) { + // Originally we ran findMapping at the very beginning of the request. This turned out to have + // application-crashing side-effects with grails. That is why we don't add all HandlerMapping + // classes here. Although now that we run findMapping after the request, and only when server + // span name has not been updated by a controller, the probability of bad side-effects is much + // reduced even if we did add all HandlerMapping classes here. + if (mapping instanceof RequestMappingHandlerMapping) { + handlerMappings.add(mapping); + if (mapping.usesPathPatterns()) { + this.parseRequestPath = true; + } + } + } + if (!handlerMappings.isEmpty()) { + this.handlerMappings = handlerMappings; + } + } + + @Nullable + String getHttpRoute(Context context, HttpServletRequest request) { + boolean parsePath = this.parseRequestPath; + Object previousValue = null; + if (parsePath) { + previousValue = request.getAttribute(PATH_ATTRIBUTE); + // sets new value for PATH_ATTRIBUTE of request + ServletRequestPathUtils.parseAndCache(request); + } + try { + if (findMapping(request)) { + // Name the parent span based on the matching pattern + // Let the parent span resource name be set with the attribute set in findMapping. + Object bestMatchingPattern = + request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); + if (bestMatchingPattern != null) { + return prependContextPath(request, bestMatchingPattern.toString()); + } + } + } finally { + // mimic spring DispatcherServlet and restore the previous value of PATH_ATTRIBUTE + if (parsePath) { + if (previousValue == null) { + request.removeAttribute(PATH_ATTRIBUTE); + } else { + request.setAttribute(PATH_ATTRIBUTE, previousValue); + } + } + } + return null; + } + + /** + * When a HandlerMapping matches a request, it sets HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE + * as an attribute on the request. This attribute set as the HTTP route. + */ + private boolean findMapping(HttpServletRequest request) { + try { + // handlerMapping already null-checked above + for (HandlerMapping mapping : requireNonNull(handlerMappings)) { + HandlerExecutionChain handler = mapping.getHandler(request); + if (handler != null) { + return true; + } + } + } catch (Exception ignored) { + // mapping.getHandler() threw exception. Ignore + } + return false; + } + + private static String prependContextPath(HttpServletRequest request, String route) { + String contextPath = request.getContextPath(); + if (contextPath == null) { + return route; + } + return contextPath + (route.startsWith("/") ? route : ("/" + route)); + } +} diff --git a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/JavaxHttpServletRequestGetter.java b/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/JavaxHttpServletRequestGetter.java similarity index 90% rename from instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/JavaxHttpServletRequestGetter.java rename to instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/JavaxHttpServletRequestGetter.java index 6f0174dc9a14..3fba07f6341e 100644 --- a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/JavaxHttpServletRequestGetter.java +++ b/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/JavaxHttpServletRequestGetter.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.webmvc; +package io.opentelemetry.instrumentation.spring.webmvc.v5_3; import io.opentelemetry.context.propagation.TextMapGetter; import java.util.Collections; diff --git a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcHttpAttributesGetter.java b/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/SpringWebMvcHttpAttributesGetter.java similarity index 66% rename from instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcHttpAttributesGetter.java rename to instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/SpringWebMvcHttpAttributesGetter.java index 3aafabebb10e..48411036505e 100644 --- a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcHttpAttributesGetter.java +++ b/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/SpringWebMvcHttpAttributesGetter.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.webmvc; +package io.opentelemetry.instrumentation.spring.webmvc.v5_3; import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesGetter; import java.util.ArrayList; @@ -34,16 +34,42 @@ public List requestHeader(HttpServletRequest request, String name) { @Override @Nullable public String flavor(HttpServletRequest request) { - return request.getProtocol(); + String flavor = request.getProtocol(); + if (flavor == null) { + return null; + } + if (flavor.startsWith("HTTP/")) { + flavor = flavor.substring("HTTP/".length()); + } + return flavor; } - @Override @Nullable - public Integer statusCode(HttpServletRequest request, HttpServletResponse response) { - // set in StatusCodeExtractor + @Override + public Integer statusCode(HttpServletRequest request, HttpServletResponse httpServletResponse) { + // this method is never used return null; } + @Override + public Integer statusCode( + HttpServletRequest request, HttpServletResponse response, @Nullable Throwable error) { + + int statusCode; + // if response is not committed and there is a throwable set status to 500 / + // INTERNAL_SERVER_ERROR, due to servlet spec + // https://javaee.github.io/servlet-spec/downloads/servlet-4.0/servlet-4_0_FINAL.pdf: + // "If a servlet generates an error that is not handled by the error page mechanism as + // described above, the container must ensure to send a response with status 500." + if (!response.isCommitted() && error != null) { + statusCode = 500; + } else { + statusCode = response.getStatus(); + } + + return statusCode; + } + @Override public List responseHeader( HttpServletRequest request, HttpServletResponse response, String name) { diff --git a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcNetAttributesGetter.java b/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/SpringWebMvcNetAttributesGetter.java similarity index 80% rename from instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcNetAttributesGetter.java rename to instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/SpringWebMvcNetAttributesGetter.java index 0d41652a0563..dd28787456f2 100644 --- a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcNetAttributesGetter.java +++ b/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/SpringWebMvcNetAttributesGetter.java @@ -3,15 +3,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.webmvc; +package io.opentelemetry.instrumentation.spring.webmvc.v5_3; import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesGetter; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import javax.annotation.Nullable; import javax.servlet.http.HttpServletRequest; -final class SpringWebMvcNetAttributesGetter - implements NetServerAttributesGetter { +enum SpringWebMvcNetAttributesGetter implements NetServerAttributesGetter { + INSTANCE; + @Override public String transport(HttpServletRequest request) { return SemanticAttributes.NetTransportValues.IP_TCP; diff --git a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcTelemetry.java b/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/SpringWebMvcTelemetry.java similarity index 88% rename from instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcTelemetry.java rename to instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/SpringWebMvcTelemetry.java index 161eb7444bb7..5e0d652672e9 100644 --- a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcTelemetry.java +++ b/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/SpringWebMvcTelemetry.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.webmvc; +package io.opentelemetry.instrumentation.spring.webmvc.v5_3; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; @@ -36,7 +36,7 @@ public static SpringWebMvcTelemetryBuilder builder(OpenTelemetry openTelemetry) } /** Returns a new {@link Filter} that generates telemetry for received HTTP requests. */ - public Filter newServletFilter() { - return new WebMvcTracingFilter(instrumenter); + public Filter createServletFilter() { + return new WebMvcTelemetryProducingFilter(instrumenter); } } diff --git a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcTelemetryBuilder.java b/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/SpringWebMvcTelemetryBuilder.java similarity index 94% rename from instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcTelemetryBuilder.java rename to instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/SpringWebMvcTelemetryBuilder.java index a5cb9081033d..49f27f91d4ce 100644 --- a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcTelemetryBuilder.java +++ b/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/SpringWebMvcTelemetryBuilder.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.webmvc; +package io.opentelemetry.instrumentation.spring.webmvc.v5_3; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; @@ -81,9 +81,8 @@ public SpringWebMvcTelemetry build() { HttpSpanNameExtractor.create(httpAttributesGetter)) .setSpanStatusExtractor(HttpSpanStatusExtractor.create(httpAttributesGetter)) .addAttributesExtractor(httpAttributesExtractorBuilder.build()) - .addAttributesExtractor(new StatusCodeExtractor()) .addAttributesExtractor( - NetServerAttributesExtractor.create(new SpringWebMvcNetAttributesGetter())) + NetServerAttributesExtractor.create(SpringWebMvcNetAttributesGetter.INSTANCE)) .addAttributesExtractors(additionalExtractors) .addOperationMetrics(HttpServerMetrics.get()) .addContextCustomizer(HttpRouteHolder.get()) diff --git a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/WebMvcTracingFilter.java b/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/WebMvcTelemetryProducingFilter.java similarity index 55% rename from instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/WebMvcTracingFilter.java rename to instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/WebMvcTelemetryProducingFilter.java index 44306a85a5a8..8be211250556 100644 --- a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/WebMvcTracingFilter.java +++ b/instrumentation/spring/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/WebMvcTelemetryProducingFilter.java @@ -3,11 +3,15 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.webmvc; +package io.opentelemetry.instrumentation.spring.webmvc.v5_3; + +import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource.CONTROLLER; +import static java.util.Objects.requireNonNull; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder; import java.io.IOException; import javax.servlet.FilterChain; import javax.servlet.ServletException; @@ -16,14 +20,27 @@ import org.springframework.core.Ordered; import org.springframework.web.filter.OncePerRequestFilter; -final class WebMvcTracingFilter extends OncePerRequestFilter implements Ordered { +final class WebMvcTelemetryProducingFilter extends OncePerRequestFilter implements Ordered { private final Instrumenter instrumenter; + private final HttpRouteSupport httpRouteSupport = new HttpRouteSupport(); - WebMvcTracingFilter(Instrumenter instrumenter) { + WebMvcTelemetryProducingFilter( + Instrumenter instrumenter) { this.instrumenter = instrumenter; } + @Override + public void afterPropertiesSet() { + // don't do anything, in particular do not call initFilterBean() + } + + @Override + protected void initFilterBean() { + // FilterConfig must be non-null at this point + httpRouteSupport.onFilterInit(requireNonNull(getFilterConfig())); + } + @Override public void doFilterInternal( HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) @@ -36,12 +53,18 @@ public void doFilterInternal( } Context context = instrumenter.start(parentContext, request); + Throwable error = null; try (Scope ignored = context.makeCurrent()) { filterChain.doFilter(request, response); - instrumenter.end(context, request, response, null); } catch (Throwable t) { - instrumenter.end(context, request, response, t); + error = t; throw t; + } finally { + if (httpRouteSupport.hasMappings()) { + HttpRouteHolder.updateHttpRoute( + context, CONTROLLER, httpRouteSupport::getHttpRoute, request); + } + instrumenter.end(context, request, response, error); } } diff --git a/instrumentation/spring/spring-webmvc-5.3/library/src/test/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/TestWebSpringBootApp.java b/instrumentation/spring/spring-webmvc-5.3/library/src/test/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/TestWebSpringBootApp.java new file mode 100644 index 000000000000..d0175f9d0c5d --- /dev/null +++ b/instrumentation/spring/spring-webmvc-5.3/library/src/test/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/TestWebSpringBootApp.java @@ -0,0 +1,130 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.webmvc.v5_3; + +import static io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest.controller; +import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.CAPTURE_HEADERS; +import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.ERROR; +import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.EXCEPTION; +import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.INDEXED_CHILD; +import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.PATH_PARAM; +import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.QUERY_PARAM; +import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.REDIRECT; +import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.SUCCESS; +import static java.util.Collections.singletonList; + +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest; +import java.util.Properties; +import javax.servlet.Filter; +import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestHeader; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.servlet.view.RedirectView; + +@SpringBootApplication +class TestWebSpringBootApp { + + static ConfigurableApplicationContext start(int port, String contextPath) { + Properties props = new Properties(); + props.put("server.port", port); + props.put("server.servlet.contextPath", contextPath); + + SpringApplication app = new SpringApplication(TestWebSpringBootApp.class); + app.setDefaultProperties(props); + return app.run(); + } + + @Bean + Filter telemetryFilter() { + return SpringWebMvcTelemetry.builder(GlobalOpenTelemetry.get()) + .setCapturedRequestHeaders(singletonList(AbstractHttpServerTest.TEST_REQUEST_HEADER)) + .setCapturedResponseHeaders(singletonList(AbstractHttpServerTest.TEST_RESPONSE_HEADER)) + .build() + .createServletFilter(); + } + + @Controller + static class TestController { + + @RequestMapping("/success") + @ResponseBody + String success() { + return controller(SUCCESS, SUCCESS::getBody); + } + + @RequestMapping("/query") + @ResponseBody + String query_param(@RequestParam("some") String param) { + return controller(QUERY_PARAM, () -> "some=" + param); + } + + @RequestMapping("/redirect") + @ResponseBody + RedirectView redirect() { + return controller(REDIRECT, () -> new RedirectView(REDIRECT.getBody())); + } + + @RequestMapping("/error-status") + ResponseEntity error() { + return controller( + ERROR, + () -> new ResponseEntity<>(ERROR.getBody(), HttpStatus.valueOf(ERROR.getStatus()))); + } + + @RequestMapping("/exception") + ResponseEntity exception() { + return controller( + EXCEPTION, + () -> { + throw new RuntimeException(EXCEPTION.getBody()); + }); + } + + @RequestMapping("/captureHeaders") + ResponseEntity capture_headers( + @RequestHeader("X-Test-Request") String testRequestHeader) { + return controller( + CAPTURE_HEADERS, + () -> + ResponseEntity.ok() + .header("X-Test-Response", testRequestHeader) + .body(CAPTURE_HEADERS.getBody())); + } + + @RequestMapping("/path/{id}/param") + @ResponseBody + String path_param(@PathVariable("id") int id) { + return controller(PATH_PARAM, () -> String.valueOf(id)); + } + + @RequestMapping("/child") + @ResponseBody + String indexed_child(@RequestParam("id") String id) { + return controller( + INDEXED_CHILD, + () -> { + INDEXED_CHILD.collectSpanAttributes(name -> "id".equals(name) ? id : null); + return INDEXED_CHILD.getBody(); + }); + } + + @ExceptionHandler + ResponseEntity handleException(Throwable throwable) { + return new ResponseEntity<>(throwable.getMessage(), HttpStatus.INTERNAL_SERVER_ERROR); + } + } +} diff --git a/instrumentation/spring/spring-webmvc-5.3/library/src/test/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/WebMvcHttpServerTest.java b/instrumentation/spring/spring-webmvc-5.3/library/src/test/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/WebMvcHttpServerTest.java new file mode 100644 index 000000000000..9283a640d125 --- /dev/null +++ b/instrumentation/spring/spring-webmvc-5.3/library/src/test/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/WebMvcHttpServerTest.java @@ -0,0 +1,48 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.webmvc.v5_3; + +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest; +import io.opentelemetry.instrumentation.testing.junit.http.HttpServerInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.HttpServerTestOptions; +import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.springframework.context.ConfigurableApplicationContext; + +class WebMvcHttpServerTest extends AbstractHttpServerTest { + + private static final String CONTEXT_PATH = "/test"; + + @RegisterExtension + static final InstrumentationExtension testing = HttpServerInstrumentationExtension.forLibrary(); + + @Override + protected ConfigurableApplicationContext setupServer() { + return TestWebSpringBootApp.start(port, CONTEXT_PATH); + } + + @Override + protected void stopServer(ConfigurableApplicationContext applicationContext) { + applicationContext.close(); + } + + @Override + protected void configure(HttpServerTestOptions options) { + options.setContextPath(CONTEXT_PATH); + options.setTestPathParam(true); + // servlet filters don't capture exceptions thrown in controllers + options.setTestException(false); + + options.setExpectedHttpRoute( + endpoint -> { + if (endpoint == ServerEndpoint.PATH_PARAM) { + return CONTEXT_PATH + "/path/{id}/param"; + } + return expectedHttpRoute(endpoint); + }); + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index aecf27bcfc96..749c1e44884d 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -440,8 +440,8 @@ include(":instrumentation:spring:spring-web-3.1:javaagent") include(":instrumentation:spring:spring-web-3.1:library") include(":instrumentation:spring:spring-web-3.1:testing") 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-webmvc-5.3:library") include(":instrumentation:spring:spring-webflux-5.0:javaagent") include(":instrumentation:spring:spring-webflux-5.0:library") include(":instrumentation:spring:spring-ws-2.0:javaagent")