From db79d1d2b9b28fa179a6bfd4669b49f78dd73751 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 2 Nov 2022 19:02:31 +0100 Subject: [PATCH] Set matching pattern in reactive server observation context This commit fixes the observation instrumentation for the reactive HTTP server by setting the best matching pattern determined by the web framework into the `ServerRequestObservationContext`. This information is required by the observation convention for creating the expected `KeyValue` for the matching pattern. Prior to this commit, the information was missing and resulted in an UNKNOWN key value. Fixes gh-29422 --- .../server/support/RouterFunctionMapping.java | 3 +++ .../handler/AbstractUrlHandlerMapping.java | 3 +++ .../method/RequestMappingInfoHandlerMapping.java | 3 +++ .../server/support/RouterFunctionMappingTests.java | 11 ++++++++++- .../RequestMappingInfoHandlerMappingTests.java | 14 ++++++++++++++ 5 files changed, 33 insertions(+), 1 deletion(-) 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 c2a14390de8..ae214f41b34 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 @@ -28,6 +28,7 @@ import org.springframework.http.codec.HttpMessageReader; import org.springframework.http.codec.ServerCodecConfigurer; import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; +import org.springframework.web.filter.reactive.ServerHttpObservationFilter; import org.springframework.web.reactive.function.server.HandlerFunction; import org.springframework.web.reactive.function.server.RouterFunction; import org.springframework.web.reactive.function.server.RouterFunctions; @@ -170,6 +171,8 @@ public class RouterFunctionMapping extends AbstractHandlerMapping implements Ini PathPattern matchingPattern = (PathPattern) attributes.get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE); if (matchingPattern != null) { attributes.put(BEST_MATCHING_PATTERN_ATTRIBUTE, matchingPattern); + ServerHttpObservationFilter.findObservationContext(serverRequest.exchange()) + .ifPresent(context -> context.setPathPattern(matchingPattern)); } 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 bf39a051163..39fd49080fa 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 @@ -30,6 +30,7 @@ import org.springframework.http.server.PathContainer; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; +import org.springframework.web.filter.reactive.ServerHttpObservationFilter; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.util.pattern.PathPattern; @@ -165,6 +166,8 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { exchange.getAttributes().put(BEST_MATCHING_HANDLER_ATTRIBUTE, handler); exchange.getAttributes().put(BEST_MATCHING_PATTERN_ATTRIBUTE, pattern); + ServerHttpObservationFilter.findObservationContext(exchange) + .ifPresent(context -> context.setPathPattern(pattern)); 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 1a1ee0073aa..d16ee75ca9e 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 @@ -38,6 +38,7 @@ import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.util.Assert; import org.springframework.util.MultiValueMap; import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.filter.reactive.ServerHttpObservationFilter; import org.springframework.web.method.HandlerMethod; import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.reactive.result.condition.NameValueExpression; @@ -139,6 +140,8 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe exchange.getAttributes().put(BEST_MATCHING_HANDLER_ATTRIBUTE, handlerMethod); exchange.getAttributes().put(BEST_MATCHING_PATTERN_ATTRIBUTE, bestPattern); + ServerHttpObservationFilter.findObservationContext(exchange) + .ifPresent(context -> context.setPathPattern(bestPattern)); 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 7056922b9c1..344c57da3c1 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 @@ -22,6 +22,8 @@ import reactor.test.StepVerifier; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.http.codec.ServerCodecConfigurer; +import org.springframework.http.observation.reactive.ServerRequestObservationContext; +import org.springframework.web.filter.reactive.ServerHttpObservationFilter; import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.reactive.function.server.HandlerFunction; import org.springframework.web.reactive.function.server.RouterFunction; @@ -34,8 +36,10 @@ 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}. * @author Arjen Poutsma * @author Brian Clozel */ @@ -132,6 +136,8 @@ class RouterFunctionMappingTests { PathPattern matchingPattern = exchange.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); assertThat(matchingPattern).isNotNull(); assertThat(matchingPattern.getPatternString()).isEqualTo("/match"); + assertThat(ServerHttpObservationFilter.findObservationContext(exchange)) + .hasValueSatisfying(context -> assertThat(context.getPathPattern()).isEqualTo(matchingPattern)); ServerRequest serverRequest = exchange.getAttribute(RouterFunctions.REQUEST_ATTRIBUTE); assertThat(serverRequest).isNotNull(); @@ -141,7 +147,10 @@ class RouterFunctionMappingTests { } private ServerWebExchange createExchange(String urlTemplate) { - return MockServerWebExchange.from(MockServerHttpRequest.get(urlTemplate)); + MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(urlTemplate)); + ServerRequestObservationContext observationContext = new ServerRequestObservationContext(exchange); + exchange.getAttributes().put(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 1862a4dd163..acf0cf9ce98 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 @@ -36,6 +36,7 @@ import org.springframework.core.annotation.AnnotationUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; +import org.springframework.http.observation.reactive.ServerRequestObservationContext; import org.springframework.lang.Nullable; import org.springframework.stereotype.Controller; import org.springframework.util.ClassUtils; @@ -63,6 +64,7 @@ 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; @@ -257,6 +259,18 @@ public class RequestMappingInfoHandlerMappingTests { assertThat(mapped).isSameAs(handlerMethod); } + @Test + public void handleMatchBestMatchingPatternAttributeInObservationContext() { + RequestMappingInfo key = paths("/{path1}/2", "/**").build(); + ServerWebExchange exchange = MockServerWebExchange.from(get("/1/2")); + ServerRequestObservationContext observationContext = new ServerRequestObservationContext(exchange); + exchange.getAttributes().put(CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observationContext); + this.handlerMapping.handleMatch(key, handlerMethod, exchange); + + assertThat(observationContext.getPathPattern()).isNotNull(); + assertThat(observationContext.getPathPattern().toString()).isEqualTo("/{path1}/2"); + } + @Test // gh-22543 public void handleMatchBestMatchingPatternAttributeNoPatternsDefined() { ServerWebExchange exchange = MockServerWebExchange.from(get(""));