Marvel: Client nodes should be able to send metrics

closes elastic/elasticsearch#897

Original commit: elastic/x-pack-elasticsearch@1ebcc9de14

marvel/src/main/java/org/elasticsearch/marvel/agent/collector/node/NodeStatsCollector.java

@@ -55,7 +55,13 @@ public class NodeStatsCollector extends AbstractCollector<NodeStatsCollector> {
 
     @Override
     protected boolean shouldCollect() {
-        return super.shouldCollect() && nodeEnvironment.hasNodeFile();
+        // In some cases, the collector starts to collect nodes stats but the
+        // NodeEnvironment is not fully initialized (NodePath is null) and can fail.
+        // This why we need to check for nodeEnvironment.hasNodeFile() here, but only
+        // for nodes that can hold data. Client nodes can collect nodes stats because
+        // elasticsearch correctly handles the nodes stats for client nodes.
+        return super.shouldCollect()
+                && (clusterService.localNode().isDataNode() == false || nodeEnvironment.hasNodeFile());
     }
 
     @Override

marvel/src/test/java/org/elasticsearch/marvel/MarvelF.java

@@ -11,6 +11,7 @@ import org.elasticsearch.common.util.CollectionUtils;
 import org.elasticsearch.license.plugin.LicensePlugin;
 import org.elasticsearch.node.MockNode;
 import org.elasticsearch.node.Node;
+import org.elasticsearch.shield.ShieldPlugin;
 
 import java.util.Arrays;
 import java.util.concurrent.CountDownLatch;
@@ -37,7 +38,7 @@ public class MarvelF {
         }
 
         final CountDownLatch latch = new CountDownLatch(1);
-        final Node node = new MockNode(settings.build(), Version.CURRENT, Arrays.asList(MarvelPlugin.class, LicensePlugin.class));
+        final Node node = new MockNode(settings.build(), Version.CURRENT, Arrays.asList(MarvelPlugin.class, LicensePlugin.class, ShieldPlugin.class));
         Runtime.getRuntime().addShutdownHook(new Thread() {
 
             @Override

marvel/src/test/java/org/elasticsearch/marvel/agent/renderer/node/MultiNodesStatsTests.java

@@ -0,0 +1,90 @@
+/*
+ * 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.marvel.agent.renderer.node;
+
+import org.elasticsearch.action.search.SearchResponse;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.index.query.QueryBuilders;
+import org.elasticsearch.marvel.agent.collector.node.NodeStatsCollector;
+import org.elasticsearch.marvel.agent.settings.MarvelSettings;
+import org.elasticsearch.marvel.test.MarvelIntegTestCase;
+import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
+import org.elasticsearch.test.ESIntegTestCase.Scope;
+import org.junit.After;
+
+import java.util.concurrent.TimeUnit;
+
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoTimeout;
+import static org.hamcrest.Matchers.*;
+
+@ClusterScope(scope = Scope.TEST, numDataNodes = 0, numClientNodes = 0, transportClientRatio = 0.0)
+public class MultiNodesStatsTests extends MarvelIntegTestCase {
+
+    @Override
+    protected Settings nodeSettings(int nodeOrdinal) {
+        return Settings.builder()
+                .put(super.nodeSettings(nodeOrdinal))
+                .put(MarvelSettings.INTERVAL, "-1")
+                .put("marvel.agent.exporters.default_local.type", "local")
+                .put("marvel.agent.exporters.default_local.template.settings.index.number_of_replicas", 0)
+                .build();
+    }
+
+    @After
+    public void cleanup() throws Exception {
+        updateMarvelInterval(-1, TimeUnit.SECONDS);
+        wipeMarvelIndices();
+    }
+
+    public void testMultipleNodes() throws Exception {
+        logger.debug("--> starting a master only node");
+        internalCluster().startMasterOnlyNodeAsync();
+
+        logger.debug("--> starting a data only node");
+        internalCluster().startDataOnlyNodeAsync();
+
+        logger.debug("--> starting a client node");
+        internalCluster().startNodeClient(Settings.EMPTY);
+
+        logger.debug("--> starting few other nodes");
+        int extraNodes = randomIntBetween(2, 5);
+        for (int i = 0; i < extraNodes; i++) {
+            if (randomBoolean()) {
+                internalCluster().startNodeAsync();
+            } else {
+                internalCluster().startNodeClient(Settings.EMPTY);
+            }
+        }
+
+        final int nbNodes = 3 +  extraNodes;
+        logger.debug("--> waiting for {} nodes to be available", nbNodes);
+        assertBusy(new Runnable() {
+            @Override
+            public void run() {
+                assertNoTimeout(client().admin().cluster().prepareHealth().setWaitForNodes(Integer.toString(nbNodes)).get());
+            }
+        });
+
+        updateMarvelInterval(3L, TimeUnit.SECONDS);
+        waitForMarvelIndices();
+
+        assertBusy(new Runnable() {
+            @Override
+            public void run() {
+                securedFlush();
+
+                for (String nodeName : internalCluster().getNodeNames()) {
+                    SearchResponse response = client(nodeName).prepareSearch()
+                            .setTypes(NodeStatsCollector.TYPE)
+                            .setQuery(QueryBuilders.termQuery("node_stats.node_id", internalCluster().clusterService(nodeName).localNode().getId()))
+                            .get();
+                    assertThat(response.getHits().getTotalHits(), greaterThan(0L));
+                }
+            }
+        });
+    }
+
+}

marvel/src/test/java/org/elasticsearch/marvel/agent/renderer/node/NodeStatsTests.java

@@ -21,7 +21,8 @@ import java.util.concurrent.TimeUnit;
 
 import static org.hamcrest.Matchers.greaterThan;
 
-@ClusterScope(scope = Scope.TEST)
+// numClientNodes is set to 0 because Client nodes don't have Filesystem stats
+@ClusterScope(scope = Scope.TEST, numClientNodes = 0, transportClientRatio = 0.0)
 public class NodeStatsTests extends MarvelIntegTestCase {
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {