Skip to content

Commit

Permalink
Allow not configure logging without config
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jasontedor authored Aug 14, 2017
1 parent e968762 commit 447d92e
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,15 @@ static int main(final String[] args, final Elasticsearch elasticsearch, final Te
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) {
Expand Down
20 changes: 16 additions & 4 deletions core/src/main/java/org/elasticsearch/cli/Command.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,12 @@ public final int main(String[] args, Terminal terminal) throws Exception {
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);
Expand All @@ -100,6 +102,16 @@ public final int main(String[] args, Terminal terminal) throws Exception {
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.
*/
Expand Down

0 comments on commit 447d92e

Please sign in to comment.