Fix deprecation logs throttling for deprecated routes (#73051)

So far when a deprecated route was executed it only emitted deprecation
warning once. All subsequent deprecated routes (even when path and
method were different) were throttled because the key was the same -
deprecated_route

This commit suffixes the deprecation key with path and method.

closes #73002

server/src/main/java/org/elasticsearch/rest/DeprecationRestHandler.java

@@ -25,12 +25,15 @@ public class DeprecationRestHandler implements RestHandler {
     private final String deprecationMessage;
     private final DeprecationLogger deprecationLogger;
     private final boolean compatibleVersionWarning;
+    private final String deprecationKey;
 
     /**
      * Create a {@link DeprecationRestHandler} that encapsulates the {@code handler} using the {@code deprecationLogger} to log
      * deprecation {@code warning}.
      *
      * @param handler The rest handler to deprecate (it's possible that the handler is reused with a different name!)
+     * @param method a method of a deprecated endpoint
+     * @param path a path of a deprecated endpoint
      * @param deprecationMessage The message to warn users with when they use the {@code handler}
      * @param deprecationLogger The deprecation logger
      * @param compatibleVersionWarning set to false so that a deprecation warning will be issued for the handled request,
@@ -39,12 +42,13 @@ public class DeprecationRestHandler implements RestHandler {
      * @throws NullPointerException if any parameter except {@code deprecationMessage} is {@code null}
      * @throws IllegalArgumentException if {@code deprecationMessage} is not a valid header
      */
-    public DeprecationRestHandler(RestHandler handler, String deprecationMessage, DeprecationLogger deprecationLogger,
-                                  boolean compatibleVersionWarning) {
+    public DeprecationRestHandler(RestHandler handler, RestRequest.Method method, String path, String deprecationMessage,
+                                  DeprecationLogger deprecationLogger, boolean compatibleVersionWarning) {
         this.handler = Objects.requireNonNull(handler);
         this.deprecationMessage = requireValidHeader(deprecationMessage);
         this.deprecationLogger = Objects.requireNonNull(deprecationLogger);
         this.compatibleVersionWarning = compatibleVersionWarning;
+        this.deprecationKey = DEPRECATED_ROUTE_KEY + "_" + method + "_" + path;
     }
 
     /**
@@ -55,9 +59,9 @@ public class DeprecationRestHandler implements RestHandler {
     @Override
     public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
         if (compatibleVersionWarning == false) {
-            deprecationLogger.deprecate(DeprecationCategory.API, DEPRECATED_ROUTE_KEY, deprecationMessage);
+            deprecationLogger.deprecate(DeprecationCategory.API, deprecationKey, deprecationMessage);
         } else {
-            deprecationLogger.compatibleApiWarning(DEPRECATED_ROUTE_KEY, deprecationMessage);
+            deprecationLogger.compatibleApiWarning(deprecationKey, deprecationMessage);
         }
 
         handler.handleRequest(request, channel, client);

server/src/main/java/org/elasticsearch/rest/RestController.java

@@ -112,10 +112,12 @@ public class RestController implements HttpServerTransport.Dispatcher {
         assert (handler instanceof DeprecationRestHandler) == false;
         if (version == RestApiVersion.current()) {
             // e.g. it was marked as deprecated in 8.x, and we're currently running 8.x
-            registerHandler(method, path, version, new DeprecationRestHandler(handler, deprecationMessage, deprecationLogger, false));
+            registerHandler(method, path, version,
+                new DeprecationRestHandler(handler, method, path, deprecationMessage, deprecationLogger, false));
         } else if (version == RestApiVersion.minimumSupported()) {
             // e.g. it was marked as deprecated in 7.x, and we're currently running 8.x
-            registerHandler(method, path, version, new DeprecationRestHandler(handler, deprecationMessage, deprecationLogger, true));
+            registerHandler(method, path, version,
+                new DeprecationRestHandler(handler, method, path, deprecationMessage, deprecationLogger, true));
         } else {
             // e.g. it was marked as deprecated in 7.x, and we're currently running *9.x*
             logger.debug("Deprecated route [" + method + " " + path + "] for handler [" + handler.getClass() + "] " +

server/src/test/java/org/elasticsearch/rest/DeprecationRestHandlerTests.java

@@ -34,6 +34,8 @@ public class DeprecationRestHandlerTests extends ESTestCase {
      */
     private final String deprecationMessage = randomAlphaOfLengthBetween(1, 30);
     private DeprecationLogger deprecationLogger;
+    private static final RestRequest.Method METHOD = RestRequest.Method.GET;
+    private static final String PATH = "/some/path";
 
     @Before
     public void setup() {
@@ -42,18 +44,19 @@ public class DeprecationRestHandlerTests extends ESTestCase {
     }
 
     public void testNullHandler() {
-        expectThrows(NullPointerException.class, () -> new DeprecationRestHandler(null, deprecationMessage, deprecationLogger, false));
+        expectThrows(NullPointerException.class, () -> new DeprecationRestHandler(null, METHOD, PATH, deprecationMessage,
+            deprecationLogger, false));
     }
 
     public void testInvalidDeprecationMessageThrowsException() {
         String invalidDeprecationMessage = randomFrom("", null, "     ");
 
         expectThrows(IllegalArgumentException.class,
-                     () -> new DeprecationRestHandler(handler, invalidDeprecationMessage, deprecationLogger, false));
+                     () -> new DeprecationRestHandler(handler, METHOD, PATH, invalidDeprecationMessage, deprecationLogger, false));
     }
 
     public void testNullDeprecationLogger() {
-        expectThrows(NullPointerException.class, () -> new DeprecationRestHandler(handler, deprecationMessage, null, false));
+        expectThrows(NullPointerException.class, () -> new DeprecationRestHandler(handler, METHOD, PATH, deprecationMessage, null, false));
     }
 
     public void testHandleRequestLogsThenForwards() throws Exception {
@@ -62,8 +65,8 @@ public class DeprecationRestHandlerTests extends ESTestCase {
             RestChannel channel = mock(RestChannel.class);
             NodeClient client = mock(NodeClient.class);
 
-            DeprecationRestHandler deprecatedHandler = new DeprecationRestHandler(handler, deprecationMessage, deprecationLogger,
-                compatibleVersionWarning);
+            DeprecationRestHandler deprecatedHandler = new DeprecationRestHandler(handler, METHOD, PATH, deprecationMessage,
+                deprecationLogger, compatibleVersionWarning);
 
             // test it
             deprecatedHandler.handleRequest(request, channel, client);
@@ -72,9 +75,11 @@ public class DeprecationRestHandlerTests extends ESTestCase {
 
             // log, then forward
             if (compatibleVersionWarning) {
-                inOrder.verify(deprecationLogger).compatibleApiWarning("deprecated_route", deprecationMessage);
+                inOrder.verify(deprecationLogger)
+                    .compatibleApiWarning("deprecated_route_GET_/some/path", deprecationMessage);
             } else {
-                inOrder.verify(deprecationLogger).deprecate(DeprecationCategory.API, "deprecated_route", deprecationMessage);
+                inOrder.verify(deprecationLogger)
+                    .deprecate(DeprecationCategory.API, "deprecated_route_GET_/some/path", deprecationMessage);
             }
 
             inOrder.verify(handler).handleRequest(request, channel, client);
@@ -124,12 +129,14 @@ public class DeprecationRestHandlerTests extends ESTestCase {
 
     public void testSupportsContentStreamTrue() {
         when(handler.supportsContentStream()).thenReturn(true);
-        assertTrue(new DeprecationRestHandler(handler, deprecationMessage, deprecationLogger, false).supportsContentStream());
+        assertTrue(new DeprecationRestHandler(handler, METHOD, PATH, deprecationMessage, deprecationLogger,
+            false).supportsContentStream());
     }
 
     public void testSupportsContentStreamFalse() {
         when(handler.supportsContentStream()).thenReturn(false);
-        assertFalse(new DeprecationRestHandler(handler, deprecationMessage, deprecationLogger, false).supportsContentStream());
+        assertFalse(new DeprecationRestHandler(handler, METHOD, PATH, deprecationMessage, deprecationLogger,
+            false).supportsContentStream());
     }
 
     /**

x-pack/plugin/deprecation/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/deprecation/DeprecationHttpIT.java

@@ -231,6 +231,85 @@ public class DeprecationHttpIT extends ESRestTestCase {
         }
     }
 
+    public void testDeprecationRouteThrottling() throws Exception {
+        try {
+            configureWriteDeprecationLogsToIndex(true);
+
+            final Request getRequest = createTestRequest("GET");
+            assertOK(client().performRequest(getRequest));
+
+            assertOK(client().performRequest(getRequest));
+
+            final Request postRequest = createTestRequest("POST");
+            assertOK(client().performRequest(postRequest));
+
+            assertBusy(() -> {
+                Response response;
+                try {
+                    client().performRequest(new Request("POST", "/" + DATA_STREAM_NAME + "/_refresh?ignore_unavailable=true"));
+                    response = client().performRequest(new Request("GET", "/" + DATA_STREAM_NAME + "/_search"));
+                } catch (Exception e) {
+                    // It can take a moment for the index to be created. If it doesn't exist then the client
+                    // throws an exception. Translate it into an assertion error so that assertBusy() will
+                    // continue trying.
+                    throw new AssertionError(e);
+                }
+                assertOK(response);
+
+                ObjectMapper mapper = new ObjectMapper();
+                final JsonNode jsonNode = mapper.readTree(response.getEntity().getContent());
+
+                final int hits = jsonNode.at("/hits/total/value").intValue();
+                assertThat(hits, greaterThan(0));
+
+                List<Map<String, Object>> documents = new ArrayList<>();
+
+                for (int i = 0; i < hits; i++) {
+                    final JsonNode hit = jsonNode.at("/hits/hits/" + i + "/_source");
+
+                    final Map<String, Object> document = new HashMap<>();
+                    hit.fields().forEachRemaining(entry -> document.put(entry.getKey(), entry.getValue().textValue()));
+
+                    documents.add(document);
+                }
+
+                logger.warn(documents);
+                assertThat(documents, hasSize(3));
+
+                assertThat(
+                    documents,
+                    hasItems(
+                        allOf(
+                            hasEntry(KEY_FIELD_NAME, "deprecated_route_POST_/_test_cluster/deprecated_settings"),
+                            hasEntry("message", "[/_test_cluster/deprecated_settings] exists for deprecated tests")
+                        ),
+                        allOf(
+                            hasEntry(KEY_FIELD_NAME, "deprecated_route_GET_/_test_cluster/deprecated_settings"),
+                            hasEntry("message", "[/_test_cluster/deprecated_settings] exists for deprecated tests")
+                        ),
+                        allOf(
+                            hasEntry(KEY_FIELD_NAME, "deprecated_settings"),
+                            hasEntry("message", "[deprecated_settings] usage is deprecated. use [settings] instead")
+                        )
+                    )
+                );
+            }, 30, TimeUnit.SECONDS);
+        } finally {
+            configureWriteDeprecationLogsToIndex(null);
+            client().performRequest(new Request("DELETE", "_data_stream/" + DATA_STREAM_NAME));
+        }
+    }
+
+    private Request createTestRequest(String method) throws IOException {
+        final Request getRequest = new Request(method, "/_test_cluster/deprecated_settings");
+        final RequestOptions options = getRequest.getOptions().toBuilder().addHeader("X-Opaque-Id", "some xid").build();
+        getRequest.setOptions(options);
+        getRequest.setEntity(
+            buildSettingsRequest(Collections.singletonList(TestDeprecationHeaderRestAction.TEST_DEPRECATED_SETTING_TRUE1), true)
+        );
+        return getRequest;
+    }
+
     /**
      * Check that deprecation messages can be recorded to an index
      */
@@ -312,7 +391,7 @@ public class DeprecationHttpIT extends ESRestTestCase {
                             hasEntry("data_stream.namespace", "default"),
                             hasEntry("data_stream.type", "logs"),
                             hasEntry("ecs.version", "1.7"),
-                            hasEntry(KEY_FIELD_NAME, "deprecated_route"),
+                            hasEntry(KEY_FIELD_NAME, "deprecated_route_GET_/_test_cluster/deprecated_settings"),
                             hasEntry("event.dataset", "deprecation.elasticsearch"),
                             hasEntry("log.level", "DEPRECATION"),
                             hasKey("log.logger"),
@@ -445,7 +524,7 @@ public class DeprecationHttpIT extends ESRestTestCase {
                             hasEntry("data_stream.namespace", "default"),
                             hasEntry("data_stream.type", "logs"),
                             hasEntry("ecs.version", "1.7"),
-                            hasEntry(KEY_FIELD_NAME, "deprecated_route"),
+                            hasEntry(KEY_FIELD_NAME, "deprecated_route_GET_/_test_cluster/deprecated_settings"),
                             hasEntry("event.dataset", "deprecation.elasticsearch"),
                             hasEntry("log.level", "DEPRECATION"),
                             hasKey("log.logger"),

x-pack/plugin/deprecation/qa/rest/src/main/java/org/elasticsearch/xpack/deprecation/TestDeprecationHeaderRestAction.java

@@ -25,6 +25,7 @@ import java.util.List;
 import java.util.Map;
 
 import static org.elasticsearch.rest.RestRequest.Method.GET;
+import static org.elasticsearch.rest.RestRequest.Method.POST;
 
 /**
  * Enables testing {@code DeprecationRestHandler} via integration tests by guaranteeing a deprecated REST endpoint.
@@ -85,7 +86,8 @@ public class TestDeprecationHeaderRestAction extends BaseRestHandler {
         return List.of(
             // note: RestApiVersion.current() is acceptable here because this is test code -- ordinary callers of `.deprecated(...)`
             // should use an actual version
-            Route.builder(GET, "/_test_cluster/deprecated_settings").deprecated(DEPRECATED_ENDPOINT, RestApiVersion.current()).build()
+            Route.builder(GET, "/_test_cluster/deprecated_settings").deprecated(DEPRECATED_ENDPOINT, RestApiVersion.current()).build(),
+            Route.builder(POST, "/_test_cluster/deprecated_settings").deprecated(DEPRECATED_ENDPOINT, RestApiVersion.current()).build()
         );
     }