From 497902de4eeac867befd1c7f1ee3682a9a0f4f42 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 19 Jun 2018 14:37:08 +0100 Subject: [PATCH] Only bridge JUL into SLF4J when JUL has not be customized MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, Slf4jLoggingSystem would install SLF4JBridgeHandler into JUL but would only remove a single root handler that was a ConsoleHandler. If there were was than one root handler or the single root handler was of a different type, they would not be uninstalled. When deploying an application to Tomcat, this led to duplicate log messages appearing in Tomcat’s console output and to logging from other application or Tomcat itself being routed into an application-specific log file enabled using the logging.file configuration property. A secondary, related problem was that LogbackLoggingSystem installs a LevelChangePropagator so that Logback’s log level configuration is propagated into JUL. This meant that an individual Boot app with custom log level configuration could change the log levels of Tomcat itself and of any other applications that had been deployed to Tomcat and use JUL. This commit updates both Slf4jLoggingSystem and LogbackLoggingSystem so that they only change JUL’s configuration if it hasn’t already been customized. The configuration is deemed to have not been customised if there’s a single root handler and its a console handler. Closes gh-13470 --- .../boot/logging/Slf4JLoggingSystem.java | 30 +++++++++++++++---- .../logging/logback/LogbackLoggingSystem.java | 14 ++++++++- .../log4j2/Log4J2LoggingSystemTests.java | 8 +++++ .../logback/LogbackLoggingSystemTests.java | 1 + 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/Slf4JLoggingSystem.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/Slf4JLoggingSystem.java index 22161d2ba26..59e8a7675f2 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/Slf4JLoggingSystem.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/Slf4JLoggingSystem.java @@ -48,7 +48,10 @@ public abstract class Slf4JLoggingSystem extends AbstractLoggingSystem { @Override public void cleanUp() { - removeJdkLoggingBridgeHandler(); + if (isBridgeHandlerAvailable()) { + removeJdkLoggingBridgeHandler(); + reinstateConsoleHandlerIfNecessary(); + } } @Override @@ -62,7 +65,7 @@ public abstract class Slf4JLoggingSystem extends AbstractLoggingSystem { private void configureJdkLoggingBridgeHandler() { try { - if (isBridgeHandlerAvailable()) { + if (isBridgeJulIntoSlf4j()) { removeJdkLoggingBridgeHandler(); SLF4JBridgeHandler.install(); } @@ -72,16 +75,24 @@ public abstract class Slf4JLoggingSystem extends AbstractLoggingSystem { } } + protected final boolean isBridgeJulIntoSlf4j() { + return isBridgeHandlerAvailable() && isJulUsingItsDefaultConfiguration(); + } + protected final boolean isBridgeHandlerAvailable() { return ClassUtils.isPresent(BRIDGE_HANDLER, getClassLoader()); } + private boolean isJulUsingItsDefaultConfiguration() { + Logger rootLogger = LogManager.getLogManager().getLogger(""); + Handler[] handlers = rootLogger.getHandlers(); + return handlers.length == 1 && handlers[0] instanceof ConsoleHandler; + } + private void removeJdkLoggingBridgeHandler() { try { - if (isBridgeHandlerAvailable()) { - removeDefaultRootHandler(); - SLF4JBridgeHandler.uninstall(); - } + removeDefaultRootHandler(); + SLF4JBridgeHandler.uninstall(); } catch (Throwable ex) { // Ignore and continue @@ -101,4 +112,11 @@ public abstract class Slf4JLoggingSystem extends AbstractLoggingSystem { } } + private void reinstateConsoleHandlerIfNecessary() { + Logger rootLogger = LogManager.getLogManager().getLogger(""); + if (rootLogger.getHandlers().length == 0) { + rootLogger.addHandler(new ConsoleHandler()); + } + } + } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java index d25ad8850c1..2c333c132a0 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java @@ -22,6 +22,8 @@ import java.security.ProtectionDomain; import java.util.ArrayList; import java.util.List; import java.util.Set; +import java.util.logging.Handler; +import java.util.logging.LogManager; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.LoggerContext; @@ -35,6 +37,7 @@ import ch.qos.logback.core.status.Status; import org.slf4j.ILoggerFactory; import org.slf4j.Logger; import org.slf4j.Marker; +import org.slf4j.bridge.SLF4JBridgeHandler; import org.slf4j.impl.StaticLoggerBinder; import org.springframework.boot.logging.LogFile; @@ -184,11 +187,20 @@ public class LogbackLoggingSystem extends Slf4JLoggingSystem { private void stopAndReset(LoggerContext loggerContext) { loggerContext.stop(); loggerContext.reset(); - if (isBridgeHandlerAvailable()) { + if (isBridgeHandlerInstalled()) { addLevelChangePropagator(loggerContext); } } + private boolean isBridgeHandlerInstalled() { + if (!isBridgeHandlerAvailable()) { + return false; + } + java.util.logging.Logger rootLogger = LogManager.getLogManager().getLogger(""); + Handler[] handlers = rootLogger.getHandlers(); + return handlers.length == 1 && SLF4JBridgeHandler.class.isInstance(handlers[0]); + } + private void addLevelChangePropagator(LoggerContext loggerContext) { LevelChangePropagator levelChangePropagator = new LevelChangePropagator(); levelChangePropagator.setResetJUL(true); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java index e68e4064da3..327d9ea4bf9 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java @@ -32,6 +32,7 @@ import org.apache.logging.log4j.core.LoggerContext; import org.apache.logging.log4j.core.config.Configuration; import org.hamcrest.Matcher; import org.hamcrest.Matchers; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -77,6 +78,13 @@ public class Log4J2LoggingSystemTests extends AbstractLoggingSystemTests { this.logger = LogManager.getLogger(getClass()); } + @Override + @After + public void clear() { + super.clear(); + this.loggingSystem.cleanUp(); + } + @Test public void noFile() { this.loggingSystem.beforeInitialize(); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java index ea9638000e5..6b1c37268c2 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java @@ -104,6 +104,7 @@ public class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { @Override @After public void clear() { + super.clear(); this.loggingSystem.cleanUp(); }