Logging: Configure the node name when we have it (#32983)

Change the logging infrastructure to handle when the node name isn't
available in `elasticsearch.yml`. In that case the node name is not
available until long after logging is configured. The biggest change is
that the node name logging no longer fixed at pattern build time.
Instead it is read from a `SetOnce` on every print. If it is unset it is
printed as `unknown` so we have something that fits in the pattern.
On normal startup we don't log anything until the node name is available
so we never see the `unknown`s.

Author: nik9000

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy

@@ -337,7 +337,13 @@ class ClusterFormationTasks {
         if (node.nodeVersion.major >= 7) {
             esConfig['indices.breaker.total.use_real_memory'] = false
         }
-        esConfig.putAll(node.config.settings)
+        for (Map.Entry<String, Object> setting : node.config.settings) {
+            if (setting.value == null) {
+                esConfig.remove(setting.key)
+            } else {
+                esConfig.put(setting.key, setting.value)
+            }
+        }
 
         Task writeConfig = project.tasks.create(name: name, type: DefaultTask, dependsOn: setup)
         writeConfig.doFirst {

distribution/archives/integ-test-zip/build.gradle

@@ -1,2 +1,23 @@
-// This file is intentionally blank. All configuration of the
-// distribution is done in the parent project.
+/*
+ * 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.
+ */
+
+integTestRunner {
+  systemProperty 'tests.logfile',
+    "${ -> integTest.nodes[0].homeDir}/logs/${ -> integTest.nodes[0].clusterName }.log"
+}

distribution/archives/integ-test-zip/src/test/java/org/elasticsearch/test/rest/NodeNameInLogsIT.java

@@ -0,0 +1,43 @@
+/*
+ * 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.unconfigurednodename;
+
+import org.elasticsearch.common.logging.NodeNameInLogsIntegTestCase;
+
+import java.io.IOException;
+import java.io.BufferedReader;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+
+public class NodeNameInLogsIT extends NodeNameInLogsIntegTestCase {
+    @Override
+    protected BufferedReader openReader(Path logFile) throws IOException {
+        return AccessController.doPrivileged((PrivilegedAction<BufferedReader>) () -> {
+            try {
+                return Files.newBufferedReader(logFile, StandardCharsets.UTF_8);
+            } catch (IOException e) {
+                throw new RuntimeException(e);
+            }
+        });
+    }
+}

distribution/archives/integ-test-zip/src/test/resources/plugin-security.policy

@@ -0,0 +1,4 @@
+grant {
+  // Needed to read the log file
+  permission java.io.FilePermission "${tests.logfile}", "read";
+};

qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java

@@ -357,7 +357,7 @@ public void testProperties() throws IOException, UserException {
         }
     }
 
-    public void testNoNodeNameWarning() throws IOException, UserException {
+    public void testNoNodeNameInPatternWarning() throws IOException, UserException {
         setupLogging("no_node_name");
 
         final String path =
@@ -368,7 +368,7 @@ public void testNoNodeNameWarning() throws IOException, UserException {
         assertThat(events.size(), equalTo(2));
         final String location = "org.elasticsearch.common.logging.LogConfigurator";
         // the first message is a warning for unsupported configuration files
-        assertLogLine(events.get(0), Level.WARN, location, "\\[null\\] Some logging configurations have %marker but don't "
+        assertLogLine(events.get(0), Level.WARN, location, "\\[unknown\\] Some logging configurations have %marker but don't "
                 + "have %node_name. We will automatically add %node_name to the pattern to ease the migration for users "
                 + "who customize log4j2.properties but will stop this behavior in 7.0. You should manually replace "
                 + "`%node_name` with `\\[%node_name\\]%marker ` in these locations:");

qa/evil-tests/src/test/java/org/elasticsearch/env/NodeEnvironmentEvilTests.java

@@ -52,7 +52,7 @@ public void testMissingWritePermission() throws IOException {
                     .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath().toString())
                     .putList(Environment.PATH_DATA_SETTING.getKey(), tempPaths).build();
             IOException ioException = expectThrows(IOException.class, () -> {
-                new NodeEnvironment(build, TestEnvironment.newEnvironment(build));
+                new NodeEnvironment(build, TestEnvironment.newEnvironment(build), nodeId -> {});
             });
             assertTrue(ioException.getMessage(), ioException.getMessage().startsWith(path.toString()));
         }
@@ -72,7 +72,7 @@ public void testMissingWritePermissionOnIndex() throws IOException {
                 .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath().toString())
                 .putList(Environment.PATH_DATA_SETTING.getKey(), tempPaths).build();
             IOException ioException = expectThrows(IOException.class, () -> {
-                new NodeEnvironment(build, TestEnvironment.newEnvironment(build));
+                new NodeEnvironment(build, TestEnvironment.newEnvironment(build), nodeId -> {});
             });
             assertTrue(ioException.getMessage(), ioException.getMessage().startsWith("failed to test writes in data directory"));
         }
@@ -97,7 +97,7 @@ public void testMissingWritePermissionOnShard() throws IOException {
                 .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath().toString())
                 .putList(Environment.PATH_DATA_SETTING.getKey(), tempPaths).build();
             IOException ioException = expectThrows(IOException.class, () -> {
-                new NodeEnvironment(build, TestEnvironment.newEnvironment(build));
+                new NodeEnvironment(build, TestEnvironment.newEnvironment(build), nodeId -> {});
             });
             assertTrue(ioException.getMessage(), ioException.getMessage().startsWith("failed to test writes in data directory"));
         }

qa/unconfigured-node-name/build.gradle

@@ -0,0 +1,30 @@
+/*
+ * 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.
+ */
+
+apply plugin: 'elasticsearch.standalone-rest-test'
+apply plugin: 'elasticsearch.rest-test'
+
+integTestCluster {
+  setting 'node.name', null
+}
+
+integTestRunner {
+  systemProperty 'tests.logfile',
+    "${ -> integTest.nodes[0].homeDir}/logs/${ -> integTest.nodes[0].clusterName }.log"
+}

qa/unconfigured-node-name/src/test/java/org/elasticsearch/unconfigured_node_name/NodeNameInLogsIT.java

@@ -0,0 +1,43 @@
+/*
+ * 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.unconfigured_node_name;
+
+import org.elasticsearch.common.logging.NodeNameInLogsIntegTestCase;
+
+import java.io.IOException;
+import java.io.BufferedReader;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+
+public class NodeNameInLogsIT extends NodeNameInLogsIntegTestCase {
+    @Override
+    protected BufferedReader openReader(Path logFile) throws IOException {
+        return AccessController.doPrivileged((PrivilegedAction<BufferedReader>) () -> {
+            try {
+                return Files.newBufferedReader(logFile, StandardCharsets.UTF_8);
+            } catch (IOException e) {
+                throw new RuntimeException(e);
+            }
+        });
+    }
+}

qa/unconfigured-node-name/src/test/resources/plugin-security.policy

@@ -0,0 +1,4 @@
+grant {
+  // Needed to read the log file
+  permission java.io.FilePermission "${tests.logfile}", "read";
+};

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

@@ -37,7 +37,6 @@
 import org.elasticsearch.common.logging.ESLoggerFactory;
 import org.elasticsearch.common.logging.LogConfigurator;
 import org.elasticsearch.common.logging.Loggers;
-import org.elasticsearch.common.logging.NodeNamePatternConverter;
 import org.elasticsearch.common.network.IfConfig;
 import org.elasticsearch.common.settings.KeyStoreWrapper;
 import org.elasticsearch.common.settings.SecureSettings;
@@ -217,6 +216,11 @@ protected void validateNodeBeforeAcceptingRequests(
                 final BoundTransportAddress boundTransportAddress, List<BootstrapCheck> checks) throws NodeValidationException {
                 BootstrapChecks.check(context, boundTransportAddress, checks);
             }
+
+            @Override
+            protected void registerDerivedNodeNameWithLogger(String nodeName) {
+                LogConfigurator.setNodeName(nodeName);
+            }
         };
     }
 
@@ -289,9 +293,9 @@ static void init(
         final SecureSettings keystore = loadSecureSettings(initialEnv);
         final Environment environment = createEnvironment(pidFile, keystore, initialEnv.settings(), initialEnv.configFile());
 
-        String nodeName = Node.NODE_NAME_SETTING.get(environment.settings());
-        NodeNamePatternConverter.setNodeName(nodeName);
-
+        if (Node.NODE_NAME_SETTING.exists(environment.settings())) {
+            LogConfigurator.setNodeName(Node.NODE_NAME_SETTING.get(environment.settings()));
+        }
         try {
             LogConfigurator.configure(environment);
         } catch (IOException e) {