Consolidating logging initialization in cli launcher

distribution/tools/cli-launcher/src/main/java/org/elasticsearch/launcher/CliToolLauncher.java

@@ -8,9 +8,12 @@
 
 package org.elasticsearch.launcher;
 
+import org.apache.logging.log4j.Level;
 import org.elasticsearch.cli.CliToolProvider;
 import org.elasticsearch.cli.Command;
 import org.elasticsearch.cli.Terminal;
+import org.elasticsearch.common.logging.LogConfigurator;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.core.SuppressForbidden;
 
 import java.util.Map;
@@ -43,6 +46,10 @@ class CliToolLauncher {
      */
     public static void main(String[] args) throws Exception {
         Map<String, String> sysprops = getSystemProperties();
+
+        // configure logging as early as possible
+        configureLoggingWithoutConfig(sysprops);
+
         String toolname = getToolName(sysprops);
         String libs = sysprops.getOrDefault("cli.libs", "");
 
@@ -76,4 +83,15 @@ private static Map<String, String> getSystemProperties() {
     private static void exit(int status) {
         System.exit(status);
     }
+
+    /**
+     * 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.
+     */
+    private static void configureLoggingWithoutConfig(Map<String, String> sysprops) {
+        // initialize default for es.logger.level because we will not read the log4j2.properties
+        final String loggerLevel = sysprops.getOrDefault("es.logger.level", Level.INFO.name());
+        final Settings settings = Settings.builder().put("logger.level", loggerLevel).build();
+        LogConfigurator.configureWithoutConfig(settings);
+    }
 }

distribution/tools/geoip-cli/src/main/java/org/elasticsearch/geoip/GeoIpCli.java

@@ -47,7 +47,7 @@ public class GeoIpCli extends Command {
     private final OptionSpec<String> targetDirectory;
 
     public GeoIpCli() {
-        super("A CLI tool to prepare local GeoIp database service", () -> {});
+        super("A CLI tool to prepare local GeoIp database service");
         sourceDirectory = parser.acceptsAll(Arrays.asList("s", "source"), "Source directory").withRequiredArg().required();
         targetDirectory = parser.acceptsAll(Arrays.asList("t", "target"), "Target directory").withRequiredArg();
 

distribution/tools/keystore-cli/src/main/java/org/elasticsearch/cli/keystore/KeyStoreCli.java

@@ -8,12 +8,12 @@
 
 package org.elasticsearch.cli.keystore;
 
-import org.elasticsearch.common.cli.LoggingAwareMultiCommand;
+import org.elasticsearch.cli.MultiCommand;
 
 /**
  * A cli tool for managing secrets in the elasticsearch keystore.
  */
-class KeyStoreCli extends LoggingAwareMultiCommand {
+class KeyStoreCli extends MultiCommand {
 
     KeyStoreCli() {
         super("A tool for managing settings stored in the elasticsearch keystore");

distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/cli/PluginCli.java

@@ -9,7 +9,7 @@
 package org.elasticsearch.plugins.cli;
 
 import org.elasticsearch.cli.Command;
-import org.elasticsearch.common.cli.LoggingAwareMultiCommand;
+import org.elasticsearch.cli.MultiCommand;
 import org.elasticsearch.core.internal.io.IOUtils;
 
 import java.io.IOException;
@@ -19,7 +19,7 @@
 /**
  * A cli tool for adding, removing and listing plugins for elasticsearch.
  */
-class PluginCli extends LoggingAwareMultiCommand {
+class PluginCli extends MultiCommand {
 
     private final Collection<Command> commands;
 

libs/cli/src/main/java/org/elasticsearch/cli/Command.java

@@ -33,8 +33,6 @@ public abstract class Command implements Closeable {
     /** A description of the command, used in the help output. */
     protected final String description;
 
-    private final Runnable beforeMain;
-
     // these are the system properties and env vars from the environment,
     // but they can be overriden by tests. Really though Command should be stateless,
     // so the signature of main should take them in, which can happen once the entrypoint
@@ -52,13 +50,11 @@ public abstract class Command implements Closeable {
 
     /**
      * Construct the command with the specified command description and runnable to execute before main is invoked.
+     *  @param description the command description
      *
-     * @param description the command description
-     * @param beforeMain the before-main runnable
      */
-    public Command(final String description, final Runnable beforeMain) {
+    public Command(final String description) {
         this.description = description;
-        this.beforeMain = beforeMain;
         this.sysprops = captureSystemProperties();
         this.envVars = captureEnvironmentVariables();
     }
@@ -86,8 +82,6 @@ public final int main(String[] args, Terminal terminal) throws Exception {
             Runtime.getRuntime().addShutdownHook(shutdownHookThread);
         }
 
-        beforeMain.run();
-
         try {
             mainWithoutErrorHandling(args, terminal);
         } catch (OptionException e) {

libs/cli/src/main/java/org/elasticsearch/cli/MultiCommand.java

@@ -34,12 +34,11 @@ public class MultiCommand extends Command {
 
     /**
      * Construct the multi-command with the specified command description and runnable to execute before main is invoked.
+     *  @param description the multi-command description
      *
-     * @param description the multi-command description
-     * @param beforeMain the before-main runnable
      */
-    public MultiCommand(final String description, final Runnable beforeMain) {
-        super(description, beforeMain);
+    public MultiCommand(final String description) {
+        super(description);
         this.settingOption = parser.accepts("E", "Configure a setting").withRequiredArg().ofType(KeyValuePair.class);
         parser.posixlyCorrect(true);
     }

qa/evil-tests/src/test/java/org/elasticsearch/cli/EvilCommandTests.java

@@ -24,7 +24,7 @@ public class EvilCommandTests extends ESTestCase {
     public void testCommandShutdownHook() throws Exception {
         final AtomicBoolean closed = new AtomicBoolean();
         final boolean shouldThrow = randomBoolean();
-        final Command command = new Command("test-command-shutdown-hook", () -> {}) {
+        final Command command = new Command("test-command-shutdown-hook") {
             @Override
             protected void execute(Terminal terminal, OptionSet options) throws Exception {
 

server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java

@@ -42,7 +42,7 @@ class Elasticsearch extends EnvironmentAwareCommand {
 
     // visible for testing
     Elasticsearch() {
-        super("Starts Elasticsearch", () -> {}); // we configure logging later so we override the base class from configuring logging
+        super("Starts Elasticsearch"); // we configure logging later so we override the base class from configuring logging
         versionOption = parser.acceptsAll(Arrays.asList("V", "version"), "Prints Elasticsearch version information and exits");
         daemonizeOption = parser.acceptsAll(Arrays.asList("d", "daemonize"), "Starts Elasticsearch in the background")
             .availableUnless(versionOption);

server/src/main/java/org/elasticsearch/cluster/coordination/NodeToolCli.java

@@ -8,22 +8,13 @@
 package org.elasticsearch.cluster.coordination;
 
 import org.elasticsearch.cli.MultiCommand;
-import org.elasticsearch.common.cli.CommandLoggingConfigurator;
 import org.elasticsearch.env.NodeRepurposeCommand;
 import org.elasticsearch.env.OverrideNodeVersionCommand;
 
-// 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
 class NodeToolCli extends MultiCommand {
 
     NodeToolCli() {
-        super("A CLI tool to do unsafe cluster and index manipulations on current node", () -> {});
-        CommandLoggingConfigurator.configureLoggingWithoutConfig();
+        super("A CLI tool to do unsafe cluster and index manipulations on current node");
         subcommands.put("repurpose", new NodeRepurposeCommand());
         subcommands.put("unsafe-bootstrap", new UnsafeBootstrapMasterCommand());
         subcommands.put("detach-cluster", new DetachClusterCommand());

server/src/main/java/org/elasticsearch/common/cli/EnvironmentAwareCommand.java

@@ -39,18 +39,7 @@ public abstract class EnvironmentAwareCommand extends Command {
      * @param description the command description
      */
     public EnvironmentAwareCommand(final String description) {
-        this(description, CommandLoggingConfigurator::configureLoggingWithoutConfig);
-    }
-
-    /**
-     * Construct the command with the specified command description and runnable to execute before main is invoked. Commands constructed
-     * with this constructor must take ownership of configuring logging.
-     *
-     * @param description the command description
-     * @param beforeMain the before-main runnable
-     */
-    public EnvironmentAwareCommand(final String description, final Runnable beforeMain) {
-        super(description, beforeMain);
+        super(description);
         this.settingOption = parser.accepts("E", "Configure a setting").withRequiredArg().ofType(KeyValuePair.class);
     }
 

server/src/main/java/org/elasticsearch/index/shard/ShardToolCli.java

@@ -7,12 +7,12 @@
  */
 package org.elasticsearch.index.shard;
 
-import org.elasticsearch.common.cli.LoggingAwareMultiCommand;
+import org.elasticsearch.cli.MultiCommand;
 
 /**
  * Class encapsulating and dispatching commands from the {@code elasticsearch-shard} command line tool
  */
-class ShardToolCli extends LoggingAwareMultiCommand {
+class ShardToolCli extends MultiCommand {
 
     ShardToolCli() {
         super("A CLI tool to remove corrupted parts of unrecoverable shards");

server/src/test/java/org/elasticsearch/cli/CommandTests.java

@@ -34,7 +34,7 @@ static class DummyCommand extends Command {
         Exception exception = null;
 
         DummyCommand() {
-            super("Does nothing", () -> {});
+            super("Does nothing");
         }
 
         @Override

server/src/test/java/org/elasticsearch/cli/MultiCommandTests.java

@@ -30,7 +30,7 @@ static class DummyMultiCommand extends MultiCommand {
         final AtomicBoolean closed = new AtomicBoolean();
 
         DummyMultiCommand() {
-            super("A dummy multi command", () -> {});
+            super("A dummy multi command");
         }
 
         @Override
@@ -56,7 +56,7 @@ static class DummySubCommand extends Command {
         }
 
         DummySubCommand(final boolean throwsExceptionOnClose) {
-            super("A dummy subcommand", () -> {});
+            super("A dummy subcommand");
             this.throwsExceptionOnClose = throwsExceptionOnClose;
         }
 
@@ -203,7 +203,7 @@ public void testCloseWhenSubCommandCloseThrowsException() throws Exception {
 
     static class ErrorThrowingSubCommand extends Command {
         ErrorThrowingSubCommand() {
-            super("error throwing", () -> {});
+            super("error throwing");
         }
 
         @Override

x-pack/license-tools/src/main/java/org/elasticsearch/license/licensor/tools/KeyPairGeneratorTool.java

@@ -9,10 +9,10 @@
 import joptsimple.OptionSet;
 import joptsimple.OptionSpec;
 
+import org.elasticsearch.cli.Command;
 import org.elasticsearch.cli.ExitCodes;
 import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.cli.UserException;
-import org.elasticsearch.common.cli.LoggingAwareCommand;
 import org.elasticsearch.core.PathUtils;
 import org.elasticsearch.core.SuppressForbidden;
 
@@ -24,7 +24,7 @@
 
 import static org.elasticsearch.license.CryptUtils.writeEncryptedPrivateKey;
 
-public class KeyPairGeneratorTool extends LoggingAwareCommand {
+public class KeyPairGeneratorTool extends Command {
 
     private final OptionSpec<String> publicKeyPathOption;
     private final OptionSpec<String> privateKeyPathOption;

x-pack/license-tools/src/main/java/org/elasticsearch/license/licensor/tools/LicenseGeneratorTool.java

@@ -9,12 +9,12 @@
 import joptsimple.OptionSet;
 import joptsimple.OptionSpec;
 
+import org.elasticsearch.cli.Command;
 import org.elasticsearch.cli.ExitCodes;
 import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.cli.UserException;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesArray;
-import org.elasticsearch.common.cli.LoggingAwareCommand;
 import org.elasticsearch.core.PathUtils;
 import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.license.License;
@@ -28,7 +28,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 
-public class LicenseGeneratorTool extends LoggingAwareCommand {
+public class LicenseGeneratorTool extends Command {
 
     private final OptionSpec<String> publicKeyPathOption;
     private final OptionSpec<String> privateKeyPathOption;