[Monitoring] Deprecate setting interval to -1 (elastic/x-pack-elasticsearch#4023)

This deprecates setting `xpack.monitoring.collection.interval` to `-1`.

Original commit: elastic/x-pack-elasticsearch@7c5fa35aad

docs/en/settings/monitoring-settings.asciidoc

@@ -75,10 +75,14 @@ Sets the timeout for collecting the recovery information. Defaults to `10s`.
 
 `xpack.monitoring.collection.interval`::
 
+Setting to `-1` to disable data collection has been deprecated. added[6.3.0,
+Use `xpack.monitoring.collection.enabled` set to `false` instead.]
+
 Controls how often data samples are collected. Defaults to `10s`. If you
 modify the collection interval, set the `xpack.monitoring.min_interval_seconds`
-option in `kibana.yml` to the same value. Set to `-1` to temporarily disable
+option in `kibana.yml` to the same value.
-data collection. You can update this setting through the
++
+You can update this setting through the
 <<cluster-update-settings,Cluster Update Settings API>>.
 
 `xpack.monitoring.history.duration`::

plugin/build.gradle

@@ -94,7 +94,6 @@ integTestCluster {
   // Integration tests are supposed to enable/disable exporters before/after each test
   setting 'xpack.monitoring.exporters._local.type', 'local'
   setting 'xpack.monitoring.exporters._local.enabled', 'false'
-  setting 'xpack.monitoring.collection.interval', '-1'
   setting 'xpack.security.authc.token.enabled', 'true'
   setting 'xpack.security.transport.ssl.enabled', 'true'
   setting 'xpack.security.transport.ssl.keystore.path', nodeKeystore.name

plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java

@@ -5,12 +5,14 @@
  */
 package org.elasticsearch.xpack.monitoring;
 
+import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.apache.logging.log4j.util.Supplier;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.component.AbstractLifecycleComponent;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
@@ -38,6 +40,24 @@ import java.util.concurrent.atomic.AtomicBoolean;
  */
 public class MonitoringService extends AbstractLifecycleComponent {
 
+    /**
+     * Log a deprecation warning if {@code value} is -1.
+     * <p>
+     * This should be removed in 7.0.
+     *
+     * @param value The value being set for the collection interval.
+     */
+    private static void deprecateMinusOne(final TimeValue value) {
+        if (TimeValue.MINUS_ONE.equals(value)) {
+            final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(MonitoringService.class));
+
+            deprecationLogger.deprecated(
+                "Setting [xpack.monitoring.collection.interval] to [-1] has been deprecated as the way to disable collection. Use " +
+                "[xpack.monitoring.collection.enabled] set to [false] instead."
+            );
+        }
+    }
+
     /**
      * Minimum value for sampling interval (1 second)
      */
@@ -57,6 +77,8 @@ public class MonitoringService extends AbstractLifecycleComponent {
             (s) -> {
                 TimeValue value = TimeValue.parseTimeValue(s, null, "xpack.monitoring.collection.interval");
                 if (TimeValue.MINUS_ONE.equals(value) || value.millis() >= MIN_INTERVAL.millis()) {
+                    deprecateMinusOne(value);
+
                     return value;
                 }
                 throw new IllegalArgumentException("Failed to parse monitoring interval [" + s + "], value must be >= " + MIN_INTERVAL);

plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java

@@ -107,6 +107,10 @@ public class MonitoringServiceTests extends ESTestCase {
         CountingExporter exporter = new CountingExporter();
         monitoringService = new MonitoringService(settings, clusterService, threadPool, emptySet(), exporter);
 
+        assertWarnings(
+            "Setting [xpack.monitoring.collection.interval] to [-1] has been deprecated as the way to disable collection. Use " +
+            "[xpack.monitoring.collection.enabled] set to [false] instead.");
+
         monitoringService.start();
         assertBusy(() -> assertTrue(monitoringService.isStarted()));
         assertFalse("interval -1 does not start the monitoring execution", monitoringService.isMonitoringActive());

qa/rolling-upgrade/build.gradle

@@ -131,7 +131,6 @@ subprojects {
       minimumMasterNodes = { 2 }
       clusterName = 'rolling-upgrade'
       waitCondition = waitWithAuth
-      setting 'xpack.monitoring.collection.interval', '-1'
       setting 'xpack.monitoring.exporters._http.type', 'http'
       setting 'xpack.monitoring.exporters._http.enabled', 'false'
       setting 'xpack.monitoring.exporters._http.auth.username', 'test_user'
@@ -178,7 +177,6 @@ subprojects {
       minimumMasterNodes = { 2 }
       dataDir = { nodeNumber -> oldClusterTest.nodes[1].dataDir }
       waitCondition = waitWithAuth
-      setting 'xpack.monitoring.collection.interval', '-1'
       setting 'xpack.monitoring.exporters._http.type', 'http'
       setting 'xpack.monitoring.exporters._http.enabled', 'false'
       setting 'xpack.monitoring.exporters._http.auth.username', 'test_user'
@@ -219,7 +217,6 @@ subprojects {
       minimumMasterNodes = { 2 }
       dataDir = { nodeNumber -> oldClusterTest.nodes[0].dataDir }
       waitCondition = waitWithAuth
-      setting 'xpack.monitoring.collection.interval', '-1'
       setting 'xpack.monitoring.exporters._http.type', 'http'
       setting 'xpack.monitoring.exporters._http.enabled', 'false'
       setting 'xpack.monitoring.exporters._http.auth.username', 'test_user'

qa/smoke-test-plugins-ssl/build.gradle

@@ -173,7 +173,7 @@ project.rootProject.subprojects.findAll { it.path.startsWith(':plugins:') }.each
 }
 
 integTestCluster {
-  setting 'xpack.monitoring.collection.interval', '3s'
+  setting 'xpack.monitoring.collection.interval', '1s'
   setting 'xpack.monitoring.exporters._http.type', 'http'
   setting 'xpack.monitoring.exporters._http.enabled', 'false'
   setting 'xpack.monitoring.exporters._http.ssl.truststore.path', clientKeyStore.name