From 0a7a283f4595536bd1a56aa2d977ae570cc0b2b6 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 28 Mar 2016 10:56:07 -0700 Subject: [PATCH] Apply logging system properties on reinitialize Restore Spring Boot 1.3.2 behavior of re-applying system properties when SLF4J based loggers are re-initialized. Reapplying system properties is required when using the Spring Cloud config server since PropertySourceBootstrapConfiguration directly calls the system initialize method. Fixes gh-5491 --- .../boot/logging/AbstractLoggingSystem.java | 7 +- .../logging/LoggingApplicationListener.java | 35 ++------- .../boot/logging/LoggingSytemProperties.java | 76 +++++++++++++++++++ .../boot/logging/Slf4JLoggingSystem.java | 12 ++- .../logging/log4j/Log4JLoggingSystem.java | 1 + .../logging/log4j2/Log4J2LoggingSystem.java | 1 + .../logging/logback/LogbackLoggingSystem.java | 2 +- .../logging/AbstractLoggingSystemTests.java | 9 ++- .../logback/LogbackLoggingSystemTests.java | 19 ++++- 9 files changed, 127 insertions(+), 35 deletions(-) create mode 100644 spring-boot/src/main/java/org/springframework/boot/logging/LoggingSytemProperties.java diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/AbstractLoggingSystem.java b/spring-boot/src/main/java/org/springframework/boot/logging/AbstractLoggingSystem.java index 10c652598f9..bb4d36b75e5 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/AbstractLoggingSystem.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/AbstractLoggingSystem.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * Copyright 2012-2016 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. @@ -16,6 +16,7 @@ package org.springframework.boot.logging; +import org.springframework.core.env.Environment; import org.springframework.core.io.ClassPathResource; import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; @@ -168,4 +169,8 @@ public abstract class AbstractLoggingSystem extends LoggingSystem { return defaultPath; } + protected final void applySystemProperties(Environment environment, LogFile logFile) { + new LoggingSytemProperties(environment).apply(logFile); + } + } diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java b/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java index 4c63db9403e..6ee94ef66cb 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java @@ -25,7 +25,6 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; -import org.springframework.boot.ApplicationPid; import org.springframework.boot.SpringApplication; import org.springframework.boot.bind.RelaxedPropertyResolver; import org.springframework.boot.context.event.ApplicationEnvironmentPreparedEvent; @@ -102,12 +101,12 @@ public class LoggingApplicationListener implements GenericApplicationListener { /** * The name of the System property that contains the process ID. */ - public static final String PID_KEY = "PID"; + public static final String PID_KEY = LoggingSytemProperties.PID_KEY; /** * The name of the System property that contains the exception conversion word. */ - public static final String EXCEPTION_CONVERSION_WORD = "LOG_EXCEPTION_CONVERSION_WORD"; + public static final String EXCEPTION_CONVERSION_WORD = LoggingSytemProperties.EXCEPTION_CONVERSION_WORD; /** * The name of the System property that contains the log file. @@ -122,17 +121,17 @@ public class LoggingApplicationListener implements GenericApplicationListener { /** * The name of the System property that contains the console log pattern. */ - public static final String CONSOLE_LOG_PATTERN = "CONSOLE_LOG_PATTERN"; + public static final String CONSOLE_LOG_PATTERN = LoggingSytemProperties.CONSOLE_LOG_PATTERN; /** * The name of the System property that contains the file log pattern. */ - public static final String FILE_LOG_PATTERN = "FILE_LOG_PATTERN"; + public static final String FILE_LOG_PATTERN = LoggingSytemProperties.FILE_LOG_PATTERN; /** * The name of the System property that contains the log level pattern. */ - public static final String LOG_LEVEL_PATTERN = "LOG_LEVEL_PATTERN"; + public static final String LOG_LEVEL_PATTERN = LoggingSytemProperties.LOG_LEVEL_PATTERN; /** * The name of the {@link LoggingSystem} bean. @@ -247,7 +246,7 @@ public class LoggingApplicationListener implements GenericApplicationListener { */ protected void initialize(ConfigurableEnvironment environment, ClassLoader classLoader) { - setSystemProperties(environment); + new LoggingSytemProperties(environment).apply(); LogFile logFile = LogFile.get(environment); if (logFile != null) { logFile.applyToSystemProperties(); @@ -258,28 +257,6 @@ public class LoggingApplicationListener implements GenericApplicationListener { registerShutdownHookIfNecessary(environment, this.loggingSystem); } - private void setSystemProperties(ConfigurableEnvironment environment) { - RelaxedPropertyResolver propertyResolver = new RelaxedPropertyResolver( - environment, "logging."); - setSystemProperty(propertyResolver, EXCEPTION_CONVERSION_WORD, - "exception-conversion-word"); - setSystemProperty(propertyResolver, CONSOLE_LOG_PATTERN, "pattern.console"); - setSystemProperty(propertyResolver, FILE_LOG_PATTERN, "pattern.file"); - setSystemProperty(propertyResolver, LOG_LEVEL_PATTERN, "pattern.level"); - setSystemProperty(PID_KEY, new ApplicationPid().toString()); - } - - private void setSystemProperty(RelaxedPropertyResolver propertyResolver, - String systemPropertyName, String propertyName) { - setSystemProperty(systemPropertyName, propertyResolver.getProperty(propertyName)); - } - - private void setSystemProperty(String name, String value) { - if (System.getProperty(name) == null && value != null) { - System.setProperty(name, value); - } - } - private void initializeEarlyLoggingLevel(ConfigurableEnvironment environment) { if (this.parseArgs && this.springBootLogging == null) { if (environment.containsProperty("debug")) { diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/LoggingSytemProperties.java b/spring-boot/src/main/java/org/springframework/boot/logging/LoggingSytemProperties.java new file mode 100644 index 00000000000..d0d196e2860 --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/logging/LoggingSytemProperties.java @@ -0,0 +1,76 @@ +/* + * Copyright 2012-2016 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; + +import org.springframework.boot.ApplicationPid; +import org.springframework.boot.bind.RelaxedPropertyResolver; +import org.springframework.core.env.Environment; + +/** + * Utility to set system properties that can later be used by log configuration files. + * + * @author Andy Wilkinson + * @author Phillip Webb + */ +class LoggingSytemProperties { + + static final String PID_KEY = "PID"; + + static final String EXCEPTION_CONVERSION_WORD = "LOG_EXCEPTION_CONVERSION_WORD"; + + static final String CONSOLE_LOG_PATTERN = "CONSOLE_LOG_PATTERN"; + + static final String FILE_LOG_PATTERN = "FILE_LOG_PATTERN"; + + static final String LOG_LEVEL_PATTERN = "LOG_LEVEL_PATTERN"; + + private final Environment environment; + + LoggingSytemProperties(Environment environment) { + this.environment = environment; + } + + public void apply() { + apply(null); + } + + public void apply(LogFile logFile) { + RelaxedPropertyResolver propertyResolver = new RelaxedPropertyResolver( + this.environment, "logging."); + setSystemProperty(propertyResolver, EXCEPTION_CONVERSION_WORD, + "exception-conversion-word"); + setSystemProperty(propertyResolver, CONSOLE_LOG_PATTERN, "pattern.console"); + setSystemProperty(propertyResolver, FILE_LOG_PATTERN, "pattern.file"); + setSystemProperty(propertyResolver, LOG_LEVEL_PATTERN, "pattern.level"); + setSystemProperty(PID_KEY, new ApplicationPid().toString()); + if (logFile != null) { + logFile.applyToSystemProperties(); + } + } + + private void setSystemProperty(RelaxedPropertyResolver propertyResolver, + String systemPropertyName, String propertyName) { + setSystemProperty(systemPropertyName, propertyResolver.getProperty(propertyName)); + } + + private void setSystemProperty(String name, String value) { + if (System.getProperty(name) == null && value != null) { + System.setProperty(name, value); + } + } + +} diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/Slf4JLoggingSystem.java b/spring-boot/src/main/java/org/springframework/boot/logging/Slf4JLoggingSystem.java index 21acec72854..97c871e242b 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/Slf4JLoggingSystem.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/Slf4JLoggingSystem.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * Copyright 2012-2016 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. @@ -18,6 +18,7 @@ package org.springframework.boot.logging; import org.slf4j.bridge.SLF4JBridgeHandler; +import org.springframework.util.Assert; import org.springframework.util.ClassUtils; /** @@ -45,6 +46,15 @@ public abstract class Slf4JLoggingSystem extends AbstractLoggingSystem { removeJdkLoggingBridgeHandler(); } + @Override + protected void loadConfiguration(LoggingInitializationContext initializationContext, + String location, LogFile logFile) { + Assert.notNull(location, "Location must not be null"); + if (initializationContext != null) { + applySystemProperties(initializationContext.getEnvironment(), logFile); + } + } + private void configureJdkLoggingBridgeHandler() { try { if (isBridgeHandlerAvailable()) { diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/log4j/Log4JLoggingSystem.java b/spring-boot/src/main/java/org/springframework/boot/logging/log4j/Log4JLoggingSystem.java index f59ea80938c..bcbc2be824f 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/log4j/Log4JLoggingSystem.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/log4j/Log4JLoggingSystem.java @@ -88,6 +88,7 @@ public class Log4JLoggingSystem extends Slf4JLoggingSystem { @Override protected void loadConfiguration(LoggingInitializationContext initializationContext, String location, LogFile logFile) { + super.loadConfiguration(initializationContext, location, logFile); loadConfiguration(location, logFile); } diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java b/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java index ed8c8f02654..f50c70eda2e 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java @@ -152,6 +152,7 @@ public class Log4J2LoggingSystem extends Slf4JLoggingSystem { @Override protected void loadConfiguration(LoggingInitializationContext initializationContext, String location, LogFile logFile) { + super.loadConfiguration(initializationContext, location, logFile); loadConfiguration(location, logFile); } diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java b/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java index 28b1e2d7aba..231ca22ecd7 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java @@ -128,7 +128,7 @@ public class LogbackLoggingSystem extends Slf4JLoggingSystem { @Override protected void loadConfiguration(LoggingInitializationContext initializationContext, String location, LogFile logFile) { - Assert.notNull(location, "Location must not be null"); + super.loadConfiguration(initializationContext, location, logFile); LoggerContext loggerContext = getLoggerContext(); stopAndReset(loggerContext); try { diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/AbstractLoggingSystemTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/AbstractLoggingSystemTests.java index 01724c44524..a55ae11e26a 100644 --- a/spring-boot/src/test/java/org/springframework/boot/logging/AbstractLoggingSystemTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/logging/AbstractLoggingSystemTests.java @@ -62,8 +62,15 @@ public abstract class AbstractLoggingSystemTests { } protected final LogFile getLogFile(String file, String path) { + return getLogFile(file, path, true); + } + + protected final LogFile getLogFile(String file, String path, + boolean applyToSystemProperties) { LogFile logFile = new LogFile(file, path); - logFile.applyToSystemProperties(); + if (applyToSystemProperties) { + logFile.applyToSystemProperties(); + } return logFile; } 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 57b10dfa108..d8990d1e42b 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 @@ -45,6 +45,7 @@ import org.springframework.util.FileCopyUtils; import org.springframework.util.StringUtils; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -73,11 +74,13 @@ public class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { private LoggingInitializationContext initializationContext; + private MockEnvironment environment; + @Before public void setup() { this.logger = new SLF4JLogFactory().getInstance(getClass().getName()); - this.initializationContext = new LoggingInitializationContext( - new MockEnvironment()); + this.environment = new MockEnvironment(); + this.initializationContext = new LoggingInitializationContext(this.environment); } @Override @@ -318,6 +321,18 @@ public class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { } } + @Test + public void reinitializeShouldSetSytemProperty() throws Exception { + // gh-5491 + this.loggingSystem.beforeInitialize(); + this.logger.info("Hidden"); + this.loggingSystem.initialize(this.initializationContext, null, null); + LogFile logFile = getLogFile(tmpDir() + "/example.log", null, false); + this.loggingSystem.initialize(this.initializationContext, + "classpath:logback-nondefault.xml", logFile); + assertThat(System.getProperty("LOG_FILE"), endsWith("example.log")); + } + private String getLineWithText(File file, String outputSearch) throws Exception { return getLineWithText(FileCopyUtils.copyToString(new FileReader(file)), outputSearch);