瀏覽代碼

Allow specifying deprecation level for REST deprecation handler (#80629)

This allows specifying an optional level to be used for the deprecation handler registering a route.

The default remains if the level is unspecified.

Relates to #80167
Lee Hinman 3 年之前
父節點
當前提交
debb882e0d

+ 12 - 2
server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java

@@ -107,10 +107,20 @@ public class DeprecationLogger {
      * so that it can be returned to the client.
      */
     public DeprecationLogger compatibleCritical(final String key, final String msg, final Object... params) {
+        return compatible(CRITICAL, key, msg, params);
+    }
+
+    /**
+     * Used for handling previous version RestApiCompatible logic.
+     * Logs a message at the given level
+     * that has been broken in previous version.
+     * The message is also sent to the header warning logger,
+     * so that it can be returned to the client.
+     */
+    public DeprecationLogger compatible(final Level level, final String key, final String msg, final Object... params) {
         String opaqueId = HeaderWarning.getXOpaqueId();
         ESLogMessage deprecationMessage = DeprecatedMessage.compatibleDeprecationMessage(key, opaqueId, msg, params);
-        logger.log(CRITICAL, deprecationMessage);
+        logger.log(level, deprecationMessage);
         return this;
     }
-
 }

+ 24 - 2
server/src/main/java/org/elasticsearch/rest/DeprecationRestHandler.java

@@ -7,10 +7,12 @@
  */
 package org.elasticsearch.rest;
 
+import org.apache.logging.log4j.Level;
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.logging.DeprecationCategory;
 import org.elasticsearch.common.logging.DeprecationLogger;
+import org.elasticsearch.core.Nullable;
 
 import java.util.Objects;
 
@@ -26,6 +28,8 @@ public class DeprecationRestHandler implements RestHandler {
     private final DeprecationLogger deprecationLogger;
     private final boolean compatibleVersionWarning;
     private final String deprecationKey;
+    @Nullable
+    private final Level deprecationLevel;
 
     /**
      * Create a {@link DeprecationRestHandler} that encapsulates the {@code handler} using the {@code deprecationLogger} to log
@@ -46,6 +50,7 @@ public class DeprecationRestHandler implements RestHandler {
         RestHandler handler,
         RestRequest.Method method,
         String path,
+        @Nullable Level deprecationLevel,
         String deprecationMessage,
         DeprecationLogger deprecationLogger,
         boolean compatibleVersionWarning
@@ -55,6 +60,12 @@ public class DeprecationRestHandler implements RestHandler {
         this.deprecationLogger = Objects.requireNonNull(deprecationLogger);
         this.compatibleVersionWarning = compatibleVersionWarning;
         this.deprecationKey = DEPRECATED_ROUTE_KEY + "_" + method + "_" + path;
+        if (deprecationLevel != null && (deprecationLevel != Level.WARN && deprecationLevel != DeprecationLogger.CRITICAL)) {
+            throw new IllegalArgumentException(
+                "unexpected deprecation logger level: " + deprecationLevel + ", expected either 'CRITICAL' or 'WARN'"
+            );
+        }
+        this.deprecationLevel = deprecationLevel;
     }
 
     /**
@@ -65,9 +76,20 @@ public class DeprecationRestHandler implements RestHandler {
     @Override
     public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
         if (compatibleVersionWarning == false) {
-            deprecationLogger.warn(DeprecationCategory.API, deprecationKey, deprecationMessage);
+            // The default value for deprecated requests without a version warning is WARN
+            if (deprecationLevel == null || deprecationLevel == Level.WARN) {
+                deprecationLogger.warn(DeprecationCategory.API, deprecationKey, deprecationMessage);
+            } else {
+                deprecationLogger.critical(DeprecationCategory.API, deprecationKey, deprecationMessage);
+            }
         } else {
-            deprecationLogger.compatibleCritical(deprecationKey, deprecationMessage);
+            // The default value for deprecated requests with a version warning is CRITICAL,
+            // because they have a specific version where the endpoint is removed
+            if (deprecationLevel == null || deprecationLevel == DeprecationLogger.CRITICAL) {
+                deprecationLogger.compatibleCritical(deprecationKey, deprecationMessage);
+            } else {
+                deprecationLogger.compatible(Level.WARN, deprecationKey, deprecationMessage);
+            }
         }
 
         handler.handleRequest(request, channel, client);

+ 26 - 3
server/src/main/java/org/elasticsearch/rest/RestController.java

@@ -8,6 +8,7 @@
 
 package org.elasticsearch.rest;
 
+import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
@@ -124,6 +125,27 @@ public class RestController implements HttpServerTransport.Dispatcher {
         RestApiVersion version,
         RestHandler handler,
         String deprecationMessage
+    ) {
+        registerAsDeprecatedHandler(method, path, version, handler, deprecationMessage, null);
+    }
+
+    /**
+     * Registers a REST handler to be executed when the provided {@code method} and {@code path} match the request.
+     *
+     * @param method GET, POST, etc.
+     * @param path Path to handle (e.g. "/{index}/{type}/_bulk")
+     * @param version API version to handle (e.g. RestApiVersion.V_8)
+     * @param handler The handler to actually execute
+     * @param deprecationMessage The message to log and send as a header in the response
+     * @param deprecationLevel The deprecation log level to use for the deprecation warning, either WARN or CRITICAL
+     */
+    protected void registerAsDeprecatedHandler(
+        RestRequest.Method method,
+        String path,
+        RestApiVersion version,
+        RestHandler handler,
+        String deprecationMessage,
+        @Nullable Level deprecationLevel
     ) {
         assert (handler instanceof DeprecationRestHandler) == false;
         if (version == RestApiVersion.current()) {
@@ -132,7 +154,7 @@ public class RestController implements HttpServerTransport.Dispatcher {
                 method,
                 path,
                 version,
-                new DeprecationRestHandler(handler, method, path, deprecationMessage, deprecationLogger, false)
+                new DeprecationRestHandler(handler, method, path, deprecationLevel, 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
@@ -140,7 +162,7 @@ public class RestController implements HttpServerTransport.Dispatcher {
                 method,
                 path,
                 version,
-                new DeprecationRestHandler(handler, method, path, deprecationMessage, deprecationLogger, true)
+                new DeprecationRestHandler(handler, method, path, deprecationLevel, deprecationMessage, deprecationLogger, true)
             );
         } else {
             // e.g. it was marked as deprecated in 7.x, and we're currently running *9.x*
@@ -255,7 +277,8 @@ public class RestController implements HttpServerTransport.Dispatcher {
                 route.getPath(),
                 route.getRestApiVersion(),
                 handler,
-                route.getDeprecationMessage()
+                route.getDeprecationMessage(),
+                route.getDeprecationLevel()
             );
         } else {
             // it's just a normal route

+ 48 - 6
server/src/main/java/org/elasticsearch/rest/RestHandler.java

@@ -8,7 +8,9 @@
 
 package org.elasticsearch.rest;
 
+import org.apache.logging.log4j.Level;
 import org.elasticsearch.client.node.NodeClient;
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.RestApiVersion;
 import org.elasticsearch.rest.RestRequest.Method;
 import org.elasticsearch.xcontent.MediaType;
@@ -85,10 +87,19 @@ public interface RestHandler {
         private final RestApiVersion restApiVersion;
 
         private final String deprecationMessage;
+        @Nullable
+        private final Level deprecationLevel;
 
         private final Route replacedRoute;
 
-        private Route(Method method, String path, RestApiVersion restApiVersion, String deprecationMessage, Route replacedRoute) {
+        private Route(
+            Method method,
+            String path,
+            RestApiVersion restApiVersion,
+            String deprecationMessage,
+            Level deprecationLevel,
+            Route replacedRoute
+        ) {
             this.method = Objects.requireNonNull(method);
             this.path = Objects.requireNonNull(path);
             this.restApiVersion = Objects.requireNonNull(restApiVersion);
@@ -96,6 +107,7 @@ public interface RestHandler {
             // a deprecated route will have a deprecation message, and the restApiVersion
             // will represent the version when the route was deprecated
             this.deprecationMessage = deprecationMessage;
+            this.deprecationLevel = deprecationLevel;
 
             // a route that replaces another route will have a reference to the route that was replaced
             this.replacedRoute = replacedRoute;
@@ -110,7 +122,7 @@ public interface RestHandler {
          * @param path   the path, e.g. "/"
          */
         public Route(Method method, String path) {
-            this(method, path, RestApiVersion.current(), null, null);
+            this(method, path, RestApiVersion.current(), null, null, null);
         }
 
         public static class RouteBuilder {
@@ -120,6 +132,8 @@ public interface RestHandler {
             private RestApiVersion restApiVersion;
 
             private String deprecationMessage;
+            @Nullable
+            private Level deprecationLevel;
 
             private Route replacedRoute;
 
@@ -150,6 +164,29 @@ public interface RestHandler {
                 return this;
             }
 
+            /**
+             * Marks that the route being built has been deprecated (for some reason -- the deprecationMessage), and notes the major
+             * version in which that deprecation occurred.
+             * <p>
+             * For example:
+             * <pre> {@code
+             * Route.builder(GET, "_upgrade")
+             *  .deprecated("The _upgrade API is no longer useful and will be removed.", RestApiVersion.V_7)
+             *  .build()}</pre>
+             *
+             * @param deprecationMessage the user-visible explanation of this deprecation
+             * @param deprecationLevel the level at which to log the deprecation
+             * @param deprecatedInVersion the major version in which the deprecation occurred
+             * @return a reference to this object.
+             */
+            public RouteBuilder deprecated(String deprecationMessage, Level deprecationLevel, RestApiVersion deprecatedInVersion) {
+                assert this.replacedRoute == null;
+                this.restApiVersion = Objects.requireNonNull(deprecatedInVersion);
+                this.deprecationMessage = Objects.requireNonNull(deprecationMessage);
+                this.deprecationLevel = deprecationLevel;
+                return this;
+            }
+
             /**
              * Marks that the route being built replaces another route, and notes the major version in which that replacement occurred.
              * <p>
@@ -165,18 +202,18 @@ public interface RestHandler {
              */
             public RouteBuilder replaces(Method method, String path, RestApiVersion replacedInVersion) {
                 assert this.deprecationMessage == null;
-                this.replacedRoute = new Route(method, path, replacedInVersion, null, null);
+                this.replacedRoute = new Route(method, path, replacedInVersion, null, null, null);
                 return this;
             }
 
             public Route build() {
                 if (replacedRoute != null) {
-                    return new Route(method, path, restApiVersion, null, replacedRoute);
+                    return new Route(method, path, restApiVersion, null, null, replacedRoute);
                 } else if (deprecationMessage != null) {
-                    return new Route(method, path, restApiVersion, deprecationMessage, null);
+                    return new Route(method, path, restApiVersion, deprecationMessage, deprecationLevel, null);
                 } else {
                     // this is a little silly, but perfectly legal
-                    return new Route(method, path, restApiVersion, null, null);
+                    return new Route(method, path, restApiVersion, null, null, null);
                 }
             }
         }
@@ -201,6 +238,11 @@ public interface RestHandler {
             return deprecationMessage;
         }
 
+        @Nullable
+        public Level getDeprecationLevel() {
+            return deprecationLevel;
+        }
+
         public boolean isDeprecated() {
             return deprecationMessage != null;
         }

+ 3 - 2
server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutIndexTemplateAction.java

@@ -8,6 +8,7 @@
 
 package org.elasticsearch.rest.action.admin.indices;
 
+import org.apache.logging.log4j.Level;
 import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateRequest;
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.common.Strings;
@@ -38,8 +39,8 @@ public class RestPutIndexTemplateAction extends BaseRestHandler {
     @Override
     public List<Route> routes() {
         return List.of(
-            Route.builder(POST, "/_template/{name}").deprecated(DEPRECATION_WARNING, DEPRECATION_VERSION).build(),
-            Route.builder(PUT, "/_template/{name}").deprecated(DEPRECATION_WARNING, DEPRECATION_VERSION).build()
+            Route.builder(POST, "/_template/{name}").deprecated(DEPRECATION_WARNING, Level.WARN, DEPRECATION_VERSION).build(),
+            Route.builder(PUT, "/_template/{name}").deprecated(DEPRECATION_WARNING, Level.WARN, DEPRECATION_VERSION).build()
         );
     }
 

+ 25 - 7
server/src/test/java/org/elasticsearch/rest/DeprecationRestHandlerTests.java

@@ -9,6 +9,7 @@ package org.elasticsearch.rest;
 
 import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator;
 
+import org.apache.logging.log4j.Level;
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.common.logging.DeprecationCategory;
 import org.elasticsearch.common.logging.DeprecationLogger;
@@ -45,7 +46,7 @@ public class DeprecationRestHandlerTests extends ESTestCase {
     public void testNullHandler() {
         expectThrows(
             NullPointerException.class,
-            () -> new DeprecationRestHandler(null, METHOD, PATH, deprecationMessage, deprecationLogger, false)
+            () -> new DeprecationRestHandler(null, METHOD, PATH, null, deprecationMessage, deprecationLogger, false)
         );
     }
 
@@ -54,12 +55,15 @@ public class DeprecationRestHandlerTests extends ESTestCase {
 
         expectThrows(
             IllegalArgumentException.class,
-            () -> new DeprecationRestHandler(handler, METHOD, PATH, invalidDeprecationMessage, deprecationLogger, false)
+            () -> new DeprecationRestHandler(handler, METHOD, PATH, null, invalidDeprecationMessage, deprecationLogger, false)
         );
     }
 
     public void testNullDeprecationLogger() {
-        expectThrows(NullPointerException.class, () -> new DeprecationRestHandler(handler, METHOD, PATH, deprecationMessage, null, false));
+        expectThrows(
+            NullPointerException.class,
+            () -> new DeprecationRestHandler(handler, METHOD, PATH, null, deprecationMessage, null, false)
+        );
     }
 
     public void testHandleRequestLogsThenForwards() throws Exception {
@@ -68,10 +72,13 @@ public class DeprecationRestHandlerTests extends ESTestCase {
             RestChannel channel = mock(RestChannel.class);
             NodeClient client = mock(NodeClient.class);
 
+            final Level deprecationLevel = randomBoolean() ? null : randomFrom(Level.WARN, DeprecationLogger.CRITICAL);
+
             DeprecationRestHandler deprecatedHandler = new DeprecationRestHandler(
                 handler,
                 METHOD,
                 PATH,
+                deprecationLevel,
                 deprecationMessage,
                 deprecationLogger,
                 compatibleVersionWarning
@@ -84,9 +91,18 @@ public class DeprecationRestHandlerTests extends ESTestCase {
 
             // log, then forward
             if (compatibleVersionWarning) {
-                inOrder.verify(deprecationLogger).compatibleCritical("deprecated_route_GET_/some/path", deprecationMessage);
+                if (deprecationLevel == null || deprecationLevel == DeprecationLogger.CRITICAL) {
+                    inOrder.verify(deprecationLogger).compatibleCritical("deprecated_route_GET_/some/path", deprecationMessage);
+                } else {
+                    inOrder.verify(deprecationLogger).compatible(Level.WARN, "deprecated_route_GET_/some/path", deprecationMessage);
+                }
             } else {
-                inOrder.verify(deprecationLogger).warn(DeprecationCategory.API, "deprecated_route_GET_/some/path", deprecationMessage);
+                if (deprecationLevel == null || deprecationLevel == Level.WARN) {
+                    inOrder.verify(deprecationLogger).warn(DeprecationCategory.API, "deprecated_route_GET_/some/path", deprecationMessage);
+                } else {
+                    inOrder.verify(deprecationLogger)
+                        .critical(DeprecationCategory.API, "deprecated_route_GET_/some/path", deprecationMessage);
+                }
             }
 
             inOrder.verify(handler).handleRequest(request, channel, client);
@@ -141,13 +157,15 @@ public class DeprecationRestHandlerTests extends ESTestCase {
 
     public void testSupportsContentStreamTrue() {
         when(handler.supportsContentStream()).thenReturn(true);
-        assertTrue(new DeprecationRestHandler(handler, METHOD, PATH, deprecationMessage, deprecationLogger, false).supportsContentStream());
+        assertTrue(
+            new DeprecationRestHandler(handler, METHOD, PATH, null, deprecationMessage, deprecationLogger, false).supportsContentStream()
+        );
     }
 
     public void testSupportsContentStreamFalse() {
         when(handler.supportsContentStream()).thenReturn(false);
         assertFalse(
-            new DeprecationRestHandler(handler, METHOD, PATH, deprecationMessage, deprecationLogger, false).supportsContentStream()
+            new DeprecationRestHandler(handler, METHOD, PATH, null, deprecationMessage, deprecationLogger, false).supportsContentStream()
         );
     }
 

+ 2 - 1
server/src/test/java/org/elasticsearch/rest/RestControllerTests.java

@@ -234,7 +234,8 @@ public class RestControllerTests extends ESTestCase {
 
         // don't want to test everything -- just that it actually wraps the handler
         doCallRealMethod().when(controller).registerHandler(route, handler);
-        doCallRealMethod().when(controller).registerAsDeprecatedHandler(method, path, deprecatedInVersion, handler, deprecationMessage);
+        doCallRealMethod().when(controller)
+            .registerAsDeprecatedHandler(method, path, deprecatedInVersion, handler, deprecationMessage, null);
 
         controller.registerHandler(route, handler);