Skip to content

Commit

Permalink
Log index name when updating index settings (#49969)
Browse files Browse the repository at this point in the history
Today we log changes to index settings like this:

    updating [index.setting.blah] from [A] to [B]

The identity of the index whose settings were updated is conspicuously absent
from this message. This commit addresses this by adding the index name to these
messages.

Fixes #49818.
  • Loading branch information
kkewwei authored and DaveCTurner committed Jan 3, 2020
1 parent d466efb commit 5655d6a
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public abstract class AbstractScopedSettings {
private static final Pattern GROUP_KEY_PATTERN = Pattern.compile("^(?:[-\\w]+[.])+$");
private static final Pattern AFFIX_KEY_PATTERN = Pattern.compile("^(?:[-\\w]+[.])+[*](?:[.][-\\w]+)+$");

protected final Logger logger = LogManager.getLogger(this.getClass());
private final Logger logger;

private final Settings settings;
private final List<SettingUpdater<?>> settingUpdaters = new CopyOnWriteArrayList<>();
Expand All @@ -70,6 +70,7 @@ protected AbstractScopedSettings(
final Set<Setting<?>> settingsSet,
final Set<SettingUpgrader<?>> settingUpgraders,
final Setting.Property scope) {
this.logger = LogManager.getLogger(this.getClass());
this.settings = settings;
this.lastSettingsApplied = Settings.EMPTY;

Expand Down Expand Up @@ -110,7 +111,8 @@ protected void validateSettingKey(Setting<?> setting) {
}
}

protected AbstractScopedSettings(Settings nodeSettings, Settings scopeSettings, AbstractScopedSettings other) {
protected AbstractScopedSettings(Settings nodeSettings, Settings scopeSettings, AbstractScopedSettings other, Logger logger) {
this.logger = logger;
this.settings = nodeSettings;
this.lastSettingsApplied = scopeSettings;
this.scope = other.scope;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.cluster.routing.allocation.decider.EnableAllocationDecider;
import org.elasticsearch.cluster.routing.allocation.decider.MaxRetryAllocationDecider;
import org.elasticsearch.cluster.routing.allocation.decider.ShardsLimitAllocationDecider;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.index.IndexModule;
import org.elasticsearch.index.IndexSettings;
Expand Down Expand Up @@ -190,7 +191,7 @@ public IndexScopedSettings(Settings settings, Set<Setting<?>> settingsSet) {
}

private IndexScopedSettings(Settings settings, IndexScopedSettings other, IndexMetaData metaData) {
super(settings, metaData.getSettings(), other);
super(settings, metaData.getSettings(), other, Loggers.getLogger(IndexScopedSettings.class, metaData.getIndex()));
}

public IndexScopedSettings copy(Settings settings, IndexMetaData metaData) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,23 @@
*/
package org.elasticsearch.common.settings;

import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.core.LogEvent;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.AbstractScopedSettings.SettingUpdater;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.unit.ByteSizeUnit;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.monitor.jvm.JvmInfo;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.MockLogAppender;
import org.elasticsearch.test.junit.annotations.TestLogging;

import java.util.Arrays;
import java.util.Collections;
Expand All @@ -40,6 +49,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.elasticsearch.index.IndexSettingsTests.newIndexMeta;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
Expand Down Expand Up @@ -1085,4 +1095,35 @@ public void testNonSecureSettingInKeystore() {
assertThat(e.getMessage(), containsString("must be stored inside elasticsearch.yml"));
}

@TestLogging(value="org.elasticsearch.common.settings.IndexScopedSettings:INFO",
reason="to ensure we log INFO-level messages from IndexScopedSettings")
public void testLogSettingUpdate() throws Exception {
final IndexMetaData metaData = newIndexMeta("index1",
Settings.builder().put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "20s").build());
final IndexSettings settings = new IndexSettings(metaData, Settings.EMPTY);

final MockLogAppender mockLogAppender = new MockLogAppender();
mockLogAppender.addExpectation(new MockLogAppender.SeenEventExpectation(
"message",
"org.elasticsearch.common.settings.IndexScopedSettings",
Level.INFO,
"updating [index.refresh_interval] from [20s] to [10s]") {
@Override
public boolean innerMatch(LogEvent event) {
return event.getMarker().getName().equals(" [index1]");
}
});
mockLogAppender.start();
final Logger logger = LogManager.getLogger(IndexScopedSettings.class);
try {
Loggers.addAppender(logger, mockLogAppender);
settings.updateIndexMetaData(newIndexMeta("index1",
Settings.builder().put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "10s").build()));

mockLogAppender.assertAllExpectationsMatched();
} finally {
Loggers.removeAppender(logger, mockLogAppender);
mockLogAppender.stop();
}
}
}

0 comments on commit 5655d6a

Please sign in to comment.