From 258eb29978bcefc5bd14514b5bf50509cd73167b Mon Sep 17 00:00:00 2001 From: Dmytro Nosan Date: Wed, 22 Jan 2025 16:26:53 +0200 Subject: [PATCH] Register Logback StatusListener when using custom Logback file Update `LogbackLoggingSystem` so that the `OnErrorConsoleStatusListener` is also registered when loading a custom Logback configuration file. See gh-43931 Signed-off-by: Dmytro Nosan --- .../logging/logback/LogbackLoggingSystem.java | 4 +- .../logback/LogbackLoggingSystemTests.java | 58 ++++++++++++------- .../logback-include-status-listener.xml | 13 +++++ .../src/main/resources/application.properties | 4 ++ .../src/main/resources/custom-logback.xml | 11 ++++ ...mpleStructuredLoggingApplicationTests.java | 7 +++ 6 files changed, 75 insertions(+), 22 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/test/resources/logback-include-status-listener.xml create mode 100644 spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-structured-logging/src/main/resources/custom-logback.xml 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 7fcc812cd8..3b7cad7428 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 @@ -216,6 +216,7 @@ public class LogbackLoggingSystem extends AbstractLoggingSystem implements BeanF LoggerContext loggerContext = getLoggerContext(); stopAndReset(loggerContext); withLoggingSuppressed(() -> putInitializationContextObjects(loggerContext, initializationContext)); + addOnErrorConsoleStatusListener(loggerContext); SpringBootJoranConfigurator configurator = new SpringBootJoranConfigurator(initializationContext); configurator.setContext(loggerContext); boolean configuredUsingAotGeneratedArtifacts = configurator.configureUsingAotGeneratedArtifacts(); @@ -261,6 +262,7 @@ public class LogbackLoggingSystem extends AbstractLoggingSystem implements BeanF if (initializationContext != null) { applySystemProperties(initializationContext.getEnvironment(), logFile); } + addOnErrorConsoleStatusListener(loggerContext); try { Resource resource = ApplicationResourceLoader.get().getResource(location); configureByResourceUrl(initializationContext, loggerContext, resource.getURL()); @@ -493,7 +495,7 @@ public class LogbackLoggingSystem extends AbstractLoggingSystem implements BeanF private void addOnErrorConsoleStatusListener(LoggerContext context) { FilteringStatusListener listener = new FilteringStatusListener(new OnErrorConsoleStatusListener(), - Status.ERROR); + Status.WARN); listener.setContext(context); if (context.getStatusManager().add(listener)) { listener.start(); 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 966b0f593b..79133332c0 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 @@ -43,7 +43,6 @@ import ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy; import ch.qos.logback.core.status.OnConsoleStatusListener; import ch.qos.logback.core.status.OnErrorConsoleStatusListener; import ch.qos.logback.core.status.Status; -import ch.qos.logback.core.status.StatusListener; import ch.qos.logback.core.util.DynamicClassLoadingException; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -656,10 +655,8 @@ class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { .contains("SizeAndTimeBasedFileNamingAndTriggeringPolicy") .contains("DebugLogbackConfigurator"); LoggerContext loggerContext = this.logger.getLoggerContext(); - List statusListeners = loggerContext.getStatusManager().getCopyOfStatusListenerList(); - assertThat(statusListeners).hasSize(1); - StatusListener statusListener = statusListeners.get(0); - assertThat(statusListener).isInstanceOf(OnConsoleStatusListener.class); + assertThat(loggerContext.getStatusManager().getCopyOfStatusListenerList()) + .allSatisfy((listener) -> assertThat(listener).isInstanceOf(OnConsoleStatusListener.class)); } finally { System.clearProperty("logback.debug"); @@ -671,25 +668,35 @@ class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { this.loggingSystem.beforeInitialize(); initialize(this.initializationContext, null, getLogFile(tmpDir() + "/tmp.log", null)); LoggerContext loggerContext = this.logger.getLoggerContext(); - List statusListeners = loggerContext.getStatusManager().getCopyOfStatusListenerList(); - assertThat(statusListeners).hasSize(1); - StatusListener statusListener = statusListeners.get(0); - assertThat(statusListener).isInstanceOf(FilteringStatusListener.class); - assertThat(statusListener).hasFieldOrPropertyWithValue("levelThreshold", Status.ERROR); - assertThat(statusListener).extracting("delegate").isInstanceOf(OnErrorConsoleStatusListener.class); - AppenderBase appender = new AppenderBase<>() { - - @Override - protected void append(ILoggingEvent eventObject) { - throw new IllegalStateException("Fail to append"); - } - - }; - this.logger.addAppender(appender); + assertThat(loggerContext.getStatusManager().getCopyOfStatusListenerList()).allSatisfy((listener) -> { + assertThat(listener).isInstanceOf(FilteringStatusListener.class); + assertThat(listener).hasFieldOrPropertyWithValue("levelThreshold", Status.WARN); + assertThat(listener).extracting("delegate").isInstanceOf(OnErrorConsoleStatusListener.class); + }); + AlwaysFailAppender appender = new AlwaysFailAppender(); appender.setContext(loggerContext); appender.start(); + this.logger.addAppender(appender); this.logger.info("Hello world"); - assertThat(output).contains("Fail to append").contains("Hello world"); + assertThat(output).contains("Always Fail Appender").contains("Hello world"); + } + + @Test + void logbackErrorStatusListenerShouldBeRegisteredWhenUsingCustomLogbackXml(CapturedOutput output) { + this.loggingSystem.beforeInitialize(); + initialize(this.initializationContext, "classpath:logback-include-defaults.xml", null); + LoggerContext loggerContext = this.logger.getLoggerContext(); + assertThat(loggerContext.getStatusManager().getCopyOfStatusListenerList()).allSatisfy((listener) -> { + assertThat(listener).isInstanceOf(FilteringStatusListener.class); + assertThat(listener).hasFieldOrPropertyWithValue("levelThreshold", Status.WARN); + assertThat(listener).extracting("delegate").isInstanceOf(OnErrorConsoleStatusListener.class); + }); + AlwaysFailAppender appender = new AlwaysFailAppender(); + appender.setContext(loggerContext); + appender.start(); + this.logger.addAppender(appender); + this.logger.info("Hello world"); + assertThat(output).contains("Always Fail Appender").contains("Hello world"); } @Test @@ -1042,4 +1049,13 @@ class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { return (SizeAndTimeBasedRollingPolicy) getFileAppender().getRollingPolicy(); } + private static final class AlwaysFailAppender extends AppenderBase { + + @Override + protected void append(ILoggingEvent eventObject) { + throw new RuntimeException("Always Fail Appender"); + } + + } + } diff --git a/spring-boot-project/spring-boot/src/test/resources/logback-include-status-listener.xml b/spring-boot-project/spring-boot/src/test/resources/logback-include-status-listener.xml new file mode 100644 index 0000000000..6be18d7a37 --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/resources/logback-include-status-listener.xml @@ -0,0 +1,13 @@ + + + + + + + [%p] - %m%n + + + + + + diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-structured-logging/src/main/resources/application.properties b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-structured-logging/src/main/resources/application.properties index 000769604f..6a26dd0049 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-structured-logging/src/main/resources/application.properties +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-structured-logging/src/main/resources/application.properties @@ -10,3 +10,7 @@ logging.structured.format.console=smoketest.structuredlogging.CustomStructuredLo #--- spring.config.activate.on-profile=on-error logging.structured.json.customizer=smoketest.structuredlogging.DuplicateJsonMembersCustomizer +#--- +logging.config=classpath:custom-logback.xml +spring.config.activate.on-profile=on-error-custom-logback-file +logging.structured.json.customizer=smoketest.structuredlogging.DuplicateJsonMembersCustomizer diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-structured-logging/src/main/resources/custom-logback.xml b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-structured-logging/src/main/resources/custom-logback.xml new file mode 100644 index 0000000000..fd274d8cba --- /dev/null +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-structured-logging/src/main/resources/custom-logback.xml @@ -0,0 +1,11 @@ + + + + ecs + UTF-8 + + + + + + diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-structured-logging/src/test/java/smoketest/structuredlogging/SampleStructuredLoggingApplicationTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-structured-logging/src/test/java/smoketest/structuredlogging/SampleStructuredLoggingApplicationTests.java index 2d60bb8c8f..a6f8b8fbd1 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-structured-logging/src/test/java/smoketest/structuredlogging/SampleStructuredLoggingApplicationTests.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-structured-logging/src/test/java/smoketest/structuredlogging/SampleStructuredLoggingApplicationTests.java @@ -71,4 +71,11 @@ class SampleStructuredLoggingApplicationTests { assertThat(output).contains("The name 'test' has already been written"); } + @Test + void shouldCaptureCustomizerErrorWhenUsingCustomLogbackFile(CapturedOutput output) { + SampleStructuredLoggingApplication + .main(new String[] { "--spring.profiles.active=on-error-custom-logback-file" }); + assertThat(output).contains("The name 'test' has already been written"); + } + }