From 447d92e4824f9b4086bfeed0f4f02cb30422d7c7 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 14 Aug 2017 19:39:14 -0400 Subject: [PATCH] Allow not configure logging without config For CLI tools, we configure logging without reading the log4j2.properties file. This because any log statements in a CLI tool should dump to the console while reading from the log4j2.properties file would cause them to dump whereever the log configuration there indicates (e.g., possibly a remote machine). To do this, we added some code to the base implementation of all CLI tools to configure logging without a config file. This code is also executed when Elasticsearch starts up. In the past this was fine yet we previously added detection to Elasticsearch to find cases where we use logging before it is configured. Because of configuring logging without a config, this means we only catch uses of logging before the logging without config is performed. To correct this, we enable a CLI tool to skip enabling logging without a config and then in the Elasticsearch CLI we indeed utilize this to skip configuring logging without a config. Relates #26209 --- .../bootstrap/Elasticsearch.java | 9 +++++++++ .../java/org/elasticsearch/cli/Command.java | 20 +++++++++++++++---- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java index 62b01f3fe85..643ad812972 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java @@ -92,6 +92,15 @@ class Elasticsearch extends EnvironmentAwareCommand { return elasticsearch.main(args, terminal); } + @Override + protected boolean shouldConfigureLoggingWithoutConfig() { + /* + * If we allow logging to be configured without a config before we ready to read the log4j2.properties file, then we will fail to + * detect uses of logging before it is properly configured. + */ + return false; + } + @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws UserException { if (options.nonOptionArguments().isEmpty() == false) { diff --git a/core/src/main/java/org/elasticsearch/cli/Command.java b/core/src/main/java/org/elasticsearch/cli/Command.java index 99a9a7e43d9..a60dece2611 100644 --- a/core/src/main/java/org/elasticsearch/cli/Command.java +++ b/core/src/main/java/org/elasticsearch/cli/Command.java @@ -79,10 +79,12 @@ public abstract class Command implements Closeable { Runtime.getRuntime().addShutdownHook(shutdownHookThread.get()); } - // initialize default for es.logger.level because we will not read the log4j2.properties - final String loggerLevel = System.getProperty("es.logger.level", Level.INFO.name()); - final Settings settings = Settings.builder().put("logger.level", loggerLevel).build(); - LogConfigurator.configureWithoutConfig(settings); + if (shouldConfigureLoggingWithoutConfig()) { + // initialize default for es.logger.level because we will not read the log4j2.properties + final String loggerLevel = System.getProperty("es.logger.level", Level.INFO.name()); + final Settings settings = Settings.builder().put("logger.level", loggerLevel).build(); + LogConfigurator.configureWithoutConfig(settings); + } try { mainWithoutErrorHandling(args, terminal); @@ -100,6 +102,16 @@ public abstract class Command implements Closeable { return ExitCodes.OK; } + /** + * Indicate whether or not logging should be configured without reading a log4j2.properties. Most commands should do this because we do + * not configure logging for CLI tools. Only commands that configure logging on their own should not do this. + * + * @return true if logging should be configured without reading a log4j2.properties file + */ + protected boolean shouldConfigureLoggingWithoutConfig() { + return true; + } + /** * Executes the command, but all errors are thrown. */