Skip to content

Commit b96c0c1

Browse files
committed
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.
1 parent f9b22e9 commit b96c0c1

File tree

22 files changed

+369
-80
lines changed

22 files changed

+369
-80
lines changed

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,13 @@ class ClusterFormationTasks {
332332
} else {
333333
esConfig['script.max_compilations_per_minute'] = 2048
334334
}
335-
esConfig.putAll(node.config.settings)
335+
for (Map.Entry<String, Object> setting : node.config.settings) {
336+
if (setting.value == null) {
337+
esConfig.remove(setting.key)
338+
} else {
339+
esConfig.put(setting.key, setting.value)
340+
}
341+
}
336342

337343
Task writeConfig = project.tasks.create(name: name, type: DefaultTask, dependsOn: setup)
338344
writeConfig.doFirst {
Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,23 @@
1-
// This file is intentionally blank. All configuration of the
2-
// distribution is done in the parent project.
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
integTestRunner {
21+
systemProperty 'tests.logfile',
22+
"${ -> integTest.nodes[0].homeDir}/logs/${ -> integTest.nodes[0].clusterName }.log"
23+
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.unconfigurednodename;
21+
22+
import org.elasticsearch.common.logging.NodeNameInLogsIntegTestCase;
23+
24+
import java.io.IOException;
25+
import java.io.BufferedReader;
26+
import java.nio.charset.StandardCharsets;
27+
import java.nio.file.Files;
28+
import java.nio.file.Path;
29+
import java.security.AccessController;
30+
import java.security.PrivilegedAction;
31+
32+
public class NodeNameInLogsIT extends NodeNameInLogsIntegTestCase {
33+
@Override
34+
protected BufferedReader openReader(Path logFile) throws IOException {
35+
return AccessController.doPrivileged((PrivilegedAction<BufferedReader>) () -> {
36+
try {
37+
return Files.newBufferedReader(logFile, StandardCharsets.UTF_8);
38+
} catch (IOException e) {
39+
throw new RuntimeException(e);
40+
}
41+
});
42+
}
43+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
grant {
2+
// Needed to read the log file
3+
permission java.io.FilePermission "${tests.logfile}", "read";
4+
};

modules/tribe/src/main/java/org/elasticsearch/tribe/TribePlugin.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,12 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
8585
}
8686

8787
protected Function<Settings, Node> nodeBuilder(Path configPath) {
88-
return settings -> new Node(new Environment(settings, configPath));
88+
return settings -> new Node(new Environment(settings, configPath)) {
89+
@Override
90+
protected void registerDerivedNodeNameWithLogger(String nodeName) {
91+
// don't register the node name because we've done it in the main node
92+
}
93+
};
8994
}
9095

9196
@Override

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ public void testProperties() throws IOException, UserException {
357357
}
358358
}
359359

360-
public void testNoNodeNameWarning() throws IOException, UserException {
360+
public void testNoNodeNameInPatternWarning() throws IOException, UserException {
361361
setupLogging("no_node_name");
362362

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

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public void testMissingWritePermission() throws IOException {
5252
.put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath().toString())
5353
.putList(Environment.PATH_DATA_SETTING.getKey(), tempPaths).build();
5454
IOException ioException = expectThrows(IOException.class, () -> {
55-
new NodeEnvironment(build, TestEnvironment.newEnvironment(build));
55+
new NodeEnvironment(build, TestEnvironment.newEnvironment(build), nodeId -> {});
5656
});
5757
assertTrue(ioException.getMessage(), ioException.getMessage().startsWith(path.toString()));
5858
}
@@ -72,7 +72,7 @@ public void testMissingWritePermissionOnIndex() throws IOException {
7272
.put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath().toString())
7373
.putList(Environment.PATH_DATA_SETTING.getKey(), tempPaths).build();
7474
IOException ioException = expectThrows(IOException.class, () -> {
75-
new NodeEnvironment(build, TestEnvironment.newEnvironment(build));
75+
new NodeEnvironment(build, TestEnvironment.newEnvironment(build), nodeId -> {});
7676
});
7777
assertTrue(ioException.getMessage(), ioException.getMessage().startsWith("failed to test writes in data directory"));
7878
}
@@ -97,7 +97,7 @@ public void testMissingWritePermissionOnShard() throws IOException {
9797
.put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath().toString())
9898
.putList(Environment.PATH_DATA_SETTING.getKey(), tempPaths).build();
9999
IOException ioException = expectThrows(IOException.class, () -> {
100-
new NodeEnvironment(build, TestEnvironment.newEnvironment(build));
100+
new NodeEnvironment(build, TestEnvironment.newEnvironment(build), nodeId -> {});
101101
});
102102
assertTrue(ioException.getMessage(), ioException.getMessage().startsWith("failed to test writes in data directory"));
103103
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
apply plugin: 'elasticsearch.standalone-rest-test'
21+
apply plugin: 'elasticsearch.rest-test'
22+
23+
integTestCluster {
24+
setting 'node.name', null
25+
}
26+
27+
integTestRunner {
28+
systemProperty 'tests.logfile',
29+
"${ -> integTest.nodes[0].homeDir}/logs/${ -> integTest.nodes[0].clusterName }.log"
30+
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.unconfigured_node_name;
21+
22+
import org.elasticsearch.common.logging.NodeNameInLogsIntegTestCase;
23+
24+
import java.io.IOException;
25+
import java.io.BufferedReader;
26+
import java.nio.charset.StandardCharsets;
27+
import java.nio.file.Files;
28+
import java.nio.file.Path;
29+
import java.security.AccessController;
30+
import java.security.PrivilegedAction;
31+
32+
public class NodeNameInLogsIT extends NodeNameInLogsIntegTestCase {
33+
@Override
34+
protected BufferedReader openReader(Path logFile) throws IOException {
35+
return AccessController.doPrivileged((PrivilegedAction<BufferedReader>) () -> {
36+
try {
37+
return Files.newBufferedReader(logFile, StandardCharsets.UTF_8);
38+
} catch (IOException e) {
39+
throw new RuntimeException(e);
40+
}
41+
});
42+
}
43+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
grant {
2+
// Needed to read the log file
3+
permission java.io.FilePermission "${tests.logfile}", "read";
4+
};

0 commit comments

Comments
 (0)