From 1bfa83cb733517804db55157346c7f9aa7bd3eaf Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Mon, 9 Jan 2023 15:31:57 +0100 Subject: [PATCH] Register per-repository pattern for observability purposes. The standard Spring MVC observability integration registers the plain request pattern for observations. For our repository controllers that would result in one pattern registered for all individual repository resources (e.g. /{repository}/{id} etc.). However, the insights users would like to gain rather follows the individual repositories exposed. That's why we have so far exposed repository specific path pattern (/myrepo/{id}) via a custom request attribute. To adhere to the new observability integration of Spring Framework 6, we need to expose that particular pattern on the ServerRequestObservationContext, too. Fixes #2212. --- .../tests/AbstractWebIntegrationTests.java | 38 ++++++++++++++++++- .../data/rest/webmvc/jpa/JpaWebTests.java | 8 ++++ .../webmvc/RepositoryRestHandlerMapping.java | 9 ++++- 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/AbstractWebIntegrationTests.java b/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/AbstractWebIntegrationTests.java index d3b24a103..237ff4642 100755 --- a/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/AbstractWebIntegrationTests.java +++ b/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/AbstractWebIntegrationTests.java @@ -20,8 +20,17 @@ import static org.hamcrest.CoreMatchers.*; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; +import io.micrometer.observation.ObservationRegistry; +import jakarta.servlet.Filter; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import net.minidev.json.JSONArray; +import java.io.IOException; import java.util.Collections; import java.util.Map; import java.util.Optional; @@ -35,6 +44,7 @@ import org.springframework.hateoas.LinkRelation; import org.springframework.hateoas.client.LinkDiscoverers; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; +import org.springframework.http.server.observation.ServerRequestObservationContext; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; @@ -47,6 +57,7 @@ import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; import org.springframework.web.context.WebApplicationContext; +import org.springframework.web.filter.ServerHttpObservationFilter; import org.springframework.web.servlet.config.annotation.DelegatingWebMvcConfiguration; import com.jayway.jsonpath.InvalidPathException; @@ -74,6 +85,7 @@ public abstract class AbstractWebIntegrationTests { protected TestMvcClient client; protected MockMvc mvc; + protected ServerRequestObservationContext observationContext; @BeforeEach public void setUp() { @@ -82,8 +94,10 @@ public abstract class AbstractWebIntegrationTests { } protected void setupMockMvc() { - this.mvc = MockMvcBuilders.webAppContextSetup(context)// - .defaultRequest(get("/").accept(TestMvcClient.DEFAULT_MEDIA_TYPE)).build(); + this.mvc = MockMvcBuilders.webAppContextSetup(context) // + .defaultRequest(get("/").accept(TestMvcClient.DEFAULT_MEDIA_TYPE)) // + .addFilters(new FilterImplementation()) // + .build(); } protected MockHttpServletResponse postAndGet(Link link, Object payload, MediaType mediaType) throws Exception { @@ -263,4 +277,24 @@ public abstract class AbstractWebIntegrationTests { protected MultiValueMap getRootAndLinkedResources() { return new LinkedMultiValueMap(0); } + + /** + * Unconditionally registers a {@link ServerRequestObservationContext}. Required to be done explicitly as + * {@link ServerHttpObservationFilter} avoids the context registration in case of a NoOp-{@link ObservationRegistry}. + * + * @author Oliver Drotbohm + */ + private class FilterImplementation implements Filter { + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) + throws IOException, ServletException { + + observationContext = new ServerRequestObservationContext((HttpServletRequest) request, + (HttpServletResponse) response); + request.setAttribute(ServerHttpObservationFilter.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observationContext); + + chain.doFilter(request, response); + } + } } diff --git a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java index 89bd476a6..acd919fbe 100755 --- a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java +++ b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java @@ -753,6 +753,14 @@ public class JpaWebTests extends CommonWebTests { .andExpect(status().isOk()); } + @Test // #2212 + void exposesRepositoryPatternForObservation() throws Exception { + + mvc.perform(get("/authors/42")); + + assertThat(observationContext.getPathPattern()).isEqualTo("/authors/{id}"); + } + private List preparePersonResources(Person primary, Person... persons) throws Exception { Link peopleLink = client.discoverUnique(LinkRelation.of("people")); diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java index 2c6fc23d2..9faef3c89 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java @@ -42,6 +42,7 @@ import org.springframework.util.StringValueResolver; import org.springframework.web.bind.annotation.CrossOrigin; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.cors.CorsConfiguration; +import org.springframework.web.filter.ServerHttpObservationFilter; import org.springframework.web.method.HandlerMethod; import org.springframework.web.servlet.mvc.condition.PathPatternsRequestCondition; import org.springframework.web.servlet.mvc.condition.ProducesRequestCondition; @@ -242,8 +243,12 @@ public class RepositoryRestHandlerMapping extends BasePathAwareHandlerMapping { PathPatternParser parser = getPatternParser(); parser = parser != null ? parser : PARSER; - request.setAttribute(EFFECTIVE_LOOKUP_PATH_ATTRIBUTE, - parser.parse(pattern.replace("/{repository}", repositoryBasePath))); + var repositorySpecificPattern = pattern.replace("/{repository}", repositoryBasePath); + + ServerHttpObservationFilter.findObservationContext(request) + .ifPresent(context -> context.setPathPattern(repositorySpecificPattern)); + + request.setAttribute(EFFECTIVE_LOOKUP_PATH_ATTRIBUTE, parser.parse(repositorySpecificPattern)); } private static String getPattern(RequestMappingInfo info, HttpServletRequest request) {