diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index 5b8c14aa3fb..542444b4097 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -195,7 +195,7 @@ final class Bootstrap { private static void setupLogging(Settings settings, Environment environment) { try { Class.forName("org.apache.log4j.Logger"); - LogConfigurator.configure(settings); + LogConfigurator.configure(settings, true); } catch (ClassNotFoundException e) { // no log4j } catch (NoClassDefFoundError e) { diff --git a/core/src/main/java/org/elasticsearch/common/logging/log4j/LogConfigurator.java b/core/src/main/java/org/elasticsearch/common/logging/log4j/LogConfigurator.java index 8f92864ea0f..a715d3434e3 100644 --- a/core/src/main/java/org/elasticsearch/common/logging/log4j/LogConfigurator.java +++ b/core/src/main/java/org/elasticsearch/common/logging/log4j/LogConfigurator.java @@ -70,6 +70,7 @@ public class LogConfigurator { .put("socketHub", "org.apache.log4j.net.SocketHubAppender") .put("syslog", "org.apache.log4j.net.SyslogAppender") .put("telnet", "org.apache.log4j.net.TelnetAppender") + .put("terminal", "org.elasticsearch.common.logging.log4j.TerminalAppender") // policies .put("timeBased", "org.apache.log4j.rolling.TimeBasedRollingPolicy") .put("sizeBased", "org.apache.log4j.rolling.SizeBasedTriggeringPolicy") @@ -83,15 +84,24 @@ public class LogConfigurator { .put("xml", "org.apache.log4j.XMLLayout") .immutableMap(); - public static void configure(Settings settings) { + /** + * Consolidates settings and converts them into actual log4j settings, then initializes loggers and appenders. + * + * @param settings custom settings that should be applied + * @param resolveConfig controls whether the logging conf file should be read too or not. + */ + public static void configure(Settings settings, boolean resolveConfig) { if (loaded) { return; } loaded = true; // TODO: this is partly a copy of InternalSettingsPreparer...we should pass in Environment and not do all this... Environment environment = new Environment(settings); + Settings.Builder settingsBuilder = settingsBuilder(); - resolveConfig(environment, settingsBuilder); + if (resolveConfig) { + resolveConfig(environment, settingsBuilder); + } settingsBuilder .putProperties("elasticsearch.", System.getProperties()) .putProperties("es.", System.getProperties()); diff --git a/core/src/main/java/org/elasticsearch/common/logging/log4j/TerminalAppender.java b/core/src/main/java/org/elasticsearch/common/logging/log4j/TerminalAppender.java new file mode 100644 index 00000000000..3c60c44d3e3 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/common/logging/log4j/TerminalAppender.java @@ -0,0 +1,44 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + + +package org.elasticsearch.common.logging.log4j; + +import org.apache.log4j.AppenderSkeleton; +import org.apache.log4j.spi.LoggingEvent; +import org.elasticsearch.common.cli.Terminal; + +/** + * TerminalAppender logs event to Terminal.DEFAULT. It is used for example by the PluginManagerCliParser. + * */ +public class TerminalAppender extends AppenderSkeleton { + @Override + protected void append(LoggingEvent event) { + Terminal.DEFAULT.println(event.getRenderedMessage()); + } + + @Override + public void close() { + } + + @Override + public boolean requiresLayout() { + return false; + } +} diff --git a/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java b/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java index bd15a6ae1b9..7aacde5283f 100644 --- a/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java +++ b/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java @@ -73,7 +73,7 @@ public class InternalSettingsPreparer { * and then replacing all property placeholders. If a {@link Terminal} is provided and configuration settings are loaded, * settings with a value of ${prompt.text} or ${prompt.secret} will result in a prompt for * the setting to the user. - * @param input The initial settings to use + * @param input The custom settings to use. These are not overwritten by settings in the configuration file. * @param terminal the Terminal to use for input/output * @return the {@link Settings} and {@link Environment} as a {@link Tuple} */ diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginManagerCliParser.java b/core/src/main/java/org/elasticsearch/plugins/PluginManagerCliParser.java index db6afd17647..5d83fa2656f 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginManagerCliParser.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginManagerCliParser.java @@ -24,7 +24,6 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.cli.CliTool; import org.elasticsearch.common.cli.CliToolConfig; import org.elasticsearch.common.cli.Terminal; -import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.logging.log4j.LogConfigurator; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; @@ -39,7 +38,6 @@ import java.util.Locale; import static org.elasticsearch.common.cli.CliToolConfig.Builder.cmd; import static org.elasticsearch.common.cli.CliToolConfig.Builder.option; -import static org.elasticsearch.common.settings.Settings.EMPTY; public class PluginManagerCliParser extends CliTool { @@ -51,8 +49,21 @@ public class PluginManagerCliParser extends CliTool { .build(); public static void main(String[] args) { - Environment env = InternalSettingsPreparer.prepareEnvironment(EMPTY, Terminal.DEFAULT); - LogConfigurator.configure(env.settings()); + // initialize default for es.logger.level because we will not read the logging.yml + String loggerLevel = System.getProperty("es.logger.level", "INFO"); + // Set the appender for all potential log files to terminal so that other components that use the logger print out the + // same terminal. + // The reason for this is that the plugin cli cannot be configured with a file appender because when the plugin command is + // executed there is no way of knowing where the logfiles should be placed. For example, if elasticsearch + // is run as service then the logs should be at /var/log/elasticsearch but when started from the tar they should be at es.home/logs. + // Therefore we print to Terminal. + Environment env = InternalSettingsPreparer.prepareEnvironment(Settings.builder() + .put("appender.terminal.type", "terminal") + .put("rootLogger", "${es.logger.level}, terminal") + .put("es.logger.level", loggerLevel) + .build(), Terminal.DEFAULT); + // configure but do not read the logging conf file + LogConfigurator.configure(env.settings(), false); int status = new PluginManagerCliParser().execute(args).status(); System.exit(status); } diff --git a/core/src/test/java/org/elasticsearch/common/logging/log4j/Log4jESLoggerTests.java b/core/src/test/java/org/elasticsearch/common/logging/log4j/Log4jESLoggerTests.java index c1bdf00bd3c..d0cd3879dbc 100644 --- a/core/src/test/java/org/elasticsearch/common/logging/log4j/Log4jESLoggerTests.java +++ b/core/src/test/java/org/elasticsearch/common/logging/log4j/Log4jESLoggerTests.java @@ -57,7 +57,7 @@ public class Log4jESLoggerTests extends ESTestCase { .put("path.conf", configDir.toAbsolutePath()) .put("path.home", createTempDir().toString()) .build(); - LogConfigurator.configure(settings); + LogConfigurator.configure(settings, true); esTestLogger = Log4jESLoggerFactory.getLogger("test"); Logger testLogger = ((Log4jESLogger) esTestLogger).logger(); diff --git a/core/src/test/java/org/elasticsearch/common/logging/log4j/LoggingConfigurationTests.java b/core/src/test/java/org/elasticsearch/common/logging/log4j/LoggingConfigurationTests.java index 199f94c3151..2b84bec93e9 100644 --- a/core/src/test/java/org/elasticsearch/common/logging/log4j/LoggingConfigurationTests.java +++ b/core/src/test/java/org/elasticsearch/common/logging/log4j/LoggingConfigurationTests.java @@ -32,8 +32,10 @@ import org.junit.Test; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.OpenOption; import java.nio.file.Path; import java.nio.file.StandardOpenOption; +import java.util.Arrays; import static org.hamcrest.Matchers.*; @@ -56,7 +58,7 @@ public class LoggingConfigurationTests extends ESTestCase { .put("path.conf", configDir.toAbsolutePath()) .put("path.home", createTempDir().toString()) .build(); - LogConfigurator.configure(settings); + LogConfigurator.configure(settings, true); ESLogger esLogger = Log4jESLoggerFactory.getLogger("test"); Logger logger = ((Log4jESLogger) esLogger).logger(); @@ -157,20 +159,20 @@ public class LoggingConfigurationTests extends ESTestCase { public void testResolveOrder() throws Exception { Path tmpDir = createTempDir(); Path loggingConf = tmpDir.resolve(loggingConfiguration("yaml")); - Files.write(loggingConf, "logger.test: INFO, file\n".getBytes(StandardCharsets.UTF_8)); + Files.write(loggingConf, "logger.test_resolve_order: INFO, file\n".getBytes(StandardCharsets.UTF_8)); Files.write(loggingConf, "appender.file.type: file\n".getBytes(StandardCharsets.UTF_8), StandardOpenOption.APPEND); Environment environment = InternalSettingsPreparer.prepareEnvironment( Settings.builder() .put("path.conf", tmpDir.toAbsolutePath()) .put("path.home", createTempDir().toString()) - .put("logger.test", "TRACE, console") + .put("logger.test_resolve_order", "TRACE, console") .put("appender.console.type", "console") .put("appender.console.layout.type", "consolePattern") .put("appender.console.layout.conversionPattern", "[%d{ISO8601}][%-5p][%-25c] %m%n") .build(), new CliToolTestCase.MockTerminal()); - LogConfigurator.configure(environment.settings()); + LogConfigurator.configure(environment.settings(), true); // args should overwrite whatever is in the config - ESLogger esLogger = Log4jESLoggerFactory.getLogger("test"); + ESLogger esLogger = Log4jESLoggerFactory.getLogger("test_resolve_order"); Logger logger = ((Log4jESLogger) esLogger).logger(); Appender appender = logger.getAppender("console"); assertThat(appender, notNullValue()); @@ -179,6 +181,31 @@ public class LoggingConfigurationTests extends ESTestCase { assertThat(appender, nullValue()); } + // tests that config file is not read when we call LogConfigurator.configure(Settings, false) + @Test + public void testConfigNotRead() throws Exception { + Path tmpDir = createTempDir(); + Path loggingConf = tmpDir.resolve(loggingConfiguration("yaml")); + Files.write(loggingConf, + Arrays.asList( + "logger.test_config_not_read: INFO, console", + "appender.console.type: console"), + StandardCharsets.UTF_8); + Environment environment = InternalSettingsPreparer.prepareEnvironment( + Settings.builder() + .put("path.conf", tmpDir.toAbsolutePath()) + .put("path.home", createTempDir().toString()) + .build(), new CliToolTestCase.MockTerminal()); + LogConfigurator.configure(environment.settings(), false); + ESLogger esLogger = Log4jESLoggerFactory.getLogger("test_config_not_read"); + + assertNotNull(esLogger); + Logger logger = ((Log4jESLogger) esLogger).logger(); + Appender appender = logger.getAppender("console"); + // config was not read + assertNull(appender); + } + private static String loggingConfiguration(String suffix) { return "logging." + randomAsciiOfLength(randomIntBetween(0, 10)) + "." + suffix; } diff --git a/qa/vagrant/src/test/resources/packaging/scripts/plugin_test_cases.bash b/qa/vagrant/src/test/resources/packaging/scripts/plugin_test_cases.bash index ec2f177d3ca..a199cb989ab 100644 --- a/qa/vagrant/src/test/resources/packaging/scripts/plugin_test_cases.bash +++ b/qa/vagrant/src/test/resources/packaging/scripts/plugin_test_cases.bash @@ -352,3 +352,26 @@ fi @test "[$GROUP] stop elasticsearch" { stop_elasticsearch_service } + +@test "[$GROUP] install jvm-example with different logging modes and check output" { + local relativePath=${1:-$(readlink -m jvm-example-*.zip)} + sudo -E -u $ESPLUGIN_COMMAND_USER "$ESHOME/bin/plugin" install "file://$relativePath" > /tmp/plugin-cli-output + local loglines=$(cat /tmp/plugin-cli-output | wc -l) + [ "$loglines" = "6" ] || { + echo "Expected 6 lines but the output was:" + cat /tmp/plugin-cli-output + false + } + remove_jvm_example + + local relativePath=${1:-$(readlink -m jvm-example-*.zip)} + sudo -E -u $ESPLUGIN_COMMAND_USER "$ESHOME/bin/plugin" install "file://$relativePath" -Des.logger.level=DEBUG > /tmp/plugin-cli-output + local loglines=$(cat /tmp/plugin-cli-output | wc -l) + [ "$loglines" -gt "6" ] || { + echo "Expected more than 6 lines but the output was:" + cat /tmp/plugin-cli-output + false + } + remove_jvm_example +} + diff --git a/qa/vagrant/src/test/resources/packaging/scripts/plugins.bash b/qa/vagrant/src/test/resources/packaging/scripts/plugins.bash index 2af60c8a5bd..31e302c5e95 100644 --- a/qa/vagrant/src/test/resources/packaging/scripts/plugins.bash +++ b/qa/vagrant/src/test/resources/packaging/scripts/plugins.bash @@ -36,6 +36,8 @@ install_plugin() { assert_file_exist "$ESPLUGINS/$name" assert_file_exist "$ESPLUGINS/$name/plugin-descriptor.properties" + #check we did not accidentially create a log file as root as /usr/share/elasticsearch + assert_file_not_exist "/usr/share/elasticsearch/logs" # At some point installing or removing plugins caused elasticsearch's logs # to be owned by root. This is bad so we want to make sure it doesn't