A fairly common request is to add one or more metrics tags based on the contents of the response body. For example, you may have a REST endpoint that returns a customer with address information, and you want to tag with the customer's state/province and country.

Because the response body can only be read once, we'd want to use Spring's ContentCachingResponseWrapper to read the response body and extract the data necessary for the tag, but WebMvcMetricsFilter doesn't do this cache wrapping, and it's too late to do this in a custom WebMvcTagsProvider.

The only pattern I can suggest (as demonstrated in this sample) right now is for the user to add another OncePerRequestFilter that caches off the tags per response somewhere for later consumption in a custom WebMvcTagsProvider, but this seems a bit awkward?

Would it be reasonable for WebMvcMetricsFilter (and the corollary for WebFlux) to do this response wrapping, passing ContentCachingResponseWrapper to WebMvcTagsProvider? Is there a way we can do even better, passing the model object returned by the API endpoint (e.g. Person in the sample) to the tags provider so the user doesn't have to manually deserialize the response body?

Comment From: bclozel

In my opinion any option involving the raw response body is wasteful and will significantly hinder performance. Also, deserializing the response body is hard to achieve because the response content-type doesn't tell you exactly which message converter/codec was used. For WebFlux, things are even harder with allocation management and streaming responses.

We've recently disabled by default ContentCachingResponseWrapper, HiddenHttpMethodFilter and other instances of filters reading/caching the request/response as they have a significant cost.

We could consider another approach with HandlerInterceptor for Spring MVC, we would need to rely on the matched pattern (present as a request attribute) and not simply match on a particular URL. This approach is still flawed for handlers that don't use such mapping patterns.

Currently we can't replicate this with Servlet.fn, since I don't think the required infrastructure is present. Same thing for WebFlux - we would need some interception mechanism around HandlerResultHandler to make this work, but I'm not sure this would be enough.

With the current state of things, I don't think we can solve that problem in Spring Boot in a consistent manner. Since the problem is still very specific (specific mappings, casting return types and extracting information, etc) - maybe a custom HandlerInterceptor in the application is still the best way to achieve that right now.

Comment From: jkschneider

any option involving the raw response body is wasteful and will significantly hinder performance

Yes, I imagine this is true!

At some point the framework has the domain object (Person, Mono<Person>, etc.) prior to serializing it to the response body. It would be awesome to have an interceptor there.

@GetMapping("/api/person/{id}")
@Timed("person.requests")
public Person person(@PathVariable String id) {
 ...
}

There would be no extra cost associated in peeking at an object that has already been constructed.

Some sort of type-specific interceptor would help me tag metrics produced from all API endpoints that return a particular domain object like Person with country code, etc. Such an interceptor with request, response, and domain object would be enough?

Somehow this may all be a bad idea.

Comment From: philwebb

@rstoyanchev Is there any interception point in MVC to apply this kind of cross-cutting logic? I couldn't find one from a brief scan of the code. I guess AOP might be an option if not.

Comment From: jkschneider

I guess AOP might be an option if not.

Thinking of the interaction between TimedAspect, this AOP interceptor, WebMvcMetricsFilter, etc... I shouldn't have read this right before going to bed @philwebb :)

Comment From: philwebb

You're going to have some weird nightmares tonight!

Comment From: bclozel

We've discussed this point during the team call, and we think that this is not something we can address at the Spring Boot level.

In general, hook points for web applications should be considered at the Spring Framework level. For new features and performance improvements for instrumentation, the web frameworks are definitely the place to start.

In my previous comment I mentioned HandlerInterceptor, but this contract is only valid for ModelAndView return values (i.e. view rendering). For @ResponseBody variants, we can use the ResponseBodyAdvice; it is not its primary goal, so even this solution feels strange.

I managed to implement a naive version of this feature with:

A ResponseBodyAdvice implementation that serves as a @ControllerAdvice for all controllers annotated with @RestController. This advice will be called for all User return types and will add the user as a request parameter:

@Component
@ControllerAdvice(annotations = RestController.class)
public class UserMetricsControllerAdvice implements ResponseBodyAdvice<User> {

    public static String USER_METRICS_REQUEST_ATTRIBUTE = "UserMetricsControllerAdvice.REQUEST_ATTRIBUTE";

    @Override
    public boolean supports(MethodParameter methodParameter, Class<? extends HttpMessageConverter<?>> aClass) {
        return methodParameter.getParameterType().equals(User.class);
    }

    @Override
    public User beforeBodyWrite(User user, MethodParameter methodParameter, MediaType mediaType, Class<? extends HttpMessageConverter<?>> aClass,
            ServerHttpRequest serverHttpRequest, ServerHttpResponse serverHttpResponse) {
        if (serverHttpRequest instanceof ServletServerHttpRequest) {
            HttpServletRequest servletRequest = ((ServletServerHttpRequest) serverHttpRequest).getServletRequest();
            servletRequest.setAttribute(USER_METRICS_REQUEST_ATTRIBUTE, user);
        }
        return user;
    }

}

I'm also providing a WebMvcTagsContributor which will get the user from the request and, if available, will contribute the user Locale as a Tag.

@Component
public class UserMetricsTagContributor implements WebMvcTagsContributor {

    @Override
    public Iterable<Tag> getTags(HttpServletRequest request, HttpServletResponse response, Object handler, Throwable exception) {
        Object attribute = request.getAttribute(UserMetricsControllerAdvice.USER_METRICS_REQUEST_ATTRIBUTE);
        if (attribute instanceof User) {
            return Tags.of("COUNTRY", ((User)attribute).getLocale().getCountry());
        }
        return Tags.empty();
    }

    @Override
    public Iterable<Tag> getLongRequestTags(HttpServletRequest request, Object handler) {
        return Tags.empty();
    }
}

There are many issues with this approach:

  • it does not support the many other ways to return a User: ResponseEntity<User>, streaming and asynchronous types (see supported return types)
  • this works for the annotation variant, but I'm not sure there's a way to achieve the same thing with Servlet.fn (same for WebFlux)
  • in case of errors during serialization, we'd need to properly clean up the request attribute otherwise we'd add those tags anyway - depending on the use case, I'm not sure we'd want to have those for error pages
  • I did not test the performance implications of this setup

I'm sure I'm missing more here, but this example tells me that this is a complex matter that is hard to extract as a generic feature. I'm closing this issue as a result for now. For specific hook points and improvements, I think Spring Framework issues should be the best place to start.

I hope this sample will help you to make progress on this subject! Cheers!

Comment From: rstoyanchev

Sorry coming to this late. I would mention ResponseBodyAdvice as well. It should work for both @ResponseBody/@RestController as well as for ResponeEntity. That should catch all places where an Object is rendered to the response (including async ones with DeferredResult, Mono etc).

As for other return types, I'm guessing that a HTML response with ModelAndView is not of interest and likewise streaming responses might not be the primary target of this use case anyway.