Skip to content

Commit

Permalink
Allow to manually tag request metrics with exceptions
Browse files Browse the repository at this point in the history
Prior to this commit, some exceptions handled at the controller or
handler function level would:

* not bubble up to the Spring Boot error handling support
* not be tagged as part of the request metrics

This situation is inconsistent because in general, exceptions handled at
the controller level can be considered as expected behavior.
Also, depending on how the exception is handled, the request metrics
might not be tagged with the exception.
This will be reconsidered in gh-23795.

This commit prepares a transition to the new situation. Developers can
now opt-in and set the handled exception as a request attribute. This
well-known attribute will be later read by the metrics support and used
for tagging the request metrics with the exception provided.

This mechanism is automatically used by the error handling support in
Spring Boot.

Closes gh-24028
  • Loading branch information
bclozel committed Apr 1, 2021
1 parent 66e9619 commit 72a1eb6
Show file tree
Hide file tree
Showing 14 changed files with 177 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import reactor.core.publisher.Mono;

import org.springframework.boot.actuate.metrics.AutoTimer;
import org.springframework.boot.web.reactive.error.ErrorAttributes;
import org.springframework.core.Ordered;
import org.springframework.core.annotation.Order;
import org.springframework.http.server.reactive.ServerHttpResponse;
Expand Down Expand Up @@ -93,6 +94,9 @@ private void onTerminalSignal(ServerWebExchange exchange, Throwable cause, long
}

private void record(ServerWebExchange exchange, Throwable cause, long start) {
if (cause == null) {
cause = exchange.getAttribute(ErrorAttributes.ERROR_ATTRIBUTE);
}
Iterable<Tag> tags = this.tagsProvider.httpRequestTags(exchange, cause);
this.autoTimer.builder(this.metricName).tags(tags).register(this.registry).record(System.nanoTime() - start,
TimeUnit.NANOSECONDS);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2021 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.
Expand Down Expand Up @@ -33,6 +33,7 @@
import io.micrometer.core.instrument.Timer.Sample;

import org.springframework.boot.actuate.metrics.AutoTimer;
import org.springframework.boot.web.servlet.error.ErrorAttributes;
import org.springframework.core.annotation.MergedAnnotationCollectors;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.http.HttpStatus;
Expand Down Expand Up @@ -96,7 +97,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
// If async was started by something further down the chain we wait
// until the second filter invocation (but we'll be using the
// TimingContext that was attached to the first)
Throwable exception = (Throwable) request.getAttribute(DispatcherServlet.EXCEPTION_ATTRIBUTE);
Throwable exception = fetchException(request);
record(timingContext, request, response, exception);
}
}
Expand All @@ -118,6 +119,14 @@ private TimingContext startAndAttachTimingContext(HttpServletRequest request) {
return timingContext;
}

private Throwable fetchException(HttpServletRequest request) {
Throwable exception = (Throwable) request.getAttribute(ErrorAttributes.ERROR_ATTRIBUTE);
if (exception == null) {
exception = (Throwable) request.getAttribute(DispatcherServlet.EXCEPTION_ATTRIBUTE);
}
return exception;
}

private void record(TimingContext timingContext, HttpServletRequest request, HttpServletResponse response,
Throwable exception) {
Object handler = getHandler(request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import reactor.test.StepVerifier;

import org.springframework.boot.actuate.metrics.AutoTimer;
import org.springframework.boot.web.reactive.error.ErrorAttributes;
import org.springframework.mock.http.server.reactive.MockServerHttpRequest;
import org.springframework.mock.web.server.MockServerWebExchange;
import org.springframework.web.reactive.HandlerMapping;
Expand Down Expand Up @@ -94,6 +95,18 @@ void filterAddsNonEmptyTagsToRegistryForAnonymousExceptions() {
assertMetricsContainsTag("exception", anonymous.getClass().getName());
}

@Test
void filterAddsTagsToRegistryForHandledExceptions() {
MockServerWebExchange exchange = createExchange("/projects/spring-boot", "/projects/{project}");
this.webFilter.filter(exchange, (serverWebExchange) -> {
exchange.getAttributes().put(ErrorAttributes.ERROR_ATTRIBUTE, new IllegalStateException("test error"));
return exchange.getResponse().setComplete();
}).block(Duration.ofSeconds(30));
assertMetricsContainsTag("uri", "/projects/{project}");
assertMetricsContainsTag("status", "200");
assertMetricsContainsTag("exception", "IllegalStateException");
}

@Test
void filterAddsTagsToRegistryForExceptionsAndCommittedResponse() {
MockServerWebExchange exchange = createExchange("/projects/spring-boot", "/projects/{project}");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2021 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.
Expand Down Expand Up @@ -57,6 +57,7 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.boot.actuate.metrics.AutoTimer;
import org.springframework.boot.web.servlet.error.ErrorAttributes;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
Expand Down Expand Up @@ -264,7 +265,8 @@ void asyncCompletableFutureRequest() throws Exception {
@Test
void endpointThrowsError() throws Exception {
this.mvc.perform(get("/api/c1/error/10")).andExpect(status().is4xxClientError());
assertThat(this.registry.get("http.server.requests").tags("status", "422").timer().count()).isEqualTo(1L);
assertThat(this.registry.get("http.server.requests").tags("status", "422", "exception", "IllegalStateException")
.timer().count()).isEqualTo(1L);
}

@Test
Expand Down Expand Up @@ -491,6 +493,8 @@ String meta(@PathVariable String id) {
@ExceptionHandler(IllegalStateException.class)
@ResponseStatus(HttpStatus.UNPROCESSABLE_ENTITY)
ModelAndView defaultErrorHandler(HttpServletRequest request, Exception e) {
// this is done by ErrorAttributes implementations
request.setAttribute(ErrorAttributes.ERROR_ATTRIBUTE, e);
return new ModelAndView("myerror");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2188,6 +2188,8 @@ By default, Spring MVC-related metrics are tagged with the following information
To add to the default tags, provide one or more ``@Bean``s that implement `WebMvcTagsContributor`.
To replace the default tags, provide a `@Bean` that implements `WebMvcTagsProvider`.
TIP: In some cases, exceptions handled in Web controllers are not recorded as request metrics tags.
Applications can opt-in and record exceptions by <<spring-boot-features.adoc#boot-features-error-handling, setting handled exceptions as request parameters>>.
[[production-ready-metrics-web-flux]]
Expand Down Expand Up @@ -2222,7 +2224,8 @@ By default, WebFlux-related metrics are tagged with the following information:
To add to the default tags, provide one or more ``@Bean``s that implement `WebFluxTagsContributor`.
To replace the default tags, provide a `@Bean` that implements `WebFluxTagsProvider`.
TIP: In some cases, exceptions handled in controllers and handler functions are not recorded as request metrics tags.
Applications can opt-in and record exceptions by <<spring-boot-features.adoc#boot-features-webflux-error-handling, setting handled exceptions as request parameters>>.
[[production-ready-metrics-jersey-server]]
==== Jersey Server Metrics
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2566,7 +2566,13 @@ include::{include-springbootfeatures}/webapplications/servlet/MyControllerAdvice

In the preceding example, if `YourException` is thrown by a controller defined in the same package as `AcmeController`, a JSON representation of the `CustomErrorType` POJO is used instead of the `ErrorAttributes` representation.

In some cases, errors handled at the controller level are not recorded by the <<production-ready-features.adoc#production-ready-metrics-spring-mvc, metrics infrastructure>>.
Applications can ensure that such exceptions are recorded with the request metrics by setting the handled exception as a request attribute:

[source,java,indent=0,subs="verbatim,quotes,attributes"]
----
include::{include-springbootfeatures}/webapplications/servlet/MyController.java[]
----

[[boot-features-error-handling-custom-error-pages]]
===== Custom Error Pages
Expand Down Expand Up @@ -2823,6 +2829,14 @@ include::{include-springbootfeatures}/webapplications/webflux/CustomErrorWebExce

For a more complete picture, you can also subclass `DefaultErrorWebExceptionHandler` directly and override specific methods.

In some cases, errors handled at the controller or handler function level are not recorded by the <<production-ready-features.adoc#production-ready-metrics-web-flux, metrics infrastructure>>.
Applications can ensure that such exceptions are recorded with the request metrics by setting the handled exception as a request attribute:

[source,java,indent=0,subs="verbatim,quotes,attributes"]
----
include::{include-springbootfeatures}/webapplications/webflux/ExceptionHandlingController.java[]
----



[[boot-features-webflux-error-handling-custom-error-pages]]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright 2012-2021 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.boot.docs.springbootfeatures.webapplications.servlet;

import javax.servlet.http.HttpServletRequest;

import org.springframework.boot.web.servlet.error.ErrorAttributes;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.ExceptionHandler;

@Controller
public class MyController {

@ExceptionHandler(CustomException.class)
String handleCustomException(HttpServletRequest request, CustomException ex) {
request.setAttribute(ErrorAttributes.ERROR_ATTRIBUTE, ex);
return "errorView";
}

}
// @chomp:file

class CustomException extends RuntimeException {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright 2012-2021 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.boot.docs.springbootfeatures.webapplications.webflux;

import org.springframework.boot.web.reactive.error.ErrorAttributes;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.reactive.result.view.Rendering;
import org.springframework.web.server.ServerWebExchange;

@Controller
public class ExceptionHandlingController {

@GetMapping("/profile")
public Rendering userProfile() {
// ..
throw new IllegalStateException();
// ...
}

@ExceptionHandler(IllegalStateException.class)
Rendering handleIllegalState(ServerWebExchange exchange, IllegalStateException exc) {
exchange.getAttributes().putIfAbsent(ErrorAttributes.ERROR_ATTRIBUTE, exc);
return Rendering.view("errorView").modelAttribute("message", exc.getMessage()).build();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.Date;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Optional;

import org.springframework.boot.web.error.ErrorAttributeOptions;
import org.springframework.boot.web.error.ErrorAttributeOptions.Include;
Expand Down Expand Up @@ -61,7 +62,7 @@
*/
public class DefaultErrorAttributes implements ErrorAttributes {

private static final String ERROR_ATTRIBUTE = DefaultErrorAttributes.class.getName() + ".ERROR";
private static final String ERROR_INTERNAL_ATTRIBUTE = DefaultErrorAttributes.class.getName() + ".ERROR";

@Override
public Map<String, Object> getErrorAttributes(ServerRequest request, ErrorAttributeOptions options) {
Expand Down Expand Up @@ -147,13 +148,15 @@ private void handleException(Map<String, Object> errorAttributes, Throwable erro

@Override
public Throwable getError(ServerRequest request) {
return (Throwable) request.attribute(ERROR_ATTRIBUTE)
Optional<Object> error = request.attribute(ERROR_INTERNAL_ATTRIBUTE);
error.ifPresent((value) -> request.attributes().putIfAbsent(ErrorAttributes.ERROR_ATTRIBUTE, value));
return (Throwable) error
.orElseThrow(() -> new IllegalStateException("Missing exception attribute in ServerWebExchange"));
}

@Override
public void storeErrorInformation(Throwable error, ServerWebExchange exchange) {
exchange.getAttributes().putIfAbsent(ERROR_ATTRIBUTE, error);
exchange.getAttributes().putIfAbsent(ERROR_INTERNAL_ATTRIBUTE, error);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@
*/
public interface ErrorAttributes {

/**
* Name of the {@link ServerRequest#attribute(String)} Request attribute} holding the
* error resolved by the {@code ErrorAttributes} implementation.
* @since 2.5.0
*/
String ERROR_ATTRIBUTE = ErrorAttributes.class.getName() + ".error";

/**
* Return a {@link Map} of the error attributes. The map can be used as the model of
* an error page, or returned as a {@link ServerResponse} body.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
@Order(Ordered.HIGHEST_PRECEDENCE)
public class DefaultErrorAttributes implements ErrorAttributes, HandlerExceptionResolver, Ordered {

private static final String ERROR_ATTRIBUTE = DefaultErrorAttributes.class.getName() + ".ERROR";
private static final String ERROR_INTERNAL_ATTRIBUTE = DefaultErrorAttributes.class.getName() + ".ERROR";

@Override
public int getOrder() {
Expand All @@ -83,7 +83,7 @@ public ModelAndView resolveException(HttpServletRequest request, HttpServletResp
}

private void storeErrorAttributes(HttpServletRequest request, Exception ex) {
request.setAttribute(ERROR_ATTRIBUTE, ex);
request.setAttribute(ERROR_INTERNAL_ATTRIBUTE, ex);
}

@Override
Expand Down Expand Up @@ -216,8 +216,14 @@ private void addPath(Map<String, Object> errorAttributes, RequestAttributes requ

@Override
public Throwable getError(WebRequest webRequest) {
Throwable exception = getAttribute(webRequest, ERROR_ATTRIBUTE);
return (exception != null) ? exception : getAttribute(webRequest, RequestDispatcher.ERROR_EXCEPTION);
Throwable exception = getAttribute(webRequest, ERROR_INTERNAL_ATTRIBUTE);
if (exception == null) {
exception = getAttribute(webRequest, RequestDispatcher.ERROR_EXCEPTION);
}
// store the exception in a well-known attribute to make it available to metrics
// instrumentation.
webRequest.setAttribute(ErrorAttributes.ERROR_ATTRIBUTE, exception, WebRequest.SCOPE_REQUEST);
return exception;
}

@SuppressWarnings("unchecked")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@
*/
public interface ErrorAttributes {

/**
* Name of the {@link javax.servlet.http.HttpServletRequest#getAttribute(String)
* Request attribute} holding the error resolved by the {@code ErrorAttributes}
* implementation.
* @since 2.5.0
*/
String ERROR_ATTRIBUTE = ErrorAttributes.class.getName() + ".error";

/**
* Returns a {@link Map} of the error attributes. The map can be used as the model of
* an error page {@link ModelAndView}, or returned as a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ void includeException() {
Map<String, Object> attributes = this.errorAttributes.getErrorAttributes(serverRequest,
ErrorAttributeOptions.of(Include.EXCEPTION, Include.MESSAGE));
assertThat(this.errorAttributes.getError(serverRequest)).isSameAs(error);
assertThat(serverRequest.attribute(ErrorAttributes.ERROR_ATTRIBUTE)).containsSame(error);
assertThat(attributes.get("exception")).isEqualTo(RuntimeException.class.getName());
assertThat(attributes.get("message")).isEqualTo("Test");
}
Expand All @@ -177,6 +178,7 @@ void processResponseStatusException() {
assertThat(attributes.get("message")).isEqualTo("invalid request");
assertThat(attributes.get("exception")).isEqualTo(RuntimeException.class.getName());
assertThat(this.errorAttributes.getError(serverRequest)).isSameAs(error);
assertThat(serverRequest.attribute(ErrorAttributes.ERROR_ATTRIBUTE)).containsSame(error);
}

@Test
Expand All @@ -192,6 +194,7 @@ void processResponseStatusExceptionWithNoNestedCause() {
assertThat(attributes.get("message")).isEqualTo("could not process request");
assertThat(attributes.get("exception")).isEqualTo(ResponseStatusException.class.getName());
assertThat(this.errorAttributes.getError(serverRequest)).isSameAs(error);
assertThat(serverRequest.attribute(ErrorAttributes.ERROR_ATTRIBUTE)).containsSame(error);
}

@Test
Expand Down
Loading

0 comments on commit 72a1eb6

Please sign in to comment.