Allow for Log4J to be configured for peons but still ensure console logging is enforced

indexing-service/src/main/java/org/apache/druid/indexing/common/tasklogs/ConsoleLoggingEnforcementConfigurationFactory.java

@@ -19,6 +19,7 @@
 
 package org.apache.druid.indexing.common.tasklogs;
 
+import org.apache.druid.java.util.common.logger.Logger;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.core.Appender;
 import org.apache.logging.log4j.core.Filter;
@@ -28,6 +29,7 @@
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.ConfigurationFactory;
 import org.apache.logging.log4j.core.config.ConfigurationSource;
+import org.apache.logging.log4j.core.config.Configurator;
 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;
@@ -48,6 +50,9 @@
  */
 public class ConsoleLoggingEnforcementConfigurationFactory extends ConfigurationFactory
 {
+
+  private static final Logger log = new Logger(ConsoleLoggingEnforcementConfigurationFactory.class);
+
   /**
    * Valid file extensions for XML files.
    */
@@ -95,6 +100,10 @@ protected void doConfigure()
       loggerConfigList.add(this.getRootLogger());
       loggerConfigList.addAll(this.getLoggers().values());
 
+      // Alter log level for this class to be warning. This needs to happen because the logger is using the default
+      // config, which is level error and appends to console, since the logger is being configured here.
+      Configurator.setLevel(log.getName(), Level.WARN);
+
       //
       // For all logger configuration, check if its appender is ConsoleAppender.
       // If not, replace it's appender to ConsoleAppender.
@@ -117,16 +126,16 @@ private Appender findConsoleAppender()
     }
 
     /**
-     * remove all appenders from a logger and append a console appender to it
+     * 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)
     {
-      if (logger.getAppenderRefs().size() == 1
-          && logger.getAppenderRefs().get(0).getRef().equals(consoleAppender.getName())) {
-        // this logger has only one appender and its the console appender
-        return;
+      for (AppenderRef appenderRef : logger.getAppenderRefs()) {
+        if (consoleAppender.getName().equals(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;
 
@@ -143,6 +152,7 @@ private void applyConsoleAppender(LoggerConfig logger, Appender consoleAppender)
         // use the first appender's definition
         level = appenderRef.getLevel();
         filter = appenderRef.getFilter();
+        log.warn("Clearing all configured appenders for logger %s. Using ConsoleAppender instead.", logger.getName());
       }
 
       // add ConsoleAppender to this logger

indexing-service/src/test/java/org/apache/druid/indexing/common/tasklogs/ConsoleLoggingEnforcementTest.java

@@ -24,6 +24,7 @@
 import org.apache.logging.log4j.core.Logger;
 import org.apache.logging.log4j.core.LoggerContext;
 import org.apache.logging.log4j.core.appender.ConsoleAppender;
+import org.apache.logging.log4j.core.appender.HttpAppender;
 import org.apache.logging.log4j.core.config.ConfigurationSource;
 import org.apache.logging.log4j.core.layout.PatternLayout;
 import org.junit.Assert;
@@ -79,9 +80,9 @@ public void testNoConsoleAppender() throws IOException
     // this logger configuration has no console logger appender, a default one will be created
     String log4jConfiguration = "<Configuration status=\"WARN\">\n"
                                 + "  <Appenders>\n"
-                                + "    <RollingRandomAccessFile name=\"FileAppender\" fileName=\"a.log\">\n"
-                                + "      <PatternLayout pattern=\"%d{ISO8601} %p [%t] %c - %m%n\"/>\n"
-                                + "    </RollingRandomAccessFile>\n"
+                                + "    <Http name=\"Http\" url=\"http://localhost:9200/\">\n"
+                                + "      <JsonLayout properties=\"true\"/>\n"
+                                + "    </Http>\n"
                                 + "  </Appenders>\n"
                                 + "  <Loggers>\n"
                                 + "    <Root level=\"info\">\n"
@@ -115,13 +116,13 @@ public void testHasConsoleAppenderButNotUsed() throws IOException
                                 + "    <Console name=\"Console\" target=\"SYSTEM_OUT\">\n"
                                 + "      <PatternLayout pattern=\"%m\"/>\n"
                                 + "    </Console>\n"
-                                + "    <RollingRandomAccessFile name=\"FileAppender\" fileName=\"a.log\">\n"
-                                + "      <PatternLayout pattern=\"%d{ISO8601} %p [%t] %c - %m%n\"/>\n"
-                                + "    </RollingRandomAccessFile>\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=\"FileAppender\"/>\n"
+                                + "      <AppenderRef ref=\"Http\"/>\n"
                                 + "    </Root>\n"
                                 + "  </Loggers>\n"
                                 + "</Configuration>";
@@ -152,17 +153,17 @@ public void testMultipleAppender() throws IOException
                                 + "    <Console name=\"Console\" target=\"SYSTEM_OUT\">\n"
                                 + "      <PatternLayout pattern=\"%m\"/>\n"
                                 + "    </Console>\n"
-                                + "    <RollingRandomAccessFile name=\"FileAppender\" fileName=\"a.log\">\n"
-                                + "      <PatternLayout pattern=\"%d{ISO8601} %p [%t] %c - %m%n\"/>\n"
-                                + "    </RollingRandomAccessFile>\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=\"FileAppender\"/>\n"
+                                + "      <AppenderRef ref=\"Http\"/>\n"
                                 + "      <AppenderRef ref=\"Console\"/>\n"
                                 + "    </Root>\n"
                                 + "    <Logger level=\"debug\" name=\"org.apache.druid\" additivity=\"false\">\n"
-                                + "      <AppenderRef ref=\"FileAppender\"/>\n"
+                                + "      <AppenderRef ref=\"Http\"/>\n"
                                 + "      <AppenderRef ref=\"Console\"/>\n"
                                 + "    </Logger>\n"
                                 + "  </Loggers>\n"
@@ -171,10 +172,10 @@ public void testMultipleAppender() throws IOException
     LoggerContext context = enforceConsoleLogger(log4jConfiguration);
 
     // this logger is not defined in configuration, it derivates ROOT logger configuration
-    assertHasOnlyOneConsoleAppender(getLogger(context, "name_not_in_config"), Level.INFO);
+    assertHasConsoleAppenderAndHttpAppender(getLogger(context, "name_not_in_config"), Level.INFO);
 
-    assertHasOnlyOneConsoleAppender(getLogger(context, "org.apache.druid"), Level.DEBUG);
-    assertHasOnlyOneConsoleAppender(getLogger(context, ROOT), Level.INFO);
+    assertHasConsoleAppenderAndHttpAppender(getLogger(context, "org.apache.druid"), Level.DEBUG);
+    assertHasConsoleAppenderAndHttpAppender(getLogger(context, ROOT), Level.INFO);
 
     // the ConsoleAppender should be exactly the same as it's in the configuration
     PatternLayout layout = (PatternLayout) getLogger(context, "anything").getAppenders()
@@ -195,15 +196,15 @@ public void testEmptyAppender() throws IOException
                                 + "    <Console name=\"Console\" target=\"SYSTEM_OUT\">\n"
                                 + "      <PatternLayout pattern=\"%m\"/>\n"
                                 + "    </Console>\n"
-                                + "    <RollingRandomAccessFile name=\"FileAppender\" fileName=\"a.log\">\n"
-                                + "      <PatternLayout pattern=\"%d{ISO8601} %p [%t] %c - %m%n\"/>\n"
-                                + "    </RollingRandomAccessFile>\n"
+                                + "    <Http name=\"Http\" url=\"http://localhost:9200/\">\n"
+                                + "      <JsonLayout properties=\"true\"/>\n"
+                                + "    </Http>\n"
                                 + "  </Appenders>\n"
                                 + "  <Loggers>\n"
                                 + "    <Root level=\"info\">\n"
                                 + "    </Root>\n"
                                 + "    <Logger level=\"debug\" name=\"org.apache.druid\" additivity=\"false\">\n"
-                                + "      <AppenderRef ref=\"FileAppender\"/>\n"
+                                + "      <AppenderRef ref=\"Http\"/>\n"
                                 + "      <AppenderRef ref=\"Console\"/>\n"
                                 + "    </Logger>\n"
                                 + "  </Loggers>\n"
@@ -214,7 +215,7 @@ public void testEmptyAppender() throws IOException
     // this logger is not defined in configuration, it derivates ROOT logger configuration
     assertHasOnlyOneConsoleAppender(getLogger(context, "name_not_in_config"), Level.INFO);
 
-    assertHasOnlyOneConsoleAppender(getLogger(context, "org.apache.druid"), Level.DEBUG);
+    assertHasConsoleAppenderAndHttpAppender(getLogger(context, "org.apache.druid"), Level.DEBUG);
     assertHasOnlyOneConsoleAppender(getLogger(context, ROOT), Level.INFO);
 
     // the ConsoleAppender should be exactly the same as it's in the configuration
@@ -237,6 +238,20 @@ private LoggerContext enforceConsoleLogger(String configuration) throws IOExcept
     return context;
   }
 
+  private void assertHasConsoleAppenderAndHttpAppender(Logger logger, Level level)
+  {
+    // 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());
+
+    if (level != null) {
+      Assert.assertEquals(level, logger.getLevel());
+    }
+  }
+
   private void assertHasOnlyOneConsoleAppender(Logger logger, Level level)
   {
     // there's only one appender

processing/src/main/java/org/apache/druid/java/util/common/logger/Logger.java

@@ -224,6 +224,10 @@ private void logException(BiConsumer<String, Throwable> fn, Throwable t, String
     }
   }
 
+  public String getName() {
+    return this.log.getName();
+  }
+
   /**
    * Logs all the segment ids you could ever want, {@link #SEGMENTS_PER_LOG_MESSAGE} at a time, as a comma separated
    * list.