From 9ae5410ea641bdcc077bbd9f62004f9167bbc32c Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 16 Jan 2017 07:30:21 -0500 Subject: [PATCH] Do not configure a logger named level When logger.level is set, we end up configuring a logger named "level" because we look for all settings of the form "logger\..+" as configuring a logger. Yet, logger.level is special and is meant to only configure the default logging level. This commit causes is to avoid not configuring a logger named level. Relates #22624 --- .../common/logging/LogConfigurator.java | 17 +++++++--- .../logging/EvilLoggerConfigurationTests.java | 31 +++++++++++++++++++ .../common/logging/minimal/log4j2.properties | 7 +++++ 3 files changed, 50 insertions(+), 5 deletions(-) create mode 100644 qa/evil-tests/src/test/resources/org/elasticsearch/common/logging/minimal/log4j2.properties diff --git a/core/src/main/java/org/elasticsearch/common/logging/LogConfigurator.java b/core/src/main/java/org/elasticsearch/common/logging/LogConfigurator.java index 428e3ce7964..237755ce2f1 100644 --- a/core/src/main/java/org/elasticsearch/common/logging/LogConfigurator.java +++ b/core/src/main/java/org/elasticsearch/common/logging/LogConfigurator.java @@ -122,20 +122,27 @@ public class LogConfigurator { Configurator.initialize(builder.build()); } - private static void configureLoggerLevels(Settings settings) { + /** + * Configures the logging levels for loggers configured in the specified settings. + * + * @param settings the settings from which logger levels will be extracted + */ + private static void configureLoggerLevels(final Settings settings) { if (ESLoggerFactory.LOG_DEFAULT_LEVEL_SETTING.exists(settings)) { final Level level = ESLoggerFactory.LOG_DEFAULT_LEVEL_SETTING.get(settings); Loggers.setLevel(ESLoggerFactory.getRootLogger(), level); } final Map levels = settings.filter(ESLoggerFactory.LOG_LEVEL_SETTING::match).getAsMap(); - for (String key : levels.keySet()) { - final Level level = ESLoggerFactory.LOG_LEVEL_SETTING.getConcreteSetting(key).get(settings); - Loggers.setLevel(ESLoggerFactory.getLogger(key.substring("logger.".length())), level); + for (final String key : levels.keySet()) { + // do not set a log level for a logger named level (from the default log setting) + if (!key.equals(ESLoggerFactory.LOG_DEFAULT_LEVEL_SETTING.getKey())) { + final Level level = ESLoggerFactory.LOG_LEVEL_SETTING.getConcreteSetting(key).get(settings); + Loggers.setLevel(ESLoggerFactory.getLogger(key.substring("logger.".length())), level); + } } } - @SuppressForbidden(reason = "sets system property for logging configuration") private static void setLogConfigurationSystemProperty(final Path logsPath, final Settings settings) { System.setProperty("es.logs", logsPath.resolve(ClusterName.CLUSTER_NAME_SETTING.get(settings).value()).toString()); diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerConfigurationTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerConfigurationTests.java index 7ee2120c36f..9cd6ec630ad 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerConfigurationTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerConfigurationTests.java @@ -34,9 +34,11 @@ import org.elasticsearch.test.ESTestCase; import java.io.IOException; import java.nio.file.Path; +import java.util.Map; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasToString; import static org.hamcrest.Matchers.notNullValue; @@ -151,4 +153,33 @@ public class EvilLoggerConfigurationTests extends ESTestCase { assertThat(e, hasToString(containsString("no log4j2.properties found; tried"))); } + public void testLoggingLevelsFromSettings() throws IOException, UserException { + final Level rootLevel = randomFrom(Level.TRACE, Level.DEBUG, Level.INFO, Level.WARN, Level.ERROR); + final Level fooLevel = randomFrom(Level.TRACE, Level.DEBUG, Level.INFO, Level.WARN, Level.ERROR); + final Level barLevel = randomFrom(Level.TRACE, Level.DEBUG, Level.INFO, Level.WARN, Level.ERROR); + final Path configDir = getDataPath("minimal"); + final Settings settings = Settings.builder() + .put(Environment.PATH_CONF_SETTING.getKey(), configDir.toAbsolutePath()) + .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()) + .put("logger.level", rootLevel.name()) + .put("logger.foo", fooLevel.name()) + .put("logger.bar", barLevel.name()) + .build(); + final Environment environment = new Environment(settings); + LogConfigurator.configure(environment); + + final LoggerContext ctx = (LoggerContext) LogManager.getContext(false); + final Configuration config = ctx.getConfiguration(); + final Map loggerConfigs = config.getLoggers(); + assertThat(loggerConfigs.size(), equalTo(3)); + assertThat(loggerConfigs, hasKey("")); + assertThat(loggerConfigs.get("").getLevel(), equalTo(rootLevel)); + assertThat(loggerConfigs, hasKey("foo")); + assertThat(loggerConfigs.get("foo").getLevel(), equalTo(fooLevel)); + assertThat(loggerConfigs, hasKey("bar")); + assertThat(loggerConfigs.get("bar").getLevel(), equalTo(barLevel)); + + assertThat(ctx.getLogger(randomAsciiOfLength(16)).getLevel(), equalTo(rootLevel)); + } + } diff --git a/qa/evil-tests/src/test/resources/org/elasticsearch/common/logging/minimal/log4j2.properties b/qa/evil-tests/src/test/resources/org/elasticsearch/common/logging/minimal/log4j2.properties new file mode 100644 index 00000000000..f245dde979c --- /dev/null +++ b/qa/evil-tests/src/test/resources/org/elasticsearch/common/logging/minimal/log4j2.properties @@ -0,0 +1,7 @@ +appender.console.type = Console +appender.console.name = console +appender.console.layout.type = PatternLayout +appender.console.layout.pattern = %m%n + +rootLogger.level = info +rootLogger.appenderRef.console.ref = console