Deprecate /_xpack/monitoring/* in favor of /monitoring/*

This commit is part of our plan to deprecate and ultimately remove the use of _xpack in the REST APIs. * Add deprecation for /_xpack/monitoring/* in favor of /monitoring/* * Removed xpack from the rest-api-spec and tests * Removed xpack from the Action name * Removed MonitoringRestHandler as an unnecessary abstraction * Minor corrections to comments Relates elastic#35958
jakelandis · Nov 30, 2018 · 5f115e1 · 5f115e1
1 parent d567f89
commit 5f115e1
Show file tree

Hide file tree

Showing 10 changed files with 38 additions and 54 deletions.
diff --git a/...c/main/java/org/elasticsearch/xpack/core/monitoring/exporter/MonitoringTemplateUtils.java b/...c/main/java/org/elasticsearch/xpack/core/monitoring/exporter/MonitoringTemplateUtils.java
@@ -35,7 +35,7 @@ public final class MonitoringTemplateUtils {
      */
     public static final String TEMPLATE_VERSION = "6";
     /**
-     * The previous version of templates, which we still support via the REST _xpack/monitoring/_bulk endpoint because
+     * The previous version of templates, which we still support via the REST /monitoring/_bulk endpoint because
      * nothing changed for those documents.
      */
     public static final String OLD_TEMPLATE_VERSION = "2";

diff --git a/...nitoring/src/main/java/org/elasticsearch/xpack/monitoring/rest/MonitoringRestHandler.java b/...nitoring/src/main/java/org/elasticsearch/xpack/monitoring/rest/MonitoringRestHandler.java
diff --git a/...rc/main/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkAction.java b/...rc/main/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkAction.java
@@ -5,8 +5,10 @@
  */
 package org.elasticsearch.xpack.monitoring.rest.action;
 
+import org.apache.logging.log4j.LogManager;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.rest.BytesRestResponse;
@@ -19,7 +21,7 @@
 import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkRequestBuilder;
 import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkResponse;
 import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils;
-import org.elasticsearch.xpack.monitoring.rest.MonitoringRestHandler;
+import org.elasticsearch.xpack.core.rest.XPackRestHandler;
 
 import java.io.IOException;
 import java.util.Arrays;
@@ -33,20 +35,28 @@
 import static org.elasticsearch.rest.RestRequest.Method.POST;
 import static org.elasticsearch.rest.RestRequest.Method.PUT;
 
-public class RestMonitoringBulkAction extends MonitoringRestHandler {
+public class RestMonitoringBulkAction extends XPackRestHandler {
 
     public static final String MONITORING_ID = "system_id";
     public static final String MONITORING_VERSION = "system_api_version";
     public static final String INTERVAL = "interval";
+    private static String URI_BASE = "/monitoring";
+    private static String DEPRECATED_URI_BASE = XPackRestHandler.URI_BASE + URI_BASE;
+    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestMonitoringBulkAction.class));
 
     private final Map<MonitoredSystem, List<String>> supportedApiVersions;
 
     public RestMonitoringBulkAction(Settings settings, RestController controller) {
         super(settings);
-        controller.registerHandler(POST, URI_BASE + "/_bulk", this);
-        controller.registerHandler(PUT, URI_BASE + "/_bulk", this);
-        controller.registerHandler(POST, URI_BASE + "/{type}/_bulk", this);
-        controller.registerHandler(PUT, URI_BASE + "/{type}/_bulk", this);
+        // TODO: remove deprecated endpoint in 8.0.0
+        controller.registerWithDeprecatedHandler(POST, URI_BASE + "/_bulk", this,
+            POST, DEPRECATED_URI_BASE + "/_bulk", deprecationLogger);
+        controller.registerWithDeprecatedHandler(PUT, URI_BASE + "/_bulk", this,
+            PUT, DEPRECATED_URI_BASE + "/_bulk", deprecationLogger);
+        controller.registerWithDeprecatedHandler(POST, URI_BASE + "/{type}/_bulk", this,
+            POST, DEPRECATED_URI_BASE + "/{type}/_bulk", deprecationLogger);
+        controller.registerWithDeprecatedHandler(PUT, URI_BASE + "/{type}/_bulk", this,
+            PUT, DEPRECATED_URI_BASE + "/{type}/_bulk", deprecationLogger);
 
         final List<String> allVersions = Arrays.asList(
                 MonitoringTemplateUtils.TEMPLATE_VERSION,
@@ -63,7 +73,7 @@ public RestMonitoringBulkAction(Settings settings, RestController controller) {
 
     @Override
     public String getName() {
-        return "xpack_monitoring_bulk_action";
+        return "monitoring_bulk_action";
     }
 
     @Override

diff --git a/.../test/java/org/elasticsearch/xpack/monitoring/cleaner/AbstractIndicesCleanerTestCase.java b/.../test/java/org/elasticsearch/xpack/monitoring/cleaner/AbstractIndicesCleanerTestCase.java
@@ -74,8 +74,7 @@ public void testDoesNotIgnoreIndicesInOtherVersions() throws Exception {
         createTimestampedIndex(now().minusYears(1), MonitoringTemplateUtils.OLD_TEMPLATE_VERSION);
         // In the past, this index would not be deleted, but starting in 6.x the monitoring cluster
         // will be required to be a newer template version than the production cluster, so the index
-        // pushed to it will never be "unknown" in terms of their version (relates to the
-        // _xpack/monitoring/_setup API)
+        // pushed to it will never be "unknown" in terms of their version
         createTimestampedIndex(now().minusDays(10), String.valueOf(Integer.MAX_VALUE));
 
         // Won't be deleted

diff --git a/.../test/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporterIntegTests.java b/.../test/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporterIntegTests.java
@@ -84,7 +84,7 @@ public void testExport() throws Exception {
                 indexRandom(true, indexRequestBuilders);
             }
 
-            // start the monitoring service so that _xpack/monitoring/_bulk is not ignored
+            // start the monitoring service so that /monitoring/_bulk is not ignored
             final Settings.Builder exporterSettings = Settings.builder()
                     .put(MonitoringService.ENABLED.getKey(), true)
                     .put("xpack.monitoring.exporters._local.enabled", true)

diff --git a/...monitoring/src/test/java/org/elasticsearch/xpack/monitoring/integration/MonitoringIT.java b/...monitoring/src/test/java/org/elasticsearch/xpack/monitoring/integration/MonitoringIT.java
@@ -113,23 +113,17 @@ private String createBulkEntity() {
     }
 
     /**
-     * Monitoring Bulk API test:
+     * Monitoring Bulk test:
      *
-     * This test uses the Monitoring Bulk API to index document as an external application like Kibana would do. It
-     * then ensure that the documents were correctly indexed and have the expected information.
+     * This test uses the Monitoring Bulk Request to index documents. It then ensure that the documents were correctly
+     * indexed and have the expected information. REST API tests (like how this is really called) are handled as part of the
+     * XPackRest tests.
      */
     public void testMonitoringBulk() throws Exception {
         whenExportersAreReady(() -> {
             final MonitoredSystem system = randomSystem();
             final TimeValue interval = TimeValue.timeValueSeconds(randomIntBetween(1, 20));
 
-            // REST is the realistic way that these operations happen, so it's the most realistic way to integration test it too
-            // Use Monitoring Bulk API to index 3 documents
-            //final Request bulkRequest = new Request("POST", "/_xpack/monitoring/_bulk");
-            //<<add all parameters>
-            //bulkRequest.setJsonEntity(createBulkEntity());
-            //final Response bulkResponse = getRestClient().performRequest(request);
-
             final MonitoringBulkResponse bulkResponse =
                     new MonitoringBulkRequestBuilder(client())
                             .add(system, null, new BytesArray(createBulkEntity().getBytes("UTF-8")), XContentType.JSON,

diff --git a/...st/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkActionTests.java b/...st/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkActionTests.java
@@ -52,7 +52,7 @@ public class RestMonitoringBulkActionTests extends ESTestCase {
 
     public void testGetName() {
         // Are you sure that you want to change the name?
-        assertThat(action.getName(), is("xpack_monitoring_bulk_action"));
+        assertThat(action.getName(), is("monitoring_bulk_action"));
     }
 
     public void testSupportsContentStream() {

diff --git a/...t-api-spec/api/xpack.monitoring.bulk.json → ...es/rest-api-spec/api/monitoring.bulk.json b/...t-api-spec/api/xpack.monitoring.bulk.json → ...es/rest-api-spec/api/monitoring.bulk.json
@@ -1,10 +1,10 @@
 {
-  "xpack.monitoring.bulk": {
+  "monitoring.bulk": {
     "documentation": "http://www.elastic.co/guide/en/monitoring/current/appendix-api-bulk.html",
     "methods": ["POST", "PUT"],
     "url": {
-      "path": "/_xpack/monitoring/_bulk",
-      "paths": ["/_xpack/monitoring/_bulk", "/_xpack/monitoring/{type}/_bulk"],
+      "path": "/monitoring/_bulk",
+      "paths": ["/monitoring/_bulk", "/monitoring/{type}/_bulk"],
       "parts": {
         "type": {
           "type" : "string",

diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/monitoring/bulk/10_basic.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/monitoring/bulk/10_basic.yml
@@ -2,7 +2,7 @@
 "Bulk indexing of monitoring data":
 
   - do:
-      xpack.monitoring.bulk:
+      monitoring.bulk:
         system_id:          "kibana"
         system_api_version: "6"
         interval:           "10s"
@@ -37,7 +37,7 @@
   - match: { hits.total: 2 }
 
   - do:
-      xpack.monitoring.bulk:
+      monitoring.bulk:
         system_id:          "kibana"
         system_api_version: "6"
         interval:           "123456ms"
@@ -83,7 +83,7 @@
 
   # Old system_api_version should still be accepted
   - do:
-      xpack.monitoring.bulk:
+      monitoring.bulk:
         system_id:          "kibana"
         system_api_version: "2"
         interval:           "10000ms"
@@ -127,7 +127,7 @@
   # Missing a system_id causes it to fail
   - do:
       catch: bad_request
-      xpack.monitoring.bulk:
+      monitoring.bulk:
         system_api_version: "6"
         interval:           "10s"
         type:               "default_type"
@@ -138,7 +138,7 @@
   # Missing a system_api_version causes it to fail
   - do:
       catch: bad_request
-      xpack.monitoring.bulk:
+      monitoring.bulk:
         system_id:          "kibana"
         interval:           "10s"
         type:               "default_type"
@@ -149,7 +149,7 @@
   # Missing an interval causes it to fail
   - do:
       catch: bad_request
-      xpack.monitoring.bulk:
+      monitoring.bulk:
         system_id:          "kibana"
         system_api_version: "6"
         type:               "default_type"
@@ -161,7 +161,7 @@
 "Bulk indexing of monitoring data on closed indices should throw an export exception":
 
   - do:
-      xpack.monitoring.bulk:
+      monitoring.bulk:
         system_id:          "beats"
         system_api_version: "6"
         interval:           "5s"
@@ -193,7 +193,7 @@
 
   - do:
       catch: /export_exception/
-      xpack.monitoring.bulk:
+      monitoring.bulk:
         system_id:          "beats"
         system_api_version: "6"
         interval:           "5s"

diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/monitoring/bulk/20_privileges.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/monitoring/bulk/20_privileges.yml
@@ -82,7 +82,7 @@ teardown:
       headers:
         # Authorization: logstash_agent
         Authorization: "Basic bG9nc3Rhc2hfYWdlbnQ6czNrcml0"
-      xpack.monitoring.bulk:
+      monitoring.bulk:
         system_id:          "logstash"
         system_api_version: "6"
         interval:           "10s"
@@ -118,7 +118,7 @@ teardown:
       headers:
         # Authorization: unknown_agent
         Authorization: "Basic dW5rbm93bl9hZ2VudDpzM2tyaXQ="
-      xpack.monitoring.bulk:
+      monitoring.bulk:
         system_id:          "logstash"
         system_api_version: "6"
         interval:           "10s"