Skip to content

Commit

Permalink
Getting deprecation.skip_deprecated_settings to work with dynamic set…
Browse files Browse the repository at this point in the history
…tings (#81836) (#84523) (#84527)

Previously, deprecation.skip_deprecated_settings did not work with dynamic settings. The reason is that when
the deprecation check was done, only the current settings were available. When the setting is a node setting that is
fine because deprecation.skip_deprecated_settings is also a node setting. However when the setting is dynamic,
deprecation.skip_deprecated_settings is not in the Settings object.
  • Loading branch information
masseyke authored Mar 1, 2022
1 parent 78c51d5 commit 24554c5
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.settings.Settings;

import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Collections;
import java.util.List;

/**
* A logger that logs deprecation notices. Logger should be initialized with a class or name which will be used
Expand All @@ -35,7 +39,7 @@ public class DeprecationLogger {
* More serious that WARN by 1, but less serious than ERROR
*/
public static Level CRITICAL = Level.forName("CRITICAL", Level.WARN.intLevel() - 1);

private static volatile List<String> skipTheseDeprecations = Collections.emptyList();
private final Logger logger;

/**
Expand All @@ -56,6 +60,19 @@ public static DeprecationLogger getLogger(String name) {
return new DeprecationLogger(name);
}

/**
* The DeprecationLogger uses the "deprecation.skip_deprecated_settings" setting to decide whether to log a deprecation for a setting.
* This is a node setting. This method initializes the DeprecationLogger class with the node settings for the node in order to read the
* "deprecation.skip_deprecated_settings" setting. This only needs to be called once per JVM. If it is not called, the default behavior
* is to assume that the "deprecation.skip_deprecated_settings" setting is not set.
* @param nodeSettings The settings for this node
*/
public static void initialize(Settings nodeSettings) {
skipTheseDeprecations = nodeSettings == null
? Collections.emptyList()
: nodeSettings.getAsList("deprecation.skip_deprecated_settings");
}

private DeprecationLogger(String parentLoggerName) {
this.logger = LogManager.getLogger(getLoggerName(parentLoggerName));
}
Expand Down Expand Up @@ -95,12 +112,14 @@ public DeprecationLogger warn(final DeprecationCategory category, final String k
}

private DeprecationLogger logDeprecation(Level level, DeprecationCategory category, String key, String msg, Object[] params) {
assert category != DeprecationCategory.COMPATIBLE_API
: "DeprecationCategory.COMPATIBLE_API should be logged with compatibleApiWarning method";
String opaqueId = HeaderWarning.getXOpaqueId();
String productOrigin = HeaderWarning.getProductOrigin();
ESLogMessage deprecationMessage = DeprecatedMessage.of(category, key, opaqueId, productOrigin, msg, params);
doPrivilegedLog(level, deprecationMessage);
if (Regex.simpleMatch(skipTheseDeprecations, key) == false) {
assert category != DeprecationCategory.COMPATIBLE_API
: "DeprecationCategory.COMPATIBLE_API should be logged with compatibleApiWarning method";
String opaqueId = HeaderWarning.getXOpaqueId();
String productOrigin = HeaderWarning.getProductOrigin();
ESLogMessage deprecationMessage = DeprecatedMessage.of(category, key, opaqueId, productOrigin, msg, params);
doPrivilegedLog(level, deprecationMessage);
}
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,14 +577,11 @@ void checkDeprecation(Settings settings) {
if (this.isDeprecated() && this.exists(settings)) {
// It would be convenient to show its replacement key, but replacement is often not so simple
final String key = getKey();
List<String> skipTheseDeprecations = settings.getAsList("deprecation.skip_deprecated_settings");
if (Regex.simpleMatch(skipTheseDeprecations, key) == false) {
String message = "[{}] setting was deprecated in Elasticsearch and will be removed in a future release.";
if (this.isDeprecatedWarningOnly()) {
Settings.DeprecationLoggerHolder.deprecationLogger.warn(DeprecationCategory.SETTINGS, key, message, key);
} else {
Settings.DeprecationLoggerHolder.deprecationLogger.critical(DeprecationCategory.SETTINGS, key, message, key);
}
String message = "[{}] setting was deprecated in Elasticsearch and will be removed in a future release.";
if (this.isDeprecatedWarningOnly()) {
Settings.DeprecationLoggerHolder.deprecationLogger.warn(DeprecationCategory.SETTINGS, key, message, key);
} else {
Settings.DeprecationLoggerHolder.deprecationLogger.critical(DeprecationCategory.SETTINGS, key, message, key);
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions server/src/main/java/org/elasticsearch/node/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,8 @@ protected Node(
final List<Closeable> resourcesToClose = new ArrayList<>(); // register everything we need to release in the case of an error
boolean success = false;
try {
// Pass the node settings to the DeprecationLogger class so that it can have the deprecation.skip_deprecated_settings setting:
DeprecationLogger.initialize(initialEnvironment.settings());
Settings tmpSettings = Settings.builder()
.put(initialEnvironment.settings())
.put(Client.CLIENT_TYPE_SETTING_S.getKey(), CLIENT_TYPE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.core.LogEvent;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.AbstractScopedSettings.SettingUpdater;
import org.elasticsearch.common.settings.Setting.Property;
Expand Down Expand Up @@ -1396,7 +1397,7 @@ public void testCheckForDeprecation() {
}

public void testCheckForDeprecationWithSkipSetting() {
final String settingName = "foo.bar";
final String settingName = "foo.bar.hide.this";
final String settingValue = "blat";
final Setting<String> setting = Setting.simpleString(settingName, settingValue);
final Settings settings = Settings.builder().put(settingName, settingValue).build();
Expand All @@ -1409,6 +1410,7 @@ public void testCheckForDeprecationWithSkipSetting() {
.put(settingName, settingValue)
.putList("deprecation.skip_deprecated_settings", settingName)
.build();
DeprecationLogger.initialize(settingsWithSkipDeprecationSetting);
deprecatedSetting.checkDeprecation(settingsWithSkipDeprecationSetting);
ensureNoWarnings();
}
Expand Down

0 comments on commit 24554c5

Please sign in to comment.