From eeb407881a8d5f52aca77960f9ab2f9f6c2f84b3 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 20 Oct 2015 15:29:45 -0700 Subject: [PATCH] Restore "exception first" logging Effectively revert commit b396d0d2 to restore exception first logging of exceptions for Logback and Log4j2. This commit also introduces new extended versions of "whitespace" padding exception loggers so that source jar information is still included in the stacktrace. Fixes gh-4247 --- ...edWhitespaceThrowablePatternConverter.java | 66 +++++++++++++++++++ .../WhitespaceThrowablePatternConverter.java | 21 +++--- .../logback/DefaultLogbackConfiguration.java | 5 +- ...ndedWhitespaceThrowableProxyConverter.java | 39 +++++++++++ .../boot/logging/log4j2/log4j2-file.xml | 2 +- .../boot/logging/log4j2/log4j2.xml | 2 +- .../boot/logging/logback/defaults.xml | 5 +- ...tespaceThrowablePatternConverterTests.java | 59 +++++++++++++++++ .../log4j2/Log4J2LoggingSystemTests.java | 14 ---- ...tespaceThrowablePatternConverterTests.java | 3 +- ...hitespaceThrowableProxyConverterTests.java | 56 ++++++++++++++++ .../logback/LogbackLoggingSystemTests.java | 15 ----- 12 files changed, 240 insertions(+), 47 deletions(-) create mode 100644 spring-boot/src/main/java/org/springframework/boot/logging/log4j2/ExtendedWhitespaceThrowablePatternConverter.java create mode 100644 spring-boot/src/main/java/org/springframework/boot/logging/logback/ExtendedWhitespaceThrowableProxyConverter.java create mode 100644 spring-boot/src/test/java/org/springframework/boot/logging/log4j2/ExtendedWhitespaceThrowablePatternConverterTests.java create mode 100644 spring-boot/src/test/java/org/springframework/boot/logging/logback/ExtendedWhitespaceThrowableProxyConverterTests.java diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/ExtendedWhitespaceThrowablePatternConverter.java b/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/ExtendedWhitespaceThrowablePatternConverter.java new file mode 100644 index 0000000000..6ba9b26537 --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/ExtendedWhitespaceThrowablePatternConverter.java @@ -0,0 +1,66 @@ +/* + * Copyright 2012-2015 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.logging.log4j2; + +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.config.plugins.Plugin; +import org.apache.logging.log4j.core.pattern.ConverterKeys; +import org.apache.logging.log4j.core.pattern.ExtendedThrowablePatternConverter; +import org.apache.logging.log4j.core.pattern.PatternConverter; +import org.apache.logging.log4j.core.pattern.ThrowablePatternConverter; + +/** + * {@link ThrowablePatternConverter} that adds some additional whitespace around the stack + * trace. + * + * @author Vladimir Tsanev + * @author Phillip Webb + * @since 1.3.0 + */ +@Plugin(name = "WhitespaceThrowablePatternConverter", category = PatternConverter.CATEGORY) +@ConverterKeys({ "xwEx", "xwThrowable", "xwException" }) +public final class ExtendedWhitespaceThrowablePatternConverter + extends ThrowablePatternConverter { + + private final ExtendedThrowablePatternConverter delegate; + + private ExtendedWhitespaceThrowablePatternConverter(String[] options) { + super("WhitespaceExtendedThrowable", "throwable", options); + this.delegate = ExtendedThrowablePatternConverter.newInstance(options); + } + + @Override + public void format(LogEvent event, StringBuilder buffer) { + if (event.getThrown() != null) { + buffer.append(this.options.getSeparator()); + this.delegate.format(event, buffer); + buffer.append(this.options.getSeparator()); + } + } + + /** + * Creates a new instance of the class. Required by Log4J2. + * @param options pattern options, may be null. If first element is "short", only the + * first line of the throwable will be formatted. + * @return a new {@code WhitespaceThrowablePatternConverter} + */ + public static ExtendedWhitespaceThrowablePatternConverter newInstance( + String[] options) { + return new ExtendedWhitespaceThrowablePatternConverter(options); + } + +} diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/WhitespaceThrowablePatternConverter.java b/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/WhitespaceThrowablePatternConverter.java index fd751b3f2c..6fde11dce0 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/WhitespaceThrowablePatternConverter.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/WhitespaceThrowablePatternConverter.java @@ -37,17 +37,6 @@ public final class WhitespaceThrowablePatternConverter extends ThrowablePatternC super("WhitespaceThrowable", "throwable", options); } - /** - * Creates a new instance of the class. Required by Log4J2. - * - * @param options pattern options, may be null. If first element is "short", only the - * first line of the throwable will be formatted. - * @return a new {@code WhitespaceThrowablePatternConverter} - */ - public static WhitespaceThrowablePatternConverter newInstance(String[] options) { - return new WhitespaceThrowablePatternConverter(options); - } - @Override public void format(LogEvent event, StringBuilder buffer) { if (event.getThrown() != null) { @@ -57,4 +46,14 @@ public final class WhitespaceThrowablePatternConverter extends ThrowablePatternC } } + /** + * Creates a new instance of the class. Required by Log4J2. + * @param options pattern options, may be null. If first element is "short", only the + * first line of the throwable will be formatted. + * @return a new {@code WhitespaceThrowablePatternConverter} + */ + public static WhitespaceThrowablePatternConverter newInstance(String[] options) { + return new WhitespaceThrowablePatternConverter(options); + } + } diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java b/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java index 09f93090cb..0c5b831271 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java @@ -48,10 +48,10 @@ class DefaultLogbackConfiguration { private static final String CONSOLE_LOG_PATTERN = "%clr(%d{yyyy-MM-dd HH:mm:ss.SSS}){faint} " + "%clr(${LOG_LEVEL_PATTERN:-%5p}) %clr(${PID:- }){magenta} %clr(---){faint} " + "%clr([%15.15t]){faint} %clr(%-40.40logger{39}){cyan} " - + "%clr(:){faint} %m%n${LOG_EXCEPTION_CONVERSION_WORD:-%rEx}"; + + "%clr(:){faint} %m%n${LOG_EXCEPTION_CONVERSION_WORD:-%wEx}"; private static final String FILE_LOG_PATTERN = "%d{yyyy-MM-dd HH:mm:ss.SSS} " - + "${LOG_LEVEL_PATTERN:-%5p} ${PID:- } --- [%t] %-40.40logger{39} : %m%n${LOG_EXCEPTION_CONVERSION_WORD:-%rEx}"; + + "${LOG_LEVEL_PATTERN:-%5p} ${PID:- } --- [%t] %-40.40logger{39} : %m%n${LOG_EXCEPTION_CONVERSION_WORD:-%wEx}"; private static final Charset UTF8 = Charset.forName("UTF-8"); @@ -90,6 +90,7 @@ class DefaultLogbackConfiguration { private void base(LogbackConfigurator config) { config.conversionRule("clr", ColorConverter.class); config.conversionRule("wex", WhitespaceThrowableProxyConverter.class); + config.conversionRule("wEx", ExtendedWhitespaceThrowableProxyConverter.class); LevelRemappingAppender debugRemapAppender = new LevelRemappingAppender( "org.springframework.boot"); config.start(debugRemapAppender); diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/logback/ExtendedWhitespaceThrowableProxyConverter.java b/spring-boot/src/main/java/org/springframework/boot/logging/logback/ExtendedWhitespaceThrowableProxyConverter.java new file mode 100644 index 0000000000..896ed87330 --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/logging/logback/ExtendedWhitespaceThrowableProxyConverter.java @@ -0,0 +1,39 @@ +/* + * Copyright 2012-2013 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.logging.logback; + +import ch.qos.logback.classic.pattern.ExtendedThrowableProxyConverter; +import ch.qos.logback.classic.spi.IThrowableProxy; +import ch.qos.logback.core.CoreConstants; + +/** + * {@link ExtendedThrowableProxyConverter} that adds some additional whitespace around the + * stack trace. + * + * @author Phillip Webb + * @since 1.3.0 + */ +public class ExtendedWhitespaceThrowableProxyConverter + extends ExtendedThrowableProxyConverter { + + @Override + protected String throwableProxyToString(IThrowableProxy tp) { + return CoreConstants.LINE_SEPARATOR + super.throwableProxyToString(tp) + + CoreConstants.LINE_SEPARATOR; + } + +} diff --git a/spring-boot/src/main/resources/org/springframework/boot/logging/log4j2/log4j2-file.xml b/spring-boot/src/main/resources/org/springframework/boot/logging/log4j2/log4j2-file.xml index e6eaab5e3f..a97cf4065e 100644 --- a/spring-boot/src/main/resources/org/springframework/boot/logging/log4j2/log4j2-file.xml +++ b/spring-boot/src/main/resources/org/springframework/boot/logging/log4j2/log4j2-file.xml @@ -2,7 +2,7 @@ ???? - %rEx + %xwEx %5p %d{yyyy-MM-dd HH:mm:ss.SSS} ${LOG_LEVEL_PATTERN} ${sys:PID} --- [%t] %-40.40c{1.} : %m%n${sys:LOG_EXCEPTION_CONVERSION_WORD} diff --git a/spring-boot/src/main/resources/org/springframework/boot/logging/log4j2/log4j2.xml b/spring-boot/src/main/resources/org/springframework/boot/logging/log4j2/log4j2.xml index 7781ec0a40..dace970e8c 100644 --- a/spring-boot/src/main/resources/org/springframework/boot/logging/log4j2/log4j2.xml +++ b/spring-boot/src/main/resources/org/springframework/boot/logging/log4j2/log4j2.xml @@ -2,7 +2,7 @@ ???? - %rEx + %xwEx %5p %clr{%d{yyyy-MM-dd HH:mm:ss.SSS}}{faint} %clr{${LOG_LEVEL_PATTERN}} %clr{${sys:PID}}{magenta} %clr{---}{faint} %clr{[%15.15t]}{faint} %clr{%-40.40c{1.}}{cyan} %clr{:}{faint} %m%n${sys:LOG_EXCEPTION_CONVERSION_WORD} diff --git a/spring-boot/src/main/resources/org/springframework/boot/logging/logback/defaults.xml b/spring-boot/src/main/resources/org/springframework/boot/logging/logback/defaults.xml index 7cf9716557..58ce0d7ea0 100644 --- a/spring-boot/src/main/resources/org/springframework/boot/logging/logback/defaults.xml +++ b/spring-boot/src/main/resources/org/springframework/boot/logging/logback/defaults.xml @@ -8,9 +8,10 @@ initialization performed by Boot + - - + + org.springframework.boot diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/ExtendedWhitespaceThrowablePatternConverterTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/ExtendedWhitespaceThrowablePatternConverterTests.java new file mode 100644 index 0000000000..7d2e0db1e0 --- /dev/null +++ b/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/ExtendedWhitespaceThrowablePatternConverterTests.java @@ -0,0 +1,59 @@ +/* + * Copyright 2012-2015 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.logging.log4j2; + +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.impl.Log4jLogEvent; +import org.apache.logging.log4j.core.pattern.ThrowablePatternConverter; +import org.junit.Test; + +import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.Assert.assertThat; + +/** + * Tests for {@link ExtendedWhitespaceThrowablePatternConverter}. + * + * @author Vladimir Tsanev + * @author Phillip Webb + */ +public class ExtendedWhitespaceThrowablePatternConverterTests { + + private static final String LINE_SEPARATOR = System.getProperty("line.separator"); + + private final ThrowablePatternConverter converter = ExtendedWhitespaceThrowablePatternConverter + .newInstance(new String[] {}); + + @Test + public void noStackTrace() throws Exception { + LogEvent event = Log4jLogEvent.newBuilder().build(); + StringBuilder builder = new StringBuilder(); + this.converter.format(event, builder); + assertThat(builder.toString(), equalTo("")); + } + + @Test + public void withStackTrace() throws Exception { + LogEvent event = Log4jLogEvent.newBuilder().setThrown(new Exception()).build(); + StringBuilder builder = new StringBuilder(); + this.converter.format(event, builder); + assertThat(builder.toString(), startsWith(LINE_SEPARATOR)); + assertThat(builder.toString(), endsWith(LINE_SEPARATOR)); + } + +} diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java index 9e3a958476..fb0aa0e566 100644 --- a/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java @@ -203,20 +203,6 @@ public class Log4J2LoggingSystemTests extends AbstractLoggingSystemTests { assertThat(fileContents, is(expectedOutput)); } - @Test - public void rootCauseIsLoggedFirst() throws Exception { - this.loggingSystem.beforeInitialize(); - this.loggingSystem.initialize(null, null, getLogFile(null, tmpDir())); - Matcher expectedOutput = containsString( - "Wrapped by: " + "java.lang.RuntimeException: Expected"); - this.output.expect(expectedOutput); - this.logger.warn("Expected exception", - new RuntimeException("Expected", new RuntimeException("Cause"))); - String fileContents = FileCopyUtils - .copyToString(new FileReader(new File(tmpDir() + "/spring.log"))); - assertThat(fileContents, is(expectedOutput)); - } - @Test public void customExceptionConversionWord() throws Exception { System.setProperty("LOG_EXCEPTION_CONVERSION_WORD", "%ex"); diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/WhitespaceThrowablePatternConverterTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/WhitespaceThrowablePatternConverterTests.java index 3ed75ff0ec..c3bf041f32 100644 --- a/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/WhitespaceThrowablePatternConverterTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/WhitespaceThrowablePatternConverterTests.java @@ -18,6 +18,7 @@ package org.springframework.boot.logging.log4j2; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.impl.Log4jLogEvent; +import org.apache.logging.log4j.core.pattern.ThrowablePatternConverter; import org.junit.Test; import static org.hamcrest.Matchers.endsWith; @@ -34,7 +35,7 @@ public class WhitespaceThrowablePatternConverterTests { private static final String LINE_SEPARATOR = System.getProperty("line.separator"); - private final WhitespaceThrowablePatternConverter converter = WhitespaceThrowablePatternConverter + private final ThrowablePatternConverter converter = WhitespaceThrowablePatternConverter .newInstance(new String[] {}); @Test diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/logback/ExtendedWhitespaceThrowableProxyConverterTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/logback/ExtendedWhitespaceThrowableProxyConverterTests.java new file mode 100644 index 0000000000..c8d36110fc --- /dev/null +++ b/spring-boot/src/test/java/org/springframework/boot/logging/logback/ExtendedWhitespaceThrowableProxyConverterTests.java @@ -0,0 +1,56 @@ +/* + * Copyright 2012-2013 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.logging.logback; + +import ch.qos.logback.classic.spi.LoggingEvent; +import ch.qos.logback.classic.spi.ThrowableProxy; +import org.junit.Test; + +import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.Assert.assertThat; + +/** + * Tests for {@link ExtendedWhitespaceThrowableProxyConverter}. + * + * @author Phillip Webb + * @author Chanwit Kaewkasi + */ +public class ExtendedWhitespaceThrowableProxyConverterTests { + + private static final String LINE_SEPARATOR = System.getProperty("line.separator"); + + private final ExtendedWhitespaceThrowableProxyConverter converter = new ExtendedWhitespaceThrowableProxyConverter(); + + private final LoggingEvent event = new LoggingEvent(); + + @Test + public void noStackTrace() throws Exception { + String s = this.converter.convert(this.event); + assertThat(s, equalTo("")); + } + + @Test + public void withStackTrace() throws Exception { + this.event.setThrowableProxy(new ThrowableProxy(new RuntimeException())); + String s = this.converter.convert(this.event); + assertThat(s, startsWith(LINE_SEPARATOR)); + assertThat(s, endsWith(LINE_SEPARATOR)); + } + +} diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java index 8042ba66b8..d56e55f28f 100644 --- a/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java @@ -295,21 +295,6 @@ public class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { assertThat(fileContents, is(expectedOutput)); } - @Test - public void rootCauseIsLoggedFirst() throws Exception { - this.loggingSystem.beforeInitialize(); - this.loggingSystem.initialize(this.initializationContext, null, - getLogFile(null, tmpDir())); - Matcher expectedOutput = containsString( - "Wrapped by: " + "java.lang.RuntimeException: Expected"); - this.output.expect(expectedOutput); - this.logger.warn("Expected exception", - new RuntimeException("Expected", new RuntimeException("Cause"))); - String fileContents = FileCopyUtils - .copyToString(new FileReader(new File(tmpDir() + "/spring.log"))); - assertThat(fileContents, is(expectedOutput)); - } - @Test public void customExceptionConversionWord() throws Exception { System.setProperty("LOG_EXCEPTION_CONVERSION_WORD", "%ex");