diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/RollupField.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/RollupField.java index a784922228b0e..d0de0e02038c2 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/RollupField.java +++ b/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 SUPPORTED_METRICS = Arrays.asList(MaxAggregationBuilder.NAME, MinAggregationBuilder.NAME, + public static final List SUPPORTED_NUMERIC_METRICS = Arrays.asList(MaxAggregationBuilder.NAME, MinAggregationBuilder.NAME, SumAggregationBuilder.NAME, AvgAggregationBuilder.NAME, ValueCountAggregationBuilder.NAME); + public static final List 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 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 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 * diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/MetricConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/MetricConfig.java index 3a267e4cfa47c..46f0c7397c6e5 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/MetricConfig.java +++ b/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> fieldCa Map 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 supportedMetrics) { + List 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 + "]"; + } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/MetricConfigSerializingTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/MetricConfigSerializingTests.java index a5b8d9afead2e..bc2c9c3157be5 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/MetricConfigSerializingTests.java +++ b/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> 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> 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)); } } diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/put_job.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/put_job.yml index 23df0c5837700..cbb6f8956b14f 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/put_job.yml +++ b/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: