From 91401fcb8371c479f8a87299dba8e8a751580f9c Mon Sep 17 00:00:00 2001 From: Chris Earle Date: Fri, 16 Mar 2018 11:38:18 -0400 Subject: [PATCH] [Monitoring] Add "collection_enabled" to usage (elastic/x-pack-elasticsearch#4128) This adds an indicator to Monitoring's portion of X-Pack usage whether or not collection is actually enabled. It's no longer enough to have an exporter defined by default to know if monitoring is actually running. Original commit: elastic/x-pack-elasticsearch@b2eb881d6125b083d523f7e0c859280c98e0fbf7 --- .../monitoring/MonitoringFeatureSetUsage.java | 20 ++++++++-- .../xpack/monitoring/Monitoring.java | 1 + .../monitoring/MonitoringFeatureSet.java | 11 ++++- .../monitoring/MonitoringFeatureSetTests.java | 40 ++++++++++--------- .../cluster/ClusterStatsCollectorTests.java | 3 +- .../ClusterStatsMonitoringDocTests.java | 5 ++- 6 files changed, 52 insertions(+), 28 deletions(-) diff --git a/plugin/core/src/main/java/org/elasticsearch/xpack/core/monitoring/MonitoringFeatureSetUsage.java b/plugin/core/src/main/java/org/elasticsearch/xpack/core/monitoring/MonitoringFeatureSetUsage.java index c3f6020f27a..a8cf5b895fb 100644 --- a/plugin/core/src/main/java/org/elasticsearch/xpack/core/monitoring/MonitoringFeatureSetUsage.java +++ b/plugin/core/src/main/java/org/elasticsearch/xpack/core/monitoring/MonitoringFeatureSetUsage.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.core.monitoring; +import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -18,19 +19,24 @@ import java.util.Map; public class MonitoringFeatureSetUsage extends XPackFeatureSet.Usage { - private static final String ENABLED_EXPORTERS_XFIELD = "enabled_exporters"; - + @Nullable + private Boolean collectionEnabled; @Nullable private Map exporters; public MonitoringFeatureSetUsage(StreamInput in) throws IOException { super(in); exporters = in.readMap(); + if (in.getVersion().onOrAfter(Version.V_6_3_0)) { + collectionEnabled = in.readOptionalBoolean(); + } } - public MonitoringFeatureSetUsage(boolean available, boolean enabled, Map exporters) { + public MonitoringFeatureSetUsage(boolean available, boolean enabled, + boolean collectionEnabled, Map exporters) { super(XPackField.MONITORING, available, enabled); this.exporters = exporters; + this.collectionEnabled = collectionEnabled; } public Map getExporters() { @@ -41,13 +47,19 @@ public class MonitoringFeatureSetUsage extends XPackFeatureSet.Usage { public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeMap(exporters); + if (out.getVersion().onOrAfter(Version.V_6_3_0)) { + out.writeOptionalBoolean(collectionEnabled); + } } @Override protected void innerXContent(XContentBuilder builder, Params params) throws IOException { super.innerXContent(builder, params); + if (collectionEnabled != null) { + builder.field("collection_enabled", collectionEnabled); + } if (exporters != null) { - builder.field(ENABLED_EXPORTERS_XFIELD, exporters); + builder.field("enabled_exporters", exporters); } } } diff --git a/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java b/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java index 7028a601d45..4f9119df589 100644 --- a/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java +++ b/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java @@ -109,6 +109,7 @@ public class Monitoring extends Plugin implements ActionPlugin { modules.add(b -> { XPackPlugin.bindFeatureSet(b, MonitoringFeatureSet.class); if (transportClientMode || enabled == false) { + b.bind(MonitoringService.class).toProvider(Providers.of(null)); b.bind(Exporters.class).toProvider(Providers.of(null)); } }); diff --git a/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSet.java b/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSet.java index 71e1f639bee..347a72b4f38 100644 --- a/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSet.java +++ b/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSet.java @@ -23,12 +23,17 @@ import java.util.Map; public class MonitoringFeatureSet implements XPackFeatureSet { private final boolean enabled; + private final MonitoringService monitoring; private final XPackLicenseState licenseState; private final Exporters exporters; @Inject - public MonitoringFeatureSet(Settings settings, @Nullable XPackLicenseState licenseState, @Nullable Exporters exporters) { + public MonitoringFeatureSet(Settings settings, + @Nullable MonitoringService monitoring, + @Nullable XPackLicenseState licenseState, + @Nullable Exporters exporters) { this.enabled = XPackSettings.MONITORING_ENABLED.get(settings); + this.monitoring = monitoring; this.licenseState = licenseState; this.exporters = exporters; } @@ -60,7 +65,9 @@ public class MonitoringFeatureSet implements XPackFeatureSet { @Override public void usage(ActionListener listener) { - listener.onResponse(new MonitoringFeatureSetUsage(available(), enabled(), exportersUsage(exporters))); + final boolean collectionEnabled = monitoring != null && monitoring.isMonitoringActive(); + + listener.onResponse(new MonitoringFeatureSetUsage(available(), enabled(), collectionEnabled, exportersUsage(exporters))); } static Map exportersUsage(Exporters exporters) { diff --git a/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSetTests.java b/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSetTests.java index ad1f939ef76..c67d2e8b718 100644 --- a/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSetTests.java +++ b/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringFeatureSetTests.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.monitoring; +import org.elasticsearch.Version; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; @@ -21,7 +22,6 @@ import org.elasticsearch.xpack.monitoring.exporter.Exporter; import org.elasticsearch.xpack.monitoring.exporter.Exporters; import org.elasticsearch.xpack.monitoring.exporter.http.HttpExporter; import org.elasticsearch.xpack.monitoring.exporter.local.LocalExporter; -import org.junit.Before; import java.util.ArrayList; import java.util.Arrays; @@ -36,39 +36,36 @@ import static org.mockito.Mockito.when; public class MonitoringFeatureSetTests extends ESTestCase { - private XPackLicenseState licenseState; - private Exporters exporters; + private final MonitoringService monitoring = mock(MonitoringService.class); + private final XPackLicenseState licenseState = mock(XPackLicenseState.class); + private final Exporters exporters = mock(Exporters.class); - @Before - public void init() throws Exception { - licenseState = mock(XPackLicenseState.class); - exporters = mock(Exporters.class); - } - - public void testAvailable() throws Exception { - MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, licenseState, exporters); + public void testAvailable() { + MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, monitoring, licenseState, exporters); boolean available = randomBoolean(); when(licenseState.isMonitoringAllowed()).thenReturn(available); assertThat(featureSet.available(), is(available)); } - public void testEnabledSetting() throws Exception { + public void testEnabledSetting() { boolean enabled = randomBoolean(); Settings.Builder settings = Settings.builder(); settings.put("xpack.monitoring.enabled", enabled); - MonitoringFeatureSet featureSet = new MonitoringFeatureSet(settings.build(), licenseState, exporters); + MonitoringFeatureSet featureSet = new MonitoringFeatureSet(settings.build(), monitoring, licenseState, exporters); assertThat(featureSet.enabled(), is(enabled)); } - public void testEnabledDefault() throws Exception { - MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, licenseState, exporters); + public void testEnabledDefault() { + MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, monitoring, licenseState, exporters); assertThat(featureSet.enabled(), is(true)); } public void testUsage() throws Exception { - - List exporterList = new ArrayList<>(); + // anything prior to 6.3 does not include collection_enabled (so defaults it to null) + final Version serializedVersion = randomFrom(Version.CURRENT, Version.V_6_3_0, Version.V_6_2_2); + final boolean collectionEnabled = randomBoolean(); int localCount = randomIntBetween(0, 5); + List exporterList = new ArrayList<>(); for (int i = 0; i < localCount; i++) { Exporter exporter = mockExporter(LocalExporter.TYPE, true); exporterList.add(exporter); @@ -97,12 +94,14 @@ public class MonitoringFeatureSetTests extends ESTestCase { } } when(exporters.iterator()).thenReturn(exporterList.iterator()); + when(monitoring.isMonitoringActive()).thenReturn(collectionEnabled); - MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, licenseState, exporters); + MonitoringFeatureSet featureSet = new MonitoringFeatureSet(Settings.EMPTY, monitoring, licenseState, exporters); PlainActionFuture future = new PlainActionFuture<>(); featureSet.usage(future); XPackFeatureSet.Usage monitoringUsage = future.get(); BytesStreamOutput out = new BytesStreamOutput(); + out.setVersion(serializedVersion); monitoringUsage.writeTo(out); XPackFeatureSet.Usage serializedUsage = new MonitoringFeatureSetUsage(out.bytes().streamInput()); for (XPackFeatureSet.Usage usage : Arrays.asList(monitoringUsage, serializedUsage)) { @@ -113,6 +112,11 @@ public class MonitoringFeatureSetTests extends ESTestCase { usage.toXContent(builder, ToXContent.EMPTY_PARAMS); source = ObjectPath.createFromXContent(builder.contentType().xContent(), BytesReference.bytes(builder)); } + if (usage == monitoringUsage || serializedVersion.onOrAfter(Version.V_6_3_0)) { + assertThat(source.evaluate("collection_enabled"), is(collectionEnabled)); + } else { + assertThat(source.evaluate("collection_enabled"), is(nullValue())); + } assertThat(source.evaluate("enabled_exporters"), is(notNullValue())); if (localCount > 0) { assertThat(source.evaluate("enabled_exporters.local"), is(localCount)); diff --git a/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java b/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java index a6f8f81c9aa..22f6c00567c 100644 --- a/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java +++ b/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsCollectorTests.java @@ -198,9 +198,8 @@ public class ClusterStatsCollectorTests extends BaseCollectorTestCase { when(indexNameExpressionResolver.concreteIndices(clusterState, IndicesOptions.lenientExpandOpen(), "apm-*")) .thenReturn(indices); - // Baz - Changed from Logstash to Monitoring featureset final XPackUsageResponse xPackUsageResponse = new XPackUsageResponse( - singletonList(new MonitoringFeatureSetUsage(true, true, null))); + singletonList(new MonitoringFeatureSetUsage(true, true, false, null))); @SuppressWarnings("unchecked") final ActionFuture xPackUsageFuture = (ActionFuture) mock(ActionFuture.class); diff --git a/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java b/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java index 709fc4547b1..e26da862a58 100644 --- a/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java +++ b/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java @@ -254,7 +254,7 @@ public class ClusterStatsMonitoringDocTests extends BaseMonitoringDocTestCase usages = singletonList(new MonitoringFeatureSetUsage(false, true, null)); + final List usages = singletonList(new MonitoringFeatureSetUsage(false, true, false, null)); final NodeInfo mockNodeInfo = mock(NodeInfo.class); when(mockNodeInfo.getVersion()).thenReturn(Version.V_6_0_0_alpha2); @@ -556,7 +556,8 @@ public class ClusterStatsMonitoringDocTests extends BaseMonitoringDocTestCase