Bound the size of cache in deprecation logger (#16724) (#16770)

The current implementation of the map used to de-duplicate deprecation log
messages can grow without bound. This adds a simple fixed limit to the data
structure tracking existing loggers. Once the limit is breached new loggers will
no longer log deprecation warnings. I also added a check to skip the tracking
if the deprecation logger is disabled.


(cherry picked from commit b1bf72f)

Signed-off-by: Andrew Ross <[email protected]>

CHANGELOG.md

@@ -56,6 +56,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix permissions error on scripted query against remote snapshot ([#16544](https://github.com/opensearch-project/OpenSearch/pull/16544))
 - Fix `doc_values` only (`index:false`) IP field searching for masks ([#16628](https://github.com/opensearch-project/OpenSearch/pull/16628))
 - Fix stale cluster state custom file deletion ([#16670](https://github.com/opensearch-project/OpenSearch/pull/16670))
+- Bound the size of cache in deprecation logger ([16702](https://github.com/opensearch-project/OpenSearch/issues/16702))
 
 ### Security
 

server/src/main/java/org/opensearch/common/logging/DeprecatedMessage.java

@@ -47,12 +47,17 @@
  */
 public class DeprecatedMessage extends OpenSearchLogMessage {
     public static final String X_OPAQUE_ID_FIELD_NAME = "x-opaque-id";
-    private static final Set<String> keys = ConcurrentHashMap.newKeySet();
+
+    // Arbitrary maximum size, should be much larger than unique number of
+    // loggers, but small relative to heap size.
+    static final int MAX_DEDUPE_CACHE_ENTRIES = 16_384;
+
+    private static final Set<String> keyDedupeCache = ConcurrentHashMap.newKeySet();
     private final String keyWithXOpaqueId;
 
     public DeprecatedMessage(String key, String xOpaqueId, String messagePattern, Object... args) {
         super(fieldMap(key, xOpaqueId), messagePattern, args);
-        this.keyWithXOpaqueId = new StringBuilder().append(key).append(xOpaqueId).toString();
+        this.keyWithXOpaqueId = key + xOpaqueId;
     }
 
     /**
@@ -62,7 +67,7 @@ public DeprecatedMessage(String key, String xOpaqueId, String messagePattern, Ob
      * Otherwise, a warning can be logged by some test and the upcoming test can be impacted by it.
      */
     public static void resetDeprecatedMessageForTests() {
-        keys.clear();
+        keyDedupeCache.clear();
     }
 
     private static Map<String, Object> fieldMap(String key, String xOpaqueId) {
@@ -77,6 +82,15 @@ private static Map<String, Object> fieldMap(String key, String xOpaqueId) {
     }
 
     public boolean isAlreadyLogged() {
-        return !keys.add(keyWithXOpaqueId);
+        if (keyDedupeCache.contains(keyWithXOpaqueId)) {
+            return true;
+        }
+        if (keyDedupeCache.size() >= MAX_DEDUPE_CACHE_ENTRIES) {
+            // Stop logging if max size is breached to avoid performance problems from
+            // excessive logging. The historical logs will be full of deprecation warnings
+            // at this point anyway.
+            return true;
+        }
+        return !keyDedupeCache.add(keyWithXOpaqueId);
     }
 }

server/src/main/java/org/opensearch/common/logging/DeprecationLogger.java

@@ -116,9 +116,12 @@ public DeprecationLoggerBuilder deprecate(final String key, final String msg, fi
     public class DeprecationLoggerBuilder {
 
         public DeprecationLoggerBuilder withDeprecation(String key, String msg, Object[] params) {
-            DeprecatedMessage deprecationMessage = new DeprecatedMessage(key, HeaderWarning.getXOpaqueId(), msg, params);
-            if (!deprecationMessage.isAlreadyLogged()) {
-                logger.log(DEPRECATION, deprecationMessage);
+            // Check if the logger is enabled to skip the overhead of deduplicating messages if the logger is disabled
+            if (logger.isEnabled(DEPRECATION)) {
+                DeprecatedMessage deprecationMessage = new DeprecatedMessage(key, HeaderWarning.getXOpaqueId(), msg, params);
+                if (!deprecationMessage.isAlreadyLogged()) {
+                    logger.log(DEPRECATION, deprecationMessage);
+                }
             }
             return this;
         }

server/src/test/java/org/opensearch/common/logging/DeprecationLoggerTests.java

@@ -69,4 +69,22 @@ public void testDuplicateLogMessages() {
         // assert that only unique warnings are logged
         assertWarnings("Deprecated message 1", "Deprecated message 2", "Deprecated message 3");
     }
+
+    public void testMaximumSizeOfCache() {
+        final int maxEntries = DeprecatedMessage.MAX_DEDUPE_CACHE_ENTRIES;
+        // Fill up the cache, asserting every message is new
+        for (int i = 0; i < maxEntries; i++) {
+            DeprecatedMessage message = new DeprecatedMessage("key-" + i, "message-" + i, "");
+            assertFalse(message.toString(), message.isAlreadyLogged());
+        }
+        // Do the same thing except assert every message has been seen
+        for (int i = 0; i < maxEntries; i++) {
+            DeprecatedMessage message = new DeprecatedMessage("key-" + i, "message-" + i, "");
+            assertTrue(message.toString(), message.isAlreadyLogged());
+        }
+        // Add one more new entry, asserting it will forever been seen as already logged (cache is full)
+        DeprecatedMessage message = new DeprecatedMessage("key-new", "message-new", "");
+        assertTrue(message.toString(), message.isAlreadyLogged());
+        assertTrue(message.toString(), message.isAlreadyLogged());
+    }
 }