diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/tasklogs/ConsoleLoggingEnforcementConfigurationFactory.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/tasklogs/ConsoleLoggingEnforcementConfigurationFactory.java index a8f774177b7..a009a2133b3 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/tasklogs/ConsoleLoggingEnforcementConfigurationFactory.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/tasklogs/ConsoleLoggingEnforcementConfigurationFactory.java @@ -34,7 +34,7 @@ import org.apache.logging.log4j.core.config.LoggerConfig; import org.apache.logging.log4j.core.config.xml.XmlConfiguration; import org.apache.logging.log4j.core.layout.PatternLayout; -import javax.annotation.Nullable; +import javax.annotation.Nonnull; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -91,15 +91,19 @@ public class ConsoleLoggingEnforcementConfigurationFactory extends Configuration { super.doConfigure(); - Appender consoleAppender = findConsoleAppender(); - if (consoleAppender == null) { + List consoleAppenders = findConsoleAppenders(); + if (consoleAppenders.isEmpty()) { // create a ConsoleAppender with default pattern if no console appender is configured in the configuration file - consoleAppender = ConsoleAppender.newBuilder() - .setName("_Injected_Console_Appender_") - .setLayout(PatternLayout.newBuilder() - .withPattern("%d{ISO8601} %p [%t] %c - %m%n") - .build()) - .build(); + Appender injectedConsoleAppender = ConsoleAppender.newBuilder() + .setName("_Injected_Console_Appender_") + .setLayout( + PatternLayout.newBuilder() + .withPattern("%d{ISO8601} %p [%t] %c - %m%n") + .build() + ) + .build(); + injectedConsoleAppender.start(); + consoleAppenders.add(injectedConsoleAppender); } List loggerConfigList = new ArrayList<>(); @@ -112,36 +116,38 @@ public class ConsoleLoggingEnforcementConfigurationFactory extends Configuration // If not, replace it's appender to ConsoleAppender. // for (LoggerConfig logger : loggerConfigList) { - applyConsoleAppender(logger, consoleAppender); + applyConsoleAppender(logger, consoleAppenders); } } - @Nullable - private Appender findConsoleAppender() + @Nonnull + private List findConsoleAppenders() { + List consoleAppenders = new ArrayList<>(); for (Map.Entry entry : this.getAppenders().entrySet()) { Appender appender = entry.getValue(); if (appender instanceof ConsoleAppender) { - return appender; + consoleAppenders.add(appender); } } - return null; + return consoleAppenders; } /** * Ensure there is a console logger defined. Without a console logger peon logs wont be able to be stored in deep storage */ - private void applyConsoleAppender(LoggerConfig logger, Appender consoleAppender) + private void applyConsoleAppender(LoggerConfig logger, List consoleAppenders) { + List consoleAppenderNames = consoleAppenders.stream().map(Appender::getName).collect(Collectors.toList()); for (AppenderRef appenderRef : logger.getAppenderRefs()) { - if (consoleAppender.getName().equals(appenderRef.getRef())) { + if (consoleAppenderNames.contains(appenderRef.getRef())) { // we need a console logger no matter what, but we want to be able to define a different appender if necessary return; } } Level level = Level.INFO; Filter filter = null; - + Appender consoleAppender = consoleAppenders.get(0); if (!logger.getAppenderRefs().isEmpty()) { AppenderRef appenderRef = logger.getAppenderRefs().get(0); @@ -160,7 +166,6 @@ public class ConsoleLoggingEnforcementConfigurationFactory extends Configuration consoleAppender.getName()); } - // add ConsoleAppender to this logger logger.addAppender(consoleAppender, level, filter); } diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/tasklogs/ConsoleLoggingEnforcementTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/tasklogs/ConsoleLoggingEnforcementTest.java index e74f8774888..cf434ac89ff 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/tasklogs/ConsoleLoggingEnforcementTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/tasklogs/ConsoleLoggingEnforcementTest.java @@ -228,6 +228,73 @@ public class ConsoleLoggingEnforcementTest Assert.assertEquals("%m", layout.getConversionPattern()); } + @Test + public void testMultipleConsoleAppender() throws IOException + { + // this logger configuration contains multiple appenders and appender refers + String log4jConfiguration = "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + + LoggerContext context = enforceConsoleLogger(log4jConfiguration); + + // this logger is not defined in configuration, it derivates ROOT logger configuration + assertHasConsoleAppenderAndHttpAppender(getLogger(context, "name_not_in_config"), Level.INFO); + + assertHasConsoleAppenderAndHttpAppender(getLogger(context, "org.apache.druid"), Level.DEBUG); + assertHasConsoleAppenderAndHttpAppender(getLogger(context, ROOT), Level.INFO); + + assertHasConsoleAppenderAndHttpAppenderWithNameValidation(getLogger(context, "org.apache.mmon1"), Level.DEBUG, "Console1", "Http"); + assertHasConsoleAppenderAndHttpAppenderWithNameValidation(getLogger(context, "org.apache.mmon2"), Level.DEBUG, "Console2", "Http"); + assertHasOnlyOneConsoleAppender(getLogger(context, "org.apache.mmon3"), Level.DEBUG); + assertHasOnlyOneConsoleAppender(getLogger(context, "org.apache.mmon4"), Level.DEBUG); + + // the ConsoleAppender should be exactly the same as it's in the configuration + PatternLayout layout = (PatternLayout) getLogger(context, "anything").getAppenders() + .values() + .stream() + .findFirst() + .get() + .getLayout(); + Assert.assertEquals("%m", layout.getConversionPattern()); + } + private LoggerContext enforceConsoleLogger(String configuration) throws IOException { LoggerContext context = new LoggerContext("test"); @@ -239,19 +306,30 @@ public class ConsoleLoggingEnforcementTest } private void assertHasConsoleAppenderAndHttpAppender(Logger logger, Level level) + { + assertHasConsoleAppenderAndHttpAppenderWithNameValidation(logger, level, "Console", "Http"); + } + + private void assertHasConsoleAppenderAndHttpAppenderWithNameValidation( + Logger logger, + Level level, + String consoleName, + String httpName + ) { // there's two appenders Assert.assertEquals(2, logger.getAppenders().size()); // and the appenders must be ConsoleAppender and HttpAppender - Assert.assertEquals(ConsoleAppender.class, logger.getAppenders().get("Console").getClass()); - Assert.assertEquals(HttpAppender.class, logger.getAppenders().get("Http").getClass()); + Assert.assertEquals(ConsoleAppender.class, logger.getAppenders().get(consoleName).getClass()); + Assert.assertEquals(HttpAppender.class, logger.getAppenders().get(httpName).getClass()); if (level != null) { Assert.assertEquals(level, logger.getLevel()); } } + private void assertHasOnlyOneConsoleAppender(Logger logger, Level level) { // there's only one appender