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

Detect using logging before configuration 

It can easily happen that we touch a logger before logging is configured
due to chains of static intializers and other such scenarios. This
commit adds detection for this mechanism that will fail startup if we
touch a logger before logging is configured. This is a bug that will
cause builds to fail.

Relates #24076
This commit is contained in:
Jason Tedor 2017-04-12 21:13:08 -04:00 committed by GitHub
parent 7e1903469e
commit a1c2fe9e3a
4 changed files with 65 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java
View File

@ -24,11 +24,11 @@ import joptsimple.OptionSpec;
import joptsimple.OptionSpecBuilder;
import joptsimple.util.PathConverter;
import org.elasticsearch.Build;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.EnvironmentAwareCommand;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.logging.LogConfigurator;
import org.elasticsearch.env.Environment;
import org.elasticsearch.monitor.jvm.JvmInfo;
import org.elasticsearch.node.NodeValidationException;
@ -37,7 +37,6 @@ import java.io.IOException;
import java.nio.file.Path;
import java.security.Permission;
import java.util.Arrays;
import java.util.Map;
/**
* This class starts elasticsearch.
@ -80,6 +79,7 @@ class Elasticsearch extends EnvironmentAwareCommand {
// grant all permissions so that we can later set the security manager to the one that we want
}
});
LogConfigurator.registerErrorListener();
final Elasticsearch elasticsearch = new Elasticsearch();
int status = main(args, elasticsearch, Terminal.DEFAULT);
if (status != ExitCodes.OK) {

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

@ -30,6 +30,10 @@ 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.apache.logging.log4j.status.StatusConsoleListener;
import org.apache.logging.log4j.status.StatusData;
import org.apache.logging.log4j.status.StatusListener;
import org.apache.logging.log4j.status.StatusLogger;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.cluster.ClusterName;
@ -51,9 +55,35 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.StreamSupport;
public class LogConfigurator {
/*
* We want to detect situations where we touch logging before the configuration is loaded. If we do this, Log4j will status log an error
* message at the error level. With this error listener, we can capture if this happens. More broadly, we can detect any error-level
* status log message which likely indicates that something is broken. The listener is installed immediately on startup, and then when
* we get around to configuring logging we check that no error-level log messages have been logged by the status logger. If they have we
* fail startup and any such messages can be seen on the console.
*/
private static final AtomicBoolean error = new AtomicBoolean();
private static final StatusListener ERROR_LISTENER = new StatusConsoleListener(Level.ERROR) {
@Override
public void log(StatusData data) {
error.set(true);
super.log(data);
}
};
/**
* Registers a listener for status logger errors. This listener should be registered as early as possible to ensure that no errors are
* logged by the status logger before logging is configured.
*/
public static void registerErrorListener() {
StatusLogger.getLogger().registerListener(ERROR_LISTENER);
}
/**
* Configure logging without reading a log4j2.properties file, effectively configuring the
* status logger and all loggers to the console.
@ -79,9 +109,27 @@ public class LogConfigurator {
*/
public static void configure(final Environment environment) throws IOException, UserException {
Objects.requireNonNull(environment);
try {
// we are about to configure logging, check that the status logger did not log any error-level messages
checkErrorListener();
} finally {
// whether or not the error listener check failed we can remove the listener now
StatusLogger.getLogger().removeListener(ERROR_LISTENER);
}
configure(environment.settings(), environment.configFile(), environment.logsFile());
}
private static void checkErrorListener() {
assert errorListenerIsRegistered() : "expected error listener to be registered";
if (error.get()) {
throw new IllegalStateException("status logger logged an error before logging was configured");
}
}
private static boolean errorListenerIsRegistered() {
return StreamSupport.stream(StatusLogger.getLogger().getListeners().spliterator(), false).anyMatch(l -> l == ERROR_LISTENER);
}
private static void configure(final Settings settings, final Path configsPath, final Path logsPath) throws IOException, UserException {
Objects.requireNonNull(settings);
Objects.requireNonNull(configsPath);

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

@ -19,6 +19,14 @@
package org.elasticsearch.node;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.SettingsException;
import org.elasticsearch.env.Environment;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
@ -32,14 +40,6 @@ import java.util.function.Function;
import java.util.function.Predicate;
import java.util.function.UnaryOperator;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.SettingsException;
import org.elasticsearch.env.Environment;
import static org.elasticsearch.common.Strings.cleanPath;
public class InternalSettingsPreparer {

6
qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerConfigurationTests.java
View File

@ -44,6 +44,12 @@ import static org.hamcrest.Matchers.notNullValue;
public class EvilLoggerConfigurationTests extends ESTestCase {
@Override
public void setUp() throws Exception {
super.setUp();
LogConfigurator.registerErrorListener();
}
@Override
public void tearDown() throws Exception {
LoggerContext context = (LoggerContext) LogManager.getContext(false);