Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logging: Make node name consistent in logger #31588

Merged
merged 19 commits into from
Jul 31, 2018
Merged

Logging: Make node name consistent in logger #31588

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
6ebceb5
Logging: Make node name consistent in logger
nik9000 Jun 21, 2018
88db037
Merge branch 'master' into logger_fixer
nik9000 Jun 27, 2018
29074b2
Cleanup from review
nik9000 Jun 27, 2018
76a6607
Add back test parameters if there are any
nik9000 Jun 27, 2018
41e7be4
Merge branch 'master' into logger_fixer
nik9000 Jul 18, 2018
c924ade
Revert log4j2.properties changes
nik9000 Jul 18, 2018
a98de4a
Rever asciidoc changes
nik9000 Jul 18, 2018
c428a08
Remove another asciidoc change
nik9000 Jul 18, 2018
d1b1999
Update docs
nik9000 Jul 18, 2018
25c1b3e
Add warning
nik9000 Jul 20, 2018
048eccf
Merge branch 'master' into logger_fixer
nik9000 Jul 20, 2018
75a10e2
Checkstyle
nik9000 Jul 20, 2018
59f56ed
Merge branch 'master' into logger_fixer
nik9000 Jul 23, 2018
7122d0f
Set configuration
nik9000 Jul 23, 2018
eaea37b
Test
nik9000 Jul 23, 2018
64505e9
Merge branch 'master' into logger_fixer
nik9000 Jul 24, 2018
f1ba7c5
Move logger initialization
nik9000 Jul 24, 2018
1fb555f
Merge branch 'master' into logger_fixer
nik9000 Jul 30, 2018
9dbd4be
Fixup
nik9000 Jul 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion benchmarks/src/main/resources/log4j2.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
appender.console.type = Console
appender.console.name = console
appender.console.layout.type = PatternLayout
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker%m%n
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] [%node_name]%marker %m%n

# Do not log at all if it is not really critical - we're in a benchmark
rootLogger.level = error
Expand Down
2 changes: 1 addition & 1 deletion client/benchmark/src/main/resources/log4j2.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
appender.console.type = Console
appender.console.name = console
appender.console.layout.type = PatternLayout
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker%m%n
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] [%node_name]%marker %m%n

rootLogger.level = info
rootLogger.appenderRef.console.ref = console
10 changes: 5 additions & 5 deletions distribution/src/config/log4j2.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ logger.action.level = debug
appender.console.type = Console
appender.console.name = console
appender.console.layout.type = PatternLayout
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c{1.}] %marker%m%n
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c{1.}] [%node_name]%marker %m%n

appender.rolling.type = RollingFile
appender.rolling.name = rolling
appender.rolling.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}.log
appender.rolling.layout.type = PatternLayout
appender.rolling.layout.pattern = [%d{ISO8601}][%-5p][%-25c{1.}] %marker%.-10000m%n
appender.rolling.layout.pattern = [%d{ISO8601}][%-5p][%-25c{1.}] [%node_name]%marker %.-10000m%n
appender.rolling.filePattern = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}-%d{yyyy-MM-dd}-%i.log.gz
appender.rolling.policies.type = Policies
appender.rolling.policies.time.type = TimeBasedTriggeringPolicy
Expand All @@ -38,7 +38,7 @@ appender.deprecation_rolling.type = RollingFile
appender.deprecation_rolling.name = deprecation_rolling
appender.deprecation_rolling.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_deprecation.log
appender.deprecation_rolling.layout.type = PatternLayout
appender.deprecation_rolling.layout.pattern = [%d{ISO8601}][%-5p][%-25c{1.}] %marker%.-10000m%n
appender.deprecation_rolling.layout.pattern = [%d{ISO8601}][%-5p][%-25c{1.}] [%node_name]%marker %.-10000m%n
appender.deprecation_rolling.filePattern = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_deprecation-%i.log.gz
appender.deprecation_rolling.policies.type = Policies
appender.deprecation_rolling.policies.size.type = SizeBasedTriggeringPolicy
Expand All @@ -55,7 +55,7 @@ appender.index_search_slowlog_rolling.type = RollingFile
appender.index_search_slowlog_rolling.name = index_search_slowlog_rolling
appender.index_search_slowlog_rolling.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_index_search_slowlog.log
appender.index_search_slowlog_rolling.layout.type = PatternLayout
appender.index_search_slowlog_rolling.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker%.-10000m%n
appender.index_search_slowlog_rolling.layout.pattern = [%d{ISO8601}][%-5p][%-25c] [%node_name]%marker %.-10000m%n
appender.index_search_slowlog_rolling.filePattern = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_index_search_slowlog-%i.log.gz
appender.index_search_slowlog_rolling.policies.type = Policies
appender.index_search_slowlog_rolling.policies.size.type = SizeBasedTriggeringPolicy
Expand All @@ -72,7 +72,7 @@ appender.index_indexing_slowlog_rolling.type = RollingFile
appender.index_indexing_slowlog_rolling.name = index_indexing_slowlog_rolling
appender.index_indexing_slowlog_rolling.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_index_indexing_slowlog.log
appender.index_indexing_slowlog_rolling.layout.type = PatternLayout
appender.index_indexing_slowlog_rolling.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker%.-10000m%n
appender.index_indexing_slowlog_rolling.layout.pattern = [%d{ISO8601}][%-5p][%-25c] [%node_name]%marker %.-10000m%n
appender.index_indexing_slowlog_rolling.filePattern = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_index_indexing_slowlog-%i.log.gz
appender.index_indexing_slowlog_rolling.policies.type = Policies
appender.index_indexing_slowlog_rolling.policies.size.type = SizeBasedTriggeringPolicy
Expand Down
2 changes: 1 addition & 1 deletion docs/java-api/index.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ For example, you can add in your `src/main/resources` project dir a `log4j2.prop
appender.console.type = Console
appender.console.name = console
appender.console.layout.type = PatternLayout
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker%m%n
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker %m%n
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this example include [%node_name]?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't add it because I didn't think the transport client has a node name.


rootLogger.level = info
rootLogger.appenderRef.console.ref = console
Expand Down
4 changes: 2 additions & 2 deletions docs/reference/index-modules/slowlog.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ appender.index_search_slowlog_rolling.type = RollingFile
appender.index_search_slowlog_rolling.name = index_search_slowlog_rolling
appender.index_search_slowlog_rolling.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_index_search_slowlog.log
appender.index_search_slowlog_rolling.layout.type = PatternLayout
appender.index_search_slowlog_rolling.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker%.-10000m%n
appender.index_search_slowlog_rolling.layout.pattern = [%d{ISO8601}][%-5p][%-25c] [%node_name]%marker %.-10000m%n
appender.index_search_slowlog_rolling.filePattern = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_index_search_slowlog-%i.log.gz
appender.index_search_slowlog_rolling.policies.type = Policies
appender.index_search_slowlog_rolling.policies.size.type = SizeBasedTriggeringPolicy
Expand Down Expand Up @@ -103,7 +103,7 @@ appender.index_indexing_slowlog_rolling.type = RollingFile
appender.index_indexing_slowlog_rolling.name = index_indexing_slowlog_rolling
appender.index_indexing_slowlog_rolling.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_index_indexing_slowlog.log
appender.index_indexing_slowlog_rolling.layout.type = PatternLayout
appender.index_indexing_slowlog_rolling.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker%.-10000m%n
appender.index_indexing_slowlog_rolling.layout.pattern = [%d{ISO8601}][%-5p][%-25c] [%node_name]%marker %.-10000m%n
appender.index_indexing_slowlog_rolling.filePattern = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_index_indexing_slowlog-%i.log.gz
appender.index_indexing_slowlog_rolling.policies.type = Policies
appender.index_indexing_slowlog_rolling.policies.size.type = SizeBasedTriggeringPolicy
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/setup/logging-config.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ appender.rolling.type = RollingFile <1>
appender.rolling.name = rolling
appender.rolling.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}.log <2>
appender.rolling.layout.type = PatternLayout
appender.rolling.layout.pattern = [%d{ISO8601}][%-5p][%-25c{1.}] %marker%.-10000m%n
appender.rolling.layout.pattern = [%d{ISO8601}][%-5p][%-25c{1.}] [%node_name]%marker %.-10000m%n
appender.rolling.filePattern = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}-%d{yyyy-MM-dd}-%i.log.gz <3>
appender.rolling.policies.type = Policies
appender.rolling.policies.time.type = TimeBasedTriggeringPolicy <4>
Expand Down
2 changes: 1 addition & 1 deletion qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilBootstrapChecksTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public void testInvalidValue() {
final boolean enforceLimits = randomBoolean();
final IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> BootstrapChecks.check(new BootstrapContext(Settings.EMPTY, null), enforceLimits, emptyList(), "testInvalidValue"));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In production code this used to be the node name but it is no longer required because the node name is already part of the logger.

() -> BootstrapChecks.check(new BootstrapContext(Settings.EMPTY, null), enforceLimits, emptyList()));
final Matcher<String> matcher = containsString(
"[es.enforce.bootstrap.checks] must be [true] but was [" + value + "]");
assertThat(e, hasToString(matcher));
Expand Down
6 changes: 4 additions & 2 deletions qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,11 @@ public void testPrefixLogger() throws IOException, IllegalAccessException, UserE
assertThat(events.size(), equalTo(expectedLogLines + stackTraceLength));
for (int i = 0; i < expectedLogLines; i++) {
if (prefix == null) {
assertThat(events.get(i), startsWith("test"));
assertThat("Contents of [" + path + "] are wrong",
events.get(i), startsWith("[" + getTestName() + "] test"));
} else {
assertThat(events.get(i), startsWith("[" + prefix + "] test"));
assertThat("Contents of [" + path + "] are wrong",
events.get(i), startsWith("[" + getTestName() + "][" + prefix + "] test"));
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions qa/evil-tests/src/test/resources/org/elasticsearch/common/logging/config/log4j2.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
appender.console.type = Console
appender.console.name = console
appender.console.layout.type = PatternLayout
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker%m%n
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] [%test_thread_info]%marker %m%n

appender.file.type = File
appender.file.name = file
appender.file.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}.log
appender.file.layout.type = PatternLayout
appender.file.layout.pattern = [%p][%l] %marker%m%n
appender.file.layout.pattern = [%p][%l] [%test_thread_info]%marker %m%n

rootLogger.level = info
rootLogger.appenderRef.console.ref = console
Expand All @@ -23,10 +23,9 @@ appender.deprecation_file.type = File
appender.deprecation_file.name = deprecation_file
appender.deprecation_file.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_deprecation.log
appender.deprecation_file.layout.type = PatternLayout
appender.deprecation_file.layout.pattern = [%p][%l] %marker%m%n
appender.deprecation_file.layout.pattern = [%p][%l] [%test_thread_info]%marker %m%n

logger.deprecation.name = deprecation
logger.deprecation.level = warn
logger.deprecation.appenderRef.deprecation_file.ref = deprecation_file
logger.deprecation.additivity = false

2 changes: 1 addition & 1 deletion ...-tests/src/test/resources/org/elasticsearch/common/logging/config/third/log4j2.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
appender.console3.type = Console
appender.console3.name = console3
appender.console3.layout.type = PatternLayout
appender.console3.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker%m%n
appender.console3.layout.pattern = [%d{ISO8601}][%-5p][%-25c] [%test_thread_info]%marker %m%n

logger.third.name = third
logger.third.level = debug
Expand Down
6 changes: 3 additions & 3 deletions ...l-tests/src/test/resources/org/elasticsearch/common/logging/deprecation/log4j2.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
appender.console.type = Console
appender.console.name = console
appender.console.layout.type = PatternLayout
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker%m%n
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] [%test_thread_info]%marker %m%n

appender.file.type = File
appender.file.name = file
appender.file.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}.log
appender.file.layout.type = PatternLayout
appender.file.layout.pattern = [%p][%l] %marker%m%n
appender.file.layout.pattern = [%p][%l] [%test_thread_info]%marker %m%n

rootLogger.level = info
rootLogger.appenderRef.console.ref = console
Expand All @@ -17,7 +17,7 @@ appender.deprecation_file.type = File
appender.deprecation_file.name = deprecation_file
appender.deprecation_file.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_deprecation.log
appender.deprecation_file.layout.type = PatternLayout
appender.deprecation_file.layout.pattern = [%p][%l] %marker%m%n
appender.deprecation_file.layout.pattern = [%p][%l] [%test_thread_info]%marker %m%n

logger.deprecation.name = deprecation
logger.deprecation.level = warn
Expand Down
2 changes: 1 addition & 1 deletion ...tests/src/test/resources/org/elasticsearch/common/logging/find_appender/log4j2.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
appender.console.type = Console
appender.console.name = console
appender.console.layout.type = PatternLayout
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker%m%n
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] [%test_thread_info]%marker %m%n

logger.has_console_appender.name = has_console_appender
logger.has_console_appender.level = trace
Expand Down
4 changes: 2 additions & 2 deletions ...tests/src/test/resources/org/elasticsearch/common/logging/location_info/log4j2.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
appender.console.type = Console
appender.console.name = console
appender.console.layout.type = PatternLayout
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker%m%n
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] [%test_thread_info]%marker %m%n

appender.file.type = File
appender.file.name = file
appender.file.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}.log
appender.file.layout.type = PatternLayout
appender.file.layout.pattern = [%p][%l] %marker%m%n
appender.file.layout.pattern = [%p][%l] [%test_thread_info]%marker %m%n

rootLogger.level = info
rootLogger.appenderRef.console.ref = console
Expand Down
4 changes: 2 additions & 2 deletions qa/evil-tests/src/test/resources/org/elasticsearch/common/logging/prefix/log4j2.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
appender.console.type = Console
appender.console.name = console
appender.console.layout.type = PatternLayout
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker%m%n
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] [%test_thread_info]%marker %m%n

appender.file.type = File
appender.file.name = file
appender.file.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}.log
appender.file.layout.type = PatternLayout
appender.file.layout.pattern = %marker%m%n
appender.file.layout.pattern = [%test_thread_info]%marker %m%n

rootLogger.level = info
rootLogger.appenderRef.console.ref = console
Expand Down
6 changes: 3 additions & 3 deletions qa/evil-tests/src/test/resources/org/elasticsearch/common/logging/settings/log4j2.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
appender.console.type = Console
appender.console.name = console
appender.console.layout.type = PatternLayout
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] %marker%m%n
appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c] [%test_thread_info]%marker %m%n

appender.file.type = File
appender.file.name = file
appender.file.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}.log
appender.file.layout.type = PatternLayout
appender.file.layout.pattern = [%p][%l] %marker%m%n
appender.file.layout.pattern = [%p][%l] [%test_thread_info]%marker %m%n

rootLogger.level = info
rootLogger.appenderRef.console.ref = console
Expand All @@ -17,7 +17,7 @@ appender.deprecation_file.type = File
appender.deprecation_file.name = deprecation_file
appender.deprecation_file.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_deprecation.log
appender.deprecation_file.layout.type = PatternLayout
appender.deprecation_file.layout.pattern = [%p][%l] %marker%m%n
appender.deprecation_file.layout.pattern = [%p][%l] [%test_thread_info]%marker %m%n

logger.deprecation.name = org.elasticsearch.deprecation.common.settings
logger.deprecation.level = warn
Expand Down
8 changes: 6 additions & 2 deletions server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
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;
Expand Down Expand Up @@ -287,6 +288,10 @@ 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);

try {
LogConfigurator.configure(environment);
} catch (IOException e) {
Expand Down Expand Up @@ -317,8 +322,7 @@ static void init(
// install the default uncaught exception handler; must be done before security is
// initialized as we do not want to grant the runtime permission
// setDefaultUncaughtExceptionHandler
Thread.setDefaultUncaughtExceptionHandler(
new ElasticsearchUncaughtExceptionHandler(() -> Node.NODE_NAME_SETTING.get(environment.settings())));
Thread.setDefaultUncaughtExceptionHandler(new ElasticsearchUncaughtExceptionHandler());

INSTANCE.setup(true, environment);

Expand Down
10 changes: 3 additions & 7 deletions server/src/main/java/org/elasticsearch/bootstrap/BootstrapChecks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.elasticsearch.discovery.DiscoveryModule;
import org.elasticsearch.monitor.jvm.JvmInfo;
import org.elasticsearch.monitor.process.ProcessProbe;
import org.elasticsearch.node.Node;
import org.elasticsearch.node.NodeValidationException;

import java.io.BufferedReader;
Expand Down Expand Up @@ -74,8 +73,7 @@ static void check(final BootstrapContext context, final BoundTransportAddress bo
combinedChecks.addAll(additionalChecks);
check( context,
enforceLimits(boundTransportAddress, DiscoveryModule.DISCOVERY_TYPE_SETTING.get(context.settings)),
Collections.unmodifiableList(combinedChecks),
Node.NODE_NAME_SETTING.get(context.settings));
Collections.unmodifiableList(combinedChecks));
}

/**
Expand All @@ -86,14 +84,12 @@ static void check(final BootstrapContext context, final BoundTransportAddress bo
* @param context the current node boostrap context
* @param enforceLimits {@code true} if the checks should be enforced or otherwise warned
* @param checks the checks to execute
* @param nodeName the node name to be used as a logging prefix
*/
static void check(
final BootstrapContext context,
final boolean enforceLimits,
final List<BootstrapCheck> checks,
final String nodeName) throws NodeValidationException {
check(context, enforceLimits, checks, Loggers.getLogger(BootstrapChecks.class, nodeName));
final List<BootstrapCheck> checks) throws NodeValidationException {
check(context, enforceLimits, checks, Loggers.getLogger(BootstrapChecks.class));
}

/**
Expand Down
11 changes: 1 addition & 10 deletions server/src/main/java/org/elasticsearch/bootstrap/ElasticsearchUncaughtExceptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,9 @@
import java.io.IOError;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Objects;
import java.util.function.Supplier;

class ElasticsearchUncaughtExceptionHandler implements Thread.UncaughtExceptionHandler {

private final Supplier<String> loggingPrefixSupplier;

ElasticsearchUncaughtExceptionHandler(final Supplier<String> loggingPrefixSupplier) {
this.loggingPrefixSupplier = Objects.requireNonNull(loggingPrefixSupplier);
}
private static final Logger logger = Loggers.getLogger(ElasticsearchUncaughtExceptionHandler.class);

@Override
public void uncaughtException(Thread t, Throwable e) {
Expand Down Expand Up @@ -70,12 +63,10 @@ static boolean isFatalUncaught(Throwable e) {
}

void onFatalUncaught(final String threadName, final Throwable t) {
final Logger logger = Loggers.getLogger(ElasticsearchUncaughtExceptionHandler.class, loggingPrefixSupplier.get());
logger.error(() -> new ParameterizedMessage("fatal error in thread [{}], exiting", threadName), t);
}

void onNonFatalUncaught(final String threadName, final Throwable t) {
final Logger logger = Loggers.getLogger(ElasticsearchUncaughtExceptionHandler.class, loggingPrefixSupplier.get());
logger.warn(() -> new ParameterizedMessage("uncaught exception in thread [{}]", threadName), t);
}

Expand Down
7 changes: 7 additions & 0 deletions server/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ public static Logger getLogger(String prefix, Class<?> clazz) {
}

public static Logger getLogger(String prefix, Logger logger) {
if (prefix == null || prefix.length() == 0) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of our loggers don't actually have a prefix so I felt like we shouldn't wrap the default logger in that case. It is one less object to allocate and one less synchronized block to execute.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not fail here? These would just be noop calls that could be removed right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now everything gets routed through this method even if there isn't a prefix. How about this: I promise to remove as many of the getLogger variants as I can in a followup but I keep this like this in this PR so I don't make the change larger than it already is?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Can you at least add a comment here that this leniency should/will be removed in the future?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

/*
* In an followup we'll throw an exception in this case directing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I wasn't explicit, I meant create a GH issue and link here.

* folks to LogManger.getLogger.
*/
return logger;
}
return new PrefixLogger((ExtendedLogger)logger, logger.getName(), prefix);
}

Expand Down
Loading