Skip to content

Commit

Permalink
Logging: Further clean up logging ctors (#33378)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nik9000 committed Sep 5, 2018
1 parent 46ac8d1 commit 5c624bc
Show file tree
Hide file tree
Showing 11 changed files with 29 additions and 42 deletions.
18 changes: 6 additions & 12 deletions server/src/main/java/org/elasticsearch/common/logging/Loggers.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ public IndexSettings(final IndexMetaData indexMetaData, final Settings nodeSetti
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -22,9 +21,9 @@ public class ExecutableLoggingAction extends ExecutableAction<LoggingAction> {
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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -15,18 +14,16 @@

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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
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;
Expand Down Expand Up @@ -92,8 +91,7 @@ public void testExecute() throws Exception {
}

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);
Expand Down Expand Up @@ -126,14 +124,13 @@ public void testParser() throws Exception {
}

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);

Expand All @@ -146,8 +143,7 @@ public void testParserSelfGenerated() throws Exception {
}

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);
Expand All @@ -172,8 +168,7 @@ public void testParserBuilder() throws Exception {
}

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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ public void testParseWatchWithoutTriggerDoesNotWork() throws Exception {
private WatchParser createWatchparser() throws Exception {
LoggingAction loggingAction = new LoggingAction(new TextTemplate("foo"), null, null);
List<ActionWrapper> 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)));
Expand Down Expand Up @@ -622,7 +622,7 @@ private ActionRegistry registry(List<ActionWrapper> actions, ConditionRegistry c
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;
}
}
Expand Down

0 comments on commit 5c624bc

Please sign in to comment.