From 70f1b5c8f2394d3fa89a160530997d17da2fd0a6 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 30 Oct 2015 16:19:23 +0100 Subject: [PATCH] Marvel: Client nodes should be able to send metrics closes elastic/elasticsearch#897 Original commit: elastic/x-pack-elasticsearch@1ebcc9de141d509e8b0c02f9a1bb446dd5184d93 --- .../collector/node/NodeStatsCollector.java | 8 +- .../org/elasticsearch/marvel/MarvelF.java | 3 +- .../renderer/node/MultiNodesStatsTests.java | 90 +++++++++++++++++++ .../agent/renderer/node/NodeStatsTests.java | 3 +- 4 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 marvel/src/test/java/org/elasticsearch/marvel/agent/renderer/node/MultiNodesStatsTests.java diff --git a/marvel/src/main/java/org/elasticsearch/marvel/agent/collector/node/NodeStatsCollector.java b/marvel/src/main/java/org/elasticsearch/marvel/agent/collector/node/NodeStatsCollector.java index 1dbe472267a..43fe39cb3ac 100644 --- a/marvel/src/main/java/org/elasticsearch/marvel/agent/collector/node/NodeStatsCollector.java +++ b/marvel/src/main/java/org/elasticsearch/marvel/agent/collector/node/NodeStatsCollector.java @@ -55,7 +55,13 @@ public class NodeStatsCollector extends AbstractCollector { @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 diff --git a/marvel/src/test/java/org/elasticsearch/marvel/MarvelF.java b/marvel/src/test/java/org/elasticsearch/marvel/MarvelF.java index e0d7d390bf7..28f544d3edf 100644 --- a/marvel/src/test/java/org/elasticsearch/marvel/MarvelF.java +++ b/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 diff --git a/marvel/src/test/java/org/elasticsearch/marvel/agent/renderer/node/MultiNodesStatsTests.java b/marvel/src/test/java/org/elasticsearch/marvel/agent/renderer/node/MultiNodesStatsTests.java new file mode 100644 index 00000000000..01e5f487057 --- /dev/null +++ b/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)); + } + } + }); + } + +} diff --git a/marvel/src/test/java/org/elasticsearch/marvel/agent/renderer/node/NodeStatsTests.java b/marvel/src/test/java/org/elasticsearch/marvel/agent/renderer/node/NodeStatsTests.java index 292035797fd..d63b4d62d24 100644 --- a/marvel/src/test/java/org/elasticsearch/marvel/agent/renderer/node/NodeStatsTests.java +++ b/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) {