diff --git a/framework-docs/modules/ROOT/pages/integration/observability.adoc b/framework-docs/modules/ROOT/pages/integration/observability.adoc index 0fe7a1a8b7..7082095cf8 100644 --- a/framework-docs/modules/ROOT/pages/integration/observability.adoc +++ b/framework-docs/modules/ROOT/pages/integration/observability.adoc @@ -121,11 +121,15 @@ By default, the following `KeyValues` are created: [[observability.http-server.reactive]] === Reactive applications -Applications need to configure the `org.springframework.web.filter.reactive.ServerHttpObservationFilter` reactive `WebFilter` in their application. +Applications need to configure the `WebHttpHandlerBuilder` with a `MeterRegistry` to enable server instrumentation. +This can be done on the `WebHttpHandlerBuilder`, as follows: + +include::code:HttpHandlerConfiguration[] + It is using the `org.springframework.http.server.reactive.observation.DefaultServerRequestObservationConvention` by default, backed by the `ServerRequestObservationContext`. -This will only record an observation as an error if the `Exception` has not been handled by the web Framework and has bubbled up to the `WebFilter`. -Typically, all exceptions handled by Spring WebFlux's `@ExceptionHandler` and xref:web/webflux/ann-rest-exceptions.adoc[`ProblemDetail` support] will not be recorded with the observation. +This will only record an observation as an error if the `Exception` has not been handled by an application Controller. +Typically, all exceptions handled by Spring WebFlux's `@ExceptionHandler` and <> will not be recorded with the observation. You can, at any point during request processing, set the error field on the `ObservationContext` yourself: include-code::./UserController[] diff --git a/framework-docs/src/main/java/org/springframework/docs/integration/observability/httpserver/reactive/HttpHandlerConfiguration.java b/framework-docs/src/main/java/org/springframework/docs/integration/observability/httpserver/reactive/HttpHandlerConfiguration.java new file mode 100644 index 0000000000..dab8da25d5 --- /dev/null +++ b/framework-docs/src/main/java/org/springframework/docs/integration/observability/httpserver/reactive/HttpHandlerConfiguration.java @@ -0,0 +1,42 @@ +/* + * Copyright 2002-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.docs.integration.observability.httpserver.reactive; + +import io.micrometer.observation.ObservationRegistry; + +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.http.server.reactive.HttpHandler; +import org.springframework.web.server.adapter.WebHttpHandlerBuilder; + +@Configuration(proxyBeanMethods = false) +public class HttpHandlerConfiguration { + + private final ApplicationContext applicationContext; + + public HttpHandlerConfiguration(ApplicationContext applicationContext) { + this.applicationContext = applicationContext; + } + + @Bean + public HttpHandler httpHandler(ObservationRegistry registry) { + return WebHttpHandlerBuilder.applicationContext(this.applicationContext) + .observationRegistry(registry) + .build(); + } +} diff --git a/framework-docs/src/main/java/org/springframework/docs/integration/observability/httpserver/reactive/UserController.java b/framework-docs/src/main/java/org/springframework/docs/integration/observability/httpserver/reactive/UserController.java index 0f7d8b68a6..2cd35a755c 100644 --- a/framework-docs/src/main/java/org/springframework/docs/integration/observability/httpserver/reactive/UserController.java +++ b/framework-docs/src/main/java/org/springframework/docs/integration/observability/httpserver/reactive/UserController.java @@ -17,9 +17,9 @@ package org.springframework.docs.integration.observability.httpserver.reactive; import org.springframework.http.ResponseEntity; +import org.springframework.http.server.reactive.observation.ServerRequestObservationContext; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.ExceptionHandler; -import org.springframework.web.filter.reactive.ServerHttpObservationFilter; import org.springframework.web.server.ServerWebExchange; @Controller @@ -28,7 +28,7 @@ public class UserController { @ExceptionHandler(MissingUserException.class) ResponseEntity handleMissingUser(ServerWebExchange exchange, MissingUserException exception) { // We want to record this exception with the observation - ServerHttpObservationFilter.findObservationContext(exchange) + ServerRequestObservationContext.findCurrent(exchange) .ifPresent(context -> context.setError(exception)); return ResponseEntity.notFound().build(); } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/observation/ServerRequestObservationContext.java b/spring-web/src/main/java/org/springframework/http/server/reactive/observation/ServerRequestObservationContext.java index 070d863734..16db5ef5c8 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/observation/ServerRequestObservationContext.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/observation/ServerRequestObservationContext.java @@ -18,12 +18,14 @@ package org.springframework.http.server.reactive.observation; import java.util.Collections; import java.util.Map; +import java.util.Optional; import io.micrometer.observation.transport.RequestReplyReceiverContext; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.lang.Nullable; +import org.springframework.web.server.ServerWebExchange; /** * Context that holds information for metadata collection regarding @@ -36,6 +38,12 @@ import org.springframework.lang.Nullable; */ public class ServerRequestObservationContext extends RequestReplyReceiverContext { + /** + * Name of the request attribute holding the {@link ServerRequestObservationContext context} for the current observation. + * @since 6.1.0 + */ + public static final String CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE = ServerRequestObservationContext.class.getName() + ".context"; + private final Map attributes; @Nullable @@ -50,6 +58,16 @@ public class ServerRequestObservationContext extends RequestReplyReceiverContext this.attributes = Collections.unmodifiableMap(attributes); } + /** + * Get the current {@link ServerRequestObservationContext observation context} from the given exchange, if available. + * @param exchange the current exchange + * @return the current observation context + * @since 6.1.0 + */ + public static Optional findCurrent(ServerWebExchange exchange) { + return Optional.ofNullable(exchange.getAttribute(CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE)); + } + /** * Return an immutable map of the current request attributes. */ diff --git a/spring-web/src/main/java/org/springframework/web/filter/reactive/ServerHttpObservationFilter.java b/spring-web/src/main/java/org/springframework/web/filter/reactive/ServerHttpObservationFilter.java index ca75df7e7a..8e0bb6d22d 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/reactive/ServerHttpObservationFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/reactive/ServerHttpObservationFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -33,6 +33,7 @@ import org.springframework.http.server.reactive.observation.ServerRequestObserva import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebFilter; import org.springframework.web.server.WebFilterChain; +import org.springframework.web.server.adapter.WebHttpHandlerBuilder; /** @@ -47,7 +48,9 @@ import org.springframework.web.server.WebFilterChain; * * @author Brian Clozel * @since 6.0 + * @deprecated since 6.1.0 in favor of {@link WebHttpHandlerBuilder}. */ +@Deprecated(since = "6.1.0", forRemoval = true) public class ServerHttpObservationFilter implements WebFilter { /** diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java b/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java index 6c8405a565..2ca930100d 100644 --- a/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,8 +20,12 @@ import java.util.Map; import java.util.Set; import java.util.function.Function; +import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationRegistry; +import io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.reactivestreams.Publisher; import reactor.core.publisher.Mono; import org.springframework.context.ApplicationContext; @@ -36,11 +40,16 @@ import org.springframework.http.codec.multipart.Part; import org.springframework.http.server.reactive.HttpHandler; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.http.server.reactive.observation.DefaultServerRequestObservationConvention; +import org.springframework.http.server.reactive.observation.ServerHttpObservationDocumentation; +import org.springframework.http.server.reactive.observation.ServerRequestObservationContext; +import org.springframework.http.server.reactive.observation.ServerRequestObservationConvention; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebHandler; +import org.springframework.web.server.handler.ExceptionHandlingWebHandler; import org.springframework.web.server.handler.WebHandlerDecorator; import org.springframework.web.server.i18n.AcceptHeaderLocaleContextResolver; import org.springframework.web.server.i18n.LocaleContextResolver; @@ -55,6 +64,7 @@ import org.springframework.web.server.session.WebSessionManager; * * @author Rossen Stoyanchev * @author Sebastien Deleuze + * @author Brian Clozel * @since 5.0 */ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHandler { @@ -75,6 +85,8 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa private static final Set DISCONNECTED_CLIENT_EXCEPTIONS = Set.of("AbortedException", "ClientAbortException", "EOFException", "EofException"); + private static final ServerRequestObservationConvention DEFAULT_OBSERVATION_CONVENTION = new DefaultServerRequestObservationConvention(); + private static final Log logger = LogFactory.getLog(HttpWebHandlerAdapter.class); @@ -91,6 +103,12 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa @Nullable private ForwardedHeaderTransformer forwardedHeaderTransformer; + @Nullable + private ObservationRegistry observationRegistry; + + @Nullable + private ServerRequestObservationConvention observationConvention; + @Nullable private ApplicationContext applicationContext; @@ -192,6 +210,44 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa return this.forwardedHeaderTransformer; } + /** + * Configure a {@link ObservationRegistry} for recording server exchange observations. + * By default, a {@link ObservationRegistry#NOOP no-op} instance will be used. + * @param observationRegistry the observation registry to use + * @since 6.1.0 + */ + public void setObservationRegistry(ObservationRegistry observationRegistry) { + this.observationRegistry = observationRegistry; + } + + /** + * Return the configured {@link ObservationRegistry}. + * @since 6.1.0 + */ + @Nullable + public ObservationRegistry getObservationRegistry() { + return this.observationRegistry; + } + + /** + * Configure a {@link ServerRequestObservationConvention} for server exchanges observations. + * By default, a {@link DefaultServerRequestObservationConvention} instance will be used. + * @param observationConvention the observation convention to use + * @since 6.1.0 + */ + public void setObservationConvention(ServerRequestObservationConvention observationConvention) { + this.observationConvention = observationConvention; + } + + /** + * Return the Observation convention configured for server exchanges observations. + * @since 6.1.0 + */ + @Nullable + public ServerRequestObservationConvention getObservationConvention() { + return this.observationConvention; + } + /** * Configure the {@code ApplicationContext} associated with the web application, * if it was initialized with one via @@ -247,9 +303,12 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa exchange.getLogPrefix() + formatRequest(exchange.getRequest()) + (traceOn ? ", headers=" + formatHeaders(exchange.getRequest().getHeaders()) : "")); + ServerRequestObservationContext observationContext = new ServerRequestObservationContext(exchange.getRequest(), + exchange.getResponse(), exchange.getAttributes()); + exchange.getAttributes().put(ServerRequestObservationContext.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observationContext); + return getDelegate().handle(exchange) - .doOnSuccess(aVoid -> logResponse(exchange)) - .onErrorResume(ex -> handleUnresolvedError(exchange, ex)) + .transformDeferred(call -> transform(exchange, observationContext, call)) .then(cleanupMultipart(exchange)) .then(Mono.defer(response::setComplete)); } @@ -271,6 +330,42 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa return "HTTP " + request.getMethod() + " \"" + request.getPath() + query + "\""; } + private Publisher transform(ServerWebExchange exchange, ServerRequestObservationContext observationContext, Mono call) { + Observation observation = ServerHttpObservationDocumentation.HTTP_REACTIVE_SERVER_REQUESTS.observation(this.observationConvention, + DEFAULT_OBSERVATION_CONVENTION, () -> observationContext, this.observationRegistry); + observation.start(); + return call + .doOnSuccess(aVoid -> { + logResponse(exchange); + stopObservation(observation, exchange); + }) + .onErrorResume(ex -> handleUnresolvedError(exchange, observationContext, ex)) + .doOnCancel(() -> cancelObservation(observationContext, observation)) + .contextWrite(context -> context.put(ObservationThreadLocalAccessor.KEY, observation)); + } + + private void stopObservation(Observation observation, ServerWebExchange exchange) { + Throwable throwable = exchange.getAttribute(ExceptionHandlingWebHandler.HANDLED_WEB_EXCEPTION); + if (throwable != null) { + observation.error(throwable); + } + ServerHttpResponse response = exchange.getResponse(); + if (response.isCommitted()) { + observation.stop(); + } + else { + response.beforeCommit(() -> { + observation.stop(); + return Mono.empty(); + }); + } + } + + private void cancelObservation(ServerRequestObservationContext observationContext, Observation observation) { + observationContext.setConnectionAborted(true); + observation.stop(); + } + private void logResponse(ServerWebExchange exchange) { LogFormatUtils.traceDebug(logger, traceOn -> { HttpStatusCode status = exchange.getResponse().getStatusCode(); @@ -284,7 +379,7 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa responseHeaders.toString() : responseHeaders.isEmpty() ? "{}" : "{masked}"; } - private Mono handleUnresolvedError(ServerWebExchange exchange, Throwable ex) { + private Mono handleUnresolvedError(ServerWebExchange exchange, ServerRequestObservationContext observationContext, Throwable ex) { ServerHttpRequest request = exchange.getRequest(); ServerHttpResponse response = exchange.getResponse(); String logPrefix = exchange.getLogPrefix(); @@ -304,6 +399,7 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa lostClientLogger.debug(logPrefix + "Client went away: " + ex + " (stacktrace at TRACE level for '" + DISCONNECTED_CLIENT_LOG_CATEGORY + "')"); } + observationContext.setConnectionAborted(true); return Mono.empty(); } else { diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java b/spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java index 0a6a87bada..d64f0e11cb 100644 --- a/spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,7 @@ import java.util.List; import java.util.function.Consumer; import java.util.function.Function; +import io.micrometer.observation.ObservationRegistry; import reactor.blockhound.BlockHound; import reactor.blockhound.integration.BlockHoundIntegration; @@ -31,6 +32,8 @@ import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.http.codec.ServerCodecConfigurer; import org.springframework.http.server.reactive.HttpHandler; import org.springframework.http.server.reactive.HttpHandlerDecoratorFactory; +import org.springframework.http.server.reactive.observation.DefaultServerRequestObservationConvention; +import org.springframework.http.server.reactive.observation.ServerRequestObservationConvention; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; @@ -104,6 +107,12 @@ public final class WebHttpHandlerBuilder { @Nullable private ForwardedHeaderTransformer forwardedHeaderTransformer; + @Nullable + private ObservationRegistry observationRegistry; + + @Nullable + private ServerRequestObservationConvention observationConvention; + /** * Private constructor to use when initialized from an ApplicationContext. @@ -126,6 +135,8 @@ public final class WebHttpHandlerBuilder { this.codecConfigurer = other.codecConfigurer; this.localeContextResolver = other.localeContextResolver; this.forwardedHeaderTransformer = other.forwardedHeaderTransformer; + this.observationRegistry = other.observationRegistry; + this.observationConvention = other.observationConvention; this.httpHandlerDecorator = other.httpHandlerDecorator; } @@ -359,6 +370,28 @@ public final class WebHttpHandlerBuilder { return (this.forwardedHeaderTransformer != null); } + /** + * Configure a {@link ObservationRegistry} for recording server exchange observations. + * By default, a {@link ObservationRegistry#NOOP no-op} registry will be configured. + * @param observationRegistry the observation registry + * @since 6.1.0 + */ + public WebHttpHandlerBuilder observationRegistry(ObservationRegistry observationRegistry) { + this.observationRegistry = observationRegistry; + return this; + } + + /** + * Configure a {@link ServerRequestObservationConvention} to use for server observations. + * By default, a {@link DefaultServerRequestObservationConvention} will be used. + * @param observationConvention the convention to use for all recorded observations + * @since 6.1.0 + */ + public WebHttpHandlerBuilder observationConvention(ServerRequestObservationConvention observationConvention) { + this.observationConvention = observationConvention; + return this; + } + /** * Configure a {@link Function} to decorate the {@link HttpHandler} returned * by this builder which effectively wraps the entire @@ -404,6 +437,12 @@ public final class WebHttpHandlerBuilder { if (this.forwardedHeaderTransformer != null) { adapted.setForwardedHeaderTransformer(this.forwardedHeaderTransformer); } + if (this.observationRegistry != null) { + adapted.setObservationRegistry(this.observationRegistry); + } + if (this.observationConvention != null) { + adapted.setObservationConvention(this.observationConvention); + } if (this.applicationContext != null) { adapted.setApplicationContext(this.applicationContext); } diff --git a/spring-web/src/main/java/org/springframework/web/server/handler/ExceptionHandlingWebHandler.java b/spring-web/src/main/java/org/springframework/web/server/handler/ExceptionHandlingWebHandler.java index a1e61e1eea..b8b01951b3 100644 --- a/spring-web/src/main/java/org/springframework/web/server/handler/ExceptionHandlingWebHandler.java +++ b/spring-web/src/main/java/org/springframework/web/server/handler/ExceptionHandlingWebHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -38,6 +38,13 @@ import org.springframework.web.server.WebHandler; */ public class ExceptionHandlingWebHandler extends WebHandlerDecorator { + /** + * Name of the {@link ServerWebExchange#getAttributes() attribute} that + * contains the exception handled by {@link WebExceptionHandler WebExceptionHandlers}. + * @since 6.0.8 + */ + public static String HANDLED_WEB_EXCEPTION = ExceptionHandlingWebHandler.class.getSimpleName() + ".handledException"; + private final List exceptionHandlers; @@ -74,7 +81,8 @@ public class ExceptionHandlingWebHandler extends WebHandlerDecorator { } for (WebExceptionHandler handler : this.exceptionHandlers) { - completion = completion.onErrorResume(ex -> handler.handle(exchange, ex)); + completion = completion.doOnError(error -> exchange.getAttributes().put(HANDLED_WEB_EXCEPTION, error)) + .onErrorResume(ex -> handler.handle(exchange, ex)); } return completion; } diff --git a/spring-web/src/test/java/org/springframework/web/filter/reactive/ServerHttpObservationFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/reactive/ServerHttpObservationFilterTests.java index 4bef48c114..b6a6d98cce 100644 --- a/spring-web/src/test/java/org/springframework/web/filter/reactive/ServerHttpObservationFilterTests.java +++ b/spring-web/src/test/java/org/springframework/web/filter/reactive/ServerHttpObservationFilterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -40,6 +40,7 @@ import static org.assertj.core.api.Assertions.assertThat; * * @author Brian Clozel */ +@SuppressWarnings("removal") class ServerHttpObservationFilterTests { private final TestObservationRegistry observationRegistry = TestObservationRegistry.create(); diff --git a/spring-web/src/test/java/org/springframework/web/server/adapter/HttpWebHandlerAdapterObservabilityTests.java b/spring-web/src/test/java/org/springframework/web/server/adapter/HttpWebHandlerAdapterObservabilityTests.java new file mode 100644 index 0000000000..94d8c8b46d --- /dev/null +++ b/spring-web/src/test/java/org/springframework/web/server/adapter/HttpWebHandlerAdapterObservabilityTests.java @@ -0,0 +1,158 @@ +/* + * Copyright 2002-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.server.adapter; + + +import java.util.List; +import java.util.Optional; + +import io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor; +import io.micrometer.observation.tck.TestObservationRegistry; +import io.micrometer.observation.tck.TestObservationRegistryAssert; +import org.junit.jupiter.api.Test; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; +import reactor.util.context.ContextView; + +import org.springframework.http.HttpStatus; +import org.springframework.http.server.reactive.observation.ServerRequestObservationContext; +import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.WebExceptionHandler; +import org.springframework.web.server.WebHandler; +import org.springframework.web.server.handler.ExceptionHandlingWebHandler; +import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest; +import org.springframework.web.testfixture.http.server.reactive.MockServerHttpResponse; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Observability-related tests for {@link HttpWebHandlerAdapter}. + * @author Brian Clozel + */ +class HttpWebHandlerAdapterObservabilityTests { + + private final TestObservationRegistry observationRegistry = TestObservationRegistry.create(); + + private final MockServerHttpRequest request = MockServerHttpRequest.post("/test/resource").build(); + + private final MockServerHttpResponse response = new MockServerHttpResponse(); + + @Test + void handlerShouldSetObservationContextOnExchange() { + HttpStatusSuccessStubWebHandler targetHandler = new HttpStatusSuccessStubWebHandler(HttpStatus.OK); + createWebHandler(targetHandler).handle(this.request, this.response).block(); + assertThat(targetHandler.observationContext).isPresent(); + assertThat(targetHandler.observationContext.get().getCarrier()).isEqualTo(this.request); + assertThat(targetHandler.observationContext.get().getResponse()).isEqualTo(this.response); + assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS"); + } + + @Test + void handlerShouldSetCurrentObservationInReactorContext() { + ReactorContextWebHandler targetHandler = new ReactorContextWebHandler(); + createWebHandler(targetHandler).handle(this.request, this.response).block(); + assertThat(targetHandler.contextView.getOrEmpty(ObservationThreadLocalAccessor.KEY)).isPresent(); + assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS"); + } + + @Test + void handlerShouldSeeHandledException() { + ThrowingExceptionWebHandler throwingHandler = new ThrowingExceptionWebHandler(new IllegalStateException("testing error")); + ExceptionHandlingWebHandler targetHandler = new ExceptionHandlingWebHandler(throwingHandler, List.of(new BadRequestExceptionHandler())); + createWebHandler(targetHandler).handle(this.request, this.response).block(); + assertThat(throwingHandler.observationContext).isPresent(); + assertThat(throwingHandler.observationContext.get().getError()).isInstanceOf(IllegalStateException.class); + assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "CLIENT_ERROR"); + } + + @Test + void handlerShouldRecordObservationWhenCancelled() { + HttpStatusSuccessStubWebHandler targetHandler = new HttpStatusSuccessStubWebHandler(HttpStatus.OK); + StepVerifier.create(createWebHandler(targetHandler).handle(this.request, this.response)).thenCancel().verify(); + assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "UNKNOWN"); + } + + private HttpWebHandlerAdapter createWebHandler(WebHandler targetHandler) { + HttpWebHandlerAdapter handlerAdapter = new HttpWebHandlerAdapter(targetHandler); + handlerAdapter.setObservationRegistry(this.observationRegistry); + return handlerAdapter; + } + + private TestObservationRegistryAssert.TestObservationRegistryAssertReturningObservationContextAssert assertThatHttpObservation() { + return TestObservationRegistryAssert.assertThat(this.observationRegistry) + .hasObservationWithNameEqualTo("http.server.requests").that(); + } + + private static class HttpStatusSuccessStubWebHandler implements WebHandler { + + private final HttpStatus responseStatus; + + private Optional observationContext; + + public HttpStatusSuccessStubWebHandler(HttpStatus responseStatus) { + this.responseStatus = responseStatus; + } + + @Override + public Mono handle(ServerWebExchange exchange) { + this.observationContext = ServerRequestObservationContext.findCurrent(exchange); + exchange.getResponse().setStatusCode(this.responseStatus); + return Mono.empty(); + } + } + + private static class ReactorContextWebHandler implements WebHandler { + + ContextView contextView; + + @Override + public Mono handle(ServerWebExchange exchange) { + exchange.getResponse().setStatusCode(HttpStatus.OK); + return Mono.deferContextual(contextView -> { + this.contextView = contextView; + return Mono.empty(); + }); + } + } + + private static class ThrowingExceptionWebHandler implements WebHandler { + + private final Throwable exception; + + private Optional observationContext; + + private ThrowingExceptionWebHandler(Throwable exception) { + this.exception = exception; + } + + @Override + public Mono handle(ServerWebExchange exchange) { + this.observationContext = ServerRequestObservationContext.findCurrent(exchange); + return Mono.error(this.exception); + } + } + + private static class BadRequestExceptionHandler implements WebExceptionHandler { + + @Override + public Mono handle(ServerWebExchange exchange, Throwable ex) { + exchange.getResponse().setStatusCode(HttpStatus.BAD_REQUEST); + return exchange.getResponse().setComplete(); + } + } + +} diff --git a/spring-web/src/test/java/org/springframework/web/server/handler/ExceptionHandlingWebHandlerTests.java b/spring-web/src/test/java/org/springframework/web/server/handler/ExceptionHandlingWebHandlerTests.java index 4e27848f5f..412263cd96 100644 --- a/spring-web/src/test/java/org/springframework/web/server/handler/ExceptionHandlingWebHandlerTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/handler/ExceptionHandlingWebHandlerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -45,13 +45,13 @@ public class ExceptionHandlingWebHandlerTests { @Test - public void handleErrorSignal() throws Exception { + void handleErrorSignal() { createWebHandler(new BadRequestExceptionHandler()).handle(this.exchange).block(); assertThat(this.exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); } @Test - public void handleErrorSignalWithMultipleHttpErrorHandlers() throws Exception { + void handleErrorSignalWithMultipleHttpErrorHandlers() { createWebHandler( new UnresolvedExceptionHandler(), new UnresolvedExceptionHandler(), @@ -62,17 +62,14 @@ public class ExceptionHandlingWebHandlerTests { } @Test - public void unresolvedException() throws Exception { + void unresolvedException() { Mono mono = createWebHandler(new UnresolvedExceptionHandler()).handle(this.exchange); StepVerifier.create(mono).expectErrorMessage("boo").verify(); assertThat(this.exchange.getResponse().getStatusCode()).isNull(); } @Test - public void unresolvedExceptionWithWebHttpHandlerAdapter() throws Exception { - - // HttpWebHandlerAdapter handles unresolved errors - + void unresolvedExceptionWithWebHttpHandlerAdapter() { new HttpWebHandlerAdapter(createWebHandler(new UnresolvedExceptionHandler())) .handle(this.exchange.getRequest(), this.exchange.getResponse()).block(); @@ -80,11 +77,18 @@ public class ExceptionHandlingWebHandlerTests { } @Test - public void thrownExceptionBecomesErrorSignal() throws Exception { + void thrownExceptionBecomesErrorSignal() { createWebHandler(new BadRequestExceptionHandler()).handle(this.exchange).block(); assertThat(this.exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); } + @Test + void thrownExceptionIsStoredAsExchangeAttribute() { + createWebHandler(new BadRequestExceptionHandler()).handle(this.exchange).block(); + Exception exceptionAttribute = this.exchange.getAttribute(ExceptionHandlingWebHandler.HANDLED_WEB_EXCEPTION); + assertThat(exceptionAttribute).isInstanceOf(IllegalStateException.class); + } + private WebHandler createWebHandler(WebExceptionHandler... handlers) { return new ExceptionHandlingWebHandler(this.targetHandler, Arrays.asList(handlers)); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/support/RouterFunctionMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/support/RouterFunctionMapping.java index 2dd4a8142d..1d44e77274 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/support/RouterFunctionMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/support/RouterFunctionMapping.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,6 +26,7 @@ import reactor.core.publisher.Mono; import org.springframework.beans.factory.InitializingBean; import org.springframework.http.codec.HttpMessageReader; import org.springframework.http.codec.ServerCodecConfigurer; +import org.springframework.http.server.reactive.observation.ServerRequestObservationContext; import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; import org.springframework.web.filter.reactive.ServerHttpObservationFilter; @@ -161,7 +162,7 @@ public class RouterFunctionMapping extends AbstractHandlerMapping implements Ini } } - @SuppressWarnings("unchecked") + @SuppressWarnings({"unchecked", "removal"}) private void setAttributes( Map attributes, ServerRequest serverRequest, HandlerFunction handlerFunction) { @@ -173,6 +174,8 @@ public class RouterFunctionMapping extends AbstractHandlerMapping implements Ini attributes.put(BEST_MATCHING_PATTERN_ATTRIBUTE, matchingPattern); ServerHttpObservationFilter.findObservationContext(serverRequest.exchange()) .ifPresent(context -> context.setPathPattern(matchingPattern.toString())); + ServerRequestObservationContext.findCurrent(serverRequest.exchange()) + .ifPresent(context -> context.setPathPattern(matchingPattern.toString())); } Map uriVariables = (Map) attributes.get(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java index a4997b3ab8..0d92856404 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,6 +27,7 @@ import reactor.core.publisher.Mono; import org.springframework.beans.BeansException; import org.springframework.http.server.PathContainer; +import org.springframework.http.server.reactive.observation.ServerRequestObservationContext; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -128,6 +129,7 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { * @see org.springframework.web.util.pattern.PathPattern */ @Nullable + @SuppressWarnings("removal") protected Object lookupHandler(PathContainer lookupPath, ServerWebExchange exchange) throws Exception { List matches = null; for (PathPattern pattern : this.handlerMap.keySet()) { @@ -168,6 +170,8 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { exchange.getAttributes().put(BEST_MATCHING_PATTERN_ATTRIBUTE, pattern); ServerHttpObservationFilter.findObservationContext(exchange) .ifPresent(context -> context.setPathPattern(pattern.toString())); + ServerRequestObservationContext.findCurrent(exchange) + .ifPresent(context -> context.setPathPattern(pattern.toString())); exchange.getAttributes().put(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, pathWithinMapping); exchange.getAttributes().put(URI_TEMPLATE_VARIABLES_ATTRIBUTE, matchInfo.getUriVariables()); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java index 3494b8bb9a..5e1be8dc62 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java @@ -35,6 +35,7 @@ import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.MediaType; import org.springframework.http.server.PathContainer; import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.http.server.reactive.observation.ServerRequestObservationContext; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.MultiValueMap; @@ -113,6 +114,7 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe * @see HandlerMapping#PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE */ @Override + @SuppressWarnings("removal") protected void handleMatch(RequestMappingInfo info, HandlerMethod handlerMethod, ServerWebExchange exchange) { @@ -143,6 +145,8 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe exchange.getAttributes().put(BEST_MATCHING_PATTERN_ATTRIBUTE, bestPattern); ServerHttpObservationFilter.findObservationContext(exchange) .ifPresent(context -> context.setPathPattern(bestPattern.toString())); + ServerRequestObservationContext.findCurrent(exchange) + .ifPresent(context -> context.setPathPattern(bestPattern.toString())); exchange.getAttributes().put(URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriVariables); exchange.getAttributes().put(MATRIX_VARIABLES_ATTRIBUTE, matrixVariables); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/support/RouterFunctionMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/support/RouterFunctionMappingTests.java index c861561835..f27974eb58 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/support/RouterFunctionMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/support/RouterFunctionMappingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,7 +36,6 @@ import org.springframework.web.testfixture.server.MockServerWebExchange; import org.springframework.web.util.pattern.PathPattern; import static org.assertj.core.api.Assertions.assertThat; -import static org.springframework.web.filter.reactive.ServerHttpObservationFilter.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE; /** * Tests for {@link RouterFunctionMapping}. @@ -117,6 +116,7 @@ class RouterFunctionMappingTests { } @Test + @SuppressWarnings("removal") void mappedRequestShouldHoldAttributes() { HandlerFunction handlerFunction = request -> ServerResponse.ok().build(); RouterFunction routerFunction = RouterFunctions.route() @@ -138,6 +138,8 @@ class RouterFunctionMappingTests { assertThat(matchingPattern.getPatternString()).isEqualTo("/match"); assertThat(ServerHttpObservationFilter.findObservationContext(exchange)) .hasValueSatisfying(context -> assertThat(context.getPathPattern()).isEqualTo(matchingPattern.getPatternString())); + assertThat(ServerRequestObservationContext.findCurrent(exchange)) + .hasValueSatisfying(context -> assertThat(context.getPathPattern()).isEqualTo(matchingPattern.getPatternString())); ServerRequest serverRequest = exchange.getAttribute(RouterFunctions.REQUEST_ATTRIBUTE); assertThat(serverRequest).isNotNull(); @@ -146,10 +148,12 @@ class RouterFunctionMappingTests { assertThat(handler).isEqualTo(handlerFunction); } + @SuppressWarnings("removal") private ServerWebExchange createExchange(String urlTemplate) { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(urlTemplate)); ServerRequestObservationContext observationContext = new ServerRequestObservationContext(exchange.getRequest(), exchange.getResponse(), exchange.getAttributes()); - exchange.getAttributes().put(CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observationContext); + exchange.getAttributes().put(ServerHttpObservationFilter.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observationContext); + exchange.getAttributes().put(ServerRequestObservationContext.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observationContext); return exchange; } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java index bcdeefc7c3..b4d7394089 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java @@ -65,7 +65,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.web.bind.annotation.RequestMethod.GET; import static org.springframework.web.bind.annotation.RequestMethod.HEAD; import static org.springframework.web.bind.annotation.RequestMethod.OPTIONS; -import static org.springframework.web.filter.reactive.ServerHttpObservationFilter.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE; import static org.springframework.web.reactive.HandlerMapping.BEST_MATCHING_HANDLER_ATTRIBUTE; import static org.springframework.web.reactive.HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE; import static org.springframework.web.reactive.result.method.RequestMappingInfo.paths; @@ -261,11 +260,12 @@ public class RequestMappingInfoHandlerMappingTests { } @Test + @SuppressWarnings("removal") public void handleMatchBestMatchingPatternAttributeInObservationContext() { RequestMappingInfo key = paths("/{path1}/2", "/**").build(); ServerWebExchange exchange = MockServerWebExchange.from(get("/1/2")); ServerRequestObservationContext observationContext = new ServerRequestObservationContext(exchange.getRequest(), exchange.getResponse(), exchange.getAttributes()); - exchange.getAttributes().put(CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observationContext); + exchange.getAttributes().put(ServerRequestObservationContext.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observationContext); this.handlerMapping.handleMatch(key, handlerMethod, exchange); assertThat(observationContext.getPathPattern()).isEqualTo("/{path1}/2");