Allow to manually tag request metrics with exceptions

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

Author: bclozel

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilter.java

@@ -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;
@@ -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);

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilter.java

@@ -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.
@@ -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;
@@ -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);
 			}
 		}
@@ -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);

spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTests.java

@@ -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;
@@ -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}");

spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilterTests.java

@@ -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.
@@ -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;
@@ -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
@@ -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");
 		}
 

spring-boot-project/spring-boot-docs/src/docs/asciidoc/production-ready-features.adoc

@@ -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]]
@@ -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

spring-boot-project/spring-boot-docs/src/docs/asciidoc/spring-boot-features.adoc

@@ -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
@@ -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]]

spring-boot-project/spring-boot-docs/src/main/java/org/springframework/boot/docs/springbootfeatures/webapplications/servlet/MyController.java

@@ -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 {
+
+}

spring-boot-project/spring-boot-docs/src/main/java/org/springframework/boot/docs/springbootfeatures/webapplications/webflux/ExceptionHandlingController.java

@@ -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();
+	}
+
+}

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java

@@ -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;
@@ -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) {
@@ -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);
 	}
 
 }

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/ErrorAttributes.java

@@ -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.

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java

@@ -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() {
@@ -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
@@ -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")

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/ErrorAttributes.java

@@ -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

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java

@@ -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");
 	}
@@ -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
@@ -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