From 40f889b825dbae9638a580e00f798923de03648c Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 2 Sep 2016 18:36:57 -0400 Subject: [PATCH] Warn if unsupported logging configuration present This commit adds a warning that an unsupported logging configuration is present and points users to the new logging configuration file. Relates #20309 --- .../common/logging/LogConfigurator.java | 32 ++++++++++++++++-- .../common/logging/EvilLoggerTests.java | 33 ++++++++++++++----- .../common/logging/config/logging.yml | 2 ++ 3 files changed, 56 insertions(+), 11 deletions(-) create mode 100644 qa/evil-tests/src/test/resources/org/elasticsearch/common/logging/config/logging.yml 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 3b7ec0deb34..522ff589758 100644 --- a/core/src/main/java/org/elasticsearch/common/logging/LogConfigurator.java +++ b/core/src/main/java/org/elasticsearch/common/logging/LogConfigurator.java @@ -30,6 +30,7 @@ import org.apache.logging.log4j.core.config.builder.impl.BuiltConfiguration; import org.apache.logging.log4j.core.config.composite.CompositeConfiguration; import org.apache.logging.log4j.core.config.properties.PropertiesConfiguration; import org.apache.logging.log4j.core.config.properties.PropertiesConfigurationFactory; +import org.elasticsearch.Version; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.settings.Settings; @@ -43,6 +44,7 @@ import java.nio.file.Path; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; import java.util.ArrayList; +import java.util.Arrays; import java.util.EnumSet; import java.util.List; import java.util.Map; @@ -63,9 +65,9 @@ public class LogConfigurator { final LoggerContext context = (LoggerContext) LogManager.getContext(false); if (resolveConfig) { - final Set options = EnumSet.of(FileVisitOption.FOLLOW_LINKS); final List configurations = new ArrayList<>(); final PropertiesConfigurationFactory factory = new PropertiesConfigurationFactory(); + final Set options = EnumSet.of(FileVisitOption.FOLLOW_LINKS); Files.walkFileTree(environment.configFile(), options, Integer.MAX_VALUE, new SimpleFileVisitor() { @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { @@ -87,10 +89,36 @@ public class LogConfigurator { final Level level = ESLoggerFactory.LOG_LEVEL_SETTING.getConcreteSetting(key).get(settings); Loggers.setLevel(Loggers.getLogger(key.substring("logger.".length())), level); } + + warnIfOldConfigurationFilePresent(environment); + } + + private static void warnIfOldConfigurationFilePresent(final Environment environment) throws IOException { + // TODO: the warning for unsupported logging configurations can be removed in 6.0.0 + assert Version.CURRENT.major < 6; + final List suffixes = Arrays.asList(".yml", ".yaml", ".json", ".properties"); + final Set options = EnumSet.of(FileVisitOption.FOLLOW_LINKS); + Files.walkFileTree(environment.configFile(), options, Integer.MAX_VALUE, new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + final String fileName = file.getFileName().toString(); + if (fileName.startsWith("logging")) { + for (final String suffix : suffixes) { + if (fileName.endsWith(suffix)) { + Loggers.getLogger(LogConfigurator.class).warn( + "ignoring unsupported logging configuration file [{}], logging is configured via [{}]", + file.toString(), + file.getParent().resolve("log4j2.properties")); + } + } + } + return FileVisitResult.CONTINUE; + } + }); } @SuppressForbidden(reason = "sets system property for logging configuration") - private static void setLogConfigurationSystemProperty(Environment environment, Settings settings) { + private static void setLogConfigurationSystemProperty(final Environment environment, final Settings settings) { System.setProperty("es.logs", environment.logsFile().resolve(ClusterName.CLUSTER_NAME_SETTING.get(settings).value()).toString()); } diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java index a14ee2282c4..168453df763 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java @@ -21,15 +21,16 @@ package org.elasticsearch.common.logging; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Logger; +import org.elasticsearch.Version; import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.hamcrest.RegexMatcher; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -66,21 +67,22 @@ public class EvilLoggerTests extends ESTestCase { testLogger.trace("This is a trace message"); final String path = System.getProperty("es.logs") + ".log"; final List events = Files.readAllLines(PathUtils.get(path)); - assertThat(events.size(), equalTo(5)); + assertThat(events.size(), equalTo(6)); // the five messages me log plus a warning for unsupported configuration files final String location = "org.elasticsearch.common.logging.EvilLoggerTests.testLocationInfoTest"; - assertLogLine(events.get(0), Level.ERROR, location, "This is an error message"); - assertLogLine(events.get(1), Level.WARN, location, "This is a warning message"); - assertLogLine(events.get(2), Level.INFO, location, "This is an info message"); - assertLogLine(events.get(3), Level.DEBUG, location, "This is a debug message"); - assertLogLine(events.get(4), Level.TRACE, location, "This is a trace message"); + // the first message is a warning for unsupported configuration files + assertLogLine(events.get(1), Level.ERROR, location, "This is an error message"); + assertLogLine(events.get(2), Level.WARN, location, "This is a warning message"); + assertLogLine(events.get(3), Level.INFO, location, "This is an info message"); + assertLogLine(events.get(4), Level.DEBUG, location, "This is a debug message"); + assertLogLine(events.get(5), Level.TRACE, location, "This is a trace message"); } private void assertLogLine(final String logLine, final Level level, final String location, final String message) { final Matcher matcher = Pattern.compile("\\[(.*)\\]\\[(.*)\\(.*\\)\\] (.*)").matcher(logLine); assertTrue(logLine, matcher.matches()); assertThat(matcher.group(1), equalTo(level.toString())); - assertThat(matcher.group(2), equalTo(location)); - assertThat(matcher.group(3), equalTo(message)); + assertThat(matcher.group(2), RegexMatcher.matches(location)); + assertThat(matcher.group(3), RegexMatcher.matches(message)); } public void testDeprecationLogger() throws IOException { @@ -95,4 +97,17 @@ public class EvilLoggerTests extends ESTestCase { "This is a deprecation message"); } + public void testUnsupportedLoggingConfigurationFiles() throws IOException { + // TODO: the warning for unsupported logging configurations can be removed in 6.0.0 + assert Version.CURRENT.major < 6; + final String path = System.getProperty("es.logs") + ".log"; + final List events = Files.readAllLines(PathUtils.get(path)); + assertThat(events.size(), equalTo(1)); + assertLogLine( + events.get(0), + Level.WARN, + "org\\.elasticsearch\\.common\\.logging\\.LogConfigurator.*", + "ignoring unsupported logging configuration file \\[.*\\], logging is configured via \\[.*\\]"); + } + } diff --git a/qa/evil-tests/src/test/resources/org/elasticsearch/common/logging/config/logging.yml b/qa/evil-tests/src/test/resources/org/elasticsearch/common/logging/config/logging.yml new file mode 100644 index 00000000000..5aa98f454f3 --- /dev/null +++ b/qa/evil-tests/src/test/resources/org/elasticsearch/common/logging/config/logging.yml @@ -0,0 +1,2 @@ +logger.level: INFO +rootLogger: ${logger.level}, terminal