Randomly Enable Watcher while running Monitoring Tests

This is required to make sure that the integration for monitoring the Watcher Threadpool is actually working.

- Also added the full property name when the assertContains check fails
- Made shieldEnabled an instance level field rather than a static one
- Added watcherEnabled field in the same fashion (including enableWatcher method that by default randomly enables it)
- Added method to locally filter the expected field names based on watcher being enabled for the failing test

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

elasticsearch/x-pack/marvel/src/test/java/org/elasticsearch/marvel/agent/resolver/node/NodeStatsTests.java

@@ -11,13 +11,13 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.marvel.MarvelSettings;
 import org.elasticsearch.marvel.agent.collector.node.NodeStatsCollector;
 import org.elasticsearch.marvel.agent.exporter.local.LocalExporter;
-import org.elasticsearch.marvel.agent.resolver.node.NodeStatsResolver;
 import org.elasticsearch.marvel.test.MarvelIntegTestCase;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
 import org.elasticsearch.test.ESIntegTestCase.Scope;
 import org.junit.After;
 
+import java.util.Arrays;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
@@ -63,21 +63,37 @@ public class NodeStatsTests extends MarvelIntegTestCase {
         assertThat(response.getHits().getTotalHits(), greaterThan(0L));
 
         logger.debug("--> checking that every document contains the expected fields");
-        String[] filters = NodeStatsResolver.FILTERS;
+
         for (SearchHit searchHit : response.getHits().getHits()) {
             Map<String, Object> fields = searchHit.sourceAsMap();
 
-            for (String filter : filters) {
+            for (String filter : nodeStatsFilters(watcherEnabled)) {
                 if (Constants.WINDOWS) {
                     // load average is unavailable on Windows
                     if ("node_stats.os.cpu.load_average.1m".equals(filter)) {
                         continue;
                     }
                 }
+
                 assertContains(filter, fields);
             }
         }
 
         logger.debug("--> node stats successfully collected");
     }
+
+    /**
+     * Optionally exclude {@link NodeStatsResolver#FILTERS} that require Watcher to be enabled.
+     *
+     * @param includeWatcher {@code true} to keep watcher filters.
+     * @return Never {@code null} or empty.
+     * @see #watcherEnabled
+     */
+    private static String[] nodeStatsFilters(boolean includeWatcher) {
+        if (includeWatcher) {
+            return NodeStatsResolver.FILTERS;
+        }
+
+        return Arrays.stream(NodeStatsResolver.FILTERS).filter(s -> s.contains("watcher") == false).toArray(String[]::new);
+    }
 }

elasticsearch/x-pack/marvel/src/test/java/org/elasticsearch/marvel/test/MarvelIntegTestCase.java

@@ -68,14 +68,29 @@ public abstract class MarvelIntegTestCase extends ESIntegTestCase {
 
     public static final String MONITORING_INDICES_PREFIX = MonitoringIndexNameResolver.PREFIX + MonitoringIndexNameResolver.DELIMITER;
 
-    protected static Boolean shieldEnabled;
+    /**
+     * Enables individual tests to control the behavior.
+     * <p>
+     * Control this by overriding {@link #enableShield()}, which defaults to enabling it randomly.
+     */
+    protected Boolean shieldEnabled;
+    /**
+     * Enables individual tests to control the behavior.
+     * <p>
+     * Control this by overriding {@link #enableWatcher()}, which defaults to enabling it randomly.
+     */
+    protected Boolean watcherEnabled;
 
     @Override
     protected TestCluster buildTestCluster(Scope scope, long seed) throws IOException {
         if (shieldEnabled == null) {
             shieldEnabled = enableShield();
         }
+        if (watcherEnabled == null) {
+            watcherEnabled = enableWatcher();
+        }
         logger.info("--> shield {}", shieldEnabled ? "enabled" : "disabled");
+        logger.debug("--> watcher {}", watcherEnabled ? "enabled" : "disabled");
         return super.buildTestCluster(scope, seed);
     }
 
@@ -83,9 +98,7 @@ public abstract class MarvelIntegTestCase extends ESIntegTestCase {
     protected Settings nodeSettings(int nodeOrdinal) {
         Settings.Builder builder = Settings.builder()
                 .put(super.nodeSettings(nodeOrdinal))
-
-                //TODO: for now lets isolate monitoring tests from watcher (randomize this later)
-                .put(XPackPlugin.featureEnabledSetting(Watcher.NAME), false)
+                .put(XPackPlugin.featureEnabledSetting(Watcher.NAME), watcherEnabled)
                 // we do this by default in core, but for monitoring this isn't needed and only adds noise.
                 .put("index.store.mock.check_index_on_close", false);
 
@@ -151,12 +164,19 @@ public abstract class MarvelIntegTestCase extends ESIntegTestCase {
     }
 
     /**
-     * Override and returns {@code false} to force running without shield
+     * Override and return {@code false} to force running without Security.
      */
     protected boolean enableShield() {
         return randomBoolean();
     }
 
+    /**
+     * Override and return {@code false} to force running without Watcher.
+     */
+    protected boolean enableWatcher() {
+        return randomBoolean();
+    }
+
     protected void stopCollection() {
         for (AgentService agent : internalCluster().getInstances(AgentService.class)) {
             agent.stopCollection();
@@ -340,6 +360,15 @@ public abstract class MarvelIntegTestCase extends ESIntegTestCase {
      * it recurses to check if 'bar' exists in the sub-map.
      */
     protected void assertContains(String field, Map<String, Object> values) {
+        assertContains(field, values, null);
+    }
+
+    /**
+     * Checks if a field exist in a map of values. If the field contains a dot like 'foo.bar'
+     * it checks that 'foo' exists in the map of values and that it points to a sub-map. Then
+     * it recurses to check if 'bar' exists in the sub-map.
+     */
+    protected void assertContains(String field, Map<String, Object> values, String parent) {
         assertNotNull("field name should not be null", field);
         assertNotNull("values map should not be null", values);
 
@@ -351,21 +380,29 @@ public abstract class MarvelIntegTestCase extends ESIntegTestCase {
             assertTrue(Strings.hasText(segment));
 
             boolean fieldExists = values.containsKey(segment);
-            assertTrue("expecting field [" + segment + "] to be present in monitoring document", fieldExists);
+            assertTrue("expecting field [" + rebuildName(parent, segment) + "] to be present in monitoring document", fieldExists);
 
             Object value = values.get(segment);
             String next = field.substring(point + 1);
             if (next.length() > 0) {
                 assertTrue(value instanceof Map);
-                assertContains(next, (Map<String, Object>) value);
+                assertContains(next, (Map<String, Object>) value, rebuildName(parent, segment));
             } else {
                 assertFalse(value instanceof Map);
             }
         } else {
-            assertTrue("expecting field [" + field + "] to be present in monitoring document", values.containsKey(field));
+            assertTrue("expecting field [" + rebuildName(parent, field) + "] to be present in monitoring document", values.containsKey(field));
         }
     }
 
+    private String rebuildName(String parent, String field) {
+        if (Strings.isEmpty(parent)) {
+            return field;
+        }
+
+        return parent + "." + field;
+    }
+
     protected void updateMarvelInterval(long value, TimeUnit timeUnit) {
         assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(
                 Settings.builder().put(MarvelSettings.INTERVAL.getKey(), value, timeUnit)));