Expose agg usage in Feature Usage API (#55732)

* Expose agg usage in Feature Usage API Counts usage of the aggs and exposes them on the _nodes/usage/. Closes #53746 * Refactor to include non value sources aggregations * Fix reported values source type for parent and children aggs * Refactor SearchModule constructor * Fix subtype in TTest and IPRanges * Fix more subtypes in aggs that don't register themselves * Fix doc tests * Fix docs * Fix ScriptedMetricAggregatorTests * Fix compilation issues after merge * Fix merge fallout * This gets stale quickly... * Address review comments * Fix tests that were missing proper agg registration in the search module * Fix ScriptedMetricAggregatorTests * Address review comments Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
elastic · Apr 30, 2020 · b909cee · b909cee
1 parent 785527b
commit b909cee
Show file tree

Hide file tree

Showing 48 changed files with 539 additions and 103 deletions.
diff --git a/...vel/src/test/java/org/elasticsearch/client/transform/transforms/TransformConfigTests.java b/...vel/src/test/java/org/elasticsearch/client/transform/transforms/TransformConfigTests.java
@@ -45,7 +45,7 @@ public static TransformConfig randomTransformConfig() {
             randomSourceConfig(),
             randomDestConfig(),
             randomBoolean() ? null : TimeValue.timeValueMillis(randomIntBetween(1000, 1000000)),
-            randomBoolean() ? null : randomSyncConfig(), 
+            randomBoolean() ? null : randomSyncConfig(),
             PivotConfigTests.randomPivotConfig(),
             randomBoolean() ? null : randomAlphaOfLengthBetween(1, 100),
             randomBoolean() ? null : Instant.now(),

diff --git a/...on/archives/integ-test-zip/src/test/java/org/elasticsearch/test/rest/NodeRestUsageIT.java b/...on/archives/integ-test-zip/src/test/java/org/elasticsearch/test/rest/NodeRestUsageIT.java
@@ -19,11 +19,15 @@
 
 package org.elasticsearch.test.rest;
 
+import org.elasticsearch.client.Request;
 import org.elasticsearch.client.Response;
 import org.elasticsearch.client.ResponseException;
-import org.elasticsearch.client.Request;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.search.aggregations.AggregationBuilders;
+import org.elasticsearch.search.builder.SearchSourceBuilder;
 
 import java.io.IOException;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -41,14 +45,7 @@ public void testWithRestUsage() throws IOException {
         Response beforeResponse = client().performRequest(new Request("GET", path));
         Map<String, Object> beforeResponseBodyMap = entityAsMap(beforeResponse);
         assertThat(beforeResponseBodyMap, notNullValue());
-        Map<String, Object> before_nodesMap = (Map<String, Object>) beforeResponseBodyMap.get("_nodes");
-        assertThat(before_nodesMap, notNullValue());
-        Integer beforeTotal = (Integer) before_nodesMap.get("total");
-        Integer beforeSuccessful = (Integer) before_nodesMap.get("successful");
-        Integer beforeFailed = (Integer) before_nodesMap.get("failed");
-        assertThat(beforeTotal, greaterThan(0));
-        assertThat(beforeSuccessful, equalTo(beforeTotal));
-        assertThat(beforeFailed, equalTo(0));
+        int beforeSuccessful = assertSuccess(beforeResponseBodyMap);
 
         Map<String, Object> beforeNodesMap = (Map<String, Object>) beforeResponseBodyMap.get("nodes");
         assertThat(beforeNodesMap, notNullValue());
@@ -98,14 +95,7 @@ public void testWithRestUsage() throws IOException {
         Response response = client().performRequest(new Request("GET", "_nodes/usage"));
         Map<String, Object> responseBodyMap = entityAsMap(response);
         assertThat(responseBodyMap, notNullValue());
-        Map<String, Object> _nodesMap = (Map<String, Object>) responseBodyMap.get("_nodes");
-        assertThat(_nodesMap, notNullValue());
-        Integer total = (Integer) _nodesMap.get("total");
-        Integer successful = (Integer) _nodesMap.get("successful");
-        Integer failed = (Integer) _nodesMap.get("failed");
-        assertThat(total, greaterThan(0));
-        assertThat(successful, equalTo(total));
-        assertThat(failed, equalTo(0));
+        int successful = assertSuccess(responseBodyMap);
 
         Map<String, Object> nodesMap = (Map<String, Object>) responseBodyMap.get("nodes");
         assertThat(nodesMap, notNullValue());
@@ -143,4 +133,97 @@ public void testMetricsWithAll() throws IOException {
                 + "\"reason\":\"request [_nodes/usage/_all,rest_actions] contains _all and individual metrics [_all,rest_actions]\""));
     }
 
+    @SuppressWarnings("unchecked")
+    public void testAggregationUsage() throws IOException {
+        // First get the current usage figures
+        String path = randomFrom("_nodes/usage", "_nodes/usage/aggregations", "_nodes/usage/_all");
+        Response beforeResponse = client().performRequest(new Request("GET", path));
+        Map<String, Object> beforeResponseBodyMap = entityAsMap(beforeResponse);
+        assertThat(beforeResponseBodyMap, notNullValue());
+        int beforeSuccessful = assertSuccess(beforeResponseBodyMap);
+
+        Map<String, Object> beforeNodesMap = (Map<String, Object>) beforeResponseBodyMap.get("nodes");
+        assertThat(beforeNodesMap, notNullValue());
+        assertThat(beforeNodesMap.size(), equalTo(beforeSuccessful));
+
+        Map<String, Map<String, Long>> beforeCombinedAggsUsage = getTotalUsage(beforeNodesMap);
+        // Do some requests to get some rest usage stats
+        Request create = new Request("PUT", "/test");
+        create.setJsonEntity("{\"mappings\": {\"properties\": { \"str\": {\"type\": \"keyword\"}, " +
+            "\"foo\": {\"type\": \"keyword\"}, \"num\": {\"type\": \"long\"}, \"start\": {\"type\": \"date\"} } }}");
+        client().performRequest(create);
+
+        Request searchRequest = new Request("GET", "/test/_search");
+        SearchSourceBuilder searchSource = new SearchSourceBuilder()
+            .aggregation(AggregationBuilders.terms("str_terms").field("str.keyword"))
+            .aggregation(AggregationBuilders.terms("num_terms").field("num"))
+            .aggregation(AggregationBuilders.avg("num_avg").field("num"));
+        searchRequest.setJsonEntity(Strings.toString(searchSource));
+        searchRequest.setJsonEntity(Strings.toString(searchSource));
+        client().performRequest(searchRequest);
+
+        searchRequest = new Request("GET", "/test/_search");
+        searchSource = new SearchSourceBuilder()
+            .aggregation(AggregationBuilders.terms("start").field("start"))
+            .aggregation(AggregationBuilders.avg("num1").field("num"))
+            .aggregation(AggregationBuilders.avg("num2").field("num"))
+            .aggregation(AggregationBuilders.terms("foo").field("foo.keyword"));
+        String r = Strings.toString(searchSource);
+        searchRequest.setJsonEntity(Strings.toString(searchSource));
+        client().performRequest(searchRequest);
+
+        Response response = client().performRequest(new Request("GET", "_nodes/usage"));
+        Map<String, Object> responseBodyMap = entityAsMap(response);
+        assertThat(responseBodyMap, notNullValue());
+        int successful = assertSuccess(responseBodyMap);
+
+        Map<String, Object> nodesMap = (Map<String, Object>) responseBodyMap.get("nodes");
+        assertThat(nodesMap, notNullValue());
+        assertThat(nodesMap.size(), equalTo(successful));
+
+        Map<String, Map<String, Long>> afterCombinedAggsUsage = getTotalUsage(nodesMap);
+
+        assertDiff(beforeCombinedAggsUsage, afterCombinedAggsUsage, "terms", "numeric", 1L);
+        assertDiff(beforeCombinedAggsUsage, afterCombinedAggsUsage, "terms", "date", 1L);
+        assertDiff(beforeCombinedAggsUsage, afterCombinedAggsUsage, "terms", "bytes", 2L);
+        assertDiff(beforeCombinedAggsUsage, afterCombinedAggsUsage, "avg", "numeric", 3L);
+    }
+
+    private void assertDiff(Map<String, Map<String, Long>> before, Map<String, Map<String, Long>> after, String agg, String vst,
+                            long diff) {
+        Long valBefore = before.getOrDefault(agg, Collections.emptyMap()).getOrDefault(vst, 0L);
+        Long valAfter = after.getOrDefault(agg, Collections.emptyMap()).getOrDefault(vst, 0L);
+        assertThat(agg + "." + vst, valAfter - valBefore, equalTo(diff) );
+    }
+
+    private Map<String, Map<String, Long>> getTotalUsage(Map<String, Object> nodeUsage) {
+        Map<String, Map<String, Long>> combined = new HashMap<>();
+        for (Map.Entry<String, Object> nodeEntry : nodeUsage.entrySet()) {
+            @SuppressWarnings("unchecked")
+            Map<String, Object> beforeAggsUsage = (Map<String, Object>) ((Map<String, Object>) nodeEntry.getValue()).get("aggregations");
+            assertThat(beforeAggsUsage, notNullValue());
+            for (Map.Entry<String, Object> aggEntry : beforeAggsUsage.entrySet()) {
+                @SuppressWarnings("unchecked") Map<String, Object> aggMap = (Map<String, Object>) aggEntry.getValue();
+                Map<String, Long> combinedAggMap = combined.computeIfAbsent(aggEntry.getKey(), k -> new HashMap<>());
+                for  (Map.Entry<String, Object> valSourceEntry : aggMap.entrySet()) {
+                    combinedAggMap.put(valSourceEntry.getKey(),
+                        combinedAggMap.getOrDefault(valSourceEntry.getKey(), 0L) + ((Number) valSourceEntry.getValue()).longValue());
+                }
+            }
+        }
+        return combined;
+    }
+
+    private int assertSuccess(Map<String, Object> responseBodyMap) {
+        @SuppressWarnings("unchecked") Map<String, Object> nodesResultMap = (Map<String, Object>) responseBodyMap.get("_nodes");
+        assertThat(nodesResultMap, notNullValue());
+        Integer total = (Integer) nodesResultMap.get("total");
+        Integer successful = (Integer) nodesResultMap.get("successful");
+        Integer failed = (Integer) nodesResultMap.get("failed");
+        assertThat(total, greaterThan(0));
+        assertThat(successful, equalTo(total));
+        assertThat(failed, equalTo(0));
+        return successful;
+    }
+
 }
diff --git a/docs/reference/cluster/nodes-usage.asciidoc b/docs/reference/cluster/nodes-usage.asciidoc
@@ -31,15 +31,15 @@ of features for each node. All the nodes selective options are explained
 ==== {api-path-parms-title}
 
 `<metric>`::
-    (Optional, string) Limits the information returned to the specific metrics. 
-    A comma-separated list of the following options: 
+    (Optional, string) Limits the information returned to the specific metrics.
+    A comma-separated list of the following options:
 +
 --
     `_all`::
         Returns all stats.
-    
+
     `rest_actions`::
-        Returns the REST actions classname with a count of the number of times 
+        Returns the REST actions classname with a count of the number of times
         that action has been called on the node.
 --
 
@@ -79,11 +79,14 @@ The API returns the following response:
       "timestamp": 1492553961812, <1>
       "since": 1492553906606, <2>
       "rest_actions": {
-        "org.elasticsearch.rest.action.admin.cluster.RestNodesUsageAction": 1,
-        "org.elasticsearch.rest.action.admin.indices.RestCreateIndexAction": 1,
-        "org.elasticsearch.rest.action.document.RestGetAction": 1,
-        "org.elasticsearch.rest.action.search.RestSearchAction": 19, <3>
-        "org.elasticsearch.rest.action.admin.cluster.RestNodesInfoAction": 36
+        "nodes_usage_action": 1,
+        "create_index_action": 1,
+        "document_get_action": 1,
+        "search_action": 19, <3>
+        "nodes_info_action": 36
+      },
+      "aggregations": {
+        ...
       }
     }
   }
@@ -94,7 +97,9 @@ The API returns the following response:
 // TESTRESPONSE[s/1492553961812/$body.$_path/]
 // TESTRESPONSE[s/1492553906606/$body.$_path/]
 // TESTRESPONSE[s/"rest_actions": [^}]+}/"rest_actions": $body.$_path/]
+// TESTRESPONSE[s/"aggregations": [^}]+}/"aggregations": $body.$_path/]
 <1> Timestamp for when this nodes usage request was performed.
 <2> Timestamp for when the usage information recording was started. This is
 equivalent to the time that the node was started.
 <3> Search action has been called 19 times for this node.
+
diff --git a/...t/java/org/elasticsearch/search/aggregations/matrix/stats/MatrixStatsAggregatorTests.java b/...t/java/org/elasticsearch/search/aggregations/matrix/stats/MatrixStatsAggregatorTests.java
@@ -30,10 +30,13 @@
 import org.apache.lucene.util.NumericUtils;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.NumberFieldMapper;
+import org.elasticsearch.plugins.SearchPlugin;
 import org.elasticsearch.search.aggregations.AggregatorTestCase;
+import org.elasticsearch.search.aggregations.matrix.MatrixAggregationPlugin;
 
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
 
 public class MatrixStatsAggregatorTests extends AggregatorTestCase {
 
@@ -136,4 +139,8 @@ public void testTwoFieldsReduce() throws Exception {
         }
     }
 
+    @Override
+    protected List<SearchPlugin> getSearchPlugins() {
+        return Collections.singletonList(new MatrixAggregationPlugin());
+    }
 }
diff --git a/...painless/src/test/java/org/elasticsearch/painless/action/PainlessExecuteRequestTests.java b/...painless/src/test/java/org/elasticsearch/painless/action/PainlessExecuteRequestTests.java
@@ -73,7 +73,8 @@ public final void testFromXContent() throws Exception {
 
     @Override
     protected NamedWriteableRegistry getNamedWriteableRegistry() {
-        return new NamedWriteableRegistry(new SearchModule(Settings.EMPTY, Collections.emptyList()).getNamedWriteables());
+        return new NamedWriteableRegistry(new SearchModule(Settings.EMPTY, Collections.emptyList()
+        ).getNamedWriteables());
     }
 
     @Override

diff --git a/...ent-join/src/main/java/org/elasticsearch/join/aggregations/ChildrenAggregatorFactory.java b/...ent-join/src/main/java/org/elasticsearch/join/aggregations/ChildrenAggregatorFactory.java
@@ -36,6 +36,8 @@
 import java.io.IOException;
 import java.util.Map;
 
+import static org.elasticsearch.search.aggregations.support.AggregationUsageService.OTHER_SUBTYPE;
+
 public class ChildrenAggregatorFactory extends ValuesSourceAggregatorFactory {
 
     private final Query parentFilter;
@@ -84,4 +86,10 @@ protected Aggregator doCreateInternal(ValuesSource rawValuesSource,
             return asMultiBucketAggregator(this, searchContext, parent);
         }
     }
+
+    @Override
+    public String getStatsSubtype() {
+        // Child Aggregation is registered in non-standard way, so it might return child's values type
+        return OTHER_SUBTYPE;
+    }
 }
diff --git a/...arent-join/src/main/java/org/elasticsearch/join/aggregations/ParentAggregatorFactory.java b/...arent-join/src/main/java/org/elasticsearch/join/aggregations/ParentAggregatorFactory.java
@@ -36,6 +36,8 @@
 import java.io.IOException;
 import java.util.Map;
 
+import static org.elasticsearch.search.aggregations.support.AggregationUsageService.OTHER_SUBTYPE;
+
 public class ParentAggregatorFactory extends ValuesSourceAggregatorFactory {
 
     private final Query parentFilter;
@@ -85,4 +87,10 @@ protected Aggregator doCreateInternal(ValuesSource rawValuesSource,
             return asMultiBucketAggregator(this, searchContext, children);
         }
     }
+
+    @Override
+    public String getStatsSubtype() {
+        // Parent Aggregation is registered in non-standard way
+        return OTHER_SUBTYPE;
+    }
 }
diff --git a/...in/src/test/java/org/elasticsearch/join/aggregations/ChildrenToParentAggregatorTests.java b/...in/src/test/java/org/elasticsearch/join/aggregations/ChildrenToParentAggregatorTests.java
@@ -46,8 +46,10 @@
 import org.elasticsearch.index.mapper.NumberFieldMapper;
 import org.elasticsearch.index.mapper.Uid;
 import org.elasticsearch.index.shard.ShardId;
+import org.elasticsearch.join.ParentJoinPlugin;
 import org.elasticsearch.join.mapper.MetaJoinFieldMapper;
 import org.elasticsearch.join.mapper.ParentJoinFieldMapper;
+import org.elasticsearch.plugins.SearchPlugin;
 import org.elasticsearch.search.aggregations.Aggregation;
 import org.elasticsearch.search.aggregations.AggregationBuilder;
 import org.elasticsearch.search.aggregations.AggregatorTestCase;
@@ -328,4 +330,9 @@ private void testCaseTermsParentTerms(Query query, IndexSearcher indexSearcher,
         LongTerms result = search(indexSearcher, query, aggregationBuilder, fieldType, subFieldType);
         verify.accept(result);
     }
+
+    @Override
+    protected List<SearchPlugin> getSearchPlugins() {
+        return Collections.singletonList(new ParentJoinPlugin());
+    }
 }
diff --git a/...in/src/test/java/org/elasticsearch/join/aggregations/ParentToChildrenAggregatorTests.java b/...in/src/test/java/org/elasticsearch/join/aggregations/ParentToChildrenAggregatorTests.java
@@ -46,8 +46,10 @@
 import org.elasticsearch.index.mapper.NumberFieldMapper;
 import org.elasticsearch.index.mapper.Uid;
 import org.elasticsearch.index.shard.ShardId;
+import org.elasticsearch.join.ParentJoinPlugin;
 import org.elasticsearch.join.mapper.MetaJoinFieldMapper;
 import org.elasticsearch.join.mapper.ParentJoinFieldMapper;
+import org.elasticsearch.plugins.SearchPlugin;
 import org.elasticsearch.search.aggregations.AggregatorTestCase;
 import org.elasticsearch.search.aggregations.metrics.InternalMin;
 import org.elasticsearch.search.aggregations.metrics.MinAggregationBuilder;
@@ -187,4 +189,9 @@ private void testCase(Query query, IndexSearcher indexSearcher, Consumer<Interna
         InternalChildren result = search(indexSearcher, query, aggregationBuilder, fieldType);
         verify.accept(result);
     }
+
+    @Override
+    protected List<SearchPlugin> getSearchPlugins() {
+        return Collections.singletonList(new ParentJoinPlugin());
+    }
 }
diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedRequestsTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedRequestsTests.java
@@ -64,8 +64,8 @@ public class RatedRequestsTests extends ESTestCase {
     @BeforeClass
     public static void init() {
         xContentRegistry = new NamedXContentRegistry(
-                Stream.of(new SearchModule(Settings.EMPTY, emptyList()).getNamedXContents().stream()).flatMap(Function.identity())
-                        .collect(toList()));
+                Stream.of(new SearchModule(Settings.EMPTY, emptyList()).getNamedXContents().stream())
+                    .flatMap(Function.identity()).collect(toList()));
     }
 
     @AfterClass

diff --git a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yml b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yml
@@ -67,3 +67,9 @@
   - match: { hits.total: 1 }
   - match: { hits.hits.0._id: q3 }
 
+---
+"Verify nodes usage works":
+  - do:
+      nodes.usage: {}
+  - is_true: nodes
+  - match: { _nodes.failed: 0 }