From f2511b7fa3ceec800a2fb6adedcbf0e39c3cb253 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 16 Aug 2018 17:34:42 +0200 Subject: [PATCH] Improve Web DEBUG logging output configuration Since SPR-16946, Spring Framework revisited the DEBUG logging output developers get while working on Spring MVC and Spring WebFlux applications. This commit aligns to those changes where DEBUG output was produced in Spring Boot (especially in `DefaultErrorWebExceptionHandler`). This also enables DEBUG logging on the related packages when running an application with Spring Boot Developer Tools, providing a better development experience. This is also adding the new `spring.insights.web.log-request-details` configuration property, which logs additional information about the incoming requests at the DEBUG and TRACE levels. Since that information can be sensitive (e.g. credentials, tokens, etc.), this property is not enabled by default nor activated by the Developer Tools. Closes: gh-13511 --- .../http/codec/CodecsAutoConfiguration.java | 18 ++++++- .../insights/InsightsProperties.java | 54 +++++++++++++++++++ .../DefaultErrorWebExceptionHandler.java | 35 ++++-------- .../DispatcherServletAutoConfiguration.java | 12 ++++- ...orWebExceptionHandlerIntegrationTests.java | 21 -------- ...spatcherServletAutoConfigurationTests.java | 4 ++ ...DevToolsPropertyDefaultsPostProcessor.java | 3 ++ .../appendix-application-properties.adoc | 3 ++ .../src/main/asciidoc/using-spring-boot.adoc | 7 +++ 9 files changed, 107 insertions(+), 50 deletions(-) create mode 100644 spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/insights/InsightsProperties.java diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/codec/CodecsAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/codec/CodecsAutoConfiguration.java index 69794cf30f..dbf044dcfe 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/codec/CodecsAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/codec/CodecsAutoConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 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. @@ -22,7 +22,9 @@ import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.insights.InsightsProperties; import org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.web.codec.CodecCustomizer; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -64,4 +66,18 @@ public class CodecsAutoConfiguration { } + @Configuration + @EnableConfigurationProperties(InsightsProperties.class) + static class LoggingCodecConfiguration { + + @Bean + public CodecCustomizer loggingCodecCustomizer(InsightsProperties properties) { + return (configurer) -> { + configurer.defaultCodecs().enableLoggingRequestDetails( + properties.getWeb().isLogRequestDetails()); + }; + } + + } + } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/insights/InsightsProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/insights/InsightsProperties.java new file mode 100644 index 0000000000..9a41d943c0 --- /dev/null +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/insights/InsightsProperties.java @@ -0,0 +1,54 @@ +/* + * Copyright 2012-2018 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 + * + * http://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.autoconfigure.insights; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** + * {@link ConfigurationProperties properties} for . + * + * @author Brian Clozel + * @since 2.1.0 + */ +@ConfigurationProperties(prefix = "spring.insights") +public class InsightsProperties { + + private final Web web = new Web(); + + public Web getWeb() { + return this.web; + } + + public static class Web { + + /** + * Whether logging of (potentially sensitive) request details at DEBUG and TRACE + * level is allowed. + */ + private boolean logRequestDetails = false; + + public boolean isLogRequestDetails() { + return this.logRequestDetails; + } + + public void setLogRequestDetails(boolean logRequestDetails) { + this.logRequestDetails = logRequestDetails; + } + + } + +} diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java index 4c5b4f141e..ffd699e563 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java @@ -20,10 +20,8 @@ import java.util.Collections; import java.util.EnumMap; import java.util.List; import java.util.Map; -import java.util.function.BiConsumer; import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -31,6 +29,7 @@ import org.springframework.boot.autoconfigure.web.ErrorProperties; import org.springframework.boot.autoconfigure.web.ResourceProperties; import org.springframework.boot.web.reactive.error.ErrorAttributes; import org.springframework.context.ApplicationContext; +import org.springframework.http.HttpLogging; import org.springframework.http.HttpStatus; import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.MediaType; @@ -39,7 +38,6 @@ import org.springframework.web.reactive.function.server.RequestPredicate; import org.springframework.web.reactive.function.server.RouterFunction; import org.springframework.web.reactive.function.server.ServerRequest; import org.springframework.web.reactive.function.server.ServerResponse; -import org.springframework.web.server.ResponseStatusException; import static org.springframework.web.reactive.function.server.RequestPredicates.all; import static org.springframework.web.reactive.function.server.RouterFunctions.route; @@ -79,8 +77,8 @@ public class DefaultErrorWebExceptionHandler extends AbstractErrorWebExceptionHa private static final Map SERIES_VIEWS; - private static final Log logger = LogFactory - .getLog(DefaultErrorWebExceptionHandler.class); + private static final Log logger = HttpLogging + .forLogName(DefaultErrorWebExceptionHandler.class); static { Map views = new EnumMap<>(HttpStatus.Series.class); @@ -206,30 +204,15 @@ public class DefaultErrorWebExceptionHandler extends AbstractErrorWebExceptionHa */ protected void logError(ServerRequest request, HttpStatus errorStatus) { Throwable ex = getError(request); - log(request, ex, (errorStatus.is5xxServerError() ? logger::error : logger::warn)); - } - - private void log(ServerRequest request, Throwable ex, - BiConsumer logger) { - if (ex instanceof ResponseStatusException) { - logger.accept(buildMessage(request, ex), null); - } - else { - logger.accept(buildMessage(request, null), ex); + if (logger.isDebugEnabled()) { + logger.debug(request.exchange().getLogPrefix() + formatError(ex, request)); } } - private String buildMessage(ServerRequest request, Throwable ex) { - StringBuilder message = new StringBuilder("Failed to handle request ["); - message.append(request.methodName()); - message.append(" "); - message.append(request.uri()); - message.append("]"); - if (ex != null) { - message.append(": "); - message.append(ex.getMessage()); - } - return message.toString(); + private String formatError(Throwable ex, ServerRequest request) { + String reason = ex.getClass().getSimpleName() + ": " + ex.getMessage(); + return "Resolved [" + reason + "] for HTTP " + request.methodName() + " " + + request.path(); } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/DispatcherServletAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/DispatcherServletAutoConfiguration.java index 6293e7e0de..2e30b69a10 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/DispatcherServletAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/DispatcherServletAutoConfiguration.java @@ -36,6 +36,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication.Type; import org.springframework.boot.autoconfigure.condition.SpringBootCondition; +import org.springframework.boot.autoconfigure.insights.InsightsProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.web.servlet.ServletRegistrationBean; import org.springframework.boot.web.servlet.support.SpringBootServletInitializer; @@ -81,24 +82,31 @@ public class DispatcherServletAutoConfiguration { @Configuration @Conditional(DefaultDispatcherServletCondition.class) @ConditionalOnClass(ServletRegistration.class) - @EnableConfigurationProperties(WebMvcProperties.class) + @EnableConfigurationProperties({ WebMvcProperties.class, InsightsProperties.class }) protected static class DispatcherServletConfiguration { private final WebMvcProperties webMvcProperties; - public DispatcherServletConfiguration(WebMvcProperties webMvcProperties) { + private final InsightsProperties insightsProperties; + + public DispatcherServletConfiguration(WebMvcProperties webMvcProperties, + InsightsProperties insightsProperties) { this.webMvcProperties = webMvcProperties; + this.insightsProperties = insightsProperties; } @Bean(name = DEFAULT_DISPATCHER_SERVLET_BEAN_NAME) public DispatcherServlet dispatcherServlet() { DispatcherServlet dispatcherServlet = new DispatcherServlet(); + dispatcherServlet.setShouldHandleFailure(true); dispatcherServlet.setDispatchOptionsRequest( this.webMvcProperties.isDispatchOptionsRequest()); dispatcherServlet.setDispatchTraceRequest( this.webMvcProperties.isDispatchTraceRequest()); dispatcherServlet.setThrowExceptionIfNoHandlerFound( this.webMvcProperties.isThrowExceptionIfNoHandlerFound()); + dispatcherServlet.setEnableLoggingRequestDetails( + this.insightsProperties.getWeb().isLogRequestDetails()); return dispatcherServlet; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTests.java index 1cd64935c1..ba177ede8a 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTests.java @@ -30,7 +30,6 @@ import org.springframework.boot.autoconfigure.web.reactive.HttpHandlerAutoConfig import org.springframework.boot.autoconfigure.web.reactive.ReactiveWebServerFactoryAutoConfiguration; import org.springframework.boot.autoconfigure.web.reactive.WebFluxAutoConfiguration; import org.springframework.boot.test.context.runner.ReactiveWebApplicationContextRunner; -import org.springframework.boot.test.rule.OutputCapture; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; @@ -44,10 +43,7 @@ import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; import static org.assertj.core.api.Assertions.assertThat; -import static org.hamcrest.Matchers.allOf; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.not; /** * Integration tests for {@link DefaultErrorWebExceptionHandler} @@ -70,9 +66,6 @@ public class DefaultErrorWebExceptionHandlerIntegrationTests { @Rule public ExpectedException thrown = ExpectedException.none(); - @Rule - public OutputCapture output = new OutputCapture(); - @Test public void jsonError() { this.contextRunner.run((context) -> { @@ -85,8 +78,6 @@ public class DefaultErrorWebExceptionHandlerIntegrationTests { .jsonPath("path").isEqualTo(("/")).jsonPath("message") .isEqualTo("Expected!").jsonPath("exception").doesNotExist() .jsonPath("trace").doesNotExist(); - this.output.expect(allOf(containsString("Failed to handle request [GET /]"), - containsString("IllegalStateException"))); }); } @@ -112,8 +103,6 @@ public class DefaultErrorWebExceptionHandlerIntegrationTests { .expectHeader().contentType(MediaType.TEXT_HTML) .expectBody(String.class).returnResult().getResponseBody(); assertThat(body).contains("status: 500").contains("message: Expected!"); - this.output.expect(allOf(containsString("Failed to handle request [GET /]"), - containsString("IllegalStateException"))); }); } @@ -129,9 +118,6 @@ public class DefaultErrorWebExceptionHandlerIntegrationTests { .isEqualTo(("/bind")).jsonPath("exception").doesNotExist() .jsonPath("errors").isArray().jsonPath("message").isNotEmpty(); }); - this.output.expect(allOf(containsString("Failed to handle request [POST /bind]"), - containsString("Validation failed for argument"), - containsString("Field error in object 'dummyBody' on field 'content'"))); } @Test @@ -197,7 +183,6 @@ public class DefaultErrorWebExceptionHandlerIntegrationTests { .isEqualTo(HttpStatus.BAD_REQUEST.getReasonPhrase()) .jsonPath("exception") .isEqualTo(ResponseStatusException.class.getName()); - this.output.expect(not(containsString("ResponseStatusException"))); }); } @@ -215,9 +200,6 @@ public class DefaultErrorWebExceptionHandlerIntegrationTests { .returnResult().getResponseBody(); assertThat(body).contains("Whitelabel Error Page") .contains("
Expected!
"); - this.output.expect( - allOf(containsString("Failed to handle request [GET /]"), - containsString("IllegalStateException"))); }); } @@ -235,9 +217,6 @@ public class DefaultErrorWebExceptionHandlerIntegrationTests { .returnResult().getResponseBody(); assertThat(body).contains("Whitelabel Error Page") .doesNotContain("