honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #13933 from brwe/plugin-cli-logging 

plugin cli tool should not create empty log files
This commit is contained in:
Britta Weber 2015-10-06 14:13:59 +02:00
parent 07fe08f004 9492be65d4
commit d2ca694200
9 changed files with 131 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
View File

@ -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) {

14
core/src/main/java/org/elasticsearch/common/logging/log4j/LogConfigurator.java
View File

@ -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());

44
core/src/main/java/org/elasticsearch/common/logging/log4j/TerminalAppender.java Normal file
View File

@ -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;
}
}

2
core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java
View File

@ -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 <code>${prompt.text}</code> or <code>${prompt.secret}</code> 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}
*/

19
core/src/main/java/org/elasticsearch/plugins/PluginManagerCliParser.java
View File

@ -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);
}

2
core/src/test/java/org/elasticsearch/common/logging/log4j/Log4jESLoggerTests.java
View File

@ -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();

37
core/src/test/java/org/elasticsearch/common/logging/log4j/LoggingConfigurationTests.java
View File

@ -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;
}

23
qa/vagrant/src/test/resources/packaging/scripts/plugin_test_cases.bash
View File

@ -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
}

2
qa/vagrant/src/test/resources/packaging/scripts/plugins.bash
View File

@ -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