Make xpack.monitoring.enabled setting a no-op (#55617) (#56061)

* Make xpack.monitoring.enabled setting a no-op

This commit turns xpack.monitoring.enabled into a no-op. Mostly, this involved
removing the setting from the setup for integration tests. Monitoring may
introduce some complexity for test setup and teardown, so we should keep an eye
out for turbulence and failures

* Docs for making deprecated setting a no-op

Author: williamrandolph

docs/reference/migration/migrate_7_0/settings.asciidoc

@@ -296,9 +296,11 @@ In Elasticsearch 7.8.0, the following settings no longer have any effect, and
 have been deprecated:
 
 * `xpack.ilm.enabled`
+* `xpack.monitoring.enabled`
 
-In other words, even if `xpack.ilm.enabled` is set to `false`, the ILM APIs
-will be available.
+Previously, these settings could be set to `false` in order to disable the
+feature's APIs in a cluster. As of 7.8.0, these basic license features are
+always enabled for the {default-dist}.
 
 If you have disabled ILM so that you can use another tool to manage Watcher
 indices, the newly introduced `xpack.watcher.use_ilm_index_management` setting
@@ -308,12 +310,11 @@ Additionally, the following settings have been deprecated:
 
 * `xpack.enrich.enabled`
 * `xpack.flattened.enabled`
-* `xpack.monitoring.enabled`
 * `xpack.rollup.enabled`
 * `xpack.slm.enabled`
 * `xpack.sql.enabled`
 * `xpack.transform.enabled`
 * `xpack.vectors.enabled`
 
 In future releases, it will not be possible to disable the APIs for Enrichment,
-Flattened mappings, Monitoring, Rollup, SLM, SQL, Transforms, and Vectors.
+Flattened mappings, Rollup, SLM, SQL, Transforms, and Vectors.

docs/reference/monitoring/collecting-monitoring-data.asciidoc

@@ -30,10 +30,6 @@ To learn about monitoring in general, see <<monitor-elasticsearch-cluster>>.
 
 . Configure your cluster to collect monitoring data:
 
-.. Verify that the deprecated `xpack.monitoring.enabled` setting is not set to
-`false` on any node in the cluster. For more information, see
-<<monitoring-settings>>.
-
 .. Verify that the `xpack.monitoring.elasticsearch.collection.enabled` setting
 is `true`, which is its default value, on each node in the cluster.
 +

docs/reference/monitoring/esms.asciidoc

@@ -118,8 +118,6 @@ Add the following setting in the {kib} configuration file (`kibana.yml`):
 xpack.monitoring.kibana.collection.enabled: false
 ----------------------------------
 
-Leave the `xpack.monitoring.enabled` set to its default value (`true`). 
-
 For more information, see 
 {kibana-ref}/monitoring-settings-kb.html[Monitoring settings in {kib}].
 --

docs/reference/settings/monitoring-settings.asciidoc

@@ -28,12 +28,7 @@ For more information, see <<monitor-elasticsearch-cluster>>.
 
 `xpack.monitoring.enabled`::
 deprecated:[7.8.0,Basic License features should always be enabled] +
-Set to `true` (default) to enable {es} {monitoring} for {es} on the node.
-+
---
-NOTE: To enable data collection, you must also set `xpack.monitoring.collection.enabled`
-to `true`. Its default value is `false`.
---
+This deprecated setting has no effect.
 
 [float]
 [[monitoring-collection-settings]]

x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/CcrSingleNodeTestCase.java

@@ -45,7 +45,6 @@ public abstract class CcrSingleNodeTestCase extends ESSingleNodeTestCase {
     protected Settings nodeSettings() {
         Settings.Builder builder = Settings.builder();
         builder.put(XPackSettings.SECURITY_ENABLED.getKey(), false);
-        builder.put(XPackSettings.MONITORING_ENABLED.getKey(), false);
         builder.put(XPackSettings.WATCHER_ENABLED.getKey(), false);
         builder.put(XPackSettings.MACHINE_LEARNING_ENABLED.getKey(), false);
         builder.put(LicenseService.SELF_GENERATED_LICENSE_TYPE.getKey(), "trial");

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackSettings.java

@@ -61,7 +61,12 @@ private XPackSettings() {
     /** Setting for enabling or disabling security. Defaults to true. */
     public static final Setting<Boolean> SECURITY_ENABLED = Setting.boolSetting("xpack.security.enabled", true, Setting.Property.NodeScope);
 
-    /** Setting for enabling or disabling monitoring. */
+    /**
+     * Setting for enabling or disabling monitoring.
+     * <p>
+     * This setting is now a no-op: setting it to false is permitted, but does nothing.
+     */
+    @Deprecated // since = "7.8.0"
     public static final Setting<Boolean> MONITORING_ENABLED = Setting.boolSetting("xpack.monitoring.enabled", true,
         Property.NodeScope, Property.Deprecated);
 

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/monitoring/MonitoringFeatureSetUsage.java

@@ -32,9 +32,8 @@ public MonitoringFeatureSetUsage(StreamInput in) throws IOException {
         }
     }
 
-    public MonitoringFeatureSetUsage(boolean available, boolean enabled,
-                                     boolean collectionEnabled, Map<String, Object> exporters) {
-        super(XPackField.MONITORING, available, enabled);
+    public MonitoringFeatureSetUsage(boolean available, boolean collectionEnabled, Map<String, Object> exporters) {
+        super(XPackField.MONITORING, available, true);
         this.exporters = exporters;
         this.collectionEnabled = collectionEnabled;
     }

x-pack/plugin/core/src/test/java/org/elasticsearch/license/LicensesManagerServiceTests.java

@@ -37,7 +37,6 @@ protected Settings nodeSettings() {
         return Settings.builder()
             .put(super.nodeSettings())
             .put(XPackSettings.SECURITY_ENABLED.getKey(), false)
-            .put(XPackSettings.MONITORING_ENABLED.getKey(), false)
             .put(XPackSettings.WATCHER_ENABLED.getKey(), false)
             .put(XPackSettings.GRAPH_ENABLED.getKey(), false)
             .put(XPackSettings.MACHINE_LEARNING_ENABLED.getKey(), false)

x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/action/AbstractEqlIntegTestCase.java

@@ -25,7 +25,6 @@ public abstract class AbstractEqlIntegTestCase extends ESIntegTestCase {
     protected Settings nodeSettings(int nodeOrdinal) {
         Settings.Builder settings = Settings.builder().put(super.nodeSettings(nodeOrdinal));
         settings.put(XPackSettings.SECURITY_ENABLED.getKey(), false);
-        settings.put(XPackSettings.MONITORING_ENABLED.getKey(), false);
         settings.put(XPackSettings.WATCHER_ENABLED.getKey(), false);
         settings.put(XPackSettings.GRAPH_ENABLED.getKey(), false);
         settings.put(XPackSettings.MACHINE_LEARNING_ENABLED.getKey(), false);

x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/IndexLifecycleInitialisationTests.java

@@ -98,7 +98,6 @@ protected Settings nodeSettings(int nodeOrdinal) {
         settings.put(XPackSettings.MACHINE_LEARNING_ENABLED.getKey(), false);
         settings.put(XPackSettings.SECURITY_ENABLED.getKey(), false);
         settings.put(XPackSettings.WATCHER_ENABLED.getKey(), false);
-        settings.put(XPackSettings.MONITORING_ENABLED.getKey(), false);
         settings.put(XPackSettings.GRAPH_ENABLED.getKey(), false);
         settings.put(LifecycleSettings.LIFECYCLE_POLL_INTERVAL, "1s");
 

x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecycleInitialisationTests.java

@@ -47,7 +47,6 @@ protected Settings nodeSettings() {
         settings.put(XPackSettings.MACHINE_LEARNING_ENABLED.getKey(), false);
         settings.put(XPackSettings.SECURITY_ENABLED.getKey(), false);
         settings.put(XPackSettings.WATCHER_ENABLED.getKey(), false);
-        settings.put(XPackSettings.MONITORING_ENABLED.getKey(), false);
         settings.put(XPackSettings.GRAPH_ENABLED.getKey(), false);
         settings.put(Environment.PATH_REPO_SETTING.getKey(), repositoryLocation);
 

x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlSingleNodeTestCase.java

@@ -44,7 +44,6 @@ protected Settings nodeSettings() {
         newSettings.put(LicenseService.SELF_GENERATED_LICENSE_TYPE.getKey(), "trial");
         // Disable security otherwise delete-by-query action fails to get authorized
         newSettings.put(XPackSettings.SECURITY_ENABLED.getKey(), false);
-        newSettings.put(XPackSettings.MONITORING_ENABLED.getKey(), false);
         newSettings.put(XPackSettings.WATCHER_ENABLED.getKey(), false);
         // Disable ILM history index so that the tests don't have to clean it up
         newSettings.put(LifecycleSettings.LIFECYCLE_HISTORY_INDEX_ENABLED_SETTING.getKey(), false);

x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/AnnotationIndexIT.java

@@ -26,7 +26,6 @@ public class AnnotationIndexIT extends MlSingleNodeTestCase {
     protected Settings nodeSettings()  {
         Settings.Builder newSettings = Settings.builder();
         newSettings.put(super.nodeSettings());
-        newSettings.put(XPackSettings.MONITORING_ENABLED.getKey(), false);
         newSettings.put(XPackSettings.SECURITY_ENABLED.getKey(), false);
         newSettings.put(XPackSettings.WATCHER_ENABLED.getKey(), false);
         return newSettings.build();

x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/support/BaseMlIntegTestCase.java

@@ -95,7 +95,6 @@ protected Settings nodeSettings(int nodeOrdinal) {
         settings.put(XPackSettings.SECURITY_ENABLED.getKey(), false);
         settings.put(LicenseService.SELF_GENERATED_LICENSE_TYPE.getKey(), "trial");
         settings.put(XPackSettings.WATCHER_ENABLED.getKey(), false);
-        settings.put(XPackSettings.MONITORING_ENABLED.getKey(), false);
         settings.put(XPackSettings.GRAPH_ENABLED.getKey(), false);
         settings.put(LifecycleSettings.LIFECYCLE_HISTORY_INDEX_ENABLED_SETTING.getKey(), false);
         return settings.build();

x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java

@@ -34,7 +34,6 @@
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.watcher.ResourceWatcherService;
 import org.elasticsearch.xpack.core.XPackPlugin;
-import org.elasticsearch.xpack.core.XPackSettings;
 import org.elasticsearch.xpack.core.monitoring.MonitoringField;
 import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkAction;
 import org.elasticsearch.xpack.core.ssl.SSLService;
@@ -66,7 +65,6 @@
 import java.util.Set;
 import java.util.function.Supplier;
 
-import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static org.elasticsearch.common.settings.Setting.boolSetting;
 
@@ -85,26 +83,20 @@ public class Monitoring extends Plugin implements ActionPlugin, ReloadablePlugin
         true, Setting.Property.Dynamic, Setting.Property.NodeScope, Setting.Property.Deprecated);
 
     protected final Settings settings;
-    private final boolean enabled;
     private final boolean transportClientMode;
 
     private Exporters exporters;
 
     public Monitoring(Settings settings) {
         this.settings = settings;
         this.transportClientMode = XPackPlugin.transportClientMode(settings);
-        this.enabled = XPackSettings.MONITORING_ENABLED.get(settings);
     }
 
     // overridable by tests
     protected SSLService getSslService() { return XPackPlugin.getSharedSslService(); }
     protected XPackLicenseState getLicenseState() { return XPackPlugin.getSharedLicenseState(); }
     protected LicenseService getLicenseService() { return XPackPlugin.getSharedLicenseService(); }
 
-    boolean isEnabled() {
-        return enabled;
-    }
-
     boolean isTransportClient() {
         return transportClientMode;
     }
@@ -114,7 +106,7 @@ public Collection<Module> createGuiceModules() {
         List<Module> modules = new ArrayList<>();
         modules.add(b -> {
             XPackPlugin.bindFeatureSet(b, MonitoringFeatureSet.class);
-            if (transportClientMode || enabled == false) {
+            if (transportClientMode) {
                 b.bind(MonitoringService.class).toProvider(Providers.of(null));
                 b.bind(Exporters.class).toProvider(Providers.of(null));
             }
@@ -129,10 +121,6 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
                                                NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry,
                                                IndexNameExpressionResolver expressionResolver,
                                                Supplier<RepositoriesService> repositoriesServiceSupplier) {
-        if (enabled == false) {
-            return Collections.emptyList();
-        }
-
         final ClusterSettings clusterSettings = clusterService.getClusterSettings();
         final CleanerService cleanerService = new CleanerService(settings, clusterSettings, threadPool, getLicenseState());
         final SSLService dynamicSSLService = getSslService().createDynamicSSLService();
@@ -161,19 +149,13 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
 
     @Override
     public List<ActionHandler<? extends ActionRequest, ? extends ActionResponse>> getActions() {
-        if (false == enabled) {
-            return emptyList();
-        }
         return singletonList(new ActionHandler<>(MonitoringBulkAction.INSTANCE, TransportMonitoringBulkAction.class));
     }
 
     @Override
     public List<RestHandler> getRestHandlers(Settings settings, RestController restController, ClusterSettings clusterSettings,
             IndexScopedSettings indexScopedSettings, SettingsFilter settingsFilter, IndexNameExpressionResolver indexNameExpressionResolver,
             Supplier<DiscoveryNodes> nodesInCluster) {
-        if (false == enabled) {
-            return emptyList();
-        }
         return singletonList(new RestMonitoringBulkAction());
     }
 

x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSet.java

@@ -8,11 +8,9 @@
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.inject.Inject;
-import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.xpack.core.XPackFeatureSet;
 import org.elasticsearch.xpack.core.XPackField;
-import org.elasticsearch.xpack.core.XPackSettings;
 import org.elasticsearch.xpack.core.monitoring.MonitoringFeatureSetUsage;
 import org.elasticsearch.xpack.monitoring.exporter.Exporter;
 import org.elasticsearch.xpack.monitoring.exporter.Exporters;
@@ -22,17 +20,14 @@
 
 public class MonitoringFeatureSet implements XPackFeatureSet {
 
-    private final boolean enabled;
     private final MonitoringService monitoring;
     private final XPackLicenseState licenseState;
     private final Exporters exporters;
 
     @Inject
-    public MonitoringFeatureSet(Settings settings,
-                                @Nullable MonitoringService monitoring,
+    public MonitoringFeatureSet(@Nullable MonitoringService monitoring,
                                 @Nullable XPackLicenseState licenseState,
                                 @Nullable Exporters exporters) {
-        this.enabled = XPackSettings.MONITORING_ENABLED.get(settings);
         this.monitoring = monitoring;
         this.licenseState = licenseState;
         this.exporters = exporters;
@@ -50,7 +45,7 @@ public boolean available() {
 
     @Override
     public boolean enabled() {
-        return enabled;
+        return true;
     }
 
     @Override
@@ -62,7 +57,7 @@ public Map<String, Object> nativeCodeInfo() {
     public void usage(ActionListener<XPackFeatureSet.Usage> listener) {
         final boolean collectionEnabled = monitoring != null && monitoring.isMonitoringActive();
 
-        listener.onResponse(new MonitoringFeatureSetUsage(available(), enabled(), collectionEnabled, exportersUsage(exporters)));
+        listener.onResponse(new MonitoringFeatureSetUsage(available(), collectionEnabled, exportersUsage(exporters)));
     }
 
     static Map<String, Object> exportersUsage(Exporters exporters) {

x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSetTests.java

@@ -10,15 +10,12 @@
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
-import org.elasticsearch.common.settings.Setting;
-import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.rest.yaml.ObjectPath;
 import org.elasticsearch.xpack.core.XPackFeatureSet;
-import org.elasticsearch.xpack.core.XPackSettings;
 import org.elasticsearch.xpack.core.XPackFeatureSet.Usage;
 import org.elasticsearch.xpack.core.monitoring.MonitoringFeatureSetUsage;
 import org.elasticsearch.xpack.monitoring.exporter.Exporter;
@@ -44,23 +41,19 @@ public class MonitoringFeatureSetTests extends ESTestCase {
     private final Exporters exporters = mock(Exporters.class);
 
     public void testAvailable() {
-        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, monitoring, licenseState, exporters);
+        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(monitoring, licenseState, exporters);
         boolean available = randomBoolean();
         when(licenseState.isAllowed(XPackLicenseState.Feature.MONITORING)).thenReturn(available);
         assertThat(featureSet.available(), is(available));
     }
 
-    public void testEnabledSetting() {
-        boolean enabled = randomBoolean();
-        Settings.Builder settings = Settings.builder();
-        settings.put("xpack.monitoring.enabled", enabled);
-        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(settings.build(), monitoring, licenseState, exporters);
-        assertThat(featureSet.enabled(), is(enabled));
-        assertSettingDeprecationsAndWarnings(new Setting<?>[] { XPackSettings.MONITORING_ENABLED } );
+    public void testMonitoringEnabledByDefault() {
+        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(monitoring, licenseState, exporters);
+        assertThat(featureSet.enabled(), is(true));
     }
 
     public void testEnabledDefault() {
-        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, monitoring, licenseState, exporters);
+        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(monitoring, licenseState, exporters);
         assertThat(featureSet.enabled(), is(true));
     }
 
@@ -100,7 +93,7 @@ public void testUsage() throws Exception {
         when(exporters.getEnabledExporters()).thenReturn(exporterList);
         when(monitoring.isMonitoringActive()).thenReturn(collectionEnabled);
 
-        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, monitoring, licenseState, exporters);
+        MonitoringFeatureSet featureSet = new MonitoringFeatureSet(monitoring, licenseState, exporters);
         PlainActionFuture<Usage> future = new PlainActionFuture<>();
         featureSet.usage(future);
         XPackFeatureSet.Usage monitoringUsage = future.get();

x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringPluginClientTests.java

@@ -206,7 +206,7 @@ public void testDoCollect() throws Exception {
             .thenReturn(indices);
 
         final XPackUsageResponse xPackUsageResponse = new XPackUsageResponse(
-            singletonList(new MonitoringFeatureSetUsage(true, true, false, null)));
+            singletonList(new MonitoringFeatureSetUsage(true, false, null)));
 
         @SuppressWarnings("unchecked")
         final ActionFuture<XPackUsageResponse> xPackUsageFuture = (ActionFuture<XPackUsageResponse>) mock(ActionFuture.class);
@@ -305,7 +305,7 @@ public void testDoCollectNoLicense() throws Exception {
             when(client.admin()).thenReturn(adminClient);
 
             final XPackUsageResponse xPackUsageResponse = new XPackUsageResponse(
-                singletonList(new MonitoringFeatureSetUsage(true, true, false, null)));
+                singletonList(new MonitoringFeatureSetUsage(true, false, null)));
             @SuppressWarnings("unchecked")
             final ActionFuture<XPackUsageResponse> xPackUsageFuture = (ActionFuture<XPackUsageResponse>) mock(ActionFuture.class);
             when(client.execute(same(XPackUsageAction.INSTANCE), any(XPackUsageRequest.class))).thenReturn(xPackUsageFuture);