Register Logback OnErrorConsoleStatusListener when using custom Logba…

…ck 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 spring-projectsgh-43822 Signed-off-by: Dmytro Nosan <[email protected]>
nosan · Jan 23, 2025 · 653fd2a · 653fd2a
1 parent 766e71a
commit 653fd2a
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 23 deletions.
diff --git a/...ing-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java b/...ing-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java
@@ -221,6 +221,7 @@ private boolean initializeFromAotGeneratedArtifactsIfPossible(LoggingInitializat
 		boolean configuredUsingAotGeneratedArtifacts = configurator.configureUsingAotGeneratedArtifacts();
 		if (configuredUsingAotGeneratedArtifacts) {
 			reportConfigurationErrorsIfNecessary(loggerContext);
+			addOnErrorConsoleStatusListener(loggerContext);
 		}
 		return configuredUsingAotGeneratedArtifacts;
 	}
@@ -235,9 +236,7 @@ protected void loadDefaults(LoggingInitializationContext initializationContext,
 			if (debug) {
 				StatusListenerConfigHelper.addOnConsoleListenerInstance(loggerContext, new OnConsoleStatusListener());
 			}
-			else {
-				addOnErrorConsoleStatusListener(loggerContext);
-			}
+			addOnErrorConsoleStatusListener(loggerContext);
 			Environment environment = initializationContext.getEnvironment();
 			// Apply system properties directly in case the same JVM runs multiple apps
 			new LogbackLoggingSystemProperties(environment, getDefaultValueResolver(environment),
@@ -271,6 +270,7 @@ protected void loadConfiguration(LoggingInitializationContext initializationCont
 			loggerContext.start();
 		});
 		reportConfigurationErrorsIfNecessary(loggerContext);
+		addOnErrorConsoleStatusListener(loggerContext);
 	}
 
 	private void reportConfigurationErrorsIfNecessary(LoggerContext loggerContext) {
@@ -492,6 +492,9 @@ private void withLoggingSuppressed(Runnable action) {
 	}
 
 	private void addOnErrorConsoleStatusListener(LoggerContext context) {
+		if (StatusUtil.contextHasStatusListener(context)) {
+			return;
+		}
 		FilteringStatusListener listener = new FilteringStatusListener(new OnErrorConsoleStatusListener(),
 				Status.ERROR);
 		listener.setContext(context);

diff --git a/...oot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java b/...oot/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");
+		}
+
+	}
+
 }
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
@@ -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>
diff --git a/...tests/spring-boot-smoke-test-structured-logging/src/main/resources/application.properties b/...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/...oke-tests/spring-boot-smoke-test-structured-logging/src/main/resources/custom-logback.xml b/...oke-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>
diff --git a/...ng/src/test/java/smoketest/structuredlogging/SampleStructuredLoggingApplicationTests.java b/...ng/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");
+	}
+
 }