Register Logback OnErrorConsoleStatusListener when using custom Logback file

Prior to this commit, OnErrorConsoleStatusListener was not registered
for a custom Logback configuration file.

This commit registers OnErrorConsoleStatusListener when the Logback
configuration is loaded from a custom Logback file that does
not include any StatusListener.

See gh-43822

Signed-off-by: Dmytro Nosan <[email protected]>

Author: nosan

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java

@@ -38,6 +38,7 @@
 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.status.StatusUtil;
 import ch.qos.logback.core.util.StatusListenerConfigHelper;
 import ch.qos.logback.core.util.StatusPrinter2;
@@ -220,6 +221,7 @@ private boolean initializeFromAotGeneratedArtifactsIfPossible(LoggingInitializat
 		configurator.setContext(loggerContext);
 		boolean configuredUsingAotGeneratedArtifacts = configurator.configureUsingAotGeneratedArtifacts();
 		if (configuredUsingAotGeneratedArtifacts) {
+			addOnErrorConsoleStatusListenerIfNecessary(loggerContext);
 			reportConfigurationErrorsIfNecessary(loggerContext);
 		}
 		return configuredUsingAotGeneratedArtifacts;
@@ -235,9 +237,7 @@ protected void loadDefaults(LoggingInitializationContext initializationContext,
 			if (debug) {
 				StatusListenerConfigHelper.addOnConsoleListenerInstance(loggerContext, new OnConsoleStatusListener());
 			}
-			else {
-				addOnErrorConsoleStatusListener(loggerContext);
-			}
+			addOnErrorConsoleStatusListenerIfNecessary(loggerContext);
 			Environment environment = initializationContext.getEnvironment();
 			// Apply system properties directly in case the same JVM runs multiple apps
 			new LogbackLoggingSystemProperties(environment, getDefaultValueResolver(environment),
@@ -264,6 +264,7 @@ protected void loadConfiguration(LoggingInitializationContext initializationCont
 			try {
 				Resource resource = ApplicationResourceLoader.get().getResource(location);
 				configureByResourceUrl(initializationContext, loggerContext, resource.getURL());
+				addOnErrorConsoleStatusListenerIfNecessary(loggerContext);
 			}
 			catch (Exception ex) {
 				throw new IllegalStateException("Could not initialize Logback logging from " + location, ex);
@@ -286,7 +287,7 @@ private void reportConfigurationErrorsIfNecessary(LoggerContext loggerContext) {
 			}
 		}
 		if (errors.isEmpty()) {
-			if (!StatusUtil.contextHasStatusListener(loggerContext)) {
+			if (shouldPrintErrors(loggerContext)) {
 				this.statusPrinter.printInCaseOfErrorsOrWarnings(loggerContext);
 			}
 			return;
@@ -297,6 +298,14 @@ private void reportConfigurationErrorsIfNecessary(LoggerContext loggerContext) {
 		throw ex;
 	}
 
+	private static boolean shouldPrintErrors(LoggerContext loggerContext) {
+		if (!StatusUtil.contextHasStatusListener(loggerContext)) {
+			return true;
+		}
+		List<StatusListener> listeners = loggerContext.getStatusManager().getCopyOfStatusListenerList();
+		return listeners.stream().allMatch((listener) -> listener instanceof FilteringStatusListener);
+	}
+
 	private void configureByResourceUrl(LoggingInitializationContext initializationContext, LoggerContext loggerContext,
 			URL url) throws JoranException {
 		JoranConfigurator configurator = new SpringBootJoranConfigurator(initializationContext);
@@ -491,7 +500,10 @@ private void withLoggingSuppressed(Runnable action) {
 		}
 	}
 
-	private void addOnErrorConsoleStatusListener(LoggerContext context) {
+	private void addOnErrorConsoleStatusListenerIfNecessary(LoggerContext context) {
+		if (StatusUtil.contextHasStatusListener(context)) {
+			return;
+		}
 		FilteringStatusListener listener = new FilteringStatusListener(new OnErrorConsoleStatusListener(),
 				Status.ERROR);
 		listener.setContext(context);

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java

@@ -43,7 +43,6 @@
 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 @@ void logbackDebugPropertyIsHonored(CapturedOutput output) {
 				.contains("SizeAndTimeBasedFileNamingAndTriggeringPolicy")
 				.contains("DebugLogbackConfigurator");
 			LoggerContext loggerContext = this.logger.getLoggerContext();
-			List<StatusListener> 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,46 @@ void logbackErrorStatusListenerShouldBeRegistered(CapturedOutput output) {
 		this.loggingSystem.beforeInitialize();
 		initialize(this.initializationContext, null, getLogFile(tmpDir() + "/tmp.log", null));
 		LoggerContext loggerContext = this.logger.getLoggerContext();
-		List<StatusListener> 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<ILoggingEvent> appender = new AppenderBase<>() {
-
-			@Override
-			protected void append(ILoggingEvent eventObject) {
-				throw new IllegalStateException("Fail to append");
-			}
-
-		};
+		assertThat(loggerContext.getStatusManager().getCopyOfStatusListenerList()).allSatisfy((listener) -> {
+			assertThat(listener).isInstanceOf(FilteringStatusListener.class);
+			assertThat(listener).hasFieldOrPropertyWithValue("levelThreshold", Status.ERROR);
+			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
+	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.ERROR);
+			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 logbackErrorStatusListenerShouldNotBeRegisteredWhenCustomLogbackXmlHasStatusListener(CapturedOutput output) {
+		this.loggingSystem.beforeInitialize();
+		initialize(this.initializationContext, "classpath:logback-include-status-listener.xml", null);
+		LoggerContext loggerContext = this.logger.getLoggerContext();
+		assertThat(loggerContext.getStatusManager().getCopyOfStatusListenerList())
+			.allSatisfy((listener) -> assertThat(listener).isInstanceOf(OnConsoleStatusListener.class));
+		this.logger.info("Hello world");
+		assertThat(output).contains("Hello world");
 	}
 
 	@Test
@@ -1042,4 +1060,13 @@ private static SizeAndTimeBasedRollingPolicy<?> getRollingPolicy() {
 		return (SizeAndTimeBasedRollingPolicy<?>) getFileAppender().getRollingPolicy();
 	}
 
+	private static final class AlwaysFailAppender extends AppenderBase<ILoggingEvent> {
+
+		@Override
+		protected void append(ILoggingEvent eventObject) {
+			throw new RuntimeException("Always Fail Appender");
+		}
+
+	}
+
 }

spring-boot-project/spring-boot/src/test/resources/logback-include-status-listener.xml

@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<configuration>
+	<statusListener class="ch.qos.logback.core.status.OnConsoleStatusListener"/>
+	<include resource="org/springframework/boot/logging/logback/defaults.xml"/>
+	<appender name="CONSOLE" class="ch.qos.logback.core.ConsoleAppender">
+		<encoder>
+			<pattern>[%p] - %m%n</pattern>
+		</encoder>
+	</appender>
+	<root level="INFO">
+		<appender-ref ref="CONSOLE"/>
+	</root>
+</configuration>

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

spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-structured-logging/src/main/resources/custom-logback.xml

@@ -0,0 +1,11 @@
+<configuration>
+	<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+		<encoder class="org.springframework.boot.logging.logback.StructuredLogEncoder">
+			<format>ecs</format>
+			<charset>UTF-8</charset>
+		</encoder>
+	</appender>
+	<root level="INFO">
+		<appender-ref ref="STDOUT"/>
+	</root>
+</configuration>

spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-structured-logging/src/test/java/smoketest/structuredlogging/SampleStructuredLoggingApplicationTests.java

@@ -71,4 +71,11 @@ void shouldCaptureCustomizerError(CapturedOutput output) {
 		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");
+	}
+
 }