This is an automated email from the ASF dual-hosted git repository.
suneet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new accd5536df Allow for Log4J to be configured for peons but still ensure
console logging is enforced (#14094)
accd5536df is described below
commit accd5536dfecac3b59bf695b030c62fda8204bf8
Author: TSFenwick <[email protected]>
AuthorDate: Mon Apr 24 10:41:56 2023 -0700
Allow for Log4J to be configured for peons but still ensure console logging
is enforced (#14094)
* Allow for Log4J to be configured for peons but still ensure console
logging is enforced
This change will allow for log4j to be configured for peons but require
console logging is still
configured for them to ensure peon logs are saved to deep storage.
Also fixed the test ConsoleLoggingEnforcementTest to use a valid appender
for the non console
Config as the previous config was incorrect and would never return a logger.
* fix checkstyle
* add warning to logger when it overwrites all loggers to be console
* optimize calls for altering logging config for
ConsoleLoggingEnforcementConfigurationFactory
add getName to the druid logger class
* update docs, and error message
* edit docs to be more clear
* fix checkstyle issues
* CI fixes - LoggerTest code coverage and fix spelling issue for logging
docs
---
docs/configuration/logging.md | 14 +++---
...soleLoggingEnforcementConfigurationFactory.java | 28 ++++++++---
.../tasklogs/ConsoleLoggingEnforcementTest.java | 55 ++++++++++++++--------
.../druid/java/util/common/logger/Logger.java | 5 ++
.../druid/java/util/common/logger/LoggerTest.java | 7 +++
website/.spelling | 1 +
6 files changed, 78 insertions(+), 32 deletions(-)
diff --git a/docs/configuration/logging.md b/docs/configuration/logging.md
index c7b7242cb2..20ac6cae48 100644
--- a/docs/configuration/logging.md
+++ b/docs/configuration/logging.md
@@ -105,13 +105,15 @@ The following example log4j2.xml is based upon the micro
quickstart:
</Configuration>
```
+Peons always output logs to standard output. Middle Managers redirect task
logs from standard output to
+[long-term storage](index.md#log-long-term-storage).
+
> NOTE:
-> Although Druid shares the log4j configuration file task peon processes,
-> the appenders in this file DO NOT take effect for peon processes. Peons
always output logs to standard output.
-> Middle Managers redirect task logs from standard output to [long-term
storage](index.md#log-long-term-storage).
->
-> However, log level settings do take effect for these task peon processes.
-> This means you can configure loggers at different logging level for task
logs using `log4j2.xml`.
+> Druid shares the log4j configuration file among all services, including task
peon processes.
+> However, you must define a console appender in the logger for your peon
processes.
+> If you don't define a console appender, Druid creates and configures a new
console appender
+> that retains the log level, such as `info` or `warn`, but does not retain
any other appender
+> configuration, including non-console ones.
## Log directory
The included log4j2.xml configuration for Druid and ZooKeeper writes logs to
the `log` directory at the root of the distribution.
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 1874963f3c..a8f774177b 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
@@ -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.AppenderRef;
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,11 +50,20 @@ import java.util.stream.Collectors;
*/
public class ConsoleLoggingEnforcementConfigurationFactory extends
ConfigurationFactory
{
+
+ private static final Logger log = new
Logger(ConsoleLoggingEnforcementConfigurationFactory.class);
+
/**
* Valid file extensions for XML files.
*/
public static final String[] SUFFIXES = new String[]{".xml", "*"};
+ // Alter log level for this class to be warning. This needs to happen
because the logger is using the default
+ // config, which is always level error and appends to console, since the
logger is being configured by this class.
+ static {
+ Configurator.setLevel(log.getName(), Level.WARN);
+ }
+
@Override
public String[] getSupportedTypes()
{
@@ -95,6 +106,7 @@ public class ConsoleLoggingEnforcementConfigurationFactory
extends Configuration
loggerConfigList.add(this.getRootLogger());
loggerConfigList.addAll(this.getLoggers().values());
+
//
// For all logger configuration, check if its appender is
ConsoleAppender.
// If not, replace it's appender to ConsoleAppender.
@@ -117,16 +129,16 @@ public class
ConsoleLoggingEnforcementConfigurationFactory extends Configuration
}
/**
- * 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,8 +155,12 @@ public class ConsoleLoggingEnforcementConfigurationFactory
extends Configuration
// use the first appender's definition
level = appenderRef.getLevel();
filter = appenderRef.getFilter();
+ log.warn("Clearing all configured appenders for logger %s. Using %s
instead.",
+ logger.toString(),
+ 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 ea5622074b..e74f877488 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
@@ -24,6 +24,7 @@ import org.apache.logging.log4j.LogManager;
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 class ConsoleLoggingEnforcementTest
// 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 class ConsoleLoggingEnforcementTest
+ " <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 class ConsoleLoggingEnforcementTest
+ " <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 class ConsoleLoggingEnforcementTest
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 class ConsoleLoggingEnforcementTest
+ " <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 class ConsoleLoggingEnforcementTest
// 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 @@ public class ConsoleLoggingEnforcementTest
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
diff --git
a/processing/src/main/java/org/apache/druid/java/util/common/logger/Logger.java
b/processing/src/main/java/org/apache/druid/java/util/common/logger/Logger.java
index 07f1cd1fba..64271add16 100644
---
a/processing/src/main/java/org/apache/druid/java/util/common/logger/Logger.java
+++
b/processing/src/main/java/org/apache/druid/java/util/common/logger/Logger.java
@@ -224,6 +224,11 @@ public class Logger
}
}
+ 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.
diff --git
a/processing/src/test/java/org/apache/druid/java/util/common/logger/LoggerTest.java
b/processing/src/test/java/org/apache/druid/java/util/common/logger/LoggerTest.java
index acb549b142..3fce46638f 100644
---
a/processing/src/test/java/org/apache/druid/java/util/common/logger/LoggerTest.java
+++
b/processing/src/test/java/org/apache/druid/java/util/common/logger/LoggerTest.java
@@ -128,6 +128,13 @@ public class LoggerTest
Assert.assertEquals(expected, msgCount.getValue());
}
+ @Test
+ public void testGetName()
+ {
+ String expected = "org.apache.druid.java.util.common.logger.LoggerTest";
+ Assert.assertEquals(expected, log.getName());
+ }
+
private Logger.LogFunction getLogToListFunction(List<String> messages)
{
return (msg, format) -> messages.add(StringUtils.format(msg, format));
diff --git a/website/.spelling b/website/.spelling
index 54cdb726a6..16d48b006a 100644
--- a/website/.spelling
+++ b/website/.spelling
@@ -746,6 +746,7 @@ APPROX_QUANTILE_FIXED_BUCKETS
100x
- ../docs/configuration/logging.md
_common
+appender
appenders
- ../docs/dependencies/deep-storage.md
druid-hdfs-storage
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
