Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Register Logback OnErrorConsoleStatusListener when using custom Logback file #43931

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -220,6 +221,7 @@ private boolean initializeFromAotGeneratedArtifactsIfPossible(LoggingInitializat
configurator.setContext(loggerContext);
boolean configuredUsingAotGeneratedArtifacts = configurator.configureUsingAotGeneratedArtifacts();
if (configuredUsingAotGeneratedArtifacts) {
addOnErrorConsoleStatusListenerIfNecessary(loggerContext);
reportConfigurationErrorsIfNecessary(loggerContext);
}
return configuredUsingAotGeneratedArtifacts;
Expand All @@ -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),
Expand All @@ -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);
Expand All @@ -286,7 +287,7 @@ private void reportConfigurationErrorsIfNecessary(LoggerContext loggerContext) {
}
}
if (errors.isEmpty()) {
if (!StatusUtil.contextHasStatusListener(loggerContext)) {
if (shouldPrintErrors(loggerContext)) {
this.statusPrinter.printInCaseOfErrorsOrWarnings(loggerContext);
}
return;
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand All @@ -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
Expand Down Expand Up @@ -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");
}

}

}
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

}
Loading