[Logstash][Monitoring] Fix Registered Usage and Add Tests (elastic/x-pack-elasticsearch#4075)

This properly registers the `XPackFeatureSetUsage` for Logstash and
it tests it by invoking the Usage API in a Monitoring QA test.

Without those being properly registered, the test will consistently fail.

Original commit: elastic/x-pack-elasticsearch@2e8f2376fd

plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackClientPlugin.java

@@ -36,6 +36,7 @@ import org.elasticsearch.xpack.core.action.XPackUsageAction;
 import org.elasticsearch.xpack.core.deprecation.DeprecationInfoAction;
 import org.elasticsearch.xpack.core.graph.GraphFeatureSetUsage;
 import org.elasticsearch.xpack.core.graph.action.GraphExploreAction;
+import org.elasticsearch.xpack.core.logstash.LogstashFeatureSetUsage;
 import org.elasticsearch.xpack.core.ml.MachineLearningFeatureSetUsage;
 import org.elasticsearch.xpack.core.ml.MlMetadata;
 import org.elasticsearch.xpack.core.ml.action.CloseJobAction;
@@ -292,7 +293,7 @@ public class XPackClientPlugin extends Plugin implements ActionPlugin, NetworkPl
                 // x-pack
                 XPackInfoAction.INSTANCE,
                 XPackUsageAction.INSTANCE,
-                //rollup
+                // rollup
                 RollupSearchAction.INSTANCE,
                 PutRollupJobAction.INSTANCE,
                 StartRollupJobAction.INSTANCE,
@@ -308,6 +309,8 @@ public class XPackClientPlugin extends Plugin implements ActionPlugin, NetworkPl
         return Arrays.asList(
                 // graph
                 new NamedWriteableRegistry.Entry(XPackFeatureSet.Usage.class, XPackField.GRAPH, GraphFeatureSetUsage::new),
+                // logstash
+                new NamedWriteableRegistry.Entry(XPackFeatureSet.Usage.class, XPackField.LOGSTASH, LogstashFeatureSetUsage::new),
                 // ML - Custom metadata
                 new NamedWriteableRegistry.Entry(MetaData.Custom.class, "ml", MlMetadata::new),
                 new NamedWriteableRegistry.Entry(NamedDiff.class, "ml", MlMetadata.MlMetadataDiff::new),
@@ -344,7 +347,7 @@ public class XPackClientPlugin extends Plugin implements ActionPlugin, NetworkPl
                 // licensing
                 new NamedWriteableRegistry.Entry(MetaData.Custom.class, LicensesMetaData.TYPE, LicensesMetaData::new),
                 new NamedWriteableRegistry.Entry(NamedDiff.class, LicensesMetaData.TYPE, LicensesMetaData::readDiffFrom),
-                //rollup
+                // rollup
                 new NamedWriteableRegistry.Entry(XPackFeatureSet.Usage.class, XPackField.ROLLUP, RollupFeatureSetUsage::new),
                 new NamedWriteableRegistry.Entry(PersistentTaskParams.class, RollupJob.NAME, RollupJob::new),
                 new NamedWriteableRegistry.Entry(Task.Status.class, RollupJobStatus.NAME, RollupJobStatus::new)

plugin/core/src/main/java/org/elasticsearch/xpack/core/logstash/LogstashFeatureSetUsage.java

@@ -0,0 +1,24 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.xpack.core.logstash;
+
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.xpack.core.XPackFeatureSet;
+import org.elasticsearch.xpack.core.XPackField;
+
+import java.io.IOException;
+
+public class LogstashFeatureSetUsage extends XPackFeatureSet.Usage {
+
+    public LogstashFeatureSetUsage(StreamInput in) throws IOException {
+        super(in);
+    }
+
+    public LogstashFeatureSetUsage(boolean available, boolean enabled) {
+        super(XPackField.LOGSTASH, available, enabled);
+    }
+
+}

plugin/logstash/src/main/java/org/elasticsearch/xpack/logstash/Logstash.java

@@ -28,7 +28,6 @@ import java.util.regex.Pattern;
  */
 public class Logstash extends Plugin implements ActionPlugin {
 
-    public static final String NAME = "logstash";
     private static final String LOGSTASH_TEMPLATE_NAME = "logstash-index-template";
     private static final String TEMPLATE_VERSION_PATTERN =
             Pattern.quote("${logstash.template.version}");

plugin/logstash/src/main/java/org/elasticsearch/xpack/logstash/LogstashFeatureSet.java

@@ -14,7 +14,9 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.xpack.core.XPackFeatureSet;
+import org.elasticsearch.xpack.core.XPackField;
 import org.elasticsearch.xpack.core.XPackSettings;
+import org.elasticsearch.xpack.core.logstash.LogstashFeatureSetUsage;
 
 
 import java.io.IOException;
@@ -33,7 +35,7 @@ public class LogstashFeatureSet implements XPackFeatureSet {
 
     @Override
     public String name() {
-        return Logstash.NAME;
+        return XPackField.LOGSTASH;
     }
 
     @Override
@@ -58,25 +60,7 @@ public class LogstashFeatureSet implements XPackFeatureSet {
 
     @Override
     public void usage(ActionListener<XPackFeatureSet.Usage> listener) {
-        listener.onResponse(new LogstashFeatureSet.Usage(available(), enabled()));
+        listener.onResponse(new LogstashFeatureSetUsage(available(), enabled()));
     }
 
-    public static class Usage extends XPackFeatureSet.Usage {
-
-        public Usage(StreamInput in) throws IOException {
-            super(in);
-        }
-
-        public Usage(boolean available, boolean enabled) {
-            super(Logstash.NAME, available, enabled);
-        }
-
-        public void writeTo(StreamOutput out) throws IOException {
-            super.writeTo(out);
-        }
-
-        protected void innerXContent(XContentBuilder builder, Params params) throws IOException {
-            super.innerXContent(builder, params);
-        }
-    }
 }

plugin/logstash/src/test/java/org/elasticsearch/xpack/logstash/LogstashFeatureSetTests.java

@@ -11,6 +11,7 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.core.XPackFeatureSet;
+import org.elasticsearch.xpack.core.logstash.LogstashFeatureSetUsage;
 
 import static org.mockito.Mockito.mock;
 import static org.hamcrest.core.Is.is;
@@ -33,7 +34,7 @@ public class LogstashFeatureSetTests extends ESTestCase {
 
         BytesStreamOutput out = new BytesStreamOutput();
         usage.writeTo(out);
-        XPackFeatureSet.Usage serializedUsage = new LogstashFeatureSet.Usage(out.bytes().streamInput());
+        XPackFeatureSet.Usage serializedUsage = new LogstashFeatureSetUsage(out.bytes().streamInput());
         assertThat(serializedUsage.enabled(), is(enabled));
     }
 
@@ -57,7 +58,7 @@ public class LogstashFeatureSetTests extends ESTestCase {
 
         BytesStreamOutput out = new BytesStreamOutput();
         usage.writeTo(out);
-        XPackFeatureSet.Usage serializedUsage = new LogstashFeatureSet.Usage(out.bytes().streamInput());
+        XPackFeatureSet.Usage serializedUsage = new LogstashFeatureSetUsage(out.bytes().streamInput());
         assertThat(serializedUsage.available(), is(available));
     }
 }

plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/integration/MonitoringIT.java

@@ -516,7 +516,7 @@ public class MonitoringIT extends ESSingleNodeTestCase {
         // delete anything that may happen to already exist
         assertAcked(client().admin().indices().prepareDelete(".monitoring-*"));
 
-        assertThat("Must be no enabled exporters before enabling monitoring", getMonitoringUsageExportersDefined(), is(true));
+        assertThat("Must be no enabled exporters before enabling monitoring", getMonitoringUsageExportersDefined(), is(false));
 
         final Settings settings = Settings.builder()
                 .put("xpack.monitoring.collection.enabled", true)
@@ -525,7 +525,7 @@ public class MonitoringIT extends ESSingleNodeTestCase {
 
         assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
 
-        assertBusy(() -> assertThat("[_local] exporter not enabled yet", getMonitoringUsageExportersDefined(), is(false)));
+        assertBusy(() -> assertThat("[_local] exporter not enabled yet", getMonitoringUsageExportersDefined(), is(true)));
 
         assertBusy(() -> {
             // Monitoring uses auto_expand_replicas, so it should be green even without replicas
@@ -552,7 +552,7 @@ public class MonitoringIT extends ESSingleNodeTestCase {
 
         assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
 
-        assertBusy(() -> assertThat("Exporters are not yet stopped", getMonitoringUsageExportersDefined(), is(true)));
+        assertBusy(() -> assertThat("Exporters are not yet stopped", getMonitoringUsageExportersDefined(), is(false)));
         assertBusy(() -> {
             try {
                 // now wait until Monitoring has actually stopped
@@ -588,7 +588,7 @@ public class MonitoringIT extends ESSingleNodeTestCase {
 
         assertThat("Monitoring feature set does not exist", monitoringUsage.isPresent(), is(true));
 
-        return monitoringUsage.get().getExporters().isEmpty();
+        return monitoringUsage.get().getExporters().isEmpty() == false;
     }
 
     /**

qa/smoke-test-plugins-ssl/src/test/java/org/elasticsearch/smoketest/SmokeTestMonitoringWithSecurityIT.java

@@ -13,6 +13,9 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.xpack.core.XPackPlugin;
+import org.elasticsearch.xpack.core.action.XPackUsageRequestBuilder;
+import org.elasticsearch.xpack.core.action.XPackUsageResponse;
+import org.elasticsearch.xpack.core.monitoring.MonitoringFeatureSetUsage;
 import org.elasticsearch.xpack.core.security.SecurityField;
 import org.junit.After;
 import org.junit.Before;
@@ -21,11 +24,13 @@ import java.net.InetSocketAddress;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.is;
 
 /**
  * This test checks that a Monitoring's HTTP exporter correctly exports to a monitoring cluster
@@ -56,8 +61,9 @@ public class SmokeTestMonitoringWithSecurityIT extends ESIntegTestCase {
     @Before
     public void enableExporter() throws Exception {
         Settings exporterSettings = Settings.builder()
+                .put("xpack.monitoring.collection.enabled", true)
                 .put("xpack.monitoring.exporters._http.enabled", true)
-                .put("xpack.monitoring.exporters._http.host", "https://" + NetworkAddress.format(randomFrom(httpAddresses())))
+                .put("xpack.monitoring.exporters._http.host", "https://" + randomNodeHttpAddress())
                 .build();
         assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(exporterSettings));
     }
@@ -65,14 +71,31 @@ public class SmokeTestMonitoringWithSecurityIT extends ESIntegTestCase {
     @After
     public void disableExporter() {
         Settings exporterSettings = Settings.builder()
+                .putNull("xpack.monitoring.collection.enabled")
                 .putNull("xpack.monitoring.exporters._http.enabled")
                 .putNull("xpack.monitoring.exporters._http.host")
                 .build();
         assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(exporterSettings));
     }
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/x-pack-elasticsearch/issues/2077")
+    private boolean getMonitoringUsageExportersDefined() throws Exception {
+        final XPackUsageResponse usageResponse = new XPackUsageRequestBuilder(client()).execute().get();
+        final Optional<MonitoringFeatureSetUsage> monitoringUsage =
+                usageResponse.getUsages()
+                        .stream()
+                        .filter(usage -> usage instanceof MonitoringFeatureSetUsage)
+                        .map(usage -> (MonitoringFeatureSetUsage)usage)
+                        .findFirst();
+
+        assertThat("Monitoring feature set does not exist", monitoringUsage.isPresent(), is(true));
+
+        return monitoringUsage.get().getExporters().isEmpty() == false;
+    }
+
     public void testHTTPExporterWithSSL() throws Exception {
+        // Ensures that the exporter is actually on
+        assertBusy(() -> assertThat("[_http] exporter is not defined", getMonitoringUsageExportersDefined(), is(true)));
+        
         // Checks that the monitoring index templates have been installed
         assertBusy(() -> {
             GetIndexTemplatesResponse response = client().admin().indices().prepareGetTemplates(MONITORING_PATTERN).get();
@@ -101,7 +124,7 @@ public class SmokeTestMonitoringWithSecurityIT extends ESIntegTestCase {
         });
     }
 
-    private InetSocketAddress[] httpAddresses() {
+    private String randomNodeHttpAddress() {
         List<NodeInfo> nodes = client().admin().cluster().prepareNodesInfo().clear().setHttp(true).get().getNodes();
         assertThat(nodes.size(), greaterThan(0));
 
@@ -109,6 +132,6 @@ public class SmokeTestMonitoringWithSecurityIT extends ESIntegTestCase {
         for (int i = 0; i < nodes.size(); i++) {
             httpAddresses[i] = nodes.get(i).getHttp().address().publishAddress().address();
         }
-        return httpAddresses;
+        return NetworkAddress.format(randomFrom(httpAddresses));
     }
 }