Move logging initialization to NodeToolCli

Andrey Ershov · Andrey Ershov · commit e23f59af291f · 2019-01-23T14:18:50.000+01:00
diff --git a/server/src/main/java/org/elasticsearch/cli/CommandLoggingConfigurator.java b/server/src/main/java/org/elasticsearch/cli/CommandLoggingConfigurator.java
@@ -27,13 +27,13 @@
  * Holder class for method to configure logging without Elasticsearch configuration files for use in CLI tools that will not read such
  * files.
  */
-final class CommandLoggingConfigurator {
+public final class CommandLoggingConfigurator {
 
     /**
      * Configures logging without Elasticsearch configuration files based on the system property "es.logger.level" only. As such, any
      * logging will be written to the console.
      */
-    static void configureLoggingWithoutConfig() {
+    public static void configureLoggingWithoutConfig() {
         // initialize default for es.logger.level because we will not read the log4j2.properties
         final String loggerLevel = System.getProperty("es.logger.level", Level.INFO.name());
         final Settings settings = Settings.builder().put("logger.level", loggerLevel).build();
diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/NodeToolCli.java b/server/src/main/java/org/elasticsearch/cluster/coordination/NodeToolCli.java
@@ -18,13 +18,22 @@
  */
 package org.elasticsearch.cluster.coordination;
 
-import org.elasticsearch.cli.LoggingAwareMultiCommand;
+import org.elasticsearch.cli.CommandLoggingConfigurator;
+import org.elasticsearch.cli.MultiCommand;
 import org.elasticsearch.cli.Terminal;
 
-public class NodeToolCli extends LoggingAwareMultiCommand {
+// NodeToolCli does not extend LoggingAwareCommand, because LoggingAwareCommand performs logging initialization
+// after LoggingAwareCommand instance is constructed.
+// It's too late for us, because before UnsafeBootstrapMasterCommand is added to the list of subcommands
+// log4j2 initialization will happen, because it has static reference to Logger class.
+// Even if we avoid making a static reference to Logger class, there is no nice way to avoid declaring
+// UNSAFE_BOOTSTRAP, which depends on ClusterService, which in turn has static Logger.
+// TODO execute CommandLoggingConfigurator.configureLoggingWithoutConfig() in the constructor of commands, not in beforeMain
+public class NodeToolCli extends MultiCommand {
 
     public NodeToolCli() {
-        super("A CLI tool to unsafely recover a cluster after the permanent loss of too many master-eligible nodes");
+        super("A CLI tool to unsafely recover a cluster after the permanent loss of too many master-eligible nodes", ()->{});
+        CommandLoggingConfigurator.configureLoggingWithoutConfig();
         subcommands.put("unsafe-bootstrap", new UnsafeBootstrapMasterCommand());
     }
 
diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapMasterCommand.java b/server/src/main/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapMasterCommand.java
@@ -46,6 +46,7 @@
 
 public class UnsafeBootstrapMasterCommand extends EnvironmentAwareCommand {
 
+    private static final Logger logger = LogManager.getLogger(UnsafeBootstrapMasterCommand.class);
     private final NamedXContentRegistry namedXContentRegistry;
 
     static final String WARNING_MSG =
@@ -81,9 +82,6 @@ public class UnsafeBootstrapMasterCommand extends EnvironmentAwareCommand {
 
     @Override
     protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception {
-        // We don't use classical "private static final Logger", because log4j2 initialization happens
-        // after UnsafeBootstrapMasterCommand instance creation.
-        final Logger logger = LogManager.getLogger(UnsafeBootstrapMasterCommand.class);
         showWarning(terminal);
         String text = terminal.readText("Confirm [y/N] ");
         if (text.equalsIgnoreCase("y") == false) {
