From 5c624bc55b2cc26ec8986e4bd69c8c48c06db7f4 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 5 Sep 2018 13:04:26 -0400 Subject: [PATCH] Logging: Further clean up logging ctors (#33378) Drops and unused logging constructor, simplifies a rarely used one, and removes `Settings` from a third. There is now only a single logging ctor that takes `Settings` and we'll remove that one in a follow up change. --- .../elasticsearch/common/logging/Loggers.java | 18 ++++++------------ .../index/AbstractIndexComponent.java | 2 +- .../index/CompositeIndexEventListener.java | 2 +- .../org/elasticsearch/index/IndexSettings.java | 2 +- .../elasticsearch/index/IndexingSlowLog.java | 3 ++- .../org/elasticsearch/index/SearchSlowLog.java | 5 +++-- .../elasticsearch/xpack/watcher/Watcher.java | 2 +- .../logging/ExecutableLoggingAction.java | 7 +++---- .../actions/logging/LoggingActionFactory.java | 11 ++++------- .../actions/logging/LoggingActionTests.java | 15 +++++---------- .../xpack/watcher/watch/WatchTests.java | 4 ++-- 11 files changed, 29 insertions(+), 42 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/logging/Loggers.java b/server/src/main/java/org/elasticsearch/common/logging/Loggers.java index 35e55f5f505..57bafbbdac4 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/Loggers.java +++ b/server/src/main/java/org/elasticsearch/common/logging/Loggers.java @@ -50,31 +50,25 @@ public class Loggers { Setting.Property.NodeScope)); public static Logger getLogger(Class clazz, ShardId shardId, String... prefixes) { - return getLogger(clazz, Settings.EMPTY, - shardId.getIndex(), asArrayList(Integer.toString(shardId.id()), prefixes).toArray(new String[0])); + return getLogger(clazz, shardId.getIndex(), asArrayList(Integer.toString(shardId.id()), prefixes).toArray(new String[0])); } /** * Just like {@link #getLogger(Class, ShardId, String...)} but String loggerName instead of - * Class. + * Class and no extra prefixes. */ - public static Logger getLogger(String loggerName, ShardId shardId, String... prefixes) { - return getLogger(loggerName, Settings.EMPTY, - asArrayList(shardId.getIndexName(), Integer.toString(shardId.id()), prefixes).toArray(new String[0])); + public static Logger getLogger(String loggerName, ShardId shardId) { + return ESLoggerFactory.getLogger(formatPrefix(shardId.getIndexName(), Integer.toString(shardId.id())), loggerName); } - public static Logger getLogger(Class clazz, Settings settings, Index index, String... prefixes) { - return getLogger(clazz, settings, asArrayList(Loggers.SPACE, index.getName(), prefixes).toArray(new String[0])); + public static Logger getLogger(Class clazz, Index index, String... prefixes) { + return getLogger(clazz, Settings.EMPTY, asArrayList(Loggers.SPACE, index.getName(), prefixes).toArray(new String[0])); } public static Logger getLogger(Class clazz, Settings settings, String... prefixes) { return ESLoggerFactory.getLogger(formatPrefix(prefixes), clazz); } - public static Logger getLogger(String loggerName, Settings settings, String... prefixes) { - return ESLoggerFactory.getLogger(formatPrefix(prefixes), loggerName); - } - public static Logger getLogger(Logger parentLogger, String s) { String prefix = null; if (parentLogger instanceof PrefixLogger) { diff --git a/server/src/main/java/org/elasticsearch/index/AbstractIndexComponent.java b/server/src/main/java/org/elasticsearch/index/AbstractIndexComponent.java index 25acdd06b44..7760e104140 100644 --- a/server/src/main/java/org/elasticsearch/index/AbstractIndexComponent.java +++ b/server/src/main/java/org/elasticsearch/index/AbstractIndexComponent.java @@ -33,7 +33,7 @@ public abstract class AbstractIndexComponent implements IndexComponent { * Constructs a new index component, with the index name and its settings. */ protected AbstractIndexComponent(IndexSettings indexSettings) { - this.logger = Loggers.getLogger(getClass(), indexSettings.getSettings(), indexSettings.getIndex()); + this.logger = Loggers.getLogger(getClass(), indexSettings.getIndex()); this.deprecationLogger = new DeprecationLogger(logger); this.indexSettings = indexSettings; } diff --git a/server/src/main/java/org/elasticsearch/index/CompositeIndexEventListener.java b/server/src/main/java/org/elasticsearch/index/CompositeIndexEventListener.java index 1bdec683bfb..78cd8d462ea 100644 --- a/server/src/main/java/org/elasticsearch/index/CompositeIndexEventListener.java +++ b/server/src/main/java/org/elasticsearch/index/CompositeIndexEventListener.java @@ -51,7 +51,7 @@ final class CompositeIndexEventListener implements IndexEventListener { } } this.listeners = Collections.unmodifiableList(new ArrayList<>(listeners)); - this.logger = Loggers.getLogger(getClass(), indexSettings.getSettings(), indexSettings.getIndex()); + this.logger = Loggers.getLogger(getClass(), indexSettings.getIndex()); } @Override diff --git a/server/src/main/java/org/elasticsearch/index/IndexSettings.java b/server/src/main/java/org/elasticsearch/index/IndexSettings.java index 0dd3654ce3d..79d335a803b 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexSettings.java +++ b/server/src/main/java/org/elasticsearch/index/IndexSettings.java @@ -397,7 +397,7 @@ public final class IndexSettings { this.settings = Settings.builder().put(nodeSettings).put(indexMetaData.getSettings()).build(); this.index = indexMetaData.getIndex(); version = IndexMetaData.SETTING_INDEX_VERSION_CREATED.get(settings); - logger = Loggers.getLogger(getClass(), settings, index); + logger = Loggers.getLogger(getClass(), index); nodeName = Node.NODE_NAME_SETTING.get(settings); this.indexMetaData = indexMetaData; numberOfShards = settings.getAsInt(IndexMetaData.SETTING_NUMBER_OF_SHARDS, null); diff --git a/server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java b/server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java index 8293f873c65..e69cfd2c7af 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java +++ b/server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java @@ -20,6 +20,7 @@ package org.elasticsearch.index; import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.common.Booleans; import org.elasticsearch.common.Strings; import org.elasticsearch.common.logging.Loggers; @@ -89,7 +90,7 @@ public final class IndexingSlowLog implements IndexingOperationListener { }, Property.Dynamic, Property.IndexScope); IndexingSlowLog(IndexSettings indexSettings) { - this.indexLogger = Loggers.getLogger(INDEX_INDEXING_SLOWLOG_PREFIX + ".index", indexSettings.getSettings()); + this.indexLogger = LogManager.getLogger(INDEX_INDEXING_SLOWLOG_PREFIX + ".index"); this.index = indexSettings.getIndex(); indexSettings.getScopedSettings().addSettingsUpdateConsumer(INDEX_INDEXING_SLOWLOG_REFORMAT_SETTING, this::setReformat); diff --git a/server/src/main/java/org/elasticsearch/index/SearchSlowLog.java b/server/src/main/java/org/elasticsearch/index/SearchSlowLog.java index 10b4c4318a3..200d72f601d 100644 --- a/server/src/main/java/org/elasticsearch/index/SearchSlowLog.java +++ b/server/src/main/java/org/elasticsearch/index/SearchSlowLog.java @@ -20,6 +20,7 @@ package org.elasticsearch.index; import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.common.Strings; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Setting; @@ -82,8 +83,8 @@ public final class SearchSlowLog implements SearchOperationListener { public SearchSlowLog(IndexSettings indexSettings) { - this.queryLogger = Loggers.getLogger(INDEX_SEARCH_SLOWLOG_PREFIX + ".query", indexSettings.getSettings()); - this.fetchLogger = Loggers.getLogger(INDEX_SEARCH_SLOWLOG_PREFIX + ".fetch", indexSettings.getSettings()); + this.queryLogger = LogManager.getLogger(INDEX_SEARCH_SLOWLOG_PREFIX + ".query"); + this.fetchLogger = LogManager.getLogger(INDEX_SEARCH_SLOWLOG_PREFIX + ".fetch"); indexSettings.getScopedSettings().addSettingsUpdateConsumer(INDEX_SEARCH_SLOWLOG_THRESHOLD_QUERY_WARN_SETTING, this::setQueryWarnThreshold); this.queryWarnThreshold = indexSettings.getValue(INDEX_SEARCH_SLOWLOG_THRESHOLD_QUERY_WARN_SETTING).nanos(); diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index 96237b6e3de..330962e2167 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -300,7 +300,7 @@ public class Watcher extends Plugin implements ActionPlugin, ScriptPlugin, Reloa actionFactoryMap.put(EmailAction.TYPE, new EmailActionFactory(settings, emailService, templateEngine, emailAttachmentsParser)); actionFactoryMap.put(WebhookAction.TYPE, new WebhookActionFactory(settings, httpClient, templateEngine)); actionFactoryMap.put(IndexAction.TYPE, new IndexActionFactory(settings, client)); - actionFactoryMap.put(LoggingAction.TYPE, new LoggingActionFactory(settings, templateEngine)); + actionFactoryMap.put(LoggingAction.TYPE, new LoggingActionFactory(templateEngine)); actionFactoryMap.put(HipChatAction.TYPE, new HipChatActionFactory(settings, templateEngine, hipChatService)); actionFactoryMap.put(JiraAction.TYPE, new JiraActionFactory(settings, templateEngine, jiraService)); actionFactoryMap.put(SlackAction.TYPE, new SlackActionFactory(settings, templateEngine, slackService)); diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/logging/ExecutableLoggingAction.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/logging/ExecutableLoggingAction.java index 37f242ca499..b1cb723949d 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/logging/ExecutableLoggingAction.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/logging/ExecutableLoggingAction.java @@ -6,8 +6,7 @@ package org.elasticsearch.xpack.watcher.actions.logging; import org.apache.logging.log4j.Logger; -import org.elasticsearch.common.logging.Loggers; -import org.elasticsearch.common.settings.Settings; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.xpack.core.watcher.actions.Action; import org.elasticsearch.xpack.core.watcher.actions.ExecutableAction; import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext; @@ -22,9 +21,9 @@ public class ExecutableLoggingAction extends ExecutableAction { private final Logger textLogger; private final TextTemplateEngine templateEngine; - public ExecutableLoggingAction(LoggingAction action, Logger logger, Settings settings, TextTemplateEngine templateEngine) { + public ExecutableLoggingAction(LoggingAction action, Logger logger, TextTemplateEngine templateEngine) { super(action, logger); - this.textLogger = action.category != null ? Loggers.getLogger(action.category, settings) : logger; + this.textLogger = action.category != null ? LogManager.getLogger(action.category) : logger; this.templateEngine = templateEngine; } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/logging/LoggingActionFactory.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/logging/LoggingActionFactory.java index 44a8ace89e9..44bbbd4675a 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/logging/LoggingActionFactory.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/logging/LoggingActionFactory.java @@ -5,8 +5,7 @@ */ package org.elasticsearch.xpack.watcher.actions.logging; -import org.elasticsearch.common.logging.Loggers; -import org.elasticsearch.common.settings.Settings; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.xpack.core.watcher.actions.ActionFactory; import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine; @@ -15,18 +14,16 @@ import java.io.IOException; public class LoggingActionFactory extends ActionFactory { - private final Settings settings; private final TextTemplateEngine templateEngine; - public LoggingActionFactory(Settings settings, TextTemplateEngine templateEngine) { - super(Loggers.getLogger(ExecutableLoggingAction.class, settings)); - this.settings = settings; + public LoggingActionFactory(TextTemplateEngine templateEngine) { + super(LogManager.getLogger(ExecutableLoggingAction.class)); this.templateEngine = templateEngine; } @Override public ExecutableLoggingAction parseExecutable(String watchId, String actionId, XContentParser parser) throws IOException { LoggingAction action = LoggingAction.parse(watchId, actionId, parser); - return new ExecutableLoggingAction(action, actionLogger, settings, templateEngine); + return new ExecutableLoggingAction(action, actionLogger, templateEngine); } } diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/logging/LoggingActionTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/logging/LoggingActionTests.java index fdfd8ae0745..6ed6f524738 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/logging/LoggingActionTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/logging/LoggingActionTests.java @@ -8,7 +8,6 @@ package org.elasticsearch.xpack.watcher.actions.logging; import org.apache.logging.log4j.Logger; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.SuppressLoggerChecks; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.ESTestCase; @@ -92,8 +91,7 @@ public class LoggingActionTests extends ESTestCase { } public void testParser() throws Exception { - Settings settings = Settings.EMPTY; - LoggingActionFactory parser = new LoggingActionFactory(settings, engine); + LoggingActionFactory parser = new LoggingActionFactory(engine); String text = randomAlphaOfLength(10); TextTemplate template = new TextTemplate(text); @@ -126,14 +124,13 @@ public class LoggingActionTests extends ESTestCase { } public void testParserSelfGenerated() throws Exception { - Settings settings = Settings.EMPTY; - LoggingActionFactory parser = new LoggingActionFactory(settings, engine); + LoggingActionFactory parser = new LoggingActionFactory(engine); String text = randomAlphaOfLength(10); TextTemplate template = new TextTemplate(text); String category = randomAlphaOfLength(10); LoggingAction action = new LoggingAction(template, level, category); - ExecutableLoggingAction executable = new ExecutableLoggingAction(action, logger, settings, engine); + ExecutableLoggingAction executable = new ExecutableLoggingAction(action, logger, engine); XContentBuilder builder = jsonBuilder(); executable.toXContent(builder, Attachment.XContent.EMPTY_PARAMS); @@ -146,8 +143,7 @@ public class LoggingActionTests extends ESTestCase { } public void testParserBuilder() throws Exception { - Settings settings = Settings.EMPTY; - LoggingActionFactory parser = new LoggingActionFactory(settings, engine); + LoggingActionFactory parser = new LoggingActionFactory(engine); String text = randomAlphaOfLength(10); TextTemplate template = new TextTemplate(text); @@ -172,8 +168,7 @@ public class LoggingActionTests extends ESTestCase { } public void testParserFailure() throws Exception { - Settings settings = Settings.EMPTY; - LoggingActionFactory parser = new LoggingActionFactory(settings, engine); + LoggingActionFactory parser = new LoggingActionFactory(engine); XContentBuilder builder = jsonBuilder() .startObject().endObject(); diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/watch/WatchTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/watch/WatchTests.java index 8029b1c7a49..ae3066a3ee6 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/watch/WatchTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/watch/WatchTests.java @@ -438,7 +438,7 @@ public class WatchTests extends ESTestCase { private WatchParser createWatchparser() throws Exception { LoggingAction loggingAction = new LoggingAction(new TextTemplate("foo"), null, null); List actions = Collections.singletonList(new ActionWrapper("_logging_", randomThrottler(), null, null, - new ExecutableLoggingAction(loggingAction, logger, settings, new MockTextTemplateEngine()))); + new ExecutableLoggingAction(loggingAction, logger, new MockTextTemplateEngine()))); ScheduleRegistry scheduleRegistry = registry(new IntervalSchedule(new IntervalSchedule.Interval(1, IntervalSchedule.Interval.Unit.SECONDS))); @@ -622,7 +622,7 @@ public class WatchTests extends ESTestCase { parsers.put(WebhookAction.TYPE, new WebhookActionFactory(settings, httpClient, templateEngine)); break; case LoggingAction.TYPE: - parsers.put(LoggingAction.TYPE, new LoggingActionFactory(settings, new MockTextTemplateEngine())); + parsers.put(LoggingAction.TYPE, new LoggingActionFactory(new MockTextTemplateEngine())); break; } }