From a669f672895b5b913bcde0d941f9670f54e08b44 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Thu, 6 Sep 2018 09:41:40 -0700 Subject: [PATCH 01/17] Implement xpack.monitoring.elasticsearch.collection.enabled setting --- .../xpack/monitoring/Monitoring.java | 20 ++++++----- .../xpack/monitoring/MonitoringService.java | 33 ++++++++++++++++--- .../xpack/monitoring/collector/Collector.java | 12 ++++++- .../collector/ccr/CcrStatsCollector.java | 6 ++-- .../cluster/ClusterStatsCollector.java | 7 ++-- .../indices/IndexRecoveryCollector.java | 4 ++- .../indices/IndexStatsCollector.java | 4 ++- .../collector/ml/JobStatsCollector.java | 9 ++--- .../collector/node/NodeStatsCollector.java | 4 ++- .../collector/shards/ShardsCollector.java | 4 ++- .../monitoring/BaseCollectorTestCase.java | 3 ++ .../cluster/ClusterStatsCollectorTests.java | 13 ++++---- .../indices/IndexRecoveryCollectorTests.java | 10 +++--- .../indices/IndexStatsCollectorTests.java | 8 ++--- .../collector/ml/JobStatsCollectorTests.java | 12 +++---- .../node/NodeStatsCollectorTests.java | 8 ++--- .../shards/ShardsCollectorTests.java | 10 +++--- 17 files changed, 112 insertions(+), 55 deletions(-) diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java index bb2ed76831da2..d3c3a5738240f 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java @@ -36,6 +36,7 @@ import org.elasticsearch.xpack.core.monitoring.MonitoringField; import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkAction; import org.elasticsearch.xpack.core.ssl.SSLService; +import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.action.TransportMonitoringBulkAction; import org.elasticsearch.xpack.monitoring.cleaner.CleanerService; import org.elasticsearch.xpack.monitoring.collector.Collector; @@ -136,16 +137,18 @@ public Collection createComponents(Client client, ClusterService cluster final Exporters exporters = new Exporters(settings, exporterFactories, clusterService, getLicenseState(), threadPool.getThreadContext()); + final MonitoringService monitoringService = new MonitoringService(settings, clusterService, threadPool, exporters); + Set collectors = new HashSet<>(); - collectors.add(new IndexStatsCollector(settings, clusterService, getLicenseState(), client)); - collectors.add(new ClusterStatsCollector(settings, clusterService, getLicenseState(), client, getLicenseService())); - collectors.add(new ShardsCollector(settings, clusterService, getLicenseState())); - collectors.add(new NodeStatsCollector(settings, clusterService, getLicenseState(), client)); - collectors.add(new IndexRecoveryCollector(settings, clusterService, getLicenseState(), client)); - collectors.add(new JobStatsCollector(settings, clusterService, getLicenseState(), client)); - collectors.add(new CcrStatsCollector(settings, clusterService, getLicenseState(), client)); + collectors.add(new IndexStatsCollector(settings, clusterService, monitoringService, getLicenseState(), client)); + collectors.add(new ClusterStatsCollector(settings, clusterService, monitoringService, getLicenseState(), client, getLicenseService())); + collectors.add(new ShardsCollector(settings, clusterService, monitoringService, getLicenseState())); + collectors.add(new NodeStatsCollector(settings, clusterService, monitoringService, getLicenseState(), client)); + collectors.add(new IndexRecoveryCollector(settings, clusterService, monitoringService, getLicenseState(), client)); + collectors.add(new JobStatsCollector(settings, clusterService, monitoringService, getLicenseState(), client)); + collectors.add(new CcrStatsCollector(settings, clusterService, monitoringService, getLicenseState(), client)); - final MonitoringService monitoringService = new MonitoringService(settings, clusterService, threadPool, collectors, exporters); + monitoringService.addCollectors(collectors); return Arrays.asList(monitoringService, exporters, cleanerService); } @@ -174,6 +177,7 @@ public List> getSettings() { settings.add(MonitoringField.HISTORY_DURATION); settings.add(CLEAN_WATCHER_HISTORY); settings.add(MonitoringService.ENABLED); + settings.add(MonitoringService.ELASTICSEARCH_COLLECTION_ENABLED); settings.add(MonitoringService.INTERVAL); settings.add(Collector.INDICES); settings.add(ClusterStatsCollector.CLUSTER_STATS_TIMEOUT); diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java index 07d24826f8648..38853b5ce78cb 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java @@ -24,6 +24,7 @@ import java.io.Closeable; import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.Objects; import java.util.Set; import java.util.concurrent.Semaphore; @@ -44,7 +45,15 @@ public class MonitoringService extends AbstractLifecycleComponent { public static final TimeValue MIN_INTERVAL = TimeValue.timeValueSeconds(1L); /** - * Dynamically controls enabling or disabling the collection of Monitoring data. + * Dynamically controls enabling or disabling the collection of Monitoring data only from Elasticsearch. + */ + public static final Setting ELASTICSEARCH_COLLECTION_ENABLED = + Setting.boolSetting("xpack.monitoring.elasticsearch.collection.enabled", true, + Setting.Property.Dynamic, Setting.Property.NodeScope); + + /** + * Dynamically controls enabling or disabling the collection of Monitoring data from Elasticsearch as well as other products + * in the stack. */ public static final Setting ENABLED = Setting.boolSetting("xpack.monitoring.collection.enabled", false, @@ -68,24 +77,31 @@ public class MonitoringService extends AbstractLifecycleComponent { private final Set collectors; private final Exporters exporters; + private volatile boolean elasticsearchCollectionEnabled; private volatile boolean enabled; private volatile TimeValue interval; private volatile ThreadPool.Cancellable scheduler; - MonitoringService(Settings settings, ClusterService clusterService, ThreadPool threadPool, - Set collectors, Exporters exporters) { + MonitoringService(Settings settings, ClusterService clusterService, ThreadPool threadPool, Exporters exporters) { super(settings); this.clusterService = Objects.requireNonNull(clusterService); this.threadPool = Objects.requireNonNull(threadPool); - this.collectors = Objects.requireNonNull(collectors); + this.collectors = new HashSet(); this.exporters = Objects.requireNonNull(exporters); + this.elasticsearchCollectionEnabled = ELASTICSEARCH_COLLECTION_ENABLED.get(settings); this.enabled = ENABLED.get(settings); this.interval = INTERVAL.get(settings); + clusterService.getClusterSettings().addSettingsUpdateConsumer(ELASTICSEARCH_COLLECTION_ENABLED, this::setElasticsearchCollectionEnabled); clusterService.getClusterSettings().addSettingsUpdateConsumer(ENABLED, this::setMonitoringActive); clusterService.getClusterSettings().addSettingsUpdateConsumer(INTERVAL, this::setInterval); } + void setElasticsearchCollectionEnabled(final boolean enabled) { + this.elasticsearchCollectionEnabled = enabled; + scheduleExecution(); + } + void setMonitoringActive(final boolean enabled) { this.enabled = enabled; scheduleExecution(); @@ -96,6 +112,11 @@ void setInterval(final TimeValue interval) { scheduleExecution(); } + void addCollectors(Set collectors) { + this.collectors.addAll(Objects.requireNonNull(collectors)); + scheduleExecution(); + } + public TimeValue getInterval() { return interval; } @@ -104,6 +125,10 @@ public boolean isMonitoringActive() { return isStarted() && enabled; } + public boolean isElasticsearchCollectionEnabled() { + return this.elasticsearchCollectionEnabled; + } + private String threadPoolName() { return ThreadPool.Names.GENERIC; } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/Collector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/Collector.java index b03ce2001b73b..4cb427d44114f 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/Collector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/Collector.java @@ -20,6 +20,7 @@ import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.core.XPackField; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; +import org.elasticsearch.xpack.monitoring.MonitoringService; import java.util.Collection; import java.util.List; @@ -44,15 +45,17 @@ public abstract class Collector extends AbstractComponent { private final String name; private final Setting collectionTimeoutSetting; + private final MonitoringService monitoringService; protected final ClusterService clusterService; protected final XPackLicenseState licenseState; - public Collector(final Settings settings, final String name, final ClusterService clusterService, + public Collector(final Settings settings, final String name, final ClusterService clusterService, final MonitoringService monitoringService, final Setting timeoutSetting, final XPackLicenseState licenseState) { super(settings); this.name = name; this.clusterService = clusterService; + this.monitoringService = monitoringService; this.collectionTimeoutSetting = timeoutSetting; this.licenseState = licenseState; } @@ -76,6 +79,13 @@ protected boolean shouldCollect(final boolean isElectedMaster) { logger.trace("collector [{}] can not collect data due to invalid license", name()); return false; } + + // TODO: Check if Elasticsearch collection is enabled + if (this.monitoringService.isElasticsearchCollectionEnabled() == false) { + logger.trace("collector [{}] will not collect data because elasticsearch collection is disabled", name()); + return false; + } + return true; } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ccr/CcrStatsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ccr/CcrStatsCollector.java index 510f430d19647..6713e431f9a11 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ccr/CcrStatsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ccr/CcrStatsCollector.java @@ -39,18 +39,20 @@ public class CcrStatsCollector extends Collector { public CcrStatsCollector( final Settings settings, final ClusterService clusterService, + final MonitoringService monitoringService, final XPackLicenseState licenseState, final Client client) { - this(settings, clusterService, licenseState, new XPackClient(client).ccr(), client.threadPool().getThreadContext()); + this(settings, clusterService, monitoringService, licenseState, new XPackClient(client).ccr(), client.threadPool().getThreadContext()); } CcrStatsCollector( final Settings settings, final ClusterService clusterService, + final MonitoringService monitoringService, final XPackLicenseState licenseState, final CcrClient ccrClient, final ThreadContext threadContext) { - super(settings, TYPE, clusterService, CCR_STATS_TIMEOUT, licenseState); + super(settings, TYPE, clusterService, monitoringService, CCR_STATS_TIMEOUT, licenseState); this.ccrClient = ccrClient; this.threadContext = threadContext; } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java index 1a05b03436495..b33ce263c02c6 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java @@ -28,6 +28,7 @@ import org.elasticsearch.xpack.core.XPackFeatureSet; import org.elasticsearch.xpack.core.action.XPackUsageRequestBuilder; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; +import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.Collection; @@ -61,19 +62,21 @@ public class ClusterStatsCollector extends Collector { public ClusterStatsCollector(final Settings settings, final ClusterService clusterService, + final MonitoringService monitoringService, final XPackLicenseState licenseState, final Client client, final LicenseService licenseService) { - this(settings, clusterService, licenseState, client, licenseService, new IndexNameExpressionResolver(Settings.EMPTY)); + this(settings, clusterService, monitoringService, licenseState, client, licenseService, new IndexNameExpressionResolver(Settings.EMPTY)); } ClusterStatsCollector(final Settings settings, final ClusterService clusterService, + final MonitoringService monitoringService, final XPackLicenseState licenseState, final Client client, final LicenseService licenseService, final IndexNameExpressionResolver indexNameExpressionResolver) { - super(settings, ClusterStatsMonitoringDoc.TYPE, clusterService, CLUSTER_STATS_TIMEOUT, licenseState); + super(settings, ClusterStatsMonitoringDoc.TYPE, clusterService, monitoringService, CLUSTER_STATS_TIMEOUT, licenseState); this.client = client; this.licenseService = licenseService; diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollector.java index ea8b5065387f3..8870912c15e2f 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollector.java @@ -15,6 +15,7 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; +import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.ArrayList; @@ -48,10 +49,11 @@ public class IndexRecoveryCollector extends Collector { public IndexRecoveryCollector(final Settings settings, final ClusterService clusterService, + final MonitoringService monitoringService, final XPackLicenseState licenseState, final Client client) { - super(settings, IndexRecoveryMonitoringDoc.TYPE, clusterService, INDEX_RECOVERY_TIMEOUT, licenseState); + super(settings, IndexRecoveryMonitoringDoc.TYPE, clusterService, monitoringService, INDEX_RECOVERY_TIMEOUT, licenseState); this.client = Objects.requireNonNull(client); } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollector.java index b6f50ed9e5cfc..3203f6196599c 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollector.java @@ -18,6 +18,7 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; +import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.ArrayList; @@ -42,9 +43,10 @@ public class IndexStatsCollector extends Collector { public IndexStatsCollector(final Settings settings, final ClusterService clusterService, + final MonitoringService monitoringService, final XPackLicenseState licenseState, final Client client) { - super(settings, "index-stats", clusterService, INDEX_STATS_TIMEOUT, licenseState); + super(settings, "index-stats", clusterService, monitoringService, INDEX_STATS_TIMEOUT, licenseState); this.client = client; } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollector.java index cfbf9b7e0a4b8..aab2b1271433e 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollector.java @@ -19,6 +19,7 @@ import org.elasticsearch.xpack.core.ml.action.GetJobsStatsAction; import org.elasticsearch.xpack.core.ml.client.MachineLearningClient; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; +import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.List; @@ -45,14 +46,14 @@ public class JobStatsCollector extends Collector { private final ThreadContext threadContext; private final MachineLearningClient client; - public JobStatsCollector(final Settings settings, final ClusterService clusterService, + public JobStatsCollector(final Settings settings, final ClusterService clusterService, final MonitoringService monitoringService, final XPackLicenseState licenseState, final Client client) { - this(settings, clusterService, licenseState, new XPackClient(client).machineLearning(), client.threadPool().getThreadContext()); + this(settings, clusterService, monitoringService, licenseState, new XPackClient(client).machineLearning(), client.threadPool().getThreadContext()); } - JobStatsCollector(final Settings settings, final ClusterService clusterService, + JobStatsCollector(final Settings settings, final ClusterService clusterService, final MonitoringService monitoringService, final XPackLicenseState licenseState, final MachineLearningClient client, final ThreadContext threadContext) { - super(settings, JobStatsMonitoringDoc.TYPE, clusterService, JOB_STATS_TIMEOUT, licenseState); + super(settings, JobStatsMonitoringDoc.TYPE, clusterService, monitoringService, JOB_STATS_TIMEOUT, licenseState); this.client = client; this.threadContext = threadContext; } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollector.java index c990485e6a536..eb28ef171d96a 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollector.java @@ -18,6 +18,7 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; +import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.Collection; @@ -51,10 +52,11 @@ public class NodeStatsCollector extends Collector { public NodeStatsCollector(final Settings settings, final ClusterService clusterService, + final MonitoringService monitoringService, final XPackLicenseState licenseState, final Client client) { - super(settings, NodeStatsMonitoringDoc.TYPE, clusterService, NODE_STATS_TIMEOUT, licenseState); + super(settings, NodeStatsMonitoringDoc.TYPE, clusterService, monitoringService, NODE_STATS_TIMEOUT, licenseState); this.client = Objects.requireNonNull(client); } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollector.java index b5a3a2920e2f7..39d84ae5b79a6 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollector.java @@ -14,6 +14,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; +import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.ArrayList; @@ -32,9 +33,10 @@ public class ShardsCollector extends Collector { public ShardsCollector(final Settings settings, final ClusterService clusterService, + final MonitoringService monitoringService, final XPackLicenseState licenseState) { - super(settings, ShardMonitoringDoc.TYPE, clusterService, null, licenseState); + super(settings, ShardMonitoringDoc.TYPE, clusterService, monitoringService, null, licenseState); } @Override diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/BaseCollectorTestCase.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/BaseCollectorTestCase.java index 2a48bc66204ea..3f8f5752b4cd2 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/BaseCollectorTestCase.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/BaseCollectorTestCase.java @@ -23,6 +23,7 @@ import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.function.Function; @@ -34,6 +35,7 @@ public abstract class BaseCollectorTestCase extends ESTestCase { protected ClusterName clusterName; protected ClusterService clusterService; + protected MonitoringService monitoringService; protected ClusterState clusterState; protected DiscoveryNodes nodes; protected MetaData metaData; @@ -46,6 +48,7 @@ public void setUp() throws Exception { super.setUp(); clusterName = mock(ClusterName.class); clusterService = mock(ClusterService.class); + monitoringService = mock(MonitoringService.class); clusterState = mock(ClusterState.class); nodes = mock(DiscoveryNodes.class); metaData = mock(MetaData.class); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java index 49355d51495ec..84d8a9bd8e020 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java @@ -67,14 +67,14 @@ public void setUp() throws Exception { public void testShouldCollectReturnsFalseIfNotMaster() { final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, licenseState, client, licenseService); + new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService); assertThat(collector.shouldCollect(false), is(false)); } public void testShouldCollectReturnsTrue() { final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, licenseState, client, licenseService); + new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService); assertThat(collector.shouldCollect(true), is(true)); } @@ -86,7 +86,7 @@ public void testDoAPMIndicesExistReturnsBasedOnIndices() { when(resolver.concreteIndices(clusterState, IndicesOptions.lenientExpandOpen(), "apm-*")).thenReturn(indices); final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, licenseState, client, licenseService, resolver); + new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, resolver); assertThat(collector.doAPMIndicesExist(clusterState), is(apmIndicesExist)); } @@ -97,7 +97,7 @@ public void testDoAPMIndicesExistReturnsFalseForExpectedExceptions() { when(resolver.concreteIndices(clusterState, IndicesOptions.lenientExpandOpen(), "apm-*")).thenThrow(exception); final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, licenseState, client, licenseService, resolver); + new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, resolver); assertThat(collector.doAPMIndicesExist(clusterState), is(false)); } @@ -108,7 +108,7 @@ public void testDoAPMIndicesExistRethrowsUnexpectedExceptions() { when(resolver.concreteIndices(clusterState, IndicesOptions.lenientExpandOpen(), "apm-*")).thenThrow(exception); final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, licenseState, client, licenseService, resolver); + new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, resolver); expectThrows(RuntimeException.class, () -> collector.doAPMIndicesExist(clusterState)); } @@ -210,7 +210,7 @@ public void testDoCollect() throws Exception { when(xPackUsageFuture.actionGet()).thenReturn(xPackUsageResponse); final ClusterStatsCollector collector = - new ClusterStatsCollector(settings.build(), clusterService, licenseState, + new ClusterStatsCollector(settings.build(), clusterService, monitoringService, licenseState, client, licenseService, indexNameExpressionResolver); Assert.assertEquals(timeout, collector.getCollectionTimeout()); @@ -254,6 +254,7 @@ public void testDoCollect() throws Exception { assertThat(document.getClusterState().stateUUID(), equalTo(clusterState.stateUUID())); verify(clusterService, times(1)).getClusterName(); + verify(monitoringService, times(1)).isElasticsearchCollectionEnabled(); verify(clusterState, times(1)).metaData(); verify(metaData, times(1)).clusterUUID(); verify(licenseService, times(1)).getLicense(); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java index 47504736d26e9..055670e3cd9c0 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java @@ -55,7 +55,7 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, licenseState, client); + final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -65,14 +65,14 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { public void testShouldCollectReturnsFalseIfNotMaster() { when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, licenseState, client); + final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(false), is(false)); } public void testShouldCollectReturnsTrue() { when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, licenseState, client); + final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(true), is(true)); verify(licenseState).isMonitoringAllowed(); @@ -138,7 +138,7 @@ public void testDoCollect() throws Exception { final Client client = mock(Client.class); when(client.admin()).thenReturn(adminClient); - final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, licenseState, client); + final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertEquals(timeout, collector.getCollectionTimeout()); assertEquals(recoveryOnly, collector.getActiveRecoveriesOnly()); @@ -182,4 +182,4 @@ public void testDoCollect() throws Exception { assertThat(recoveries.shardRecoveryStates().size(), equalTo(nbRecoveries)); } } -} \ No newline at end of file +} diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java index ef96726aaee01..2cac56a6d9437 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java @@ -50,7 +50,7 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, licenseState, client); + final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -60,14 +60,14 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { public void testShouldCollectReturnsFalseIfNotMaster() { when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, licenseState, client); + final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(false), is(false)); } public void testShouldCollectReturnsTrue() { when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, licenseState, client); + final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(true), is(true)); verify(licenseState).isMonitoringAllowed(); @@ -144,7 +144,7 @@ public void testDoCollect() throws Exception { final Client client = mock(Client.class); when(client.admin()).thenReturn(adminClient); - final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, licenseState, client); + final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertEquals(timeout, collector.getCollectionTimeout()); final long interval = randomNonNegativeLong(); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java index 1d4e3d8857bf6..51a4616acf93b 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java @@ -50,7 +50,7 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { when(licenseState.isMonitoringAllowed()).thenReturn(false); when(licenseState.isMachineLearningAllowed()).thenReturn(mlAllowed); - final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, licenseState, client); + final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -67,7 +67,7 @@ public void testShouldCollectReturnsFalseIfNotMaster() { // this controls the blockage final boolean isElectedMaster = false; - final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, licenseState, client); + final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); } @@ -82,7 +82,7 @@ public void testShouldCollectReturnsFalseIfMLIsDisabled() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, licenseState, client); + final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); @@ -100,7 +100,7 @@ public void testShouldCollectReturnsFalseIfMLIsNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, licenseState, client); + final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); @@ -116,7 +116,7 @@ public void testShouldCollectReturnsTrue() { when(licenseState.isMachineLearningAllowed()).thenReturn(true); final boolean isElectedMaster = true; - final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, licenseState, client); + final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(true)); @@ -134,7 +134,7 @@ public void testDoCollect() throws Exception { final TimeValue timeout = TimeValue.timeValueSeconds(randomIntBetween(1, 120)); withCollectionTimeout(JobStatsCollector.JOB_STATS_TIMEOUT, timeout); - final JobStatsCollector collector = new JobStatsCollector(Settings.EMPTY, clusterService, licenseState, client, threadContext); + final JobStatsCollector collector = new JobStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, threadContext); assertEquals(timeout, collector.getCollectionTimeout()); final List jobStats = mockJobStats(); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java index 03692cc9d5382..5d18fb81480c2 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java @@ -43,7 +43,7 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, licenseState, client); + final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -55,7 +55,7 @@ public void testShouldCollectReturnsTrue() { when(licenseState.isMonitoringAllowed()).thenReturn(true); final boolean isElectedMaster = true; - final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, licenseState, client); + final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(true)); verify(licenseState).isMonitoringAllowed(); @@ -76,7 +76,7 @@ public void testDoCollectWithFailures() throws Exception { final Client client = mock(Client.class); thenReturnNodeStats(client, timeout, nodesStatsResponse); - final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, licenseState, client); + final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertEquals(timeout, collector.getCollectionTimeout()); final FailedNodeException e = expectThrows(FailedNodeException.class, () -> @@ -110,7 +110,7 @@ public void testDoCollect() throws Exception { final Client client = mock(Client.class); thenReturnNodeStats(client, timeout, nodesStatsResponse); - final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, licenseState, client); + final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertEquals(timeout, collector.getCollectionTimeout()); final long interval = randomNonNegativeLong(); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java index 4affc3a164397..8cad571bb0616 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java @@ -48,7 +48,7 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, licenseState); + final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -61,7 +61,7 @@ public void testShouldCollectReturnsFalseIfNotMaster() { // this controls the blockage whenLocalNodeElectedMaster(false); - final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, licenseState); + final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState); assertThat(collector.shouldCollect(false), is(false)); } @@ -70,14 +70,14 @@ public void testShouldCollectReturnsTrue() { when(licenseState.isMonitoringAllowed()).thenReturn(true); whenLocalNodeElectedMaster(true); - final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, licenseState); + final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState); assertThat(collector.shouldCollect(true), is(true)); verify(licenseState).isMonitoringAllowed(); } public void testDoCollectWhenNoClusterState() throws Exception { - final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, licenseState); + final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState); final Collection results = collector.doCollect(randomMonitoringNode(random()), randomNonNegativeLong(), null); assertThat(results, notNullValue()); @@ -105,7 +105,7 @@ public void testDoCollect() throws Exception { when(nodes.get(eq("_current"))).thenReturn(localNode); when(clusterState.getNodes()).thenReturn(nodes); - final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, licenseState); + final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState); assertNull(collector.getCollectionTimeout()); assertArrayEquals(indices, collector.getCollectionIndices()); From dfc6ebe01841f733b8434e15bbefb9a6516c987e Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Thu, 6 Sep 2018 11:37:22 -0700 Subject: [PATCH 02/17] Fixing line lengths --- .../java/org/elasticsearch/xpack/monitoring/Monitoring.java | 3 ++- .../elasticsearch/xpack/monitoring/MonitoringService.java | 3 ++- .../elasticsearch/xpack/monitoring/collector/Collector.java | 5 +++-- .../monitoring/collector/cluster/ClusterStatsCollector.java | 3 ++- .../xpack/monitoring/collector/ml/JobStatsCollector.java | 3 ++- 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java index d3c3a5738240f..a14697afbd8aa 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java @@ -141,7 +141,8 @@ public Collection createComponents(Client client, ClusterService cluster Set collectors = new HashSet<>(); collectors.add(new IndexStatsCollector(settings, clusterService, monitoringService, getLicenseState(), client)); - collectors.add(new ClusterStatsCollector(settings, clusterService, monitoringService, getLicenseState(), client, getLicenseService())); + collectors.add(new ClusterStatsCollector(settings, clusterService, monitoringService, getLicenseState(), client, + getLicenseService())); collectors.add(new ShardsCollector(settings, clusterService, monitoringService, getLicenseState())); collectors.add(new NodeStatsCollector(settings, clusterService, monitoringService, getLicenseState(), client)); collectors.add(new IndexRecoveryCollector(settings, clusterService, monitoringService, getLicenseState(), client)); diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java index 38853b5ce78cb..24d991464d58f 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java @@ -92,7 +92,8 @@ public class MonitoringService extends AbstractLifecycleComponent { this.enabled = ENABLED.get(settings); this.interval = INTERVAL.get(settings); - clusterService.getClusterSettings().addSettingsUpdateConsumer(ELASTICSEARCH_COLLECTION_ENABLED, this::setElasticsearchCollectionEnabled); + clusterService.getClusterSettings() + .addSettingsUpdateConsumer(ELASTICSEARCH_COLLECTION_ENABLED, this::setElasticsearchCollectionEnabled); clusterService.getClusterSettings().addSettingsUpdateConsumer(ENABLED, this::setMonitoringActive); clusterService.getClusterSettings().addSettingsUpdateConsumer(INTERVAL, this::setInterval); } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/Collector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/Collector.java index 4cb427d44114f..05112475ce3f3 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/Collector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/Collector.java @@ -50,8 +50,9 @@ public abstract class Collector extends AbstractComponent { protected final ClusterService clusterService; protected final XPackLicenseState licenseState; - public Collector(final Settings settings, final String name, final ClusterService clusterService, final MonitoringService monitoringService, - final Setting timeoutSetting, final XPackLicenseState licenseState) { + public Collector(final Settings settings, final String name, final ClusterService clusterService, + final MonitoringService monitoringService, final Setting timeoutSetting, + final XPackLicenseState licenseState) { super(settings); this.name = name; this.clusterService = clusterService; diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java index b33ce263c02c6..1a38ab5039040 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java @@ -66,7 +66,8 @@ public ClusterStatsCollector(final Settings settings, final XPackLicenseState licenseState, final Client client, final LicenseService licenseService) { - this(settings, clusterService, monitoringService, licenseState, client, licenseService, new IndexNameExpressionResolver(Settings.EMPTY)); + this(settings, clusterService, monitoringService, licenseState, client, licenseService, + new IndexNameExpressionResolver(Settings.EMPTY)); } ClusterStatsCollector(final Settings settings, diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollector.java index aab2b1271433e..d5668d7f7c92f 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollector.java @@ -48,7 +48,8 @@ public class JobStatsCollector extends Collector { public JobStatsCollector(final Settings settings, final ClusterService clusterService, final MonitoringService monitoringService, final XPackLicenseState licenseState, final Client client) { - this(settings, clusterService, monitoringService, licenseState, new XPackClient(client).machineLearning(), client.threadPool().getThreadContext()); + this(settings, clusterService, monitoringService, licenseState, new XPackClient(client).machineLearning(), + client.threadPool().getThreadContext()); } JobStatsCollector(final Settings settings, final ClusterService clusterService, final MonitoringService monitoringService, From 07b3bd6490aacb0fdaee9af93d18d3c9f832781f Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Thu, 6 Sep 2018 13:09:12 -0700 Subject: [PATCH 03/17] Updating constructor calls in test --- .../xpack/monitoring/MonitoringServiceTests.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java index 3d23933f8dcc0..4027a6e2a0e6b 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java @@ -68,7 +68,7 @@ public void terminate() throws Exception { } public void testIsMonitoringActive() throws Exception { - monitoringService = new MonitoringService(Settings.EMPTY, clusterService, threadPool, emptySet(), new CountingExporter()); + monitoringService = new MonitoringService(Settings.EMPTY, clusterService, threadPool, new CountingExporter()); monitoringService.start(); assertBusy(() -> assertTrue(monitoringService.isStarted())); @@ -97,7 +97,7 @@ public void testInterval() throws Exception { .build(); CountingExporter exporter = new CountingExporter(); - monitoringService = new MonitoringService(settings, clusterService, threadPool, emptySet(), exporter); + monitoringService = new MonitoringService(settings, clusterService, threadPool, exporter); monitoringService.start(); assertBusy(() -> assertTrue(monitoringService.isStarted())); @@ -123,7 +123,7 @@ public void testSkipExecution() throws Exception { .put("xpack.monitoring.collection.interval", MonitoringService.MIN_INTERVAL) .build(); - monitoringService = new MonitoringService(settings, clusterService, threadPool, emptySet(), exporter); + monitoringService = new MonitoringService(settings, clusterService, threadPool, exporter); monitoringService.start(); assertBusy(() -> assertTrue(monitoringService.isStarted())); From ccbdbcf6f1a5e30281c3364d637fa5d5ba33260b Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Thu, 6 Sep 2018 16:47:02 -0700 Subject: [PATCH 04/17] Removing unused import --- .../elasticsearch/xpack/monitoring/MonitoringServiceTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java index 4027a6e2a0e6b..2042422060085 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java @@ -25,7 +25,6 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; -import static java.util.Collections.emptySet; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.mockito.Mockito.mock; From b17ed7841b4ba3a6ee828001be582e151d764d42 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Thu, 6 Sep 2018 16:47:16 -0700 Subject: [PATCH 05/17] Fixing line lengths in test classes --- .../cluster/ClusterStatsCollectorTests.java | 9 ++++++--- .../indices/IndexRecoveryCollectorTests.java | 12 ++++++++---- .../collector/indices/IndexStatsCollectorTests.java | 12 ++++++++---- .../collector/ml/JobStatsCollectorTests.java | 3 ++- .../collector/node/NodeStatsCollectorTests.java | 12 ++++++++---- 5 files changed, 32 insertions(+), 16 deletions(-) diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java index 84d8a9bd8e020..74cb7114144bf 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java @@ -86,7 +86,8 @@ public void testDoAPMIndicesExistReturnsBasedOnIndices() { when(resolver.concreteIndices(clusterState, IndicesOptions.lenientExpandOpen(), "apm-*")).thenReturn(indices); final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, resolver); + new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, + resolver); assertThat(collector.doAPMIndicesExist(clusterState), is(apmIndicesExist)); } @@ -97,7 +98,8 @@ public void testDoAPMIndicesExistReturnsFalseForExpectedExceptions() { when(resolver.concreteIndices(clusterState, IndicesOptions.lenientExpandOpen(), "apm-*")).thenThrow(exception); final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, resolver); + new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, + resolver); assertThat(collector.doAPMIndicesExist(clusterState), is(false)); } @@ -108,7 +110,8 @@ public void testDoAPMIndicesExistRethrowsUnexpectedExceptions() { when(resolver.concreteIndices(clusterState, IndicesOptions.lenientExpandOpen(), "apm-*")).thenThrow(exception); final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, resolver); + new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, + resolver); expectThrows(RuntimeException.class, () -> collector.doAPMIndicesExist(clusterState)); } diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java index 055670e3cd9c0..91df9a1c9798b 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java @@ -55,7 +55,8 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, + licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -65,14 +66,16 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { public void testShouldCollectReturnsFalseIfNotMaster() { when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, + licenseState, client); assertThat(collector.shouldCollect(false), is(false)); } public void testShouldCollectReturnsTrue() { when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, + licenseState, client); assertThat(collector.shouldCollect(true), is(true)); verify(licenseState).isMonitoringAllowed(); @@ -138,7 +141,8 @@ public void testDoCollect() throws Exception { final Client client = mock(Client.class); when(client.admin()).thenReturn(adminClient); - final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, + licenseState, client); assertEquals(timeout, collector.getCollectionTimeout()); assertEquals(recoveryOnly, collector.getActiveRecoveriesOnly()); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java index 2cac56a6d9437..a80bd1b720425 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java @@ -50,7 +50,8 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, + client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -60,14 +61,16 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { public void testShouldCollectReturnsFalseIfNotMaster() { when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, + client); assertThat(collector.shouldCollect(false), is(false)); } public void testShouldCollectReturnsTrue() { when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, + client); assertThat(collector.shouldCollect(true), is(true)); verify(licenseState).isMonitoringAllowed(); @@ -144,7 +147,8 @@ public void testDoCollect() throws Exception { final Client client = mock(Client.class); when(client.admin()).thenReturn(adminClient); - final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, + client); assertEquals(timeout, collector.getCollectionTimeout()); final long interval = randomNonNegativeLong(); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java index 51a4616acf93b..e6e5e6521ee5c 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java @@ -134,7 +134,8 @@ public void testDoCollect() throws Exception { final TimeValue timeout = TimeValue.timeValueSeconds(randomIntBetween(1, 120)); withCollectionTimeout(JobStatsCollector.JOB_STATS_TIMEOUT, timeout); - final JobStatsCollector collector = new JobStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, threadContext); + final JobStatsCollector collector = new JobStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, + client, threadContext); assertEquals(timeout, collector.getCollectionTimeout()); final List jobStats = mockJobStats(); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java index 5d18fb81480c2..b9980a3f84287 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java @@ -43,7 +43,8 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, + client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -55,7 +56,8 @@ public void testShouldCollectReturnsTrue() { when(licenseState.isMonitoringAllowed()).thenReturn(true); final boolean isElectedMaster = true; - final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, + client); assertThat(collector.shouldCollect(isElectedMaster), is(true)); verify(licenseState).isMonitoringAllowed(); @@ -76,7 +78,8 @@ public void testDoCollectWithFailures() throws Exception { final Client client = mock(Client.class); thenReturnNodeStats(client, timeout, nodesStatsResponse); - final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, + client); assertEquals(timeout, collector.getCollectionTimeout()); final FailedNodeException e = expectThrows(FailedNodeException.class, () -> @@ -110,7 +113,8 @@ public void testDoCollect() throws Exception { final Client client = mock(Client.class); thenReturnNodeStats(client, timeout, nodesStatsResponse); - final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, + client); assertEquals(timeout, collector.getCollectionTimeout()); final long interval = randomNonNegativeLong(); From cb17765a29531e729046b38a72452d64d722f863 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 7 Sep 2018 13:35:54 -0700 Subject: [PATCH 06/17] Make monitoringService.isElasticsearchCollectionEnabled() return true for tests --- .../collector/indices/IndexRecoveryCollectorTests.java | 1 + .../monitoring/collector/indices/IndexStatsCollectorTests.java | 1 + .../xpack/monitoring/collector/ml/JobStatsCollectorTests.java | 1 + .../xpack/monitoring/collector/node/NodeStatsCollectorTests.java | 1 + .../xpack/monitoring/collector/shards/ShardsCollectorTests.java | 1 + 5 files changed, 5 insertions(+) diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java index 91df9a1c9798b..ba8b48acd8148 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java @@ -73,6 +73,7 @@ public void testShouldCollectReturnsFalseIfNotMaster() { } public void testShouldCollectReturnsTrue() { + when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); when(licenseState.isMonitoringAllowed()).thenReturn(true); final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java index a80bd1b720425..db18ff71a31fc 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java @@ -68,6 +68,7 @@ public void testShouldCollectReturnsFalseIfNotMaster() { } public void testShouldCollectReturnsTrue() { + when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); when(licenseState.isMonitoringAllowed()).thenReturn(true); final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java index e6e5e6521ee5c..519375ad78193 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java @@ -113,6 +113,7 @@ public void testShouldCollectReturnsTrue() { final Settings settings = mlEnabledSettings(); when(licenseState.isMonitoringAllowed()).thenReturn(true); + when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); when(licenseState.isMachineLearningAllowed()).thenReturn(true); final boolean isElectedMaster = true; diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java index b9980a3f84287..fd4861c2fd92f 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java @@ -54,6 +54,7 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { public void testShouldCollectReturnsTrue() { when(licenseState.isMonitoringAllowed()).thenReturn(true); + when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); final boolean isElectedMaster = true; final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java index 8cad571bb0616..6f1e4670f2668 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java @@ -68,6 +68,7 @@ public void testShouldCollectReturnsFalseIfNotMaster() { public void testShouldCollectReturnsTrue() { when(licenseState.isMonitoringAllowed()).thenReturn(true); + when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); whenLocalNodeElectedMaster(true); final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState); From e003f3108607fa4be4a12ec24ebbf56755d0cb18 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 7 Sep 2018 15:11:14 -0700 Subject: [PATCH 07/17] Remove wrong expectation --- .../monitoring/collector/cluster/ClusterStatsCollectorTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java index 74cb7114144bf..44967a5b2a084 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java @@ -257,7 +257,6 @@ public void testDoCollect() throws Exception { assertThat(document.getClusterState().stateUUID(), equalTo(clusterState.stateUUID())); verify(clusterService, times(1)).getClusterName(); - verify(monitoringService, times(1)).isElasticsearchCollectionEnabled(); verify(clusterState, times(1)).metaData(); verify(metaData, times(1)).clusterUUID(); verify(licenseService, times(1)).getLicense(); From a610e00e11aa0b8da6c978a0503a3313d9e8e11f Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 7 Sep 2018 19:07:41 -0700 Subject: [PATCH 08/17] Adding unit tests for new flag to be false --- .../collector/cluster/ClusterStatsCollectorTests.java | 11 +++++++++++ .../indices/IndexRecoveryCollectorTests.java | 11 +++++++++++ .../collector/indices/IndexStatsCollectorTests.java | 11 +++++++++++ .../collector/ml/JobStatsCollectorTests.java | 10 ++++++++++ .../collector/node/NodeStatsCollectorTests.java | 11 +++++++++++ .../collector/shards/ShardsCollectorTests.java | 10 ++++++++++ 6 files changed, 64 insertions(+) diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java index 44967a5b2a084..73870639e7cdd 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java @@ -72,6 +72,17 @@ public void testShouldCollectReturnsFalseIfNotMaster() { assertThat(collector.shouldCollect(false), is(false)); } + public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { + when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(false); + when(licenseState.isMonitoringAllowed()).thenReturn(true); + final boolean isElectedMaster = randomBoolean(); + whenLocalNodeElectedMaster(isElectedMaster); + + final ClusterStatsCollector collector = + new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService); + assertThat(collector.shouldCollect(isElectedMaster), is(false)); + } + public void testShouldCollectReturnsTrue() { final ClusterStatsCollector collector = new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java index ba8b48acd8148..14d43a8ddffb1 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java @@ -72,6 +72,17 @@ public void testShouldCollectReturnsFalseIfNotMaster() { assertThat(collector.shouldCollect(false), is(false)); } + public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { + when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(false); + when(licenseState.isMonitoringAllowed()).thenReturn(true); + final boolean isElectedMaster = randomBoolean(); + whenLocalNodeElectedMaster(isElectedMaster); + + final IndexRecoveryCollector collector = + new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + assertThat(collector.shouldCollect(isElectedMaster), is(false)); + } + public void testShouldCollectReturnsTrue() { when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); when(licenseState.isMonitoringAllowed()).thenReturn(true); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java index db18ff71a31fc..28e22daaec995 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java @@ -67,6 +67,17 @@ public void testShouldCollectReturnsFalseIfNotMaster() { assertThat(collector.shouldCollect(false), is(false)); } + public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { + when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(false); + when(licenseState.isMonitoringAllowed()).thenReturn(true); + final boolean isElectedMaster = randomBoolean(); + whenLocalNodeElectedMaster(isElectedMaster); + + final IndexStatsCollector collector = + new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + assertThat(collector.shouldCollect(isElectedMaster), is(false)); + } + public void testShouldCollectReturnsTrue() { when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); when(licenseState.isMonitoringAllowed()).thenReturn(true); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java index 519375ad78193..8f2bc9d99e7cf 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java @@ -109,6 +109,16 @@ public void testShouldCollectReturnsFalseIfMLIsNotAllowed() { } } + public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { + when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(false); + when(licenseState.isMonitoringAllowed()).thenReturn(true); + final boolean isElectedMaster = randomBoolean(); + whenLocalNodeElectedMaster(isElectedMaster); + + final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, monitoringService, licenseState, client); + assertThat(collector.shouldCollect(isElectedMaster), is(false)); + } + public void testShouldCollectReturnsTrue() { final Settings settings = mlEnabledSettings(); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java index fd4861c2fd92f..4322b761368aa 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java @@ -52,6 +52,17 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { } } + public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { + when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(false); + when(licenseState.isMonitoringAllowed()).thenReturn(true); + final boolean isElectedMaster = randomBoolean(); + whenLocalNodeElectedMaster(isElectedMaster); + + final NodeStatsCollector collector = + new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + assertThat(collector.shouldCollect(isElectedMaster), is(false)); + } + public void testShouldCollectReturnsTrue() { when(licenseState.isMonitoringAllowed()).thenReturn(true); when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java index 6f1e4670f2668..89c3c0277e3f5 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java @@ -66,6 +66,16 @@ public void testShouldCollectReturnsFalseIfNotMaster() { assertThat(collector.shouldCollect(false), is(false)); } + public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { + when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(false); + when(licenseState.isMonitoringAllowed()).thenReturn(true); + final boolean isElectedMaster = randomBoolean(); + whenLocalNodeElectedMaster(isElectedMaster); + + final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState); + assertThat(collector.shouldCollect(isElectedMaster), is(false)); + } + public void testShouldCollectReturnsTrue() { when(licenseState.isMonitoringAllowed()).thenReturn(true); when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); From 00c9e4b5209ddf33e356a84e914ba02863133146 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 7 Sep 2018 19:13:39 -0700 Subject: [PATCH 09/17] Fixing line wrapping/indentation for better readability --- .../cluster/ClusterStatsCollectorTests.java | 11 ++++------- .../indices/IndexRecoveryCollectorTests.java | 16 ++++++++-------- .../indices/IndexStatsCollectorTests.java | 18 +++++++++--------- .../collector/ml/JobStatsCollectorTests.java | 4 ++-- .../node/NodeStatsCollectorTests.java | 18 +++++++++--------- 5 files changed, 32 insertions(+), 35 deletions(-) diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java index 73870639e7cdd..120467399827d 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java @@ -79,7 +79,7 @@ public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { whenLocalNodeElectedMaster(isElectedMaster); final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService); + new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService); assertThat(collector.shouldCollect(isElectedMaster), is(false)); } @@ -97,8 +97,7 @@ public void testDoAPMIndicesExistReturnsBasedOnIndices() { when(resolver.concreteIndices(clusterState, IndicesOptions.lenientExpandOpen(), "apm-*")).thenReturn(indices); final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, - resolver); + new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, resolver); assertThat(collector.doAPMIndicesExist(clusterState), is(apmIndicesExist)); } @@ -109,8 +108,7 @@ public void testDoAPMIndicesExistReturnsFalseForExpectedExceptions() { when(resolver.concreteIndices(clusterState, IndicesOptions.lenientExpandOpen(), "apm-*")).thenThrow(exception); final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, - resolver); + new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, resolver); assertThat(collector.doAPMIndicesExist(clusterState), is(false)); } @@ -121,8 +119,7 @@ public void testDoAPMIndicesExistRethrowsUnexpectedExceptions() { when(resolver.concreteIndices(clusterState, IndicesOptions.lenientExpandOpen(), "apm-*")).thenThrow(exception); final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, - resolver); + new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, resolver); expectThrows(RuntimeException.class, () -> collector.doAPMIndicesExist(clusterState)); } diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java index 14d43a8ddffb1..efb530a5bd7e9 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java @@ -55,8 +55,8 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, - licenseState, client); + final IndexRecoveryCollector collector = + new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -66,8 +66,8 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { public void testShouldCollectReturnsFalseIfNotMaster() { when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, - licenseState, client); + final IndexRecoveryCollector collector = + new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(false), is(false)); } @@ -86,8 +86,8 @@ public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { public void testShouldCollectReturnsTrue() { when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, - licenseState, client); + final IndexRecoveryCollector collector = + new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(true), is(true)); verify(licenseState).isMonitoringAllowed(); @@ -153,8 +153,8 @@ public void testDoCollect() throws Exception { final Client client = mock(Client.class); when(client.admin()).thenReturn(adminClient); - final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, - licenseState, client); + final IndexRecoveryCollector collector = + new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertEquals(timeout, collector.getCollectionTimeout()); assertEquals(recoveryOnly, collector.getActiveRecoveriesOnly()); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java index 28e22daaec995..42f746ff8c7e9 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java @@ -50,8 +50,8 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, - client); + final IndexStatsCollector collector = + new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -61,8 +61,8 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { public void testShouldCollectReturnsFalseIfNotMaster() { when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, - client); + final IndexStatsCollector collector = + new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(false), is(false)); } @@ -74,15 +74,15 @@ public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { whenLocalNodeElectedMaster(isElectedMaster); final IndexStatsCollector collector = - new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); } public void testShouldCollectReturnsTrue() { when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, - client); + final IndexStatsCollector collector = + new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(true), is(true)); verify(licenseState).isMonitoringAllowed(); @@ -159,8 +159,8 @@ public void testDoCollect() throws Exception { final Client client = mock(Client.class); when(client.admin()).thenReturn(adminClient); - final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, - client); + final IndexStatsCollector collector = + new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertEquals(timeout, collector.getCollectionTimeout()); final long interval = randomNonNegativeLong(); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java index 8f2bc9d99e7cf..1b90e26c02ee9 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java @@ -145,8 +145,8 @@ public void testDoCollect() throws Exception { final TimeValue timeout = TimeValue.timeValueSeconds(randomIntBetween(1, 120)); withCollectionTimeout(JobStatsCollector.JOB_STATS_TIMEOUT, timeout); - final JobStatsCollector collector = new JobStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, - client, threadContext); + final JobStatsCollector collector = + new JobStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, threadContext); assertEquals(timeout, collector.getCollectionTimeout()); final List jobStats = mockJobStats(); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java index 4322b761368aa..8a0e685488a30 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java @@ -43,8 +43,8 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, - client); + final NodeStatsCollector collector = + new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -59,7 +59,7 @@ public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { whenLocalNodeElectedMaster(isElectedMaster); final NodeStatsCollector collector = - new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); } @@ -68,8 +68,8 @@ public void testShouldCollectReturnsTrue() { when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); final boolean isElectedMaster = true; - final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, - client); + final NodeStatsCollector collector = + new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(true)); verify(licenseState).isMonitoringAllowed(); @@ -90,8 +90,8 @@ public void testDoCollectWithFailures() throws Exception { final Client client = mock(Client.class); thenReturnNodeStats(client, timeout, nodesStatsResponse); - final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, - client); + final NodeStatsCollector collector = + new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertEquals(timeout, collector.getCollectionTimeout()); final FailedNodeException e = expectThrows(FailedNodeException.class, () -> @@ -125,8 +125,8 @@ public void testDoCollect() throws Exception { final Client client = mock(Client.class); thenReturnNodeStats(client, timeout, nodesStatsResponse); - final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, - client); + final NodeStatsCollector collector = + new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); assertEquals(timeout, collector.getCollectionTimeout()); final long interval = randomNonNegativeLong(); From f5cfa91631a67aa717f76d4a3c14882ce526bd4b Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 7 Sep 2018 19:56:07 -0700 Subject: [PATCH 10/17] Adding docs --- .../monitoring/configuring-monitoring.asciidoc | 9 +++++---- docs/reference/monitoring/pause-export.asciidoc | 10 ++++++++++ docs/reference/settings/monitoring-settings.asciidoc | 4 ++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/docs/reference/monitoring/configuring-monitoring.asciidoc b/docs/reference/monitoring/configuring-monitoring.asciidoc index 3bcfef2acbf29..b230540b91045 100644 --- a/docs/reference/monitoring/configuring-monitoring.asciidoc +++ b/docs/reference/monitoring/configuring-monitoring.asciidoc @@ -13,10 +13,11 @@ indices. You can also adjust how monitoring data is displayed. . To collect monitoring data about your {es} cluster: -.. Verify that the `xpack.monitoring.enabled` and -`xpack.monitoring.collection.enabled` settings are `true` on each node in the -cluster. By default, data collection is disabled. For more information, see -<>. +.. Verify that the `xpack.monitoring.enabled`, +`xpack.monitoring.collection.enabled`, and +`xpack.monitoring.elasticsearch.collection.enabled` settings are `true` on each +node in the cluster. By default, data collection is disabled. For more +information, see <>. .. Optional: Specify which indices you want to monitor. + diff --git a/docs/reference/monitoring/pause-export.asciidoc b/docs/reference/monitoring/pause-export.asciidoc index 128e72a463c2d..e46e078a01d75 100644 --- a/docs/reference/monitoring/pause-export.asciidoc +++ b/docs/reference/monitoring/pause-export.asciidoc @@ -16,6 +16,16 @@ monitoring data from other sources such as {kib}, Beats, and Logstash is ignored You can update this setting by using the {ref}/cluster-update-settings.html[Cluster Update Settings API]. +If you want to collect data from sources such as {kib}, Beats, and Logstash but +not collect data about your {es} cluster, you can disable data collection +just for {es}: + +[source,yaml] +--------------------------------------------------- +xpack.monitoring.collection.enabled: true +xpack.monitoring.elasticsearch.collection.enabled: true +--------------------------------------------------- + If you want to separately disable a specific exporter, you can specify the `enabled` setting (which defaults to `true`) per exporter. For example: diff --git a/docs/reference/settings/monitoring-settings.asciidoc b/docs/reference/settings/monitoring-settings.asciidoc index 2759944e61572..7109170544c62 100644 --- a/docs/reference/settings/monitoring-settings.asciidoc +++ b/docs/reference/settings/monitoring-settings.asciidoc @@ -66,6 +66,10 @@ option in `kibana.yml` to the same value. You can update this setting through the <>. +`xpack.monitoring.elasticsearch.collection.enabled`:: + +Controls whether statistics about your {es} cluster should be collected. Defaults to `true`. + `xpack.monitoring.collection.cluster.stats.timeout`:: Sets the timeout for collecting the cluster statistics. Defaults to `10s`. From 871fc3bf7a486ae41a7249109eb42cda390b5e38 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Mon, 10 Sep 2018 02:46:39 -0700 Subject: [PATCH 11/17] Fixing logic in ClusterStatsCollector::shouldCollect --- .../monitoring/collector/cluster/ClusterStatsCollector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java index 1a38ab5039040..56ac8f80dbdbe 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java @@ -87,7 +87,7 @@ public ClusterStatsCollector(final Settings settings, @Override protected boolean shouldCollect(final boolean isElectedMaster) { // This collector can always collect data on the master node - return isElectedMaster; + return isElectedMaster && super.shouldCollect(isElectedMaster); } @Override From cd751df078e94be942fb71002aebea25dc12e025 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Thu, 13 Sep 2018 06:31:46 -0700 Subject: [PATCH 12/17] Rebasing with master and resolving conflicts --- .../xpack/monitoring/collector/ccr/CcrStatsCollector.java | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ccr/CcrStatsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ccr/CcrStatsCollector.java index 6713e431f9a11..8abd9f51ef93c 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ccr/CcrStatsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ccr/CcrStatsCollector.java @@ -20,6 +20,7 @@ import org.elasticsearch.xpack.core.ccr.action.CcrStatsAction; import org.elasticsearch.xpack.core.ccr.client.CcrClient; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; +import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.Collection; From 79a79c7350500f55f98585bea81dc53c442e297e Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 14 Sep 2018 14:04:56 -0700 Subject: [PATCH 13/17] Simplifying implementation by gating scheduling --- .../xpack/monitoring/Monitoring.java | 22 ++++++-------- .../xpack/monitoring/MonitoringService.java | 30 +++++++++++-------- .../xpack/monitoring/collector/Collector.java | 13 +------- .../collector/ccr/CcrStatsCollector.java | 7 ++--- .../cluster/ClusterStatsCollector.java | 10 ++----- .../indices/IndexRecoveryCollector.java | 4 +-- .../indices/IndexStatsCollector.java | 4 +-- .../collector/ml/JobStatsCollector.java | 10 +++---- .../collector/node/NodeStatsCollector.java | 4 +-- .../collector/shards/ShardsCollector.java | 4 +-- .../monitoring/BaseCollectorTestCase.java | 3 -- .../monitoring/MonitoringServiceTests.java | 7 +++-- .../cluster/ClusterStatsCollectorTests.java | 23 ++++---------- .../indices/IndexRecoveryCollectorTests.java | 24 +++------------ .../indices/IndexStatsCollectorTests.java | 24 +++------------ .../collector/ml/JobStatsCollectorTests.java | 24 ++++----------- .../node/NodeStatsCollectorTests.java | 24 +++------------ .../shards/ShardsCollectorTests.java | 21 ++++--------- 18 files changed, 73 insertions(+), 185 deletions(-) diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java index a14697afbd8aa..027cb7de937f4 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java @@ -36,7 +36,6 @@ import org.elasticsearch.xpack.core.monitoring.MonitoringField; import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkAction; import org.elasticsearch.xpack.core.ssl.SSLService; -import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.action.TransportMonitoringBulkAction; import org.elasticsearch.xpack.monitoring.cleaner.CleanerService; import org.elasticsearch.xpack.monitoring.collector.Collector; @@ -137,19 +136,16 @@ public Collection createComponents(Client client, ClusterService cluster final Exporters exporters = new Exporters(settings, exporterFactories, clusterService, getLicenseState(), threadPool.getThreadContext()); - final MonitoringService monitoringService = new MonitoringService(settings, clusterService, threadPool, exporters); - Set collectors = new HashSet<>(); - collectors.add(new IndexStatsCollector(settings, clusterService, monitoringService, getLicenseState(), client)); - collectors.add(new ClusterStatsCollector(settings, clusterService, monitoringService, getLicenseState(), client, - getLicenseService())); - collectors.add(new ShardsCollector(settings, clusterService, monitoringService, getLicenseState())); - collectors.add(new NodeStatsCollector(settings, clusterService, monitoringService, getLicenseState(), client)); - collectors.add(new IndexRecoveryCollector(settings, clusterService, monitoringService, getLicenseState(), client)); - collectors.add(new JobStatsCollector(settings, clusterService, monitoringService, getLicenseState(), client)); - collectors.add(new CcrStatsCollector(settings, clusterService, monitoringService, getLicenseState(), client)); - - monitoringService.addCollectors(collectors); + collectors.add(new IndexStatsCollector(settings, clusterService, getLicenseState(), client)); + collectors.add(new ClusterStatsCollector(settings, clusterService, getLicenseState(), client, getLicenseService())); + collectors.add(new ShardsCollector(settings, clusterService, getLicenseState())); + collectors.add(new NodeStatsCollector(settings, clusterService, getLicenseState(), client)); + collectors.add(new IndexRecoveryCollector(settings, clusterService, getLicenseState(), client)); + collectors.add(new JobStatsCollector(settings, clusterService, getLicenseState(), client)); + collectors.add(new CcrStatsCollector(settings, clusterService, getLicenseState(), client)); + + final MonitoringService monitoringService = new MonitoringService(settings, clusterService, threadPool, collectors, exporters); return Arrays.asList(monitoringService, exporters, cleanerService); } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java index 24d991464d58f..9c7b4c3a02926 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java @@ -24,7 +24,6 @@ import java.io.Closeable; import java.util.ArrayList; import java.util.Collection; -import java.util.HashSet; import java.util.Objects; import java.util.Set; import java.util.concurrent.Semaphore; @@ -44,9 +43,14 @@ public class MonitoringService extends AbstractLifecycleComponent { */ public static final TimeValue MIN_INTERVAL = TimeValue.timeValueSeconds(1L); - /** + /* * Dynamically controls enabling or disabling the collection of Monitoring data only from Elasticsearch. - */ + *

+ * This should only be used while transitioning to Metricbeat-based data collection for Elasticsearch with + * {@linkplain #ENABLED} set to {@code true}. By setting this to {@code false} and that value to {@code true}, + * Kibana, Logstash, Beats, and APM Server can all continue to report their stats through this cluster until they + * are transitioned to being monitored by Metricbeat as well. + */ public static final Setting ELASTICSEARCH_COLLECTION_ENABLED = Setting.boolSetting("xpack.monitoring.elasticsearch.collection.enabled", true, Setting.Property.Dynamic, Setting.Property.NodeScope); @@ -82,11 +86,12 @@ public class MonitoringService extends AbstractLifecycleComponent { private volatile TimeValue interval; private volatile ThreadPool.Cancellable scheduler; - MonitoringService(Settings settings, ClusterService clusterService, ThreadPool threadPool, Exporters exporters) { + MonitoringService(Settings settings, ClusterService clusterService, ThreadPool threadPool, + Set collectors, Exporters exporters) { super(settings); this.clusterService = Objects.requireNonNull(clusterService); this.threadPool = Objects.requireNonNull(threadPool); - this.collectors = new HashSet(); + this.collectors = Objects.requireNonNull(collectors); this.exporters = Objects.requireNonNull(exporters); this.elasticsearchCollectionEnabled = ELASTICSEARCH_COLLECTION_ENABLED.get(settings); this.enabled = ENABLED.get(settings); @@ -113,11 +118,6 @@ void setInterval(final TimeValue interval) { scheduleExecution(); } - void addCollectors(Set collectors) { - this.collectors.addAll(Objects.requireNonNull(collectors)); - scheduleExecution(); - } - public TimeValue getInterval() { return interval; } @@ -130,6 +130,10 @@ public boolean isElasticsearchCollectionEnabled() { return this.elasticsearchCollectionEnabled; } + public boolean shouldScheduleExecution() { + return isElasticsearchCollectionEnabled() && isMonitoringActive(); + } + private String threadPoolName() { return ThreadPool.Names.GENERIC; } @@ -181,7 +185,7 @@ void scheduleExecution() { if (scheduler != null) { cancelExecution(); } - if (isMonitoringActive()) { + if (shouldScheduleExecution()) { scheduler = threadPool.scheduleWithFixedDelay(monitor, interval, threadPoolName()); } } @@ -214,7 +218,7 @@ class MonitoringExecution extends AbstractRunnable implements Closeable { @Override public void doRun() { - if (isMonitoringActive() == false) { + if (shouldScheduleExecution() == false) { logger.debug("monitoring execution is skipped"); return; } @@ -249,7 +253,7 @@ protected void doRun() throws Exception { new ParameterizedMessage("monitoring collector [{}] failed to collect data", collector.name()), e); } } - if (isMonitoringActive()) { + if (shouldScheduleExecution()) { exporters.export(results, ActionListener.wrap(r -> semaphore.release(), this::onFailure)); } else { semaphore.release(); diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/Collector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/Collector.java index 05112475ce3f3..b03ce2001b73b 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/Collector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/Collector.java @@ -20,7 +20,6 @@ import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.core.XPackField; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; -import org.elasticsearch.xpack.monitoring.MonitoringService; import java.util.Collection; import java.util.List; @@ -45,18 +44,15 @@ public abstract class Collector extends AbstractComponent { private final String name; private final Setting collectionTimeoutSetting; - private final MonitoringService monitoringService; protected final ClusterService clusterService; protected final XPackLicenseState licenseState; public Collector(final Settings settings, final String name, final ClusterService clusterService, - final MonitoringService monitoringService, final Setting timeoutSetting, - final XPackLicenseState licenseState) { + final Setting timeoutSetting, final XPackLicenseState licenseState) { super(settings); this.name = name; this.clusterService = clusterService; - this.monitoringService = monitoringService; this.collectionTimeoutSetting = timeoutSetting; this.licenseState = licenseState; } @@ -80,13 +76,6 @@ protected boolean shouldCollect(final boolean isElectedMaster) { logger.trace("collector [{}] can not collect data due to invalid license", name()); return false; } - - // TODO: Check if Elasticsearch collection is enabled - if (this.monitoringService.isElasticsearchCollectionEnabled() == false) { - logger.trace("collector [{}] will not collect data because elasticsearch collection is disabled", name()); - return false; - } - return true; } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ccr/CcrStatsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ccr/CcrStatsCollector.java index 8abd9f51ef93c..510f430d19647 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ccr/CcrStatsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ccr/CcrStatsCollector.java @@ -20,7 +20,6 @@ import org.elasticsearch.xpack.core.ccr.action.CcrStatsAction; import org.elasticsearch.xpack.core.ccr.client.CcrClient; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; -import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.Collection; @@ -40,20 +39,18 @@ public class CcrStatsCollector extends Collector { public CcrStatsCollector( final Settings settings, final ClusterService clusterService, - final MonitoringService monitoringService, final XPackLicenseState licenseState, final Client client) { - this(settings, clusterService, monitoringService, licenseState, new XPackClient(client).ccr(), client.threadPool().getThreadContext()); + this(settings, clusterService, licenseState, new XPackClient(client).ccr(), client.threadPool().getThreadContext()); } CcrStatsCollector( final Settings settings, final ClusterService clusterService, - final MonitoringService monitoringService, final XPackLicenseState licenseState, final CcrClient ccrClient, final ThreadContext threadContext) { - super(settings, TYPE, clusterService, monitoringService, CCR_STATS_TIMEOUT, licenseState); + super(settings, TYPE, clusterService, CCR_STATS_TIMEOUT, licenseState); this.ccrClient = ccrClient; this.threadContext = threadContext; } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java index 56ac8f80dbdbe..1a05b03436495 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollector.java @@ -28,7 +28,6 @@ import org.elasticsearch.xpack.core.XPackFeatureSet; import org.elasticsearch.xpack.core.action.XPackUsageRequestBuilder; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; -import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.Collection; @@ -62,22 +61,19 @@ public class ClusterStatsCollector extends Collector { public ClusterStatsCollector(final Settings settings, final ClusterService clusterService, - final MonitoringService monitoringService, final XPackLicenseState licenseState, final Client client, final LicenseService licenseService) { - this(settings, clusterService, monitoringService, licenseState, client, licenseService, - new IndexNameExpressionResolver(Settings.EMPTY)); + this(settings, clusterService, licenseState, client, licenseService, new IndexNameExpressionResolver(Settings.EMPTY)); } ClusterStatsCollector(final Settings settings, final ClusterService clusterService, - final MonitoringService monitoringService, final XPackLicenseState licenseState, final Client client, final LicenseService licenseService, final IndexNameExpressionResolver indexNameExpressionResolver) { - super(settings, ClusterStatsMonitoringDoc.TYPE, clusterService, monitoringService, CLUSTER_STATS_TIMEOUT, licenseState); + super(settings, ClusterStatsMonitoringDoc.TYPE, clusterService, CLUSTER_STATS_TIMEOUT, licenseState); this.client = client; this.licenseService = licenseService; @@ -87,7 +83,7 @@ public ClusterStatsCollector(final Settings settings, @Override protected boolean shouldCollect(final boolean isElectedMaster) { // This collector can always collect data on the master node - return isElectedMaster && super.shouldCollect(isElectedMaster); + return isElectedMaster; } @Override diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollector.java index 8870912c15e2f..ea8b5065387f3 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollector.java @@ -15,7 +15,6 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; -import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.ArrayList; @@ -49,11 +48,10 @@ public class IndexRecoveryCollector extends Collector { public IndexRecoveryCollector(final Settings settings, final ClusterService clusterService, - final MonitoringService monitoringService, final XPackLicenseState licenseState, final Client client) { - super(settings, IndexRecoveryMonitoringDoc.TYPE, clusterService, monitoringService, INDEX_RECOVERY_TIMEOUT, licenseState); + super(settings, IndexRecoveryMonitoringDoc.TYPE, clusterService, INDEX_RECOVERY_TIMEOUT, licenseState); this.client = Objects.requireNonNull(client); } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollector.java index 3203f6196599c..b6f50ed9e5cfc 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollector.java @@ -18,7 +18,6 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; -import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.ArrayList; @@ -43,10 +42,9 @@ public class IndexStatsCollector extends Collector { public IndexStatsCollector(final Settings settings, final ClusterService clusterService, - final MonitoringService monitoringService, final XPackLicenseState licenseState, final Client client) { - super(settings, "index-stats", clusterService, monitoringService, INDEX_STATS_TIMEOUT, licenseState); + super(settings, "index-stats", clusterService, INDEX_STATS_TIMEOUT, licenseState); this.client = client; } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollector.java index d5668d7f7c92f..cfbf9b7e0a4b8 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollector.java @@ -19,7 +19,6 @@ import org.elasticsearch.xpack.core.ml.action.GetJobsStatsAction; import org.elasticsearch.xpack.core.ml.client.MachineLearningClient; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; -import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.List; @@ -46,15 +45,14 @@ public class JobStatsCollector extends Collector { private final ThreadContext threadContext; private final MachineLearningClient client; - public JobStatsCollector(final Settings settings, final ClusterService clusterService, final MonitoringService monitoringService, + public JobStatsCollector(final Settings settings, final ClusterService clusterService, final XPackLicenseState licenseState, final Client client) { - this(settings, clusterService, monitoringService, licenseState, new XPackClient(client).machineLearning(), - client.threadPool().getThreadContext()); + this(settings, clusterService, licenseState, new XPackClient(client).machineLearning(), client.threadPool().getThreadContext()); } - JobStatsCollector(final Settings settings, final ClusterService clusterService, final MonitoringService monitoringService, + JobStatsCollector(final Settings settings, final ClusterService clusterService, final XPackLicenseState licenseState, final MachineLearningClient client, final ThreadContext threadContext) { - super(settings, JobStatsMonitoringDoc.TYPE, clusterService, monitoringService, JOB_STATS_TIMEOUT, licenseState); + super(settings, JobStatsMonitoringDoc.TYPE, clusterService, JOB_STATS_TIMEOUT, licenseState); this.client = client; this.threadContext = threadContext; } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollector.java index eb28ef171d96a..c990485e6a536 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollector.java @@ -18,7 +18,6 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; -import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.Collection; @@ -52,11 +51,10 @@ public class NodeStatsCollector extends Collector { public NodeStatsCollector(final Settings settings, final ClusterService clusterService, - final MonitoringService monitoringService, final XPackLicenseState licenseState, final Client client) { - super(settings, NodeStatsMonitoringDoc.TYPE, clusterService, monitoringService, NODE_STATS_TIMEOUT, licenseState); + super(settings, NodeStatsMonitoringDoc.TYPE, clusterService, NODE_STATS_TIMEOUT, licenseState); this.client = Objects.requireNonNull(client); } diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollector.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollector.java index 39d84ae5b79a6..b5a3a2920e2f7 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollector.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollector.java @@ -14,7 +14,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringDoc; -import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.ArrayList; @@ -33,10 +32,9 @@ public class ShardsCollector extends Collector { public ShardsCollector(final Settings settings, final ClusterService clusterService, - final MonitoringService monitoringService, final XPackLicenseState licenseState) { - super(settings, ShardMonitoringDoc.TYPE, clusterService, monitoringService, null, licenseState); + super(settings, ShardMonitoringDoc.TYPE, clusterService, null, licenseState); } @Override diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/BaseCollectorTestCase.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/BaseCollectorTestCase.java index 3f8f5752b4cd2..2a48bc66204ea 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/BaseCollectorTestCase.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/BaseCollectorTestCase.java @@ -23,7 +23,6 @@ import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.xpack.monitoring.MonitoringService; import org.elasticsearch.xpack.monitoring.collector.Collector; import java.util.function.Function; @@ -35,7 +34,6 @@ public abstract class BaseCollectorTestCase extends ESTestCase { protected ClusterName clusterName; protected ClusterService clusterService; - protected MonitoringService monitoringService; protected ClusterState clusterState; protected DiscoveryNodes nodes; protected MetaData metaData; @@ -48,7 +46,6 @@ public void setUp() throws Exception { super.setUp(); clusterName = mock(ClusterName.class); clusterService = mock(ClusterService.class); - monitoringService = mock(MonitoringService.class); clusterState = mock(ClusterState.class); nodes = mock(DiscoveryNodes.class); metaData = mock(MetaData.class); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java index 2042422060085..3d23933f8dcc0 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java @@ -25,6 +25,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; +import static java.util.Collections.emptySet; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.mockito.Mockito.mock; @@ -67,7 +68,7 @@ public void terminate() throws Exception { } public void testIsMonitoringActive() throws Exception { - monitoringService = new MonitoringService(Settings.EMPTY, clusterService, threadPool, new CountingExporter()); + monitoringService = new MonitoringService(Settings.EMPTY, clusterService, threadPool, emptySet(), new CountingExporter()); monitoringService.start(); assertBusy(() -> assertTrue(monitoringService.isStarted())); @@ -96,7 +97,7 @@ public void testInterval() throws Exception { .build(); CountingExporter exporter = new CountingExporter(); - monitoringService = new MonitoringService(settings, clusterService, threadPool, exporter); + monitoringService = new MonitoringService(settings, clusterService, threadPool, emptySet(), exporter); monitoringService.start(); assertBusy(() -> assertTrue(monitoringService.isStarted())); @@ -122,7 +123,7 @@ public void testSkipExecution() throws Exception { .put("xpack.monitoring.collection.interval", MonitoringService.MIN_INTERVAL) .build(); - monitoringService = new MonitoringService(settings, clusterService, threadPool, exporter); + monitoringService = new MonitoringService(settings, clusterService, threadPool, emptySet(), exporter); monitoringService.start(); assertBusy(() -> assertTrue(monitoringService.isStarted())); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java index 120467399827d..49355d51495ec 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java @@ -67,25 +67,14 @@ public void setUp() throws Exception { public void testShouldCollectReturnsFalseIfNotMaster() { final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService); + new ClusterStatsCollector(Settings.EMPTY, clusterService, licenseState, client, licenseService); assertThat(collector.shouldCollect(false), is(false)); } - public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { - when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(false); - when(licenseState.isMonitoringAllowed()).thenReturn(true); - final boolean isElectedMaster = randomBoolean(); - whenLocalNodeElectedMaster(isElectedMaster); - - final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService); - assertThat(collector.shouldCollect(isElectedMaster), is(false)); - } - public void testShouldCollectReturnsTrue() { final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService); + new ClusterStatsCollector(Settings.EMPTY, clusterService, licenseState, client, licenseService); assertThat(collector.shouldCollect(true), is(true)); } @@ -97,7 +86,7 @@ public void testDoAPMIndicesExistReturnsBasedOnIndices() { when(resolver.concreteIndices(clusterState, IndicesOptions.lenientExpandOpen(), "apm-*")).thenReturn(indices); final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, resolver); + new ClusterStatsCollector(Settings.EMPTY, clusterService, licenseState, client, licenseService, resolver); assertThat(collector.doAPMIndicesExist(clusterState), is(apmIndicesExist)); } @@ -108,7 +97,7 @@ public void testDoAPMIndicesExistReturnsFalseForExpectedExceptions() { when(resolver.concreteIndices(clusterState, IndicesOptions.lenientExpandOpen(), "apm-*")).thenThrow(exception); final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, resolver); + new ClusterStatsCollector(Settings.EMPTY, clusterService, licenseState, client, licenseService, resolver); assertThat(collector.doAPMIndicesExist(clusterState), is(false)); } @@ -119,7 +108,7 @@ public void testDoAPMIndicesExistRethrowsUnexpectedExceptions() { when(resolver.concreteIndices(clusterState, IndicesOptions.lenientExpandOpen(), "apm-*")).thenThrow(exception); final ClusterStatsCollector collector = - new ClusterStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, licenseService, resolver); + new ClusterStatsCollector(Settings.EMPTY, clusterService, licenseState, client, licenseService, resolver); expectThrows(RuntimeException.class, () -> collector.doAPMIndicesExist(clusterState)); } @@ -221,7 +210,7 @@ public void testDoCollect() throws Exception { when(xPackUsageFuture.actionGet()).thenReturn(xPackUsageResponse); final ClusterStatsCollector collector = - new ClusterStatsCollector(settings.build(), clusterService, monitoringService, licenseState, + new ClusterStatsCollector(settings.build(), clusterService, licenseState, client, licenseService, indexNameExpressionResolver); Assert.assertEquals(timeout, collector.getCollectionTimeout()); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java index efb530a5bd7e9..f4484aa5ed755 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryCollectorTests.java @@ -55,8 +55,7 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final IndexRecoveryCollector collector = - new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -66,28 +65,14 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { public void testShouldCollectReturnsFalseIfNotMaster() { when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexRecoveryCollector collector = - new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, licenseState, client); assertThat(collector.shouldCollect(false), is(false)); } - public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { - when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(false); - when(licenseState.isMonitoringAllowed()).thenReturn(true); - final boolean isElectedMaster = randomBoolean(); - whenLocalNodeElectedMaster(isElectedMaster); - - final IndexRecoveryCollector collector = - new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); - assertThat(collector.shouldCollect(isElectedMaster), is(false)); - } - public void testShouldCollectReturnsTrue() { - when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexRecoveryCollector collector = - new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, licenseState, client); assertThat(collector.shouldCollect(true), is(true)); verify(licenseState).isMonitoringAllowed(); @@ -153,8 +138,7 @@ public void testDoCollect() throws Exception { final Client client = mock(Client.class); when(client.admin()).thenReturn(adminClient); - final IndexRecoveryCollector collector = - new IndexRecoveryCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexRecoveryCollector collector = new IndexRecoveryCollector(Settings.EMPTY, clusterService, licenseState, client); assertEquals(timeout, collector.getCollectionTimeout()); assertEquals(recoveryOnly, collector.getActiveRecoveriesOnly()); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java index 42f746ff8c7e9..ef96726aaee01 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndexStatsCollectorTests.java @@ -50,8 +50,7 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final IndexStatsCollector collector = - new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -61,28 +60,14 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { public void testShouldCollectReturnsFalseIfNotMaster() { when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexStatsCollector collector = - new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, licenseState, client); assertThat(collector.shouldCollect(false), is(false)); } - public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { - when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(false); - when(licenseState.isMonitoringAllowed()).thenReturn(true); - final boolean isElectedMaster = randomBoolean(); - whenLocalNodeElectedMaster(isElectedMaster); - - final IndexStatsCollector collector = - new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); - assertThat(collector.shouldCollect(isElectedMaster), is(false)); - } - public void testShouldCollectReturnsTrue() { - when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); when(licenseState.isMonitoringAllowed()).thenReturn(true); - final IndexStatsCollector collector = - new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, licenseState, client); assertThat(collector.shouldCollect(true), is(true)); verify(licenseState).isMonitoringAllowed(); @@ -159,8 +144,7 @@ public void testDoCollect() throws Exception { final Client client = mock(Client.class); when(client.admin()).thenReturn(adminClient); - final IndexStatsCollector collector = - new IndexStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final IndexStatsCollector collector = new IndexStatsCollector(Settings.EMPTY, clusterService, licenseState, client); assertEquals(timeout, collector.getCollectionTimeout()); final long interval = randomNonNegativeLong(); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java index 1b90e26c02ee9..1d4e3d8857bf6 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/ml/JobStatsCollectorTests.java @@ -50,7 +50,7 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { when(licenseState.isMonitoringAllowed()).thenReturn(false); when(licenseState.isMachineLearningAllowed()).thenReturn(mlAllowed); - final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, monitoringService, licenseState, client); + final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -67,7 +67,7 @@ public void testShouldCollectReturnsFalseIfNotMaster() { // this controls the blockage final boolean isElectedMaster = false; - final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, monitoringService, licenseState, client); + final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); } @@ -82,7 +82,7 @@ public void testShouldCollectReturnsFalseIfMLIsDisabled() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, monitoringService, licenseState, client); + final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); @@ -100,7 +100,7 @@ public void testShouldCollectReturnsFalseIfMLIsNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, monitoringService, licenseState, client); + final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); @@ -109,25 +109,14 @@ public void testShouldCollectReturnsFalseIfMLIsNotAllowed() { } } - public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { - when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(false); - when(licenseState.isMonitoringAllowed()).thenReturn(true); - final boolean isElectedMaster = randomBoolean(); - whenLocalNodeElectedMaster(isElectedMaster); - - final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, monitoringService, licenseState, client); - assertThat(collector.shouldCollect(isElectedMaster), is(false)); - } - public void testShouldCollectReturnsTrue() { final Settings settings = mlEnabledSettings(); when(licenseState.isMonitoringAllowed()).thenReturn(true); - when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); when(licenseState.isMachineLearningAllowed()).thenReturn(true); final boolean isElectedMaster = true; - final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, monitoringService, licenseState, client); + final JobStatsCollector collector = new JobStatsCollector(settings, clusterService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(true)); @@ -145,8 +134,7 @@ public void testDoCollect() throws Exception { final TimeValue timeout = TimeValue.timeValueSeconds(randomIntBetween(1, 120)); withCollectionTimeout(JobStatsCollector.JOB_STATS_TIMEOUT, timeout); - final JobStatsCollector collector = - new JobStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client, threadContext); + final JobStatsCollector collector = new JobStatsCollector(Settings.EMPTY, clusterService, licenseState, client, threadContext); assertEquals(timeout, collector.getCollectionTimeout()); final List jobStats = mockJobStats(); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java index 8a0e685488a30..03692cc9d5382 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsCollectorTests.java @@ -43,8 +43,7 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final NodeStatsCollector collector = - new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -52,24 +51,11 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { } } - public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { - when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(false); - when(licenseState.isMonitoringAllowed()).thenReturn(true); - final boolean isElectedMaster = randomBoolean(); - whenLocalNodeElectedMaster(isElectedMaster); - - final NodeStatsCollector collector = - new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); - assertThat(collector.shouldCollect(isElectedMaster), is(false)); - } - public void testShouldCollectReturnsTrue() { when(licenseState.isMonitoringAllowed()).thenReturn(true); - when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); final boolean isElectedMaster = true; - final NodeStatsCollector collector = - new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, licenseState, client); assertThat(collector.shouldCollect(isElectedMaster), is(true)); verify(licenseState).isMonitoringAllowed(); @@ -90,8 +76,7 @@ public void testDoCollectWithFailures() throws Exception { final Client client = mock(Client.class); thenReturnNodeStats(client, timeout, nodesStatsResponse); - final NodeStatsCollector collector = - new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, licenseState, client); assertEquals(timeout, collector.getCollectionTimeout()); final FailedNodeException e = expectThrows(FailedNodeException.class, () -> @@ -125,8 +110,7 @@ public void testDoCollect() throws Exception { final Client client = mock(Client.class); thenReturnNodeStats(client, timeout, nodesStatsResponse); - final NodeStatsCollector collector = - new NodeStatsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState, client); + final NodeStatsCollector collector = new NodeStatsCollector(Settings.EMPTY, clusterService, licenseState, client); assertEquals(timeout, collector.getCollectionTimeout()); final long interval = randomNonNegativeLong(); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java index 89c3c0277e3f5..4affc3a164397 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/shards/ShardsCollectorTests.java @@ -48,7 +48,7 @@ public void testShouldCollectReturnsFalseIfMonitoringNotAllowed() { final boolean isElectedMaster = randomBoolean(); whenLocalNodeElectedMaster(isElectedMaster); - final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState); + final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, licenseState); assertThat(collector.shouldCollect(isElectedMaster), is(false)); if (isElectedMaster) { @@ -61,34 +61,23 @@ public void testShouldCollectReturnsFalseIfNotMaster() { // this controls the blockage whenLocalNodeElectedMaster(false); - final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState); + final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, licenseState); assertThat(collector.shouldCollect(false), is(false)); } - public void testShouldCollectReturnsFalseIfElasticsearchCollectionIsDisabled() { - when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(false); - when(licenseState.isMonitoringAllowed()).thenReturn(true); - final boolean isElectedMaster = randomBoolean(); - whenLocalNodeElectedMaster(isElectedMaster); - - final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState); - assertThat(collector.shouldCollect(isElectedMaster), is(false)); - } - public void testShouldCollectReturnsTrue() { when(licenseState.isMonitoringAllowed()).thenReturn(true); - when(monitoringService.isElasticsearchCollectionEnabled()).thenReturn(true); whenLocalNodeElectedMaster(true); - final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState); + final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, licenseState); assertThat(collector.shouldCollect(true), is(true)); verify(licenseState).isMonitoringAllowed(); } public void testDoCollectWhenNoClusterState() throws Exception { - final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState); + final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, licenseState); final Collection results = collector.doCollect(randomMonitoringNode(random()), randomNonNegativeLong(), null); assertThat(results, notNullValue()); @@ -116,7 +105,7 @@ public void testDoCollect() throws Exception { when(nodes.get(eq("_current"))).thenReturn(localNode); when(clusterState.getNodes()).thenReturn(nodes); - final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, monitoringService, licenseState); + final ShardsCollector collector = new ShardsCollector(Settings.EMPTY, clusterService, licenseState); assertNull(collector.getCollectionTimeout()); assertArrayEquals(indices, collector.getCollectionIndices()); From 9f6a20863ba8f3dcaaad42c4e6ec7ec39d6401ed Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Sat, 15 Sep 2018 03:22:48 -0700 Subject: [PATCH 14/17] Doc fixes / improvements --- docs/reference/monitoring/configuring-monitoring.asciidoc | 6 ++++-- docs/reference/monitoring/pause-export.asciidoc | 2 +- docs/reference/settings/monitoring-settings.asciidoc | 7 +++++++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/reference/monitoring/configuring-monitoring.asciidoc b/docs/reference/monitoring/configuring-monitoring.asciidoc index b230540b91045..6708b791036a9 100644 --- a/docs/reference/monitoring/configuring-monitoring.asciidoc +++ b/docs/reference/monitoring/configuring-monitoring.asciidoc @@ -16,8 +16,10 @@ indices. You can also adjust how monitoring data is displayed. .. Verify that the `xpack.monitoring.enabled`, `xpack.monitoring.collection.enabled`, and `xpack.monitoring.elasticsearch.collection.enabled` settings are `true` on each -node in the cluster. By default, data collection is disabled. For more -information, see <>. +node in the cluster. By default xpack.monitoring.collection.enabled is disabled +(`false`), and that overrides xpack.monitoring.elasticsearch.collection.enabled, +which defaults to being enabled (`true`). Both settings can be set dynamically +at runtime. For more information, see <>. .. Optional: Specify which indices you want to monitor. + diff --git a/docs/reference/monitoring/pause-export.asciidoc b/docs/reference/monitoring/pause-export.asciidoc index e46e078a01d75..7a8bc664ffc38 100644 --- a/docs/reference/monitoring/pause-export.asciidoc +++ b/docs/reference/monitoring/pause-export.asciidoc @@ -23,7 +23,7 @@ just for {es}: [source,yaml] --------------------------------------------------- xpack.monitoring.collection.enabled: true -xpack.monitoring.elasticsearch.collection.enabled: true +xpack.monitoring.elasticsearch.collection.enabled: false --------------------------------------------------- If you want to separately disable a specific exporter, you can specify the diff --git a/docs/reference/settings/monitoring-settings.asciidoc b/docs/reference/settings/monitoring-settings.asciidoc index 7109170544c62..405e04c31b808 100644 --- a/docs/reference/settings/monitoring-settings.asciidoc +++ b/docs/reference/settings/monitoring-settings.asciidoc @@ -69,6 +69,13 @@ You can update this setting through the `xpack.monitoring.elasticsearch.collection.enabled`:: Controls whether statistics about your {es} cluster should be collected. Defaults to `true`. +This is different from xpack.monitoring.collection.enabled, which allows you to enable or disable +all monitoring collection. However, this setting simply disables the collection of Elasticsearch +data while still allowing other data (e.g., Kibana, Logstash, Beats, or APM Server monitoring data) +to use through this cluster. ++ +You can update this setting through the +<>. `xpack.monitoring.collection.cluster.stats.timeout`:: From 8d3e9311fb48210819dbb8be8f2a8dd9e9116c6d Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Sat, 15 Sep 2018 03:23:04 -0700 Subject: [PATCH 15/17] Making methods package private --- .../elasticsearch/xpack/monitoring/MonitoringService.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java index 9c7b4c3a02926..4b9d09b3b9760 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java @@ -122,15 +122,15 @@ public TimeValue getInterval() { return interval; } - public boolean isMonitoringActive() { + boolean isMonitoringActive() { return isStarted() && enabled; } - public boolean isElasticsearchCollectionEnabled() { + boolean isElasticsearchCollectionEnabled() { return this.elasticsearchCollectionEnabled; } - public boolean shouldScheduleExecution() { + boolean shouldScheduleExecution() { return isElasticsearchCollectionEnabled() && isMonitoringActive(); } From 94749fc68a885f68a406a99eeb2f8648b62bcca1 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Mon, 17 Sep 2018 16:05:08 -0700 Subject: [PATCH 16/17] Fixing wording --- docs/reference/settings/monitoring-settings.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/settings/monitoring-settings.asciidoc b/docs/reference/settings/monitoring-settings.asciidoc index 405e04c31b808..a039084412cda 100644 --- a/docs/reference/settings/monitoring-settings.asciidoc +++ b/docs/reference/settings/monitoring-settings.asciidoc @@ -72,7 +72,7 @@ Controls whether statistics about your {es} cluster should be collected. Default This is different from xpack.monitoring.collection.enabled, which allows you to enable or disable all monitoring collection. However, this setting simply disables the collection of Elasticsearch data while still allowing other data (e.g., Kibana, Logstash, Beats, or APM Server monitoring data) -to use through this cluster. +to pass through this cluster. + You can update this setting through the <>. From 5adbb8d69f74b902d25bcb239339adc94a57b6b2 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Mon, 17 Sep 2018 16:05:42 -0700 Subject: [PATCH 17/17] Fixing method access --- .../org/elasticsearch/xpack/monitoring/MonitoringService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java index 4b9d09b3b9760..073a4cf785c41 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java @@ -122,7 +122,7 @@ public TimeValue getInterval() { return interval; } - boolean isMonitoringActive() { + public boolean isMonitoringActive() { return isStarted() && enabled; }