Rollup adding support for date field metrics (#34185)

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/RollupField.java

@@ -6,6 +6,7 @@
 package org.elasticsearch.xpack.core.rollup;
 
 import org.elasticsearch.common.ParseField;
+import org.elasticsearch.index.mapper.DateFieldMapper;
 import org.elasticsearch.index.mapper.NumberFieldMapper;
 import org.elasticsearch.search.aggregations.metrics.AvgAggregationBuilder;
 import org.elasticsearch.search.aggregations.metrics.MaxAggregationBuilder;
@@ -15,7 +16,9 @@
 import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
 
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -34,8 +37,19 @@ public class RollupField {
     public static final String TYPE_NAME = "_doc";
     public static final String AGG = "agg";
     public static final String ROLLUP_MISSING = "ROLLUP_MISSING_40710B25931745D4B0B8B310F6912A69";
-    public static final List<String> SUPPORTED_METRICS = Arrays.asList(MaxAggregationBuilder.NAME, MinAggregationBuilder.NAME,
+    public static final List<String> SUPPORTED_NUMERIC_METRICS = Arrays.asList(MaxAggregationBuilder.NAME, MinAggregationBuilder.NAME,
             SumAggregationBuilder.NAME, AvgAggregationBuilder.NAME, ValueCountAggregationBuilder.NAME);
+    public static final List<String> SUPPORTED_DATE_METRICS = Arrays.asList(MaxAggregationBuilder.NAME,
+        MinAggregationBuilder.NAME,
+        ValueCountAggregationBuilder.NAME);
+
+    // a set of ALL our supported metrics, to be a union of all other supported metric types (numeric, date, etc.)
+    public static final Set<String> SUPPORTED_METRICS;
+    static {
+        SUPPORTED_METRICS = new HashSet<>();
+        SUPPORTED_METRICS.addAll(SUPPORTED_NUMERIC_METRICS);
+        SUPPORTED_METRICS.addAll(SUPPORTED_DATE_METRICS);
+    }
 
     // these mapper types are used by the configs (metric, histo, etc) to validate field mappings
     public static final List<String> NUMERIC_FIELD_MAPPER_TYPES;
@@ -47,6 +61,8 @@ public class RollupField {
         NUMERIC_FIELD_MAPPER_TYPES = types;
     }
 
+    public static final String DATE_FIELD_MAPPER_TYPE = DateFieldMapper.CONTENT_TYPE;
+
     /**
      * Format to the appropriate Rollup field name convention
      *

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/MetricConfig.java

@@ -19,6 +19,7 @@
 import org.elasticsearch.xpack.core.rollup.RollupField;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -108,18 +109,24 @@ public void validateMappings(Map<String, Map<String, FieldCapabilities>> fieldCa
         Map<String, FieldCapabilities> fieldCaps = fieldCapsResponse.get(field);
         if (fieldCaps != null && fieldCaps.isEmpty() == false) {
             fieldCaps.forEach((key, value) -> {
+                if (value.isAggregatable() == false) {
+                    validationException.addValidationError("The field [" + field + "] must be aggregatable across all indices, " +
+                        "but is not.");
+                }
                 if (RollupField.NUMERIC_FIELD_MAPPER_TYPES.contains(key)) {
-                    if (value.isAggregatable() == false) {
-                        validationException.addValidationError("The field [" + field + "] must be aggregatable across all indices, " +
-                                "but is not.");
+                    // nothing to do as all metrics are supported by SUPPORTED_NUMERIC_METRICS currently
+                } else if (RollupField.DATE_FIELD_MAPPER_TYPE.equals(key)) {
+                    if (RollupField.SUPPORTED_DATE_METRICS.containsAll(metrics) == false) {
+                        validationException.addValidationError(
+                            buildSupportedMetricError("date", RollupField.SUPPORTED_DATE_METRICS));
                     }
                 } else {
-                    validationException.addValidationError("The field referenced by a metric group must be a [numeric] type, but found " +
-                            fieldCaps.keySet().toString() + " for field [" + field + "]");
+                    validationException.addValidationError("The field referenced by a metric group must be a [numeric] or [date] type, " +
+                        "but found " + fieldCaps.keySet().toString() + " for field [" + field + "]");
                 }
             });
         } else {
-            validationException.addValidationError("Could not find a [numeric] field with name [" + field + "] in any of the " +
+            validationException.addValidationError("Could not find a [numeric] or [date] field with name [" + field + "] in any of the " +
                     "indices matching the index pattern.");
         }
     }
@@ -166,4 +173,11 @@ public String toString() {
     public static MetricConfig fromXContent(final XContentParser parser) throws IOException {
         return PARSER.parse(parser, null);
     }
+
+    private String buildSupportedMetricError(String type, List<String> supportedMetrics) {
+        List<String> unsupportedMetrics = new ArrayList<>(metrics);
+        unsupportedMetrics.removeAll(supportedMetrics);
+        return "Only the metrics " + supportedMetrics + " are supported for [" + type + "] types," +
+            " but unsupported metrics " + unsupportedMetrics + " supplied for field [" + field + "]";
+    }
 }

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/MetricConfigSerializingTests.java

@@ -11,14 +11,17 @@
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.test.AbstractSerializingTestCase;
 import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers;
+import org.elasticsearch.xpack.core.rollup.RollupField;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
 import static java.util.Collections.singletonList;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.isIn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -45,8 +48,8 @@ public void testValidateNoMapping() {
 
         MetricConfig config = new MetricConfig("my_field", singletonList("max"));
         config.validateMappings(responseMap, e);
-        assertThat(e.validationErrors().get(0), equalTo("Could not find a [numeric] field with name [my_field] in any of the " +
-                "indices matching the index pattern."));
+        assertThat(e.validationErrors().get(0), equalTo("Could not find a [numeric] or [date] field with name [my_field] in any" +
+            " of the indices matching the index pattern."));
     }
 
     public void testValidateNomatchingField() {
@@ -59,8 +62,8 @@ public void testValidateNomatchingField() {
 
         MetricConfig config = new MetricConfig("my_field", singletonList("max"));
         config.validateMappings(responseMap, e);
-        assertThat(e.validationErrors().get(0), equalTo("Could not find a [numeric] field with name [my_field] in any of the " +
-                "indices matching the index pattern."));
+        assertThat(e.validationErrors().get(0), equalTo("Could not find a [numeric] or [date] field with name [my_field] in any" +
+            " of the indices matching the index pattern."));
     }
 
     public void testValidateFieldWrongType() {
@@ -73,8 +76,8 @@ public void testValidateFieldWrongType() {
 
         MetricConfig config = new MetricConfig("my_field", singletonList("max"));
         config.validateMappings(responseMap, e);
-        assertThat(e.validationErrors().get(0), equalTo("The field referenced by a metric group must be a [numeric] type, " +
-                "but found [keyword] for field [my_field]"));
+        assertThat("The field referenced by a metric group must be a [numeric] or [date] type," +
+            " but found [keyword] for field [my_field]", isIn(e.validationErrors()));
     }
 
     public void testValidateFieldMatchingNotAggregatable() {
@@ -91,6 +94,21 @@ public void testValidateFieldMatchingNotAggregatable() {
         assertThat(e.validationErrors().get(0), equalTo("The field [my_field] must be aggregatable across all indices, but is not."));
     }
 
+    public void testValidateDateFieldUnsupportedMetric() {
+        ActionRequestValidationException e = new ActionRequestValidationException();
+        Map<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
+
+        // Have to mock fieldcaps because the ctor's aren't public...
+        FieldCapabilities fieldCaps = mock(FieldCapabilities.class);
+        when(fieldCaps.isAggregatable()).thenReturn(true);
+        responseMap.put("my_field", Collections.singletonMap("date", fieldCaps));
+
+        MetricConfig config = new MetricConfig("my_field", Arrays.asList("avg", "max"));
+        config.validateMappings(responseMap, e);
+        assertThat(e.validationErrors().get(0), equalTo("Only the metrics " + RollupField.SUPPORTED_DATE_METRICS.toString() +
+            " are supported for [date] types, but unsupported metrics [avg] supplied for field [my_field]"));
+    }
+
     public void testValidateMatchingField() {
         ActionRequestValidationException e = new ActionRequestValidationException();
         Map<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
@@ -153,6 +171,13 @@ public void testValidateMatchingField() {
         config = new MetricConfig("my_field", singletonList("max"));
         config.validateMappings(responseMap, e);
         assertThat(e.validationErrors().size(), equalTo(0));
+
+        fieldCaps = mock(FieldCapabilities.class);
+        when(fieldCaps.isAggregatable()).thenReturn(true);
+        responseMap.put("my_field", Collections.singletonMap("date", fieldCaps));
+        config = new MetricConfig("my_field", singletonList("max"));
+        config.validateMappings(responseMap, e);
+        assertThat(e.validationErrors().size(), equalTo(0));
     }
 
 }

x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/put_job.yml

@@ -196,7 +196,7 @@ setup:
 "Validation failures":
 
   - do:
-      catch: /Could not find a \[numeric\] field with name \[field_doesnt_exist\] in any of the indices matching the index pattern/
+      catch: /Could not find a \[numeric\] or \[date\] field with name \[field_doesnt_exist\] in any of the indices matching the index pattern/
       headers:
         Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser
       xpack.rollup.put_job: