Use LRU set to reduce repeat deprecation messages

This commit adds an LRU set to used to determine if a keyed deprecation
message should be written to the deprecation logs, or only added to the
response headers on the thread context.

Relates #25474

4 files changed, 50 insertions, 41 deletions

diff --git a/core/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java b/core/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java
index ee3e3c7490..3ed1d9d30a 100644
--- a/core/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java
+++ b/core/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java
@@ -31,8 +31,10 @@ import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeFormatterBuilder;
 import java.time.format.SignStyle;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
@@ -118,12 +120,32 @@ public class DeprecationLogger {
     }
 
     /**
-     * Logs a deprecated message.
+     * Logs a deprecation message, adding a formatted warning message as a response header on the thread context.
      */
     public void deprecated(String msg, Object... params) {
         deprecated(THREAD_CONTEXT, msg, params);
     }
 
+    // LRU set of keys used to determine if a deprecation message should be emitted to the deprecation logs
+    private Set<String> keys = Collections.newSetFromMap(Collections.synchronizedMap(new LinkedHashMap<String, Boolean>() {
+        @Override
+        protected boolean removeEldestEntry(final Map.Entry eldest) {
+            return size() > 128;
+        }
+    }));
+
+    /**
+     * Adds a formatted warning message as a response header on the thread context, and logs a deprecation message if the associated key has
+     * not recently been seen.
+     *
+     * @param key    the key used to determine if this deprecation should be logged
+     * @param msg    the message to log
+     * @param params parameters to the message
+     */
+    public void deprecatedAndMaybeLog(final String key, final String msg, final Object... params) {
+        deprecated(THREAD_CONTEXT, msg, keys.add(key), params);
+    }
+
     /*
      * RFC7234 specifies the warning format as warn-code <space> warn-agent <space> "warn-text" [<space> "warn-date"]. Here, warn-code is a
      * three-digit number with various standard warn codes specified. The warn code 299 is apt for our purposes as it represents a
@@ -270,8 +292,12 @@ public class DeprecationLogger {
      * @param message The deprecation message.
      * @param params The parameters used to fill in the message, if any exist.
      */
-    @SuppressLoggerChecks(reason = "safely delegates to logger")
     void deprecated(final Set<ThreadContext> threadContexts, final String message, final Object... params) {
+        deprecated(threadContexts, message, true, params);
+    }
+
+    @SuppressLoggerChecks(reason = "safely delegates to logger")
+    void deprecated(final Set<ThreadContext> threadContexts, final String message, final boolean log, final Object... params) {
         final Iterator<ThreadContext> iterator = threadContexts.iterator();
 
         if (iterator.hasNext()) {
@@ -287,8 +313,9 @@ public class DeprecationLogger {
                     // ignored; it should be removed shortly
                 }
             }
-            logger.warn(formattedMessage);
-        } else {
+        }
+
+        if (log) {
             logger.warn(message, params);
         }
     }
diff --git a/core/src/main/java/org/elasticsearch/common/settings/Setting.java b/core/src/main/java/org/elasticsearch/common/settings/Setting.java
index 241315144a..b69da68a7e 100644
--- a/core/src/main/java/org/elasticsearch/common/settings/Setting.java
+++ b/core/src/main/java/org/elasticsearch/common/settings/Setting.java
@@ -19,6 +19,7 @@
 package org.elasticsearch.common.settings;
 
 import org.apache.logging.log4j.Logger;
+import org.apache.lucene.util.SetOnce;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.action.support.ToXContentToBytes;
@@ -342,14 +343,27 @@ public class Setting<T> extends ToXContentToBytes {
         return settings.get(getKey(), defaultValue.apply(settings));
     }
 
+    private static SetOnce<DeprecationLogger> deprecationLogger = new SetOnce<>();
+
+    // we have to initialize lazily otherwise a logger would be constructed before logging is initialized
+    private static synchronized DeprecationLogger getDeprecationLogger() {
+        if (deprecationLogger.get() == null) {
+            deprecationLogger.set(new DeprecationLogger(Loggers.getLogger(Settings.class)));
+        }
+        return deprecationLogger.get();
+    }
+
     /** Logs a deprecation warning if the setting is deprecated and used. */
-    protected void checkDeprecation(Settings settings) {
+    void checkDeprecation(Settings settings) {
         // They're using the setting, so we need to tell them to stop
-        if (this.isDeprecated() && this.exists(settings) && settings.addDeprecatedSetting(this)) {
+        if (this.isDeprecated() && this.exists(settings)) {
             // It would be convenient to show its replacement key, but replacement is often not so simple
-            final DeprecationLogger deprecationLogger = new DeprecationLogger(Loggers.getLogger(getClass()));
-            deprecationLogger.deprecated("[{}] setting was deprecated in Elasticsearch and will be removed in a future release! " +
-                "See the breaking changes documentation for the next major version.", getKey());
+            final String key = getKey();
+            getDeprecationLogger().deprecatedAndMaybeLog(
+                    key,
+                    "[{}] setting was deprecated in Elasticsearch and will be removed in a future release! "
+                            + "See the breaking changes documentation for the next major version.",
+                    key);
         }
     }
 
diff --git a/core/src/main/java/org/elasticsearch/common/settings/Settings.java b/core/src/main/java/org/elasticsearch/common/settings/Settings.java
index 8412f57fd8..1dfaccaf6d 100644
--- a/core/src/main/java/org/elasticsearch/common/settings/Settings.java
+++ b/core/src/main/java/org/elasticsearch/common/settings/Settings.java
@@ -94,22 +94,6 @@ public final class Settings implements ToXContent {
     private final SetOnce<Set<String>> firstLevelNames = new SetOnce<>();
 
     /**
-     * The set of deprecated settings tracked by this settings object.
-     */
-    private final Set<String> deprecatedSettings = Collections.newSetFromMap(new ConcurrentHashMap<>());
-
-    /**
-     * Add the setting as a tracked deprecated setting.
-     *
-     * @param setting the deprecated setting to track
-     * @return true if the setting was not already tracked as a deprecated setting, otherwise false
-     */
-    boolean addDeprecatedSetting(final Setting setting) {
-        assert setting.isDeprecated() && setting.exists(this) : setting.getKey();
-        return deprecatedSettings.add(setting.getKey());
-    }
-
-    /**
      * Setting names found in this Settings for both string and secure settings.
      * This is constructed lazily in {@link #keySet()}.
      */
diff --git a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java
index 1ac94b6caa..0bb1abb37a 100644
--- a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java
+++ b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java
@@ -154,22 +154,6 @@ public class SettingTests extends ESTestCase {
         assertNull(ab2.get());
     }
 
-    public void testDeprecatedSetting() {
-        final Setting<Boolean> deprecatedSetting = Setting.boolSetting("deprecated.foo.bar", false, Property.Deprecated);
-        final Settings settings = Settings.builder().put("deprecated.foo.bar", true).build();
-        final int iterations = randomIntBetween(0, 128);
-        for (int i = 0; i < iterations; i++) {
-            deprecatedSetting.get(settings);
-        }
-        if (iterations > 0) {
-            /*
-             * This tests that we log the deprecation warning exactly one time, otherwise we would have to assert the deprecation warning
-             * for each usage of the setting.
-             */
-            assertSettingDeprecationsAndWarnings(new Setting[]{deprecatedSetting});
-        }
-    }
-
     public void testDefault() {
         TimeValue defaultValue = TimeValue.timeValueMillis(randomIntBetween(0, 1000000));
         Setting<TimeValue> setting =