Allow multiple consoleAppender to be used in peon logging (#14521)

* Allow multiple consoleAppender to be used in peon logging

* Fix Attempted to append to non-started appender error

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();
+      List<Appender> consoleAppenders = findConsoleAppenders();
-      if (consoleAppender == null) {
+      if (consoleAppenders.isEmpty()) {
         // create a ConsoleAppender with default pattern if no console appender is configured in the configuration file
-        consoleAppender = ConsoleAppender.newBuilder()
+        Appender injectedConsoleAppender = ConsoleAppender.newBuilder()
-                                         .setName("_Injected_Console_Appender_")
+                                                          .setName("_Injected_Console_Appender_")
-                                         .setLayout(PatternLayout.newBuilder()
+                                                          .setLayout(
-                                                                 .withPattern("%d{ISO8601} %p [%t] %c - %m%n")
+                                                              PatternLayout.newBuilder()
-                                                                 .build())
+                                                                           .withPattern("%d{ISO8601} %p [%t] %c - %m%n")
-                                         .build();
+                                                                           .build()
+                                                          )
+                                                          .build();
+        injectedConsoleAppender.start();
+        consoleAppenders.add(injectedConsoleAppender);
       }
 
       List<LoggerConfig> 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
+    @Nonnull
-    private Appender findConsoleAppender()
+    private List<Appender> findConsoleAppenders()
     {
+      List<Appender> consoleAppenders = new ArrayList<>();
       for (Map.Entry<String, Appender> 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<Appender> consoleAppenders)
     {
+      List<String> 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);
     }

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 = "<Configuration status=\"WARN\">\n"
+                                + "  <Appenders>\n"
+                                + "    <Console name=\"Console\" target=\"SYSTEM_OUT\">\n"
+                                + "      <PatternLayout pattern=\"%m\"/>\n"
+                                + "    </Console>\n"
+                                + "    <Console name=\"Console1\" target=\"SYSTEM_OUT\">\n"
+                                + "      <PatternLayout pattern=\"%m\"/>\n"
+                                + "    </Console>\n"
+                                + "    <Console name=\"Console2\" target=\"SYSTEM_OUT\">\n"
+                                + "      <PatternLayout pattern=\"%m\"/>\n"
+                                + "    </Console>\n"
+                                + "    <Http name=\"Http\" url=\"http://localhost:9200/\">\n"
+                                + "      <JsonLayout properties=\"true\"/>\n"
+                                + "    </Http>\n"
+                                + "  </Appenders>\n"
+                                + "  <Loggers>\n"
+                                + "    <Root level=\"info\">\n"
+                                + "      <AppenderRef ref=\"Http\"/>\n"
+                                + "      <AppenderRef ref=\"Console\"/>\n"
+                                + "    </Root>\n"
+                                + "    <Logger level=\"debug\" name=\"org.apache.druid\" additivity=\"false\">\n"
+                                + "      <AppenderRef ref=\"Http\"/>\n"
+                                + "      <AppenderRef ref=\"Console\"/>\n"
+                                + "    </Logger>\n"
+                                + "    <Logger level=\"debug\" name=\"org.apache.mmon1\" additivity=\"false\">\n"
+                                + "      <AppenderRef ref=\"Http\"/>\n"
+                                + "      <AppenderRef ref=\"Console1\"/>\n"
+                                + "    </Logger>\n"
+                                + "    <Logger level=\"debug\" name=\"org.apache.mmon2\" additivity=\"false\">\n"
+                                + "      <AppenderRef ref=\"Http\"/>\n"
+                                + "      <AppenderRef ref=\"Console2\"/>\n"
+                                + "    </Logger>\n"
+                                + "    <Logger level=\"debug\" name=\"org.apache.mmon3\" additivity=\"false\">\n"
+                                + "      <AppenderRef ref=\"Http\"/>\n"
+                                + "    </Logger>\n"
+                                + "    <Logger level=\"debug\" name=\"org.apache.mmon4\" additivity=\"false\">\n"
+                                + "    </Logger>\n"
+                                + "  </Loggers>\n"
+                                + "</Configuration>";
+
+    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(ConsoleAppender.class, logger.getAppenders().get(consoleName).getClass());
-    Assert.assertEquals(HttpAppender.class, logger.getAppenders().get("Http").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