Skip to content

Commit af2877c

Browse files
benwtrentbenwtrent
authored
Rollup adding support for date field metrics (#34185) (#34200)
* Rollup adding support for date field metrics (#34185) * Restricting supported metrics for `date` field rollup * fixing expected error message for yaml test * Addressing PR comments
1 parent 230ad53 commit af2877c

File tree

4 files changed

+69
-14
lines changed

4 files changed

+69
-14
lines changed

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

+17-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.elasticsearch.xpack.core.rollup;
77

88
import org.elasticsearch.common.ParseField;
9+
import org.elasticsearch.index.mapper.DateFieldMapper;
910
import org.elasticsearch.index.mapper.NumberFieldMapper;
1011
import org.elasticsearch.search.aggregations.metrics.AvgAggregationBuilder;
1112
import org.elasticsearch.search.aggregations.metrics.MaxAggregationBuilder;
@@ -15,7 +16,9 @@
1516
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
1617

1718
import java.util.Arrays;
19+
import java.util.HashSet;
1820
import java.util.List;
21+
import java.util.Set;
1922
import java.util.stream.Collectors;
2023
import java.util.stream.Stream;
2124

@@ -34,8 +37,19 @@ public class RollupField {
3437
public static final String TYPE_NAME = "_doc";
3538
public static final String AGG = "agg";
3639
public static final String ROLLUP_MISSING = "ROLLUP_MISSING_40710B25931745D4B0B8B310F6912A69";
37-
public static final List<String> SUPPORTED_METRICS = Arrays.asList(MaxAggregationBuilder.NAME, MinAggregationBuilder.NAME,
40+
public static final List<String> SUPPORTED_NUMERIC_METRICS = Arrays.asList(MaxAggregationBuilder.NAME, MinAggregationBuilder.NAME,
3841
SumAggregationBuilder.NAME, AvgAggregationBuilder.NAME, ValueCountAggregationBuilder.NAME);
42+
public static final List<String> SUPPORTED_DATE_METRICS = Arrays.asList(MaxAggregationBuilder.NAME,
43+
MinAggregationBuilder.NAME,
44+
ValueCountAggregationBuilder.NAME);
45+
46+
// a set of ALL our supported metrics, to be a union of all other supported metric types (numeric, date, etc.)
47+
public static final Set<String> SUPPORTED_METRICS;
48+
static {
49+
SUPPORTED_METRICS = new HashSet<>();
50+
SUPPORTED_METRICS.addAll(SUPPORTED_NUMERIC_METRICS);
51+
SUPPORTED_METRICS.addAll(SUPPORTED_DATE_METRICS);
52+
}
3953

4054
// these mapper types are used by the configs (metric, histo, etc) to validate field mappings
4155
public static final List<String> NUMERIC_FIELD_MAPPER_TYPES;
@@ -47,6 +61,8 @@ public class RollupField {
4761
NUMERIC_FIELD_MAPPER_TYPES = types;
4862
}
4963

64+
public static final String DATE_FIELD_MAPPER_TYPE = DateFieldMapper.CONTENT_TYPE;
65+
5066
/**
5167
* Format to the appropriate Rollup field name convention
5268
*

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

+20-6
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.elasticsearch.xpack.core.rollup.RollupField;
2020

2121
import java.io.IOException;
22+
import java.util.ArrayList;
2223
import java.util.List;
2324
import java.util.Map;
2425
import java.util.Objects;
@@ -108,18 +109,24 @@ public void validateMappings(Map<String, Map<String, FieldCapabilities>> fieldCa
108109
Map<String, FieldCapabilities> fieldCaps = fieldCapsResponse.get(field);
109110
if (fieldCaps != null && fieldCaps.isEmpty() == false) {
110111
fieldCaps.forEach((key, value) -> {
112+
if (value.isAggregatable() == false) {
113+
validationException.addValidationError("The field [" + field + "] must be aggregatable across all indices, " +
114+
"but is not.");
115+
}
111116
if (RollupField.NUMERIC_FIELD_MAPPER_TYPES.contains(key)) {
112-
if (value.isAggregatable() == false) {
113-
validationException.addValidationError("The field [" + field + "] must be aggregatable across all indices, " +
114-
"but is not.");
117+
// nothing to do as all metrics are supported by SUPPORTED_NUMERIC_METRICS currently
118+
} else if (RollupField.DATE_FIELD_MAPPER_TYPE.equals(key)) {
119+
if (RollupField.SUPPORTED_DATE_METRICS.containsAll(metrics) == false) {
120+
validationException.addValidationError(
121+
buildSupportedMetricError("date", RollupField.SUPPORTED_DATE_METRICS));
115122
}
116123
} else {
117-
validationException.addValidationError("The field referenced by a metric group must be a [numeric] type, but found " +
118-
fieldCaps.keySet().toString() + " for field [" + field + "]");
124+
validationException.addValidationError("The field referenced by a metric group must be a [numeric] or [date] type, " +
125+
"but found " + fieldCaps.keySet().toString() + " for field [" + field + "]");
119126
}
120127
});
121128
} else {
122-
validationException.addValidationError("Could not find a [numeric] field with name [" + field + "] in any of the " +
129+
validationException.addValidationError("Could not find a [numeric] or [date] field with name [" + field + "] in any of the " +
123130
"indices matching the index pattern.");
124131
}
125132
}
@@ -166,4 +173,11 @@ public String toString() {
166173
public static MetricConfig fromXContent(final XContentParser parser) throws IOException {
167174
return PARSER.parse(parser, null);
168175
}
176+
177+
private String buildSupportedMetricError(String type, List<String> supportedMetrics) {
178+
List<String> unsupportedMetrics = new ArrayList<>(metrics);
179+
unsupportedMetrics.removeAll(supportedMetrics);
180+
return "Only the metrics " + supportedMetrics + " are supported for [" + type + "] types," +
181+
" but unsupported metrics " + unsupportedMetrics + " supplied for field [" + field + "]";
182+
}
169183
}

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

+31-6
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,17 @@
1111
import org.elasticsearch.common.xcontent.XContentParser;
1212
import org.elasticsearch.test.AbstractSerializingTestCase;
1313
import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers;
14+
import org.elasticsearch.xpack.core.rollup.RollupField;
1415

1516
import java.io.IOException;
17+
import java.util.Arrays;
1618
import java.util.Collections;
1719
import java.util.HashMap;
1820
import java.util.Map;
1921

2022
import static java.util.Collections.singletonList;
2123
import static org.hamcrest.Matchers.equalTo;
24+
import static org.hamcrest.Matchers.isIn;
2225
import static org.mockito.Mockito.mock;
2326
import static org.mockito.Mockito.when;
2427

@@ -45,8 +48,8 @@ public void testValidateNoMapping() {
4548

4649
MetricConfig config = new MetricConfig("my_field", singletonList("max"));
4750
config.validateMappings(responseMap, e);
48-
assertThat(e.validationErrors().get(0), equalTo("Could not find a [numeric] field with name [my_field] in any of the " +
49-
"indices matching the index pattern."));
51+
assertThat(e.validationErrors().get(0), equalTo("Could not find a [numeric] or [date] field with name [my_field] in any" +
52+
" of the indices matching the index pattern."));
5053
}
5154

5255
public void testValidateNomatchingField() {
@@ -59,8 +62,8 @@ public void testValidateNomatchingField() {
5962

6063
MetricConfig config = new MetricConfig("my_field", singletonList("max"));
6164
config.validateMappings(responseMap, e);
62-
assertThat(e.validationErrors().get(0), equalTo("Could not find a [numeric] field with name [my_field] in any of the " +
63-
"indices matching the index pattern."));
65+
assertThat(e.validationErrors().get(0), equalTo("Could not find a [numeric] or [date] field with name [my_field] in any" +
66+
" of the indices matching the index pattern."));
6467
}
6568

6669
public void testValidateFieldWrongType() {
@@ -73,8 +76,8 @@ public void testValidateFieldWrongType() {
7376

7477
MetricConfig config = new MetricConfig("my_field", singletonList("max"));
7578
config.validateMappings(responseMap, e);
76-
assertThat(e.validationErrors().get(0), equalTo("The field referenced by a metric group must be a [numeric] type, " +
77-
"but found [keyword] for field [my_field]"));
79+
assertThat("The field referenced by a metric group must be a [numeric] or [date] type," +
80+
" but found [keyword] for field [my_field]", isIn(e.validationErrors()));
7881
}
7982

8083
public void testValidateFieldMatchingNotAggregatable() {
@@ -91,6 +94,21 @@ public void testValidateFieldMatchingNotAggregatable() {
9194
assertThat(e.validationErrors().get(0), equalTo("The field [my_field] must be aggregatable across all indices, but is not."));
9295
}
9396

97+
public void testValidateDateFieldUnsupportedMetric() {
98+
ActionRequestValidationException e = new ActionRequestValidationException();
99+
Map<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
100+
101+
// Have to mock fieldcaps because the ctor's aren't public...
102+
FieldCapabilities fieldCaps = mock(FieldCapabilities.class);
103+
when(fieldCaps.isAggregatable()).thenReturn(true);
104+
responseMap.put("my_field", Collections.singletonMap("date", fieldCaps));
105+
106+
MetricConfig config = new MetricConfig("my_field", Arrays.asList("avg", "max"));
107+
config.validateMappings(responseMap, e);
108+
assertThat(e.validationErrors().get(0), equalTo("Only the metrics " + RollupField.SUPPORTED_DATE_METRICS.toString() +
109+
" are supported for [date] types, but unsupported metrics [avg] supplied for field [my_field]"));
110+
}
111+
94112
public void testValidateMatchingField() {
95113
ActionRequestValidationException e = new ActionRequestValidationException();
96114
Map<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
@@ -153,6 +171,13 @@ public void testValidateMatchingField() {
153171
config = new MetricConfig("my_field", singletonList("max"));
154172
config.validateMappings(responseMap, e);
155173
assertThat(e.validationErrors().size(), equalTo(0));
174+
175+
fieldCaps = mock(FieldCapabilities.class);
176+
when(fieldCaps.isAggregatable()).thenReturn(true);
177+
responseMap.put("my_field", Collections.singletonMap("date", fieldCaps));
178+
config = new MetricConfig("my_field", singletonList("max"));
179+
config.validateMappings(responseMap, e);
180+
assertThat(e.validationErrors().size(), equalTo(0));
156181
}
157182

158183
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ setup:
196196
"Validation failures":
197197

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

0 commit comments

Comments
 (0)