Support multiple metrics in top_metrics agg (elastic#52965)

This adds support for returning multiple metrics to the `top_metrics`
agg. It looks like:
```
POST /test/_search?filter_path=aggregations
{
  "aggs": {
    "tm": {
      "top_metrics": {
        "metrics": [
          {"field": "v"},
          {"field": "m"}
        ],
        "sort": {"s": "desc"}
      }
    }
  }
}
```

Author: nik9000

client/rest-high-level/src/main/java/org/elasticsearch/client/analytics/TopMetricsAggregationBuilder.java

@@ -32,6 +32,8 @@
 import org.elasticsearch.search.sort.SortBuilder;
 
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 
 /**
@@ -49,20 +51,20 @@ public class TopMetricsAggregationBuilder extends AbstractAggregationBuilder<Top
 
     private final SortBuilder<?> sort;
     private final int size;
-    private final String metric;
+    private final List<String> metrics;
 
     /**
      * Build the request.
      * @param name the name of the metric
      * @param sort the sort key used to select the top metrics
      * @param size number of results to return per bucket
-     * @param metric the name of the field to select
+     * @param metrics the names of the fields to select
      */
-    public TopMetricsAggregationBuilder(String name, SortBuilder<?> sort, int size, String metric) {
+    public TopMetricsAggregationBuilder(String name, SortBuilder<?> sort, int size, String... metrics) {
         super(name);
         this.sort = sort;
         this.size = size;
-        this.metric = metric;
+        this.metrics = Arrays.asList(metrics);
     }
 
     @Override
@@ -78,7 +80,11 @@ protected XContentBuilder internalXContent(XContentBuilder builder, Params param
             sort.toXContent(builder, params);
             builder.endArray();
             builder.field("size", size);
-            builder.startObject("metric").field("field", metric).endObject();
+            builder.startArray("metrics");
+            for (String metric: metrics) {
+                builder.startObject().field("field", metric).endObject();
+            }
+            builder.endArray();
         }
         return builder.endObject();
     }

client/rest-high-level/src/test/java/org/elasticsearch/client/analytics/AnalyticsAggsIT.java

@@ -74,6 +74,20 @@ public void testTopMetricsSizeOne() throws IOException {
         assertThat(metric.getMetrics(), equalTo(singletonMap("v", 3.0)));
     }
 
+    public void testTopMetricsManyMetrics() throws IOException {
+        indexTopMetricsData();
+        SearchRequest search = new SearchRequest("test");
+        search.source().aggregation(new TopMetricsAggregationBuilder(
+                "test", new FieldSortBuilder("s").order(SortOrder.DESC), 1, "v", "m"));
+        SearchResponse response = highLevelClient().search(search, RequestOptions.DEFAULT);
+        ParsedTopMetrics top = response.getAggregations().get("test");
+        assertThat(top.getTopMetrics(), hasSize(1));
+        ParsedTopMetrics.TopMetrics metric = top.getTopMetrics().get(0);
+        assertThat(metric.getSort(), equalTo(singletonList(2)));
+        assertThat(metric.getMetrics(), hasEntry("v", 3.0));
+        assertThat(metric.getMetrics(), hasEntry("m", 13.0));
+    }
+
     public void testTopMetricsSizeTwo() throws IOException {
         indexTopMetricsData();
         SearchRequest search = new SearchRequest("test");
@@ -92,8 +106,8 @@ public void testTopMetricsSizeTwo() throws IOException {
 
     private void indexTopMetricsData() throws IOException {
         BulkRequest bulk = new BulkRequest("test").setRefreshPolicy(RefreshPolicy.IMMEDIATE);
-        bulk.add(new IndexRequest().source(XContentType.JSON, "s", 1, "v", 2));
-        bulk.add(new IndexRequest().source(XContentType.JSON, "s", 2, "v", 3));
+        bulk.add(new IndexRequest().source(XContentType.JSON, "s", 1, "v", 2.0, "m", 12.0));
+        bulk.add(new IndexRequest().source(XContentType.JSON, "s", 2, "v", 3.0, "m", 13.0));
         highLevelClient().bulk(bulk, RequestOptions.DEFAULT);
     }
 }

docs/reference/aggregations/metrics/top-metrics-aggregation.asciidoc

@@ -22,7 +22,7 @@ POST /test/_search?filter_path=aggregations
   "aggs": {
     "tm": {
       "top_metrics": {
-        "metric": {"field": "v"},
+        "metrics": {"field": "v"},
         "sort": {"s": "desc"}
       }
     }
@@ -68,11 +68,61 @@ request. So,
 NOTE: This aggregation doesn't support any sort of "tie breaking". If two documents have
 the same sort values then this aggregation could return either document's fields.
 
-==== `metric`
+==== `metrics`
+
+`metrics` selects the fields to of the "top" document to return. Like most other
+aggregations, `top_metrics` casts these values cast to `double` precision
+floating point numbers. So they have to be numeric. Dates *work*, but they
+come back as a `double` precision floating point containing milliseconds since
+epoch. `keyword` fields aren't allowed.
+
+You can return multiple metrics by providing a list:
+
+[source,console,id=search-aggregations-metrics-top-metrics-list-of-metrics]
+----
+POST /test/_bulk?refresh
+{"index": {}}
+{"s": 1, "v": 3.1415, "m": 1.9}
+{"index": {}}
+{"s": 2, "v": 1.0, "m": 6.7}
+{"index": {}}
+{"s": 3, "v": 2.71828, "m": -12.2}
+POST /test/_search?filter_path=aggregations
+{
+  "aggs": {
+    "tm": {
+      "top_metrics": {
+        "metrics": [
+          {"field": "v"},
+          {"field": "m"}
+        ],
+        "sort": {"s": "desc"}
+      }
+    }
+  }
+}
+----
+
+Which returns:
+
+[source,js]
+----
+{
+  "aggregations": {
+    "tm": {
+      "top": [ {
+        "sort": [3],
+        "metrics": {
+          "v": 2.718280076980591,
+          "m": -12.199999809265137
+        }
+      } ]
+    }
+  }
+}
+----
+// TESTRESPONSE
 
-At this point `metric` supports only `{"field": "field_name"}` and all metrics
-are returned as double precision floating point numbers. Expect more to
-come here.
 
 ==== `size`
 
@@ -92,7 +142,7 @@ POST /test/_search?filter_path=aggregations
   "aggs": {
     "tm": {
       "top_metrics": {
-        "metric": {"field": "v"},
+        "metrics": {"field": "v"},
         "sort": {"s": "desc"},
         "size": 2
       }
@@ -174,7 +224,7 @@ POST /node/_search?filter_path=aggregations
       "aggs": {
         "tm": {
           "top_metrics": {
-            "metric": {"field": "v"},
+            "metrics": {"field": "v"},
             "sort": {"date": "desc"}
           }
         }
@@ -230,7 +280,7 @@ POST /node/_search?filter_path=aggregations
       "aggs": {
         "tm": {
           "top_metrics": {
-            "metric": {"field": "v"},
+            "metrics": {"field": "v"},
             "sort": {"date": "desc"}
           }
         }
@@ -292,7 +342,7 @@ POST /test*/_search?filter_path=aggregations
   "aggs": {
     "tm": {
       "top_metrics": {
-        "metric": {"field": "v"},
+        "metrics": {"field": "v"},
         "sort": {"s": "asc"}
       }
     }
@@ -325,7 +375,7 @@ POST /test*/_search?filter_path=aggregations
   "aggs": {
     "tm": {
       "top_metrics": {
-        "metric": {"field": "v"},
+        "metrics": {"field": "v"},
         "sort": {"s": {"order": "asc", "numeric_type": "double"}}
       }
     }

server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java

@@ -51,6 +51,7 @@
 import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.mapper.ContentPath;
 import org.elasticsearch.index.mapper.DateFieldMapper;
@@ -180,8 +181,9 @@ protected ScriptService getMockScriptService() {
 
     @Override
     protected QueryShardContext queryShardContextMock(IndexSearcher searcher, MapperService mapperService,
-                                                      IndexSettings indexSettings, CircuitBreakerService circuitBreakerService) {
-         this.queryShardContext = super.queryShardContextMock(searcher, mapperService, indexSettings, circuitBreakerService);
+                                                      IndexSettings indexSettings, CircuitBreakerService circuitBreakerService,
+                                                      BigArrays bigArrays) {
+         this.queryShardContext = super.queryShardContextMock(searcher, mapperService, indexSettings, circuitBreakerService, bigArrays);
          return queryShardContext;
     }
 

server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricAggregatorTests.java

@@ -421,11 +421,12 @@ public void testSelfReferencingAggStateAfterCombine() throws IOException {
     protected QueryShardContext queryShardContextMock(IndexSearcher searcher,
                                                         MapperService mapperService,
                                                         IndexSettings indexSettings,
-                                                        CircuitBreakerService circuitBreakerService) {
+                                                        CircuitBreakerService circuitBreakerService,
+                                                        BigArrays bigArrays) {
         MockScriptEngine scriptEngine = new MockScriptEngine(MockScriptEngine.NAME, SCRIPTS, Collections.emptyMap());
         Map<String, ScriptEngine> engines = Collections.singletonMap(scriptEngine.getType(), scriptEngine);
         ScriptService scriptService =  new ScriptService(Settings.EMPTY, engines, ScriptModule.CORE_CONTEXTS);
-        return new QueryShardContext(0, indexSettings, BigArrays.NON_RECYCLING_INSTANCE, null, null, mapperService, null, scriptService,
+        return new QueryShardContext(0, indexSettings, bigArrays, null, null, mapperService, null, scriptService,
                 xContentRegistry(), writableRegistry(), null, null, System::currentTimeMillis, null, null, () -> true);
     }
 }

test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java

@@ -285,7 +285,7 @@ public boolean shouldCache(Query query) {
         when(searchContext.lookup()).thenReturn(searchLookup);
 
         QueryShardContext queryShardContext =
-            queryShardContextMock(contextIndexSearcher, mapperService, indexSettings, circuitBreakerService);
+            queryShardContextMock(contextIndexSearcher, mapperService, indexSettings, circuitBreakerService, bigArrays);
         when(searchContext.getQueryShardContext()).thenReturn(queryShardContext);
         when(queryShardContext.getObjectMapper(anyString())).thenAnswer(invocation -> {
             String fieldName = (String) invocation.getArguments()[0];
@@ -336,9 +336,10 @@ protected MapperService mapperServiceMock() {
     protected QueryShardContext queryShardContextMock(IndexSearcher searcher,
                                                       MapperService mapperService,
                                                       IndexSettings indexSettings,
-                                                      CircuitBreakerService circuitBreakerService) {
+                                                      CircuitBreakerService circuitBreakerService,
+                                                      BigArrays bigArrays) {
 
-        return new QueryShardContext(0, indexSettings, BigArrays.NON_RECYCLING_INSTANCE, null,
+        return new QueryShardContext(0, indexSettings, bigArrays, null,
             getIndexFieldDataLookup(mapperService, circuitBreakerService),
             mapperService, null, getMockScriptService(), xContentRegistry(),
             writableRegistry(), null, searcher, System::currentTimeMillis, null, null, () -> true);