From 9037c4aca376fba483e6bb1e126663f143a3fcf0 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Tue, 4 Dec 2018 15:38:59 -0500 Subject: [PATCH 01/18] [Rollup] Compare timezones based on rules not string comparision The date_histogram internally converts obsolete timezones (such as "Canada/Mountain") into their modern equivalent ("America/Edmonton"). But rollup just stored the TZ as provided by the user. When checking the TZ for query validation we used a string comparison, which would fail due to the date_histo's upgrading behavior. Instead, we should convert both to a TimeZone object and check if their rules are compatible. This commit also proactively upgrades the config's TZ to the modern equivalent for good measure, although this isn't strictly necessary and old rollup indices will be fixed just by the query validation tweak. This has two side-effects: - We should not be adding a term filter on `time_zone` to the query. This was obsolete anyway due to filtering on job ID + individual msearch - Without the need for term filter on `time_zone`, there's no need for the request translator to add filtering clauses at all. This functionality was removed (although the filtered agg structure remains, can be cleaned up in a followup PR) It also exposed a bug: - After verifying the timezones are the same, we need to set the timezone on the rewritten date_histo otherwise it will treat the data as UTC and shift results. --- .../rollup/job/DateHistogramGroupConfig.java | 2 +- .../rollup/RollupJobIdentifierUtils.java | 9 +- .../xpack/rollup/RollupRequestTranslator.java | 49 +-- .../action/TransportRollupSearchAction.java | 4 +- .../rollup/RollupJobIdentifierUtilTests.java | 45 +++ .../rollup/RollupRequestTranslationTests.java | 98 +---- .../xpack/rollup/config/ConfigTests.java | 5 + .../test/rollup/rollup_search.yml | 362 ++++++++++++++++++ 8 files changed, 447 insertions(+), 127 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java index 166322b93722c..2689da9932780 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java @@ -103,7 +103,7 @@ public DateHistogramGroupConfig(final String field, this.interval = interval; this.field = field; this.delay = delay; - this.timeZone = (timeZone != null && timeZone.isEmpty() == false) ? timeZone : DEFAULT_TIMEZONE; + this.timeZone = DateTimeZone.forID((timeZone != null && timeZone.isEmpty() == false) ? timeZone : DEFAULT_TIMEZONE).toString(); // validate interval createRounding(this.interval.toString(), this.timeZone); diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java index 232034177e87b..7c64306f97db7 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TimeZone; /** * This class contains utilities to identify which jobs are the "best" for a given aggregation tree. @@ -97,11 +98,13 @@ private static void checkDateHisto(DateHistogramAggregationBuilder source, List< if (agg.get(RollupField.AGG).equals(DateHistogramAggregationBuilder.NAME)) { DateHistogramInterval interval = new DateHistogramInterval((String)agg.get(RollupField.INTERVAL)); - String thisTimezone = (String)agg.get(DateHistogramGroupConfig.TIME_ZONE); - String sourceTimeZone = source.timeZone() == null ? DateTimeZone.UTC.toString() : source.timeZone().toString(); + TimeZone thisTimezone = DateTimeZone.forID((String)agg.get(DateHistogramGroupConfig.TIME_ZONE)).toTimeZone(); + TimeZone sourceTimeZone = source.timeZone() == null + ? DateTimeZone.UTC.toTimeZone() + : source.timeZone().toTimeZone(); // Ensure we are working on the same timezone - if (thisTimezone.equalsIgnoreCase(sourceTimeZone) == false) { + if (thisTimezone.hasSameRules(sourceTimeZone) == false) { continue; } if (source.dateHistogramInterval() != null) { diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupRequestTranslator.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupRequestTranslator.java index 8b028b712e717..41f679f49086e 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupRequestTranslator.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupRequestTranslator.java @@ -11,8 +11,6 @@ import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.index.query.TermQueryBuilder; import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregationBuilder; @@ -21,7 +19,6 @@ import org.elasticsearch.search.aggregations.metrics.SumAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder; import org.elasticsearch.xpack.core.rollup.RollupField; -import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig; import org.joda.time.DateTimeZone; import java.util.ArrayList; @@ -47,7 +44,7 @@ * } * * - * The only publicly "consumable" API is {@link #translateAggregation(AggregationBuilder, List, NamedWriteableRegistry)}. + * The only publicly "consumable" API is {@link #translateAggregation(AggregationBuilder, NamedWriteableRegistry)}. */ public class RollupRequestTranslator { @@ -116,26 +113,22 @@ public class RollupRequestTranslator { * relevant method below. * * @param source The source aggregation to translate into rollup-enabled version - * @param filterConditions A list used to track any filter conditions that sub-aggs may - * require. * @param registry Registry containing the various aggregations so that we can easily * deserialize into a stream for cloning * @return Returns the fully translated aggregation tree. Note that it returns a list instead * of a single AggBuilder, since some aggregations (e.g. avg) may result in two * translated aggs (sum + count) */ - public static List translateAggregation(AggregationBuilder source, - List filterConditions, - NamedWriteableRegistry registry) { + public static List translateAggregation(AggregationBuilder source, NamedWriteableRegistry registry) { if (source.getWriteableName().equals(DateHistogramAggregationBuilder.NAME)) { - return translateDateHistogram((DateHistogramAggregationBuilder) source, filterConditions, registry); + return translateDateHistogram((DateHistogramAggregationBuilder) source, registry); } else if (source.getWriteableName().equals(HistogramAggregationBuilder.NAME)) { - return translateHistogram((HistogramAggregationBuilder) source, filterConditions, registry); + return translateHistogram((HistogramAggregationBuilder) source, registry); } else if (RollupField.SUPPORTED_METRICS.contains(source.getWriteableName())) { return translateVSLeaf((ValuesSourceAggregationBuilder.LeafOnly)source, registry); } else if (source.getWriteableName().equals(TermsAggregationBuilder.NAME)) { - return translateTerms((TermsAggregationBuilder)source, filterConditions, registry); + return translateTerms((TermsAggregationBuilder)source, registry); } else { throw new IllegalArgumentException("Unable to translate aggregation tree into Rollup. Aggregation [" + source.getName() + "] is of type [" + source.getClass().getSimpleName() + "] which is " + @@ -195,22 +188,13 @@ public static List translateAggregation(AggregationBuilder s *
  • Field: `{timestamp field}.date_histogram._count`
    • * * - *
  • Add a filter condition:
    • - *
  • - *
      - *
    • Query type: TermQuery
      • - *
    • Field: `{timestamp_field}.date_histogram.interval`
      • - *
    • Value: `{source interval}`
      • - *
    - *
    • * * */ private static List translateDateHistogram(DateHistogramAggregationBuilder source, - List filterConditions, NamedWriteableRegistry registry) { - return translateVSAggBuilder(source, filterConditions, registry, () -> { + return translateVSAggBuilder(source, registry, () -> { DateHistogramAggregationBuilder rolledDateHisto = new DateHistogramAggregationBuilder(source.getName()); @@ -220,10 +204,8 @@ private static List translateDateHistogram(DateHistogramAggr rolledDateHisto.interval(source.interval()); } - String timezone = source.timeZone() == null ? DateTimeZone.UTC.toString() : source.timeZone().toString(); - filterConditions.add(new TermQueryBuilder(RollupField.formatFieldName(source, - DateHistogramGroupConfig.TIME_ZONE), timezone)); - + DateTimeZone timezone = source.timeZone() == null ? DateTimeZone.UTC : source.timeZone(); + rolledDateHisto.timeZone(timezone); rolledDateHisto.offset(source.offset()); if (source.extendedBounds() != null) { rolledDateHisto.extendedBounds(source.extendedBounds()); @@ -245,14 +227,13 @@ private static List translateDateHistogram(DateHistogramAggr * Notably, it adds a Sum metric to calculate the doc_count in each bucket. * * Conventions are identical to a date_histogram (excepting date-specific details), so see - * {@link #translateDateHistogram(DateHistogramAggregationBuilder, List, NamedWriteableRegistry)} for + * {@link #translateDateHistogram(DateHistogramAggregationBuilder, NamedWriteableRegistry)} for * a complete list of conventions, examples, etc */ private static List translateHistogram(HistogramAggregationBuilder source, - List filterConditions, NamedWriteableRegistry registry) { - return translateVSAggBuilder(source, filterConditions, registry, () -> { + return translateVSAggBuilder(source, registry, () -> { HistogramAggregationBuilder rolledHisto = new HistogramAggregationBuilder(source.getName()); @@ -325,10 +306,9 @@ private static List translateHistogram(HistogramAggregationB * */ private static List translateTerms(TermsAggregationBuilder source, - List filterConditions, NamedWriteableRegistry registry) { - return translateVSAggBuilder(source, filterConditions, registry, () -> { + return translateVSAggBuilder(source, registry, () -> { TermsAggregationBuilder rolledTerms = new TermsAggregationBuilder(source.getName(), source.valueType()); rolledTerms.field(RollupField.formatFieldName(source, RollupField.VALUE)); @@ -356,8 +336,6 @@ private static List translateTerms(TermsAggregationBuilder s * ValueSourceBuilder. This method is called by all the agg-specific methods (e.g. translateDateHistogram()) * * @param source The source aggregation that we wish to translate - * @param filterConditions A list of existing filter conditions, in case we need to add some - * for this particular agg * @param registry Named registry for serializing leaf metrics. Not actually used by this method, * but is passed downwards for leaf usage * @param factory A factory closure that generates a new shallow clone of the `source`. E.g. if `source` is @@ -368,15 +346,14 @@ private static List translateTerms(TermsAggregationBuilder s * @return the translated multi-bucket ValueSourceAggBuilder */ private static List - translateVSAggBuilder(ValuesSourceAggregationBuilder source, List filterConditions, - NamedWriteableRegistry registry, Supplier factory) { + translateVSAggBuilder(ValuesSourceAggregationBuilder source, NamedWriteableRegistry registry, Supplier factory) { T rolled = factory.get(); // Translate all subaggs and add to the newly translated agg // NOTE: using for loop instead of stream because compiler explodes with a bug :/ for (AggregationBuilder subAgg : source.getSubAggregations()) { - List translated = translateAggregation(subAgg, filterConditions, registry); + List translated = translateAggregation(subAgg, registry); for (AggregationBuilder t : translated) { rolled.subAggregation(t); } diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java index 610275705eef8..76c6beef33704 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java @@ -176,10 +176,12 @@ static MultiSearchRequest createMSearchRequest(SearchRequest request, NamedWrite for (AggregationBuilder agg : sourceAgg.getAggregatorFactories()) { + // TODO this filter agg is now redundant given we filter on job ID + // in the query and the translator doesn't add any clauses anymore List filterConditions = new ArrayList<>(5); // Translate the agg tree, and collect any potential filtering clauses - List translatedAgg = RollupRequestTranslator.translateAggregation(agg, filterConditions, registry); + List translatedAgg = RollupRequestTranslator.translateAggregation(agg, registry); BoolQueryBuilder boolQuery = new BoolQueryBuilder(); filterConditions.forEach(boolQuery::must); diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java index 95161e0d149dc..25ba13daa08d4 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java @@ -661,6 +661,51 @@ public void testComparatorCalendar() { } } + public void testObsoleteTimezone() { + // Job has "obsolete" timezone + DateHistogramGroupConfig dateHisto = new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h"), null, "Canada/Mountain"); + GroupConfig group = new GroupConfig(dateHisto); + RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10, group, emptyList(), null); + RollupJobCaps cap = new RollupJobCaps(job); + Set caps = singletonSet(cap); + + DateHistogramAggregationBuilder builder = new DateHistogramAggregationBuilder("foo").field("foo") + .dateHistogramInterval(job.getGroupConfig().getDateHistogram().getInterval()) + .timeZone(DateTimeZone.forID("Canada/Mountain")); + + Set bestCaps = RollupJobIdentifierUtils.findBestJobs(builder, caps); + assertThat(bestCaps.size(), equalTo(1)); + + builder = new DateHistogramAggregationBuilder("foo").field("foo") + .dateHistogramInterval(job.getGroupConfig().getDateHistogram().getInterval()) + .timeZone(DateTimeZone.forID("America/Edmonton")); + + bestCaps = RollupJobIdentifierUtils.findBestJobs(builder, caps); + assertThat(bestCaps.size(), equalTo(1)); + + // now the reverse, job has "new" timezone + + dateHisto = new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h"), null, "America/Edmonton"); + group = new GroupConfig(dateHisto); + job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10, group, emptyList(), null); + cap = new RollupJobCaps(job); + caps = singletonSet(cap); + + builder = new DateHistogramAggregationBuilder("foo").field("foo") + .dateHistogramInterval(job.getGroupConfig().getDateHistogram().getInterval()) + .timeZone(DateTimeZone.forID("Canada/Mountain")); + + bestCaps = RollupJobIdentifierUtils.findBestJobs(builder, caps); + assertThat(bestCaps.size(), equalTo(1)); + + builder = new DateHistogramAggregationBuilder("foo").field("foo") + .dateHistogramInterval(job.getGroupConfig().getDateHistogram().getInterval()) + .timeZone(DateTimeZone.forID("America/Edmonton")); + + bestCaps = RollupJobIdentifierUtils.findBestJobs(builder, caps); + assertThat(bestCaps.size(), equalTo(1)); + } + private static long getMillis(RollupJobCaps cap) { for (RollupJobCaps.RollupFieldCaps fieldCaps : cap.getFieldCaps().values()) { for (Map agg : fieldCaps.getAggs()) { diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupRequestTranslationTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupRequestTranslationTests.java index 1ceac98725e8b..8bd15d59456ca 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupRequestTranslationTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupRequestTranslationTests.java @@ -9,8 +9,6 @@ import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.index.query.TermQueryBuilder; import org.elasticsearch.search.SearchModule; import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; @@ -32,7 +30,6 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.function.Function; @@ -64,9 +61,8 @@ public void testBasicDateHisto() { .extendedBounds(new ExtendedBounds(0L, 1000L)) .subAggregation(new MaxAggregationBuilder("the_max").field("max_field")) .subAggregation(new AvgAggregationBuilder("the_avg").field("avg_field")); - List filterConditions = new ArrayList<>(); - List translated = translateAggregation(histo, filterConditions, namedWriteableRegistry); + List translated = translateAggregation(histo, namedWriteableRegistry); assertThat(translated.size(), equalTo(1)); assertThat(translated.get(0), Matchers.instanceOf(DateHistogramAggregationBuilder.class)); DateHistogramAggregationBuilder translatedHisto = (DateHistogramAggregationBuilder)translated.get(0); @@ -92,22 +88,6 @@ public void testBasicDateHisto() { assertThat(subAggs.get("test_histo._count"), Matchers.instanceOf(SumAggregationBuilder.class)); assertThat(((SumAggregationBuilder)subAggs.get("test_histo._count")).field(), equalTo("foo.date_histogram._count")); - - assertThat(filterConditions.size(), equalTo(1)); - for (QueryBuilder q : filterConditions) { - if (q instanceof TermQueryBuilder) { - switch (((TermQueryBuilder) q).fieldName()) { - case "foo.date_histogram.time_zone": - assertThat(((TermQueryBuilder) q).value(), equalTo("UTC")); - break; - default: - fail("Unexpected Term Query in filter conditions: [" + ((TermQueryBuilder) q).fieldName() + "]"); - break; - } - } else { - fail("Unexpected query builder in filter conditions"); - } - } } public void testFormattedDateHisto() { @@ -117,9 +97,8 @@ public void testFormattedDateHisto() { .extendedBounds(new ExtendedBounds(0L, 1000L)) .format("yyyy-MM-dd") .subAggregation(new MaxAggregationBuilder("the_max").field("max_field")); - List filterConditions = new ArrayList<>(); - List translated = translateAggregation(histo, filterConditions, namedWriteableRegistry); + List translated = translateAggregation(histo, namedWriteableRegistry); assertThat(translated.size(), equalTo(1)); assertThat(translated.get(0), Matchers.instanceOf(DateHistogramAggregationBuilder.class)); DateHistogramAggregationBuilder translatedHisto = (DateHistogramAggregationBuilder)translated.get(0); @@ -132,7 +111,6 @@ public void testFormattedDateHisto() { public void testSimpleMetric() { int i = ESTestCase.randomIntBetween(0, 2); List translated = new ArrayList<>(); - List filterConditions = new ArrayList<>(); Class clazz = null; String fieldName = null; @@ -140,17 +118,17 @@ public void testSimpleMetric() { if (i == 0) { translated = translateAggregation(new MaxAggregationBuilder("test_metric") - .field("foo"), filterConditions, namedWriteableRegistry); + .field("foo"), namedWriteableRegistry); clazz = MaxAggregationBuilder.class; fieldName = "foo.max.value"; } else if (i == 1) { translated = translateAggregation(new MinAggregationBuilder("test_metric") - .field("foo"), filterConditions, namedWriteableRegistry); + .field("foo"), namedWriteableRegistry); clazz = MinAggregationBuilder.class; fieldName = "foo.min.value"; } else if (i == 2) { translated = translateAggregation(new SumAggregationBuilder("test_metric") - .field("foo"), filterConditions, namedWriteableRegistry); + .field("foo"), namedWriteableRegistry); clazz = SumAggregationBuilder.class; fieldName = "foo.sum.value"; } @@ -159,14 +137,12 @@ public void testSimpleMetric() { assertThat(translated.get(0), Matchers.instanceOf(clazz)); assertThat((translated.get(0)).getName(), equalTo("test_metric")); assertThat(((ValuesSourceAggregationBuilder)translated.get(0)).field(), equalTo(fieldName)); - - assertThat(filterConditions.size(), equalTo(0)); } public void testUnsupportedMetric() { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> translateAggregation(new StatsAggregationBuilder("test_metric") - .field("foo"), Collections.emptyList(), namedWriteableRegistry)); + .field("foo"), namedWriteableRegistry)); assertThat(e.getMessage(), equalTo("Unable to translate aggregation tree into Rollup. Aggregation [test_metric] is of type " + "[StatsAggregationBuilder] which is currently unsupported.")); } @@ -177,9 +153,8 @@ public void testDateHistoIntervalWithMinMax() { .field("foo") .subAggregation(new MaxAggregationBuilder("the_max").field("max_field")) .subAggregation(new AvgAggregationBuilder("the_avg").field("avg_field")); - List filterConditions = new ArrayList<>(); - List translated = translateAggregation(histo, filterConditions, namedWriteableRegistry); + List translated = translateAggregation(histo, namedWriteableRegistry); assertThat(translated.size(), equalTo(1)); assertThat(translated.get(0), instanceOf(DateHistogramAggregationBuilder.class)); DateHistogramAggregationBuilder translatedHisto = (DateHistogramAggregationBuilder)translated.get(0); @@ -205,20 +180,6 @@ public void testDateHistoIntervalWithMinMax() { assertThat(subAggs.get("test_histo._count"), instanceOf(SumAggregationBuilder.class)); assertThat(((SumAggregationBuilder)subAggs.get("test_histo._count")).field(), equalTo("foo.date_histogram._count")); - - assertThat(filterConditions.size(), equalTo(1)); - - for (QueryBuilder q : filterConditions) { - if (q instanceof TermQueryBuilder) { - if (((TermQueryBuilder) q).fieldName().equals("foo.date_histogram.time_zone")) { - assertThat(((TermQueryBuilder) q).value(), equalTo("UTC")); - } else { - fail("Unexpected Term Query in filter conditions: [" + ((TermQueryBuilder) q).fieldName() + "]"); - } - } else { - fail("Unexpected query builder in filter conditions"); - } - } } public void testDateHistoLongIntervalWithMinMax() { @@ -227,9 +188,8 @@ public void testDateHistoLongIntervalWithMinMax() { .field("foo") .subAggregation(new MaxAggregationBuilder("the_max").field("max_field")) .subAggregation(new AvgAggregationBuilder("the_avg").field("avg_field")); - List filterConditions = new ArrayList<>(); - List translated = translateAggregation(histo, filterConditions, namedWriteableRegistry); + List translated = translateAggregation(histo, namedWriteableRegistry); assertThat(translated.size(), equalTo(1)); assertThat(translated.get(0), instanceOf(DateHistogramAggregationBuilder.class)); DateHistogramAggregationBuilder translatedHisto = (DateHistogramAggregationBuilder)translated.get(0); @@ -255,26 +215,11 @@ public void testDateHistoLongIntervalWithMinMax() { assertThat(subAggs.get("test_histo._count"), instanceOf(SumAggregationBuilder.class)); assertThat(((SumAggregationBuilder)subAggs.get("test_histo._count")).field(), equalTo("foo.date_histogram._count")); - - assertThat(filterConditions.size(), equalTo(1)); - - for (QueryBuilder q : filterConditions) { - if (q instanceof TermQueryBuilder) { - if (((TermQueryBuilder) q).fieldName().equals("foo.date_histogram.time_zone")) { - assertThat(((TermQueryBuilder) q).value(), equalTo("UTC")); - } else { - fail("Unexpected Term Query in filter conditions: [" + ((TermQueryBuilder) q).fieldName() + "]"); - } - } else { - fail("Unexpected query builder in filter conditions"); - } - } } public void testAvgMetric() { - List filterConditions = new ArrayList<>(); List translated = translateAggregation(new AvgAggregationBuilder("test_metric") - .field("foo"), filterConditions, namedWriteableRegistry); + .field("foo"), namedWriteableRegistry); assertThat(translated.size(), equalTo(2)); Map metrics = translated.stream() @@ -287,8 +232,6 @@ public void testAvgMetric() { assertThat(metrics.get("test_metric._count"), Matchers.instanceOf(SumAggregationBuilder.class)); assertThat(((SumAggregationBuilder)metrics.get("test_metric._count")).field(), equalTo("foo.avg._count")); - - assertThat(filterConditions.size(), equalTo(0)); } public void testStringTerms() throws IOException { @@ -297,9 +240,8 @@ public void testStringTerms() throws IOException { terms.field("foo") .subAggregation(new MaxAggregationBuilder("the_max").field("max_field")) .subAggregation(new AvgAggregationBuilder("the_avg").field("avg_field")); - List filterConditions = new ArrayList<>(); - List translated = translateAggregation(terms, filterConditions, namedWriteableRegistry); + List translated = translateAggregation(terms, namedWriteableRegistry); assertThat(translated.size(), equalTo(1)); assertThat(translated.get(0), Matchers.instanceOf(TermsAggregationBuilder.class)); TermsAggregationBuilder translatedHisto = (TermsAggregationBuilder)translated.get(0); @@ -324,8 +266,6 @@ public void testStringTerms() throws IOException { assertThat(subAggs.get("test_string_terms._count"), Matchers.instanceOf(SumAggregationBuilder.class)); assertThat(((SumAggregationBuilder)subAggs.get("test_string_terms._count")).field(), equalTo("foo.terms._count")); - - assertThat(filterConditions.size(), equalTo(0)); } public void testBasicHisto() { @@ -336,9 +276,8 @@ public void testBasicHisto() { .extendedBounds(0.0, 1000.0) .subAggregation(new MaxAggregationBuilder("the_max").field("max_field")) .subAggregation(new AvgAggregationBuilder("the_avg").field("avg_field")); - List filterConditions = new ArrayList<>(); - List translated = translateAggregation(histo, filterConditions, namedWriteableRegistry); + List translated = translateAggregation(histo, namedWriteableRegistry); assertThat(translated.size(), equalTo(1)); assertThat(translated.get(0), Matchers.instanceOf(HistogramAggregationBuilder.class)); HistogramAggregationBuilder translatedHisto = (HistogramAggregationBuilder)translated.get(0); @@ -364,18 +303,6 @@ public void testBasicHisto() { assertThat(((SumAggregationBuilder)subAggs.get("test_histo._count")).field(), equalTo("foo.histogram._count")); - assertThat(filterConditions.size(), equalTo(0)); - for (QueryBuilder q : filterConditions) { - if (q instanceof TermQueryBuilder) { - switch (((TermQueryBuilder) q).fieldName()) { - default: - fail("Unexpected Term Query in filter conditions: [" + ((TermQueryBuilder) q).fieldName() + "]"); - break; - } - } else { - fail("Unexpected query builder in filter conditions"); - } - } } public void testUnsupportedAgg() { @@ -383,10 +310,9 @@ public void testUnsupportedAgg() { geo.field("foo") .subAggregation(new MaxAggregationBuilder("the_max").field("max_field")) .subAggregation(new AvgAggregationBuilder("the_avg").field("avg_field")); - List filterConditions = new ArrayList<>(); Exception e = expectThrows(RuntimeException.class, - () -> translateAggregation(geo, filterConditions, namedWriteableRegistry)); + () -> translateAggregation(geo, namedWriteableRegistry)); assertThat(e.getMessage(), equalTo("Unable to translate aggregation tree into Rollup. Aggregation [test_geo] is of type " + "[GeoDistanceAggregationBuilder] which is currently unsupported.")); } diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java index 86891eda669fa..fbc0a9811f522 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java @@ -89,6 +89,11 @@ public void testUnkownTimeZone() { assertThat(e.getMessage(), equalTo("The datetime zone id 'FOO' is not recognised")); } + public void testObsoleteTimeZone() { + DateHistogramGroupConfig config = new DateHistogramGroupConfig("foo", DateHistogramInterval.HOUR, null, "Canada/Mountain"); + assertThat(config.getTimeZone(), equalTo("America/Edmonton")); + } + public void testEmptyHistoField() { Exception e = expectThrows(IllegalArgumentException.class, () -> new HistogramGroupConfig(1L, (String[]) null)); assertThat(e.getMessage(), equalTo("Fields must have at least one value")); diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml index 3399376b6ab69..1d9ca45f05aae 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml @@ -914,3 +914,365 @@ setup: interval: "1h" time_zone: "UTC" +--- +"Obsolete Timezone": + + - do: + indices.create: + index: tz + body: + mappings: + _doc: + properties: + timestamp: + type: date + partition: + type: keyword + price: + type: integer + - do: + headers: + Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser + xpack.rollup.put_job: + id: tz + body: > + { + "index_pattern": "tz", + "rollup_index": "tz_rollup", + "cron": "*/30 * * * * ?", + "page_size" :10, + "groups" : { + "date_histogram": { + "field": "timestamp", + "interval": "1h", + "time_zone": "Canada/Mountain" + }, + "terms": { + "fields": ["partition"] + } + }, + "metrics": [ + { + "field": "price", + "metrics": ["max"] + } + ] + } + + - do: + headers: + Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser + bulk: + refresh: true + body: + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2017-01-01T05:00:00Z" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "America/Edmonton" + timestamp.date_histogram._count: 1 + partition.terms.value: "a" + partition.terms._count: 1 + price.max.value: 1 + "_rollup.id": "tz" + "_rollup.version": 2 + + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2017-01-01T06:00:00Z" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "America/Edmonton" + timestamp.date_histogram._count: 2 + partition.terms.value: "b" + partition.terms._count: 2 + price.max.value: 2 + "_rollup.id": "tz" + "_rollup.version": 2 + + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2017-01-01T07:00:00Z" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "America/Edmonton" + timestamp.date_histogram._count: 10 + partition.terms.value: "a" + partition.terms._count: 10 + price.max.value: 3 + "_rollup.id": "tz" + "_rollup.version": 2 + + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2017-01-01T08:00:00Z" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "America/Edmonton" + timestamp.date_histogram._count: 10 + partition.terms.value: "b" + partition.terms._count: 10 + price.max.value: 4 + "_rollup.id": "tz" + "_rollup.version": 2 + + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2017-01-01T08:00:00Z" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "America/Edmonton" + timestamp.date_histogram._count: 10 + partition.terms.value: "a" + partition.terms._count: 10 + price.max.value: 3 + "_rollup.id": "tz" + "_rollup.version": 2 + + + - do: + xpack.rollup.rollup_search: + index: "tz_rollup" + body: + size: 0 + aggs: + histo: + date_histogram: + field: "timestamp" + interval: "1h" + time_zone: "America/Edmonton" + aggs: + the_max: + max: + field: "price" + + - length: { aggregations.histo.buckets: 4 } + - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } + - match: { aggregations.histo.buckets.0.doc_count: 1 } + - match: { aggregations.histo.buckets.0.the_max.value: 1 } + - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } + - match: { aggregations.histo.buckets.1.doc_count: 2 } + - match: { aggregations.histo.buckets.1.the_max.value: 2 } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } + - match: { aggregations.histo.buckets.2.doc_count: 10 } + - match: { aggregations.histo.buckets.2.the_max.value: 3 } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } + - match: { aggregations.histo.buckets.3.doc_count: 20 } + - match: { aggregations.histo.buckets.3.the_max.value: 4 } + + - do: + xpack.rollup.rollup_search: + index: "tz_rollup" + body: + size: 0 + aggs: + histo: + date_histogram: + field: "timestamp" + interval: "1h" + time_zone: "Canada/Mountain" + aggs: + the_max: + max: + field: "price" + + - length: { aggregations.histo.buckets: 4 } + - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } + - match: { aggregations.histo.buckets.0.doc_count: 1 } + - match: { aggregations.histo.buckets.0.the_max.value: 1 } + - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } + - match: { aggregations.histo.buckets.1.doc_count: 2 } + - match: { aggregations.histo.buckets.1.the_max.value: 2 } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } + - match: { aggregations.histo.buckets.2.doc_count: 10 } + - match: { aggregations.histo.buckets.2.the_max.value: 3 } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } + - match: { aggregations.histo.buckets.3.doc_count: 20 } + - match: { aggregations.histo.buckets.3.the_max.value: 4 } + +--- +"Obsolete BWC Timezone": + + - do: + indices.create: + index: tz_rollup + body: + settings: + number_of_shards: 1 + number_of_replicas: 0 + mappings: + _doc: + properties: + partition.terms.value: + type: keyword + partition.terms._count: + type: long + timestamp.date_histogram.time_zone: + type: keyword + timestamp.date_histogram.interval: + type: keyword + timestamp.date_histogram.timestamp: + type: date + timestamp.date_histogram._count: + type: long + price.max.value: + type: double + _rollup.id: + type: keyword + _rollup.version: + type: long + _meta: + _rollup: + sensor: + cron: "* * * * * ?" + rollup_index: "tz_rollup" + index_pattern: "tz" + timeout: "20s" + page_size: 1000 + groups: + date_histogram: + field: "timestamp" + interval: "1h" + time_zone: "Canada/Mountain" + terms: + fields: + - "partition" + id: tz + metrics: + - field: "price" + metrics: + - max + + - do: + headers: + Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser + bulk: + refresh: true + body: + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2016-12-31T22:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "Canada/Mountain" + timestamp.date_histogram._count: 1 + partition.terms.value: "a" + partition.terms._count: 1 + price.max.value: 1 + "_rollup.id": "tz" + "_rollup.version": 2 + + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2016-12-31T23:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "Canada/Mountain" + timestamp.date_histogram._count: 2 + partition.terms.value: "b" + partition.terms._count: 2 + price.max.value: 2 + "_rollup.id": "tz" + "_rollup.version": 2 + + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2017-01-01T00:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "Canada/Mountain" + timestamp.date_histogram._count: 10 + partition.terms.value: "a" + partition.terms._count: 10 + price.max.value: 3 + "_rollup.id": "tz" + "_rollup.version": 2 + + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2017-01-01T01:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "Canada/Mountain" + timestamp.date_histogram._count: 10 + partition.terms.value: "b" + partition.terms._count: 10 + price.max.value: 4 + "_rollup.id": "tz" + "_rollup.version": 2 + + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2017-01-01T01:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "Canada/Mountain" + timestamp.date_histogram._count: 10 + partition.terms.value: "a" + partition.terms._count: 10 + price.max.value: 3 + "_rollup.id": "tz" + "_rollup.version": 2 + + + - do: + xpack.rollup.rollup_search: + index: "tz_rollup" + body: + size: 0 + aggs: + histo: + date_histogram: + field: "timestamp" + interval: "1h" + time_zone: "America/Edmonton" + aggs: + the_max: + max: + field: "price" + + - length: { aggregations.histo.buckets: 4 } + - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000-07:00" } + - match: { aggregations.histo.buckets.0.doc_count: 1 } + - match: { aggregations.histo.buckets.0.the_max.value: 1 } + - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000-07:00" } + - match: { aggregations.histo.buckets.1.doc_count: 2 } + - match: { aggregations.histo.buckets.1.the_max.value: 2 } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000-07:00" } + - match: { aggregations.histo.buckets.2.doc_count: 10 } + - match: { aggregations.histo.buckets.2.the_max.value: 3 } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000-07:00" } + - match: { aggregations.histo.buckets.3.doc_count: 20 } + - match: { aggregations.histo.buckets.3.the_max.value: 4 } + + - do: + xpack.rollup.rollup_search: + index: "tz_rollup" + body: + size: 0 + aggs: + histo: + date_histogram: + field: "timestamp" + interval: "1h" + time_zone: "Canada/Mountain" + aggs: + the_max: + max: + field: "price" + + - length: { aggregations.histo.buckets: 4 } + - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000-07:00" } + - match: { aggregations.histo.buckets.0.doc_count: 1 } + - match: { aggregations.histo.buckets.0.the_max.value: 1 } + - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000-07:00" } + - match: { aggregations.histo.buckets.1.doc_count: 2 } + - match: { aggregations.histo.buckets.1.the_max.value: 2 } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000-07:00" } + - match: { aggregations.histo.buckets.2.doc_count: 10 } + - match: { aggregations.histo.buckets.2.the_max.value: 3 } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000-07:00" } + - match: { aggregations.histo.buckets.3.doc_count: 20 } + - match: { aggregations.histo.buckets.3.the_max.value: 4 } + From cc6563ef9f08b0893db115fb5d45867becce3c01 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Tue, 4 Dec 2018 17:55:54 -0500 Subject: [PATCH 02/18] Fix timestamps in test Forgot to update these. :) --- .../rest-api-spec/test/rollup/rollup_search.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml index 1d9ca45f05aae..f0fd45619593c 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml @@ -1048,16 +1048,16 @@ setup: field: "price" - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } + - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000-07:00" } - match: { aggregations.histo.buckets.0.doc_count: 1 } - match: { aggregations.histo.buckets.0.the_max.value: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } + - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000-07:00" } - match: { aggregations.histo.buckets.1.doc_count: 2 } - match: { aggregations.histo.buckets.1.the_max.value: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000-07:00" } - match: { aggregations.histo.buckets.2.doc_count: 10 } - match: { aggregations.histo.buckets.2.the_max.value: 3 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000-07:00" } - match: { aggregations.histo.buckets.3.doc_count: 20 } - match: { aggregations.histo.buckets.3.the_max.value: 4 } @@ -1078,16 +1078,16 @@ setup: field: "price" - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } + - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000-07:00" } - match: { aggregations.histo.buckets.0.doc_count: 1 } - match: { aggregations.histo.buckets.0.the_max.value: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } + - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000-07:00" } - match: { aggregations.histo.buckets.1.doc_count: 2 } - match: { aggregations.histo.buckets.1.the_max.value: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000-07:00" } - match: { aggregations.histo.buckets.2.doc_count: 10 } - match: { aggregations.histo.buckets.2.the_max.value: 3 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000-07:00" } - match: { aggregations.histo.buckets.3.doc_count: 20 } - match: { aggregations.histo.buckets.3.the_max.value: 4 } From 4867d99b1b02621c8981d30606ffd72b7cf66e70 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Thu, 6 Dec 2018 15:57:57 -0500 Subject: [PATCH 03/18] Use ZoneId --- .../core/rollup/job/DateHistogramGroupConfig.java | 6 ++++-- .../xpack/rollup/RollupJobIdentifierUtils.java | 12 ++++++------ .../xpack/rollup/config/ConfigTests.java | 5 +++-- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java index 2689da9932780..73297cf8f32a8 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java @@ -25,6 +25,7 @@ import org.joda.time.DateTimeZone; import java.io.IOException; +import java.time.ZoneId; import java.util.Map; import java.util.Objects; @@ -53,7 +54,7 @@ public class DateHistogramGroupConfig implements Writeable, ToXContentObject { private static final String FIELD = "field"; public static final String TIME_ZONE = "time_zone"; public static final String DELAY = "delay"; - private static final String DEFAULT_TIMEZONE = "UTC"; + public static final String DEFAULT_TIMEZONE = "UTC"; private static final ConstructingObjectParser PARSER; static { PARSER = new ConstructingObjectParser<>(NAME, a -> @@ -103,7 +104,8 @@ public DateHistogramGroupConfig(final String field, this.interval = interval; this.field = field; this.delay = delay; - this.timeZone = DateTimeZone.forID((timeZone != null && timeZone.isEmpty() == false) ? timeZone : DEFAULT_TIMEZONE).toString(); + this.timeZone = ZoneId.of((timeZone != null && timeZone.isEmpty() == false) + ? timeZone : DEFAULT_TIMEZONE, ZoneId.SHORT_IDS).toString(); // validate interval createRounding(this.interval.toString(), this.timeZone); diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java index 7c64306f97db7..17f6c64339756 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java @@ -18,13 +18,13 @@ import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig; import org.joda.time.DateTimeZone; +import java.time.ZoneId; import java.util.ArrayList; import java.util.Comparator; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.TimeZone; /** * This class contains utilities to identify which jobs are the "best" for a given aggregation tree. @@ -98,13 +98,13 @@ private static void checkDateHisto(DateHistogramAggregationBuilder source, List< if (agg.get(RollupField.AGG).equals(DateHistogramAggregationBuilder.NAME)) { DateHistogramInterval interval = new DateHistogramInterval((String)agg.get(RollupField.INTERVAL)); - TimeZone thisTimezone = DateTimeZone.forID((String)agg.get(DateHistogramGroupConfig.TIME_ZONE)).toTimeZone(); - TimeZone sourceTimeZone = source.timeZone() == null - ? DateTimeZone.UTC.toTimeZone() - : source.timeZone().toTimeZone(); + ZoneId thisTimezone = ZoneId.of(((String)agg.get(DateHistogramGroupConfig.TIME_ZONE)), ZoneId.SHORT_IDS); + ZoneId sourceTimeZone = source.timeZone() == null + ? ZoneId.of(DateHistogramGroupConfig.DEFAULT_TIMEZONE) + : ZoneId.of(source.timeZone().toString(), ZoneId.SHORT_IDS); // Ensure we are working on the same timezone - if (thisTimezone.hasSameRules(sourceTimeZone) == false) { + if (thisTimezone.getRules().equals(sourceTimeZone.getRules()) == false) { continue; } if (source.dateHistogramInterval() != null) { diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java index fbc0a9811f522..d77eda63bb1cf 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java @@ -15,6 +15,7 @@ import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; import org.joda.time.DateTimeZone; +import java.time.zone.ZoneRulesException; import java.util.HashMap; import java.util.Map; @@ -84,9 +85,9 @@ public void testDefaultTimeZone() { } public void testUnkownTimeZone() { - Exception e = expectThrows(IllegalArgumentException.class, + ZoneRulesException e = expectThrows(ZoneRulesException.class, () -> new DateHistogramGroupConfig("foo", DateHistogramInterval.HOUR, null, "FOO")); - assertThat(e.getMessage(), equalTo("The datetime zone id 'FOO' is not recognised")); + assertThat(e.getMessage(), equalTo("Unknown time-zone ID: FOO")); } public void testObsoleteTimeZone() { From da7ebe3bfeecf25ed0b578d9c8ecf18818cc594f Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Mon, 10 Dec 2018 13:35:02 -0500 Subject: [PATCH 04/18] Fix test --- .../java/org/elasticsearch/xpack/rollup/config/ConfigTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java index d77eda63bb1cf..5249b16851f97 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java @@ -92,7 +92,7 @@ public void testUnkownTimeZone() { public void testObsoleteTimeZone() { DateHistogramGroupConfig config = new DateHistogramGroupConfig("foo", DateHistogramInterval.HOUR, null, "Canada/Mountain"); - assertThat(config.getTimeZone(), equalTo("America/Edmonton")); + assertThat(config.getTimeZone(), equalTo("Canada/Mountain")); } public void testEmptyHistoField() { From aadf68bbe5729a5f02f79ae95ecda687720d593b Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Tue, 11 Dec 2018 14:49:34 -0500 Subject: [PATCH 05/18] Deprecation warnings when using obsolete timezones --- .../elasticsearch/common/time/DateUtils.java | 121 ++++++++++++++++++ .../action/TransportPutRollupJobAction.java | 12 ++ .../test/rollup/rollup_search.yml | 7 +- 3 files changed, 139 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/time/DateUtils.java b/server/src/main/java/org/elasticsearch/common/time/DateUtils.java index c46cee881a1a0..82ae05a6b8c59 100644 --- a/server/src/main/java/org/elasticsearch/common/time/DateUtils.java +++ b/server/src/main/java/org/elasticsearch/common/time/DateUtils.java @@ -57,6 +57,127 @@ public static DateTimeZone zoneIdToDateTimeZone(ZoneId zoneId) { DEPRECATED_SHORT_TZ_IDS = tzs.keySet(); } + // Map of deprecated timezones and their recommended new counterpart + public static final Map DEPRECATED_LONG_TIMEZONES; + static { + Map tzs = new HashMap<>(); + tzs.put("Africa/Asmera","Africa/Nairobi"); + tzs.put("Africa/Timbuktu","Africa/Abidjan"); + tzs.put("America/Argentina/ComodRivadavia","America/Argentina/Catamarca"); + tzs.put("America/Atka","America/Adak"); + tzs.put("America/Buenos_Aires","America/Argentina/Buenos_Aires"); + tzs.put("America/Catamarca","America/Argentina/Catamarca"); + tzs.put("America/Coral_Harbour","America/Atikokan"); + tzs.put("America/Cordoba","America/Argentina/Cordoba"); + tzs.put("America/Ensenada","America/Tijuana"); + tzs.put("America/Fort_Wayne","America/Indiana/Indianapolis"); + tzs.put("America/Indianapolis","America/Indiana/Indianapolis"); + tzs.put("America/Jujuy","America/Argentina/Jujuy"); + tzs.put("America/Knox_IN","America/Indiana/Knox"); + tzs.put("America/Louisville","America/Kentucky/Louisville"); + tzs.put("America/Mendoza","America/Argentina/Mendoza"); + tzs.put("America/Montreal","America/Toronto"); + tzs.put("America/Porto_Acre","America/Rio_Branco"); + tzs.put("America/Rosario","America/Argentina/Cordoba"); + tzs.put("America/Santa_Isabel","America/Tijuana"); + tzs.put("America/Shiprock","America/Denver"); + tzs.put("America/Virgin","America/Port_of_Spain"); + tzs.put("Antarctica/South_Pole","Pacific/Auckland"); + tzs.put("Asia/Ashkhabad","Asia/Ashgabat"); + tzs.put("Asia/Calcutta","Asia/Kolkata"); + tzs.put("Asia/Chongqing","Asia/Shanghai"); + tzs.put("Asia/Chungking","Asia/Shanghai"); + tzs.put("Asia/Dacca","Asia/Dhaka"); + tzs.put("Asia/Harbin","Asia/Shanghai"); + tzs.put("Asia/Kashgar","Asia/Urumqi"); + tzs.put("Asia/Katmandu","Asia/Kathmandu"); + tzs.put("Asia/Macao","Asia/Macau"); + tzs.put("Asia/Rangoon","Asia/Yangon"); + tzs.put("Asia/Saigon","Asia/Ho_Chi_Minh"); + tzs.put("Asia/Tel_Aviv","Asia/Jerusalem"); + tzs.put("Asia/Thimbu","Asia/Thimphu"); + tzs.put("Asia/Ujung_Pandang","Asia/Makassar"); + tzs.put("Asia/Ulan_Bator","Asia/Ulaanbaatar"); + tzs.put("Atlantic/Faeroe","Atlantic/Faroe"); + tzs.put("Atlantic/Jan_Mayen","Europe/Oslo"); + tzs.put("Australia/ACT","Australia/Sydney"); + tzs.put("Australia/Canberra","Australia/Sydney"); + tzs.put("Australia/LHI","Australia/Lord_Howe"); + tzs.put("Australia/NSW","Australia/Sydney"); + tzs.put("Australia/North","Australia/Darwin"); + tzs.put("Australia/Queensland","Australia/Brisbane"); + tzs.put("Australia/South","Australia/Adelaide"); + tzs.put("Australia/Tasmania","Australia/Hobart"); + tzs.put("Australia/Victoria","Australia/Melbourne"); + tzs.put("Australia/West","Australia/Perth"); + tzs.put("Australia/Yancowinna","Australia/Broken_Hill"); + tzs.put("Brazil/Acre","America/Rio_Branco"); + tzs.put("Brazil/DeNoronha","America/Noronha"); + tzs.put("Brazil/East","America/Sao_Paulo"); + tzs.put("Brazil/West","America/Manaus"); + tzs.put("Canada/Atlantic","America/Halifax"); + tzs.put("Canada/Central","America/Winnipeg"); + tzs.put("Canada/East-Saskatchewan","America/Regina"); + tzs.put("Canada/Eastern","America/Toronto"); + tzs.put("Canada/Mountain","America/Edmonton"); + tzs.put("Canada/Newfoundland","America/St_Johns"); + tzs.put("Canada/Pacific","America/Vancouver"); + tzs.put("Canada/Yukon","America/Whitehorse"); + tzs.put("Chile/Continental","America/Santiago"); + tzs.put("Chile/EasterIsland","Pacific/Easter"); + tzs.put("Cuba","America/Havana"); + tzs.put("Egypt","Africa/Cairo"); + tzs.put("Eire","Europe/Dublin"); + tzs.put("Europe/Belfast","Europe/London"); + tzs.put("Europe/Tiraspol","Europe/Chisinau"); + tzs.put("GB","Europe/London"); + tzs.put("GB-Eire","Europe/London"); + tzs.put("Greenwich","Etc/GMT"); + tzs.put("Hongkong","Asia/Hong_Kong"); + tzs.put("Iceland","Atlantic/Reykjavik"); + tzs.put("Iran","Asia/Tehran"); + tzs.put("Israel","Asia/Jerusalem"); + tzs.put("Jamaica","America/Jamaica"); + tzs.put("Japan","Asia/Tokyo"); + tzs.put("Kwajalein","Pacific/Kwajalein"); + tzs.put("Libya","Africa/Tripoli"); + tzs.put("Mexico/BajaNorte","America/Tijuana"); + tzs.put("Mexico/BajaSur","America/Mazatlan"); + tzs.put("Mexico/General","America/Mexico_City"); + tzs.put("NZ","Pacific/Auckland"); + tzs.put("NZ-CHAT","Pacific/Chatham"); + tzs.put("Navajo","America/Denver"); + tzs.put("PRC","Asia/Shanghai"); + tzs.put("Pacific/Johnston","Pacific/Honolulu"); + tzs.put("Pacific/Ponape","Pacific/Pohnpei"); + tzs.put("Pacific/Samoa","Pacific/Pago_Pago"); + tzs.put("Pacific/Truk","Pacific/Chuuk"); + tzs.put("Pacific/Yap","Pacific/Chuuk"); + tzs.put("Poland","Europe/Warsaw"); + tzs.put("Portugal","Europe/Lisbon"); + tzs.put("ROC","Asia/Taipei"); + tzs.put("ROK","Asia/Seoul"); + tzs.put("Singapore","Asia/Singapore"); + tzs.put("Turkey","Europe/Istanbul"); + tzs.put("UCT","Etc/UCT"); + tzs.put("US/Alaska","America/Anchorage"); + tzs.put("US/Aleutian","America/Adak"); + tzs.put("US/Arizona","America/Phoenix"); + tzs.put("US/Central","America/Chicago"); + tzs.put("US/East-Indiana","America/Indiana/Indianapolis"); + tzs.put("US/Eastern","America/New_York"); + tzs.put("US/Hawaii","Pacific/Honolulu"); + tzs.put("US/Indiana-Starke","America/Indiana/Knox"); + tzs.put("US/Michigan","America/Detroit"); + tzs.put("US/Mountain","America/Denver"); + tzs.put("US/Pacific","America/Los_Angeles"); + tzs.put("US/Samoa","Pacific/Pago_Pago"); + tzs.put("Universal","Etc/UTC"); + tzs.put("W-SU","Europe/Moscow"); + tzs.put("Zulu","Etc/UTC"); + DEPRECATED_LONG_TIMEZONES = Collections.unmodifiableMap(tzs); + } + public static ZoneId dateTimeZoneToZoneId(DateTimeZone timeZone) { if (timeZone == null) { return null; diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java index cb04f5554b437..6ec8f65ae411d 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.rollup.action; +import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchStatusException; @@ -32,6 +33,8 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.time.DateUtils; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.license.LicenseUtils; @@ -57,6 +60,8 @@ public class TransportPutRollupJobAction extends TransportMasterNodeAction From ef0f69542419079dcb31e5e3bebf1eda5cc93634 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Tue, 18 Dec 2018 16:06:35 -0500 Subject: [PATCH 06/18] Switch DateHistoConfig over to ZoneId --- docs/reference/rollup/apis/get-job.asciidoc | 6 +- .../rollup/apis/rollup-caps.asciidoc | 2 +- .../rollup/apis/rollup-index-caps.asciidoc | 2 +- .../rollup/apis/rollup-job-config.asciidoc | 2 +- .../rollup/rollup-search-limitations.asciidoc | 2 +- .../rollup/job/DateHistogramGroupConfig.java | 50 +-- .../xpack/core/rollup/ConfigTestHelpers.java | 8 +- ...eHistogramGroupConfigSerializingTests.java | 2 + .../rollup/RollupJobIdentifierUtils.java | 21 +- .../action/TransportRollupSearchAction.java | 5 +- .../xpack/rollup/job/RollupIndexer.java | 4 +- .../xpack/rollup/config/ConfigTests.java | 6 +- .../job/RollupIndexerIndexingTests.java | 20 +- .../rest-api-spec/test/rollup/delete_job.yml | 6 +- .../rest-api-spec/test/rollup/get_jobs.yml | 6 +- .../test/rollup/get_rollup_caps.yml | 12 +- .../test/rollup/get_rollup_index_caps.yml | 22 +- .../rest-api-spec/test/rollup/put_job.yml | 2 +- .../test/rollup/rollup_search.yml | 287 +++++++++++++++++- .../test/rollup/security_tests.yml | 4 +- 20 files changed, 385 insertions(+), 84 deletions(-) diff --git a/docs/reference/rollup/apis/get-job.asciidoc b/docs/reference/rollup/apis/get-job.asciidoc index deb369907d8ad..a823e9fb5ce93 100644 --- a/docs/reference/rollup/apis/get-job.asciidoc +++ b/docs/reference/rollup/apis/get-job.asciidoc @@ -66,7 +66,7 @@ Which will yield the following response: "interval" : "1h", "delay": "7d", "field": "timestamp", - "time_zone": "UTC" + "time_zone": "Z" }, "terms" : { "fields" : [ @@ -192,7 +192,7 @@ Which will yield the following response: "interval" : "1h", "delay": "7d", "field": "timestamp", - "time_zone": "UTC" + "time_zone": "Z" }, "terms" : { "fields" : [ @@ -247,7 +247,7 @@ Which will yield the following response: "interval" : "1h", "delay": "7d", "field": "timestamp", - "time_zone": "UTC" + "time_zone": "Z" }, "terms" : { "fields" : [ diff --git a/docs/reference/rollup/apis/rollup-caps.asciidoc b/docs/reference/rollup/apis/rollup-caps.asciidoc index 274037cae8f2f..cfc03f92f30b8 100644 --- a/docs/reference/rollup/apis/rollup-caps.asciidoc +++ b/docs/reference/rollup/apis/rollup-caps.asciidoc @@ -121,7 +121,7 @@ Which will yield the following response: "timestamp" : [ { "agg" : "date_histogram", - "time_zone" : "UTC", + "time_zone" : "Z", "interval" : "1h", "delay": "7d" } diff --git a/docs/reference/rollup/apis/rollup-index-caps.asciidoc b/docs/reference/rollup/apis/rollup-index-caps.asciidoc index df314fb458b5c..999d2862dd0a2 100644 --- a/docs/reference/rollup/apis/rollup-index-caps.asciidoc +++ b/docs/reference/rollup/apis/rollup-index-caps.asciidoc @@ -117,7 +117,7 @@ This will yield the following response: "timestamp" : [ { "agg" : "date_histogram", - "time_zone" : "UTC", + "time_zone" : "Z", "interval" : "1h", "delay": "7d" } diff --git a/docs/reference/rollup/apis/rollup-job-config.asciidoc b/docs/reference/rollup/apis/rollup-job-config.asciidoc index 3a917fb59f214..98ae308858f8b 100644 --- a/docs/reference/rollup/apis/rollup-job-config.asciidoc +++ b/docs/reference/rollup/apis/rollup-job-config.asciidoc @@ -152,7 +152,7 @@ The `date_histogram` group has several parameters: `time_zone`:: Defines what time_zone the rollup documents are stored as. Unlike raw data, which can shift timezones on the fly, rolled documents have - to be stored with a specific timezone. By default, rollup documents are stored in `UTC`, but this can be changed with the `time_zone` + to be stored with a specific timezone. By default, rollup documents are stored in `UTC` (`"Z"`), but this can be changed with the `time_zone` parameter. .Calendar vs Fixed time intervals diff --git a/docs/reference/rollup/rollup-search-limitations.asciidoc b/docs/reference/rollup/rollup-search-limitations.asciidoc index c8a736450bde0..bbbf8b3576d75 100644 --- a/docs/reference/rollup/rollup-search-limitations.asciidoc +++ b/docs/reference/rollup/rollup-search-limitations.asciidoc @@ -131,5 +131,5 @@ thrown. We expect the list of support queries to grow over time as more are imp === Timezones Rollup documents are stored in the timezone of the `date_histogram` group configuration in the job. If no timezone is specified, the default -is to rollup timestamps in `UTC`. +is to rollup timestamps in `UTC` (`"Z"`). diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java index 73297cf8f32a8..b0f7501df08cd 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.core.rollup.job; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.fieldcaps.FieldCapabilities; import org.elasticsearch.common.Nullable; @@ -15,6 +16,7 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.rounding.DateTimeUnit; import org.elasticsearch.common.rounding.Rounding; +import org.elasticsearch.common.time.DateUtils; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ToXContentObject; @@ -26,6 +28,7 @@ import java.io.IOException; import java.time.ZoneId; +import java.time.ZoneOffset; import java.util.Map; import java.util.Objects; @@ -54,7 +57,7 @@ public class DateHistogramGroupConfig implements Writeable, ToXContentObject { private static final String FIELD = "field"; public static final String TIME_ZONE = "time_zone"; public static final String DELAY = "delay"; - public static final String DEFAULT_TIMEZONE = "UTC"; + public static final ZoneId DEFAULT_TIMEZONE = ZoneOffset.UTC; private static final ConstructingObjectParser PARSER; static { PARSER = new ConstructingObjectParser<>(NAME, a -> @@ -68,7 +71,7 @@ public class DateHistogramGroupConfig implements Writeable, ToXContentObject { private final String field; private final DateHistogramInterval interval; private final DateHistogramInterval delay; - private final String timeZone; + private final ZoneId timeZone; /** * Create a new {@link DateHistogramGroupConfig} using the given field and interval parameters. @@ -104,11 +107,9 @@ public DateHistogramGroupConfig(final String field, this.interval = interval; this.field = field; this.delay = delay; - this.timeZone = ZoneId.of((timeZone != null && timeZone.isEmpty() == false) - ? timeZone : DEFAULT_TIMEZONE, ZoneId.SHORT_IDS).toString(); - - // validate interval - createRounding(this.interval.toString(), this.timeZone); + // Convert to ZoneId, then validate roundings (which unfortunately requires converting to DateTimeZone) + this.timeZone = Strings.isNullOrEmpty(timeZone) ? DEFAULT_TIMEZONE : ZoneId.of(timeZone, ZoneId.SHORT_IDS); + createRounding(this.interval.toString(), DateUtils.zoneIdToDateTimeZone(this.timeZone)); if (delay != null) { // and delay TimeValue.parseTimeValue(this.delay.toString(), DELAY); @@ -119,7 +120,8 @@ public DateHistogramGroupConfig(final String field, interval = new DateHistogramInterval(in); field = in.readString(); delay = in.readOptionalWriteable(DateHistogramInterval::new); - timeZone = in.readString(); + // new ZoneId can deal with older DateTimeStrings, no need for version check + timeZone = ZoneId.of(in.readString(), ZoneId.SHORT_IDS); } @Override @@ -127,7 +129,14 @@ public void writeTo(final StreamOutput out) throws IOException { interval.writeTo(out); out.writeString(field); out.writeOptionalWriteable(delay); - out.writeString(timeZone); + // TODO change version after backport + if (out.getVersion().onOrAfter(Version.V_7_0_0)) { + out.writeString(timeZone.toString()); + } else { + // If we're on an older version, DateTime doesn't know how to + // deal with new ZoneId strings (E.g. "Z" instead of "UTC") + out.writeString(DateUtils.zoneIdToDateTimeZone(timeZone).toString()); + } } @Override @@ -139,7 +148,7 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa if (delay != null) { builder.field(DELAY, delay.toString()); } - builder.field(TIME_ZONE, timeZone); + builder.field(TIME_ZONE, timeZone.toString()); } return builder.endObject(); } @@ -169,14 +178,14 @@ public DateHistogramInterval getDelay() { * Get the timezone to apply */ public String getTimeZone() { - return timeZone; + return timeZone.toString(); } /** * Create the rounding for this date histogram */ public Rounding createRounding() { - return createRounding(interval.toString(), timeZone); + return createRounding(interval.toString(), DateUtils.zoneIdToDateTimeZone(timeZone)); } public void validateMappings(Map> fieldCapsResponse, @@ -213,12 +222,12 @@ public boolean equals(final Object other) { return Objects.equals(interval, that.interval) && Objects.equals(field, that.field) && Objects.equals(delay, that.delay) - && Objects.equals(timeZone, that.timeZone); + && timeZone.getRules().equals(((DateHistogramGroupConfig) other).timeZone.getRules()); } @Override public int hashCode() { - return Objects.hash(interval, field, delay, timeZone); + return Objects.hash(interval, field, delay, timeZone.getRules()); } @Override @@ -230,7 +239,7 @@ public static DateHistogramGroupConfig fromXContent(final XContentParser parser) return PARSER.parse(parser, null); } - private static Rounding createRounding(final String expr, final String timeZone) { + private static Rounding createRounding(final String expr, final DateTimeZone timeZone) { DateTimeUnit timeUnit = DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(expr); final Rounding.Builder rounding; if (timeUnit != null) { @@ -238,16 +247,7 @@ private static Rounding createRounding(final String expr, final String timeZone) } else { rounding = new Rounding.Builder(TimeValue.parseTimeValue(expr, "createRounding")); } - rounding.timeZone(toDateTimeZone(timeZone)); + rounding.timeZone(timeZone); return rounding.build(); } - - private static DateTimeZone toDateTimeZone(final String timezone) { - try { - return DateTimeZone.forOffsetHours(Integer.parseInt(timezone)); - } catch (NumberFormatException e) { - return DateTimeZone.forID(timezone); - } - } - } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java index d892eb550a17a..0698880fbe657 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java @@ -71,7 +71,13 @@ public static DateHistogramGroupConfig randomDateHistogramGroupConfig(final Rand final String field = randomField(random); final DateHistogramInterval interval = randomInterval(); final DateHistogramInterval delay = random.nextBoolean() ? randomInterval() : null; - final String timezone = random.nextBoolean() ? randomDateTimeZone().toString() : null; + // TODO extend this to full set of ZoneId's once we move away from DateTime + // can't do it now because some ZoneIds are not compatible with DateTime + final String timezone = random.nextBoolean() + ? (random.nextBoolean() + ? randomDateTimeZone().toString() + : "Z") + : null; return new DateHistogramGroupConfig(field, interval, delay, timezone); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfigSerializingTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfigSerializingTests.java index 415e1a00a60cf..380181797a256 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfigSerializingTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfigSerializingTests.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.core.rollup.job; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.fieldcaps.FieldCapabilities; import org.elasticsearch.common.io.stream.BytesStreamOutput; @@ -148,6 +149,7 @@ public void testBwcSerialization() throws IOException { final DateHistogramGroupConfig reference = ConfigTestHelpers.randomDateHistogramGroupConfig(random()); final BytesStreamOutput out = new BytesStreamOutput(); + out.setVersion(Version.V_6_4_0); reference.writeTo(out); // previous way to deserialize a DateHistogramGroupConfig diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java index 17f6c64339756..0eea707ea220e 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.rollup; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.rounding.DateTimeUnit; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.search.aggregations.AggregationBuilder; @@ -97,14 +98,11 @@ private static void checkDateHisto(DateHistogramAggregationBuilder source, List< for (Map agg : fieldCaps.getAggs()) { if (agg.get(RollupField.AGG).equals(DateHistogramAggregationBuilder.NAME)) { DateHistogramInterval interval = new DateHistogramInterval((String)agg.get(RollupField.INTERVAL)); - - ZoneId thisTimezone = ZoneId.of(((String)agg.get(DateHistogramGroupConfig.TIME_ZONE)), ZoneId.SHORT_IDS); - ZoneId sourceTimeZone = source.timeZone() == null - ? ZoneId.of(DateHistogramGroupConfig.DEFAULT_TIMEZONE) - : ZoneId.of(source.timeZone().toString(), ZoneId.SHORT_IDS); - + String tz = source.timeZone() == null + ? DateHistogramGroupConfig.DEFAULT_TIMEZONE.toString() + : source.timeZone().toString(); // Ensure we are working on the same timezone - if (thisTimezone.getRules().equals(sourceTimeZone.getRules()) == false) { + if (timezoneRulesCompatible(agg, tz) == false) { continue; } if (source.dateHistogramInterval() != null) { @@ -142,6 +140,15 @@ private static void checkDateHisto(DateHistogramAggregationBuilder source, List< } } + public static boolean timezoneRulesCompatible(Map caps, String targetTZ) { + ZoneId thisTimezone = ZoneId.of((String)caps.get(DateHistogramGroupConfig.TIME_ZONE), ZoneId.SHORT_IDS); + ZoneId sourceTimeZone = Strings.isNullOrEmpty(targetTZ) + ? DateHistogramGroupConfig.DEFAULT_TIMEZONE + : ZoneId.of(targetTZ, ZoneId.SHORT_IDS); + + return thisTimezone.getRules().equals(sourceTimeZone.getRules()); + } + private static boolean isCalendarInterval(DateHistogramInterval interval) { return DateHistogramAggregationBuilder.DATE_FIELD_UNITS.containsKey(interval.toString()); } diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java index 76c6beef33704..1739519a6d9a5 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java @@ -339,9 +339,8 @@ private static String rewriteFieldName(Set jobCaps, String type = (String)agg.get(RollupField.AGG); // If the cap is for a date_histo, and the query is a range, the timezones need to match - if (type.equals(DateHistogramAggregationBuilder.NAME) && timeZone != null) { - boolean matchingTZ = ((String)agg.get(DateHistogramGroupConfig.TIME_ZONE)) - .equalsIgnoreCase(timeZone); + if (type.equals(DateHistogramAggregationBuilder.NAME) && Strings.isNullOrEmpty(timeZone) == false) { + boolean matchingTZ = RollupJobIdentifierUtils.timezoneRulesCompatible(agg, timeZone); if (matchingTZ == false) { incompatibleTimeZones.add((String)agg.get(DateHistogramGroupConfig.TIME_ZONE)); } diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java index ee29e56a33169..0337cede31460 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java @@ -7,6 +7,7 @@ import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.time.DateUtils; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.RangeQueryBuilder; @@ -42,6 +43,7 @@ import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; import org.joda.time.DateTimeZone; +import java.time.ZoneId; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -214,7 +216,7 @@ public static List> createValueSourceBuilders(fi final DateHistogramValuesSourceBuilder dateHistogramBuilder = new DateHistogramValuesSourceBuilder(dateHistogramName); dateHistogramBuilder.dateHistogramInterval(dateHistogram.getInterval()); dateHistogramBuilder.field(dateHistogramField); - dateHistogramBuilder.timeZone(toDateTimeZone(dateHistogram.getTimeZone())); + dateHistogramBuilder.timeZone(DateUtils.zoneIdToDateTimeZone(ZoneId.of(dateHistogram.getTimeZone()))); return Collections.singletonList(dateHistogramBuilder); } diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java index 5249b16851f97..6b7dde72f28d4 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java @@ -71,17 +71,17 @@ public void testEmptyDateHistoInterval() { public void testNullTimeZone() { DateHistogramGroupConfig config = new DateHistogramGroupConfig("foo", DateHistogramInterval.HOUR, null, null); - assertThat(config.getTimeZone(), equalTo(DateTimeZone.UTC.getID())); + assertThat(config.getTimeZone(), equalTo("Z")); } public void testEmptyTimeZone() { DateHistogramGroupConfig config = new DateHistogramGroupConfig("foo", DateHistogramInterval.HOUR, null, ""); - assertThat(config.getTimeZone(), equalTo(DateTimeZone.UTC.getID())); + assertThat(config.getTimeZone(), equalTo("Z")); } public void testDefaultTimeZone() { DateHistogramGroupConfig config = new DateHistogramGroupConfig("foo", DateHistogramInterval.HOUR); - assertThat(config.getTimeZone(), equalTo(DateTimeZone.UTC.getID())); + assertThat(config.getTimeZone(), equalTo("Z")); } public void testUnkownTimeZone() { diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java index 1b6dd6f3a9be2..45840f3e1185b 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java @@ -115,7 +115,7 @@ public void testSimpleDateHisto() throws Exception { "the_histo.date_histogram.timestamp", 3, "the_histo.date_histogram.interval", "1ms", "the_histo.date_histogram._count", 2, - "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), + "the_histo.date_histogram.time_zone", "Z", "_rollup.id", job.getId() ) )); @@ -128,7 +128,7 @@ public void testSimpleDateHisto() throws Exception { "the_histo.date_histogram.timestamp", 7, "the_histo.date_histogram.interval", "1ms", "the_histo.date_histogram._count", 1, - "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), + "the_histo.date_histogram.time_zone", "Z", "_rollup.id", job.getId() ) )); @@ -178,7 +178,7 @@ public void testDateHistoAndMetrics() throws Exception { "counter.min.value", 10.0, "counter.max.value", 20.0, "counter.sum.value", 50.0, - "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), + "the_histo.date_histogram.time_zone", "Z", "_rollup.id", job.getId() ) )); @@ -196,7 +196,7 @@ public void testDateHistoAndMetrics() throws Exception { "counter.min.value", 32.0, "counter.max.value", 55.0, "counter.sum.value", 141.0, - "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), + "the_histo.date_histogram.time_zone", "Z", "_rollup.id", job.getId() ) )); @@ -214,7 +214,7 @@ public void testDateHistoAndMetrics() throws Exception { "counter.min.value", 55.0, "counter.max.value", 80.0, "counter.sum.value", 275.0, - "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), + "the_histo.date_histogram.time_zone", "Z", "_rollup.id", job.getId() ) )); @@ -232,7 +232,7 @@ public void testDateHistoAndMetrics() throws Exception { "counter.min.value", 80.0, "counter.max.value", 100.0, "counter.sum.value", 270.0, - "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), + "the_histo.date_histogram.time_zone", "Z", "_rollup.id", job.getId() ) )); @@ -250,7 +250,7 @@ public void testDateHistoAndMetrics() throws Exception { "counter.min.value", 120.0, "counter.max.value", 200.0, "counter.sum.value", 440.0, - "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), + "the_histo.date_histogram.time_zone", "Z", "_rollup.id", job.getId() ) )); @@ -291,7 +291,7 @@ public void testSimpleDateHistoWithDelay() throws Exception { "the_histo.date_histogram.timestamp", rounding.round(now - TimeValue.timeValueHours(5).getMillis()), "the_histo.date_histogram.interval", "1m", "the_histo.date_histogram._count", 2, - "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), + "the_histo.date_histogram.time_zone", "Z", "_rollup.id", job.getId() ) )); @@ -304,7 +304,7 @@ public void testSimpleDateHistoWithDelay() throws Exception { "the_histo.date_histogram.timestamp", rounding.round(now - TimeValue.timeValueMinutes(75).getMillis()), "the_histo.date_histogram.interval", "1m", "the_histo.date_histogram._count", 2, - "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), + "the_histo.date_histogram.time_zone", "Z", "_rollup.id", job.getId() ) )); @@ -317,7 +317,7 @@ public void testSimpleDateHistoWithDelay() throws Exception { "the_histo.date_histogram.timestamp", rounding.round(now - TimeValue.timeValueMinutes(61).getMillis()), "the_histo.date_histogram.interval", "1m", "the_histo.date_histogram._count", 1, - "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), + "the_histo.date_histogram.time_zone", "Z", "_rollup.id", job.getId() ) )); diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/delete_job.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/delete_job.yml index 2208502b3e4cf..64b5515d4e09f 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/delete_job.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/delete_job.yml @@ -57,7 +57,7 @@ setup: date_histogram: interval: "1h" field: "the_field" - time_zone: "UTC" + time_zone: "Z" metrics: - field: "value_field" metrics: @@ -110,7 +110,7 @@ setup: date_histogram: interval: "1h" field: "the_field" - time_zone: "UTC" + time_zone: "Z" metrics: - field: "value_field" metrics: @@ -163,7 +163,7 @@ setup: date_histogram: interval: "1h" field: "the_field" - time_zone: "UTC" + time_zone: "Z" metrics: - field: "value_field" metrics: diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_jobs.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_jobs.yml index 3e03ac924ec89..e1682278c3392 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_jobs.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_jobs.yml @@ -58,7 +58,7 @@ setup: date_histogram: interval: "1h" field: "the_field" - time_zone: "UTC" + time_zone: "Z" metrics: - field: "value_field" metrics: @@ -175,7 +175,7 @@ setup: date_histogram: interval: "1h" field: "the_field" - time_zone: "UTC" + time_zone: "Z" metrics: - field: "value_field" metrics: @@ -201,7 +201,7 @@ setup: date_histogram: interval: "1h" field: "the_field" - time_zone: "UTC" + time_zone: "Z" metrics: - field: "value_field" metrics: diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_caps.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_caps.yml index f39cfc6ca13a5..78bab755137c4 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_caps.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_caps.yml @@ -78,7 +78,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -125,7 +125,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -137,7 +137,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -210,7 +210,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -222,7 +222,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -237,7 +237,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_index_caps.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_index_caps.yml index 38b7303ecb3ae..536e9ec823358 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_index_caps.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_index_caps.yml @@ -78,7 +78,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -125,7 +125,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -137,7 +137,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -185,7 +185,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -258,7 +258,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -270,7 +270,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -284,7 +284,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -361,7 +361,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -373,7 +373,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -387,7 +387,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" @@ -460,7 +460,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "UTC" + time_zone: "Z" value_field: - agg: "min" - agg: "max" 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 7f3f0347ec0df..af2284dcd5c08 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 @@ -58,7 +58,7 @@ setup: date_histogram: interval: "1h" field: "the_field" - time_zone: "UTC" + time_zone: "Z" metrics: - field: "value_field" metrics: diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml index 5e867e8c5c3e3..3819039624f8b 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml @@ -213,7 +213,7 @@ setup: --- -"Search with Metric": +"Search with Metric - UTC timezone": - do: xpack.rollup.rollup_search: @@ -1306,3 +1306,288 @@ setup: - match: { aggregations.date_histogram#histo.buckets.3.max#the_max.value: 4 } +--- +"Old style UTC TZ": + + - do: + indices.create: + index: tz_rollup + body: + settings: + number_of_shards: 1 + number_of_replicas: 0 + mappings: + _doc: + properties: + partition.terms.value: + type: keyword + partition.terms._count: + type: long + timestamp.date_histogram.time_zone: + type: keyword + timestamp.date_histogram.interval: + type: keyword + timestamp.date_histogram.timestamp: + type: date + timestamp.date_histogram._count: + type: long + price.max.value: + type: double + _rollup.id: + type: keyword + _rollup.version: + type: long + _meta: + _rollup: + sensor: + cron: "* * * * * ?" + rollup_index: "tz_rollup" + index_pattern: "tz" + timeout: "20s" + page_size: 1000 + groups: + date_histogram: + field: "timestamp" + interval: "1h" + time_zone: "UTC" + terms: + fields: + - "partition" + id: tz + metrics: + - field: "price" + metrics: + - max + + - do: + headers: + Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser + bulk: + refresh: true + body: + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2016-12-31T22:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "UTC" + timestamp.date_histogram._count: 1 + partition.terms.value: "a" + partition.terms._count: 1 + price.max.value: 1 + "_rollup.id": "tz" + "_rollup.version": 2 + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2016-12-31T23:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "UTC" + timestamp.date_histogram._count: 2 + partition.terms.value: "b" + partition.terms._count: 2 + price.max.value: 2 + "_rollup.id": "tz" + "_rollup.version": 2 + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2017-01-01T00:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "UTC" + timestamp.date_histogram._count: 10 + partition.terms.value: "a" + partition.terms._count: 10 + price.max.value: 3 + "_rollup.id": "tz" + "_rollup.version": 2 + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2017-01-01T01:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "UTC" + timestamp.date_histogram._count: 10 + partition.terms.value: "b" + partition.terms._count: 10 + price.max.value: 4 + "_rollup.id": "tz" + "_rollup.version": 2 + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2017-01-01T01:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "UTC" + timestamp.date_histogram._count: 10 + partition.terms.value: "a" + partition.terms._count: 10 + price.max.value: 3 + "_rollup.id": "tz" + "_rollup.version": 2 + - do: + xpack.rollup.rollup_search: + index: "foo_rollup" + body: + size: 0 + aggs: + histo: + date_histogram: + field: "timestamp" + interval: "1h" + time_zone: "UTC" + - length: { aggregations.histo.buckets: 4 } + - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } + - match: { aggregations.histo.buckets.0.doc_count: 1 } + - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } + - match: { aggregations.histo.buckets.1.doc_count: 2 } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } + - match: { aggregations.histo.buckets.2.doc_count: 10 } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } + - match: { aggregations.histo.buckets.3.doc_count: 20 } + +--- +"New style UTC TZ": + + - do: + indices.create: + index: tz_rollup + body: + settings: + number_of_shards: 1 + number_of_replicas: 0 + mappings: + _doc: + properties: + partition.terms.value: + type: keyword + partition.terms._count: + type: long + timestamp.date_histogram.time_zone: + type: keyword + timestamp.date_histogram.interval: + type: keyword + timestamp.date_histogram.timestamp: + type: date + timestamp.date_histogram._count: + type: long + price.max.value: + type: double + _rollup.id: + type: keyword + _rollup.version: + type: long + _meta: + _rollup: + sensor: + cron: "* * * * * ?" + rollup_index: "tz_rollup" + index_pattern: "tz" + timeout: "20s" + page_size: 1000 + groups: + date_histogram: + field: "timestamp" + interval: "1h" + time_zone: "Z" + terms: + fields: + - "partition" + id: tz + metrics: + - field: "price" + metrics: + - max + + - do: + headers: + Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser + bulk: + refresh: true + body: + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2016-12-31T22:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "Z" + timestamp.date_histogram._count: 1 + partition.terms.value: "a" + partition.terms._count: 1 + price.max.value: 1 + "_rollup.id": "tz" + "_rollup.version": 2 + + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2016-12-31T23:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "Z" + timestamp.date_histogram._count: 2 + partition.terms.value: "b" + partition.terms._count: 2 + price.max.value: 2 + "_rollup.id": "tz" + "_rollup.version": 2 + + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2017-01-01T00:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "Z" + timestamp.date_histogram._count: 10 + partition.terms.value: "a" + partition.terms._count: 10 + price.max.value: 3 + "_rollup.id": "tz" + "_rollup.version": 2 + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2017-01-01T01:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "Z" + timestamp.date_histogram._count: 10 + partition.terms.value: "b" + partition.terms._count: 10 + price.max.value: 4 + "_rollup.id": "tz" + "_rollup.version": 2 + + - index: + _index: "tz_rollup" + _type: "_doc" + - timestamp.date_histogram.timestamp: "2017-01-01T01:00:00.000-07:00" + timestamp.date_histogram.interval: "1h" + timestamp.date_histogram.time_zone: "Z" + timestamp.date_histogram._count: 10 + partition.terms.value: "a" + partition.terms._count: 10 + price.max.value: 3 + "_rollup.id": "tz" + "_rollup.version": 2 + + - do: + xpack.rollup.rollup_search: + index: "foo_rollup" + body: + size: 0 + aggs: + histo: + date_histogram: + field: "timestamp" + interval: "1h" + time_zone: "UTC" + - length: { aggregations.histo.buckets: 4 } + - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } + - match: { aggregations.histo.buckets.0.doc_count: 1 } + - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } + - match: { aggregations.histo.buckets.1.doc_count: 2 } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } + - match: { aggregations.histo.buckets.2.doc_count: 10 } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } + - match: { aggregations.histo.buckets.3.doc_count: 20 } + + diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/security_tests.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/security_tests.yml index c42ab9d06f6ce..21131f3ec049f 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/security_tests.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/security_tests.yml @@ -171,7 +171,7 @@ teardown: hits.hits.0._id: "foo$VxMkzTqILshClbtbFi4-rQ" - match: hits.hits.0._source: - timestamp.date_histogram.time_zone: "UTC" + timestamp.date_histogram.time_zone: "Z" timestamp.date_histogram.timestamp: 0 value_field.max.value: 1232.0 _rollup.version: 2 @@ -334,7 +334,7 @@ teardown: hits.hits.0._id: "foo$VxMkzTqILshClbtbFi4-rQ" - match: hits.hits.0._source: - timestamp.date_histogram.time_zone: "UTC" + timestamp.date_histogram.time_zone: "Z" timestamp.date_histogram.timestamp: 0 value_field.max.value: 1232.0 _rollup.version: 2 From d049fea3b3d2e3d67c4b248cd156b83e67418000 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Tue, 18 Dec 2018 16:24:13 -0500 Subject: [PATCH 07/18] Test fix: searching the wrong index --- .../resources/rest-api-spec/test/rollup/rollup_search.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml index 3819039624f8b..0378f3aa0f92e 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml @@ -1427,7 +1427,7 @@ setup: "_rollup.version": 2 - do: xpack.rollup.rollup_search: - index: "foo_rollup" + index: "tz_rollup" body: size: 0 aggs: @@ -1571,7 +1571,7 @@ setup: - do: xpack.rollup.rollup_search: - index: "foo_rollup" + index: "tz_rollup" body: size: 0 aggs: From e9f42ef9192c3a54d7dfc09136d74acabdb2dae7 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Tue, 18 Dec 2018 16:31:33 -0500 Subject: [PATCH 08/18] checkstyle --- .../java/org/elasticsearch/xpack/rollup/config/ConfigTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java index 6b7dde72f28d4..8ac2bcc303d16 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java @@ -13,7 +13,6 @@ import org.elasticsearch.xpack.core.rollup.job.MetricConfig; import org.elasticsearch.xpack.core.rollup.job.RollupJob; import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; -import org.joda.time.DateTimeZone; import java.time.zone.ZoneRulesException; import java.util.HashMap; From 1f7802f5566776d6f995996d4652f57c5d907d44 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Tue, 18 Dec 2018 17:48:38 -0500 Subject: [PATCH 09/18] ML needs to use zones instead of strings too --- .../extractor/aggregation/RollupDataExtractorFactory.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/RollupDataExtractorFactory.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/RollupDataExtractorFactory.java index f0ee22ce85eae..6f9a3bda31e1e 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/RollupDataExtractorFactory.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/RollupDataExtractorFactory.java @@ -22,6 +22,8 @@ import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig; import org.elasticsearch.xpack.ml.datafeed.extractor.DataExtractorFactory; +import java.time.ZoneId; +import java.time.ZoneOffset; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -120,7 +122,7 @@ private static boolean validInterval(long datafeedInterval, ParsedRollupCaps rol if (rollupJobGroupConfig.hasDatehistogram() == false) { return false; } - if ("UTC".equalsIgnoreCase(rollupJobGroupConfig.getTimezone()) == false) { + if (ZoneId.of(rollupJobGroupConfig.getTimezone()).getRules().equals(ZoneOffset.UTC.getRules()) == false) { return false; } try { From 5a2cac46af12ddb0af990bd667c4acc177c73e8b Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Wed, 9 Jan 2019 17:20:54 -0500 Subject: [PATCH 10/18] Revert "Switch DateHistoConfig over to ZoneId" This reverts commit ef0f69542419079dcb31e5e3bebf1eda5cc93634. --- docs/reference/rollup/apis/get-job.asciidoc | 6 +- .../rollup/apis/rollup-caps.asciidoc | 2 +- .../rollup/apis/rollup-index-caps.asciidoc | 2 +- .../rollup/apis/rollup-job-config.asciidoc | 2 +- .../rollup/rollup-search-limitations.asciidoc | 2 +- .../rollup/job/DateHistogramGroupConfig.java | 50 +-- .../xpack/core/rollup/ConfigTestHelpers.java | 8 +- ...eHistogramGroupConfigSerializingTests.java | 2 - .../rollup/RollupJobIdentifierUtils.java | 21 +- .../action/TransportRollupSearchAction.java | 5 +- .../xpack/rollup/job/RollupIndexer.java | 4 +- .../xpack/rollup/config/ConfigTests.java | 6 +- .../job/RollupIndexerIndexingTests.java | 20 +- .../rest-api-spec/test/rollup/delete_job.yml | 6 +- .../rest-api-spec/test/rollup/get_jobs.yml | 6 +- .../test/rollup/get_rollup_caps.yml | 12 +- .../test/rollup/get_rollup_index_caps.yml | 22 +- .../rest-api-spec/test/rollup/put_job.yml | 2 +- .../test/rollup/rollup_search.yml | 287 +----------------- .../test/rollup/security_tests.yml | 4 +- 20 files changed, 84 insertions(+), 385 deletions(-) diff --git a/docs/reference/rollup/apis/get-job.asciidoc b/docs/reference/rollup/apis/get-job.asciidoc index b8439121d2541..46bdd46ead47b 100644 --- a/docs/reference/rollup/apis/get-job.asciidoc +++ b/docs/reference/rollup/apis/get-job.asciidoc @@ -66,7 +66,7 @@ Which will yield the following response: "interval" : "1h", "delay": "7d", "field": "timestamp", - "time_zone": "Z" + "time_zone": "UTC" }, "terms" : { "fields" : [ @@ -192,7 +192,7 @@ Which will yield the following response: "interval" : "1h", "delay": "7d", "field": "timestamp", - "time_zone": "Z" + "time_zone": "UTC" }, "terms" : { "fields" : [ @@ -247,7 +247,7 @@ Which will yield the following response: "interval" : "1h", "delay": "7d", "field": "timestamp", - "time_zone": "Z" + "time_zone": "UTC" }, "terms" : { "fields" : [ diff --git a/docs/reference/rollup/apis/rollup-caps.asciidoc b/docs/reference/rollup/apis/rollup-caps.asciidoc index 75aa1cfe7b6dd..bd39d701295f2 100644 --- a/docs/reference/rollup/apis/rollup-caps.asciidoc +++ b/docs/reference/rollup/apis/rollup-caps.asciidoc @@ -121,7 +121,7 @@ Which will yield the following response: "timestamp" : [ { "agg" : "date_histogram", - "time_zone" : "Z", + "time_zone" : "UTC", "interval" : "1h", "delay": "7d" } diff --git a/docs/reference/rollup/apis/rollup-index-caps.asciidoc b/docs/reference/rollup/apis/rollup-index-caps.asciidoc index a270e0acc435b..5abcbe5737678 100644 --- a/docs/reference/rollup/apis/rollup-index-caps.asciidoc +++ b/docs/reference/rollup/apis/rollup-index-caps.asciidoc @@ -117,7 +117,7 @@ This will yield the following response: "timestamp" : [ { "agg" : "date_histogram", - "time_zone" : "Z", + "time_zone" : "UTC", "interval" : "1h", "delay": "7d" } diff --git a/docs/reference/rollup/apis/rollup-job-config.asciidoc b/docs/reference/rollup/apis/rollup-job-config.asciidoc index 6210fcd58e358..b839e454e6dbf 100644 --- a/docs/reference/rollup/apis/rollup-job-config.asciidoc +++ b/docs/reference/rollup/apis/rollup-job-config.asciidoc @@ -152,7 +152,7 @@ The `date_histogram` group has several parameters: `time_zone`:: Defines what time_zone the rollup documents are stored as. Unlike raw data, which can shift timezones on the fly, rolled documents have - to be stored with a specific timezone. By default, rollup documents are stored in `UTC` (`"Z"`), but this can be changed with the `time_zone` + to be stored with a specific timezone. By default, rollup documents are stored in `UTC`, but this can be changed with the `time_zone` parameter. .Calendar vs Fixed time intervals diff --git a/docs/reference/rollup/rollup-search-limitations.asciidoc b/docs/reference/rollup/rollup-search-limitations.asciidoc index bbbf8b3576d75..c8a736450bde0 100644 --- a/docs/reference/rollup/rollup-search-limitations.asciidoc +++ b/docs/reference/rollup/rollup-search-limitations.asciidoc @@ -131,5 +131,5 @@ thrown. We expect the list of support queries to grow over time as more are imp === Timezones Rollup documents are stored in the timezone of the `date_histogram` group configuration in the job. If no timezone is specified, the default -is to rollup timestamps in `UTC` (`"Z"`). +is to rollup timestamps in `UTC`. diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java index b0f7501df08cd..73297cf8f32a8 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java @@ -5,7 +5,6 @@ */ package org.elasticsearch.xpack.core.rollup.job; -import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.fieldcaps.FieldCapabilities; import org.elasticsearch.common.Nullable; @@ -16,7 +15,6 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.rounding.DateTimeUnit; import org.elasticsearch.common.rounding.Rounding; -import org.elasticsearch.common.time.DateUtils; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ToXContentObject; @@ -28,7 +26,6 @@ import java.io.IOException; import java.time.ZoneId; -import java.time.ZoneOffset; import java.util.Map; import java.util.Objects; @@ -57,7 +54,7 @@ public class DateHistogramGroupConfig implements Writeable, ToXContentObject { private static final String FIELD = "field"; public static final String TIME_ZONE = "time_zone"; public static final String DELAY = "delay"; - public static final ZoneId DEFAULT_TIMEZONE = ZoneOffset.UTC; + public static final String DEFAULT_TIMEZONE = "UTC"; private static final ConstructingObjectParser PARSER; static { PARSER = new ConstructingObjectParser<>(NAME, a -> @@ -71,7 +68,7 @@ public class DateHistogramGroupConfig implements Writeable, ToXContentObject { private final String field; private final DateHistogramInterval interval; private final DateHistogramInterval delay; - private final ZoneId timeZone; + private final String timeZone; /** * Create a new {@link DateHistogramGroupConfig} using the given field and interval parameters. @@ -107,9 +104,11 @@ public DateHistogramGroupConfig(final String field, this.interval = interval; this.field = field; this.delay = delay; - // Convert to ZoneId, then validate roundings (which unfortunately requires converting to DateTimeZone) - this.timeZone = Strings.isNullOrEmpty(timeZone) ? DEFAULT_TIMEZONE : ZoneId.of(timeZone, ZoneId.SHORT_IDS); - createRounding(this.interval.toString(), DateUtils.zoneIdToDateTimeZone(this.timeZone)); + this.timeZone = ZoneId.of((timeZone != null && timeZone.isEmpty() == false) + ? timeZone : DEFAULT_TIMEZONE, ZoneId.SHORT_IDS).toString(); + + // validate interval + createRounding(this.interval.toString(), this.timeZone); if (delay != null) { // and delay TimeValue.parseTimeValue(this.delay.toString(), DELAY); @@ -120,8 +119,7 @@ public DateHistogramGroupConfig(final String field, interval = new DateHistogramInterval(in); field = in.readString(); delay = in.readOptionalWriteable(DateHistogramInterval::new); - // new ZoneId can deal with older DateTimeStrings, no need for version check - timeZone = ZoneId.of(in.readString(), ZoneId.SHORT_IDS); + timeZone = in.readString(); } @Override @@ -129,14 +127,7 @@ public void writeTo(final StreamOutput out) throws IOException { interval.writeTo(out); out.writeString(field); out.writeOptionalWriteable(delay); - // TODO change version after backport - if (out.getVersion().onOrAfter(Version.V_7_0_0)) { - out.writeString(timeZone.toString()); - } else { - // If we're on an older version, DateTime doesn't know how to - // deal with new ZoneId strings (E.g. "Z" instead of "UTC") - out.writeString(DateUtils.zoneIdToDateTimeZone(timeZone).toString()); - } + out.writeString(timeZone); } @Override @@ -148,7 +139,7 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa if (delay != null) { builder.field(DELAY, delay.toString()); } - builder.field(TIME_ZONE, timeZone.toString()); + builder.field(TIME_ZONE, timeZone); } return builder.endObject(); } @@ -178,14 +169,14 @@ public DateHistogramInterval getDelay() { * Get the timezone to apply */ public String getTimeZone() { - return timeZone.toString(); + return timeZone; } /** * Create the rounding for this date histogram */ public Rounding createRounding() { - return createRounding(interval.toString(), DateUtils.zoneIdToDateTimeZone(timeZone)); + return createRounding(interval.toString(), timeZone); } public void validateMappings(Map> fieldCapsResponse, @@ -222,12 +213,12 @@ public boolean equals(final Object other) { return Objects.equals(interval, that.interval) && Objects.equals(field, that.field) && Objects.equals(delay, that.delay) - && timeZone.getRules().equals(((DateHistogramGroupConfig) other).timeZone.getRules()); + && Objects.equals(timeZone, that.timeZone); } @Override public int hashCode() { - return Objects.hash(interval, field, delay, timeZone.getRules()); + return Objects.hash(interval, field, delay, timeZone); } @Override @@ -239,7 +230,7 @@ public static DateHistogramGroupConfig fromXContent(final XContentParser parser) return PARSER.parse(parser, null); } - private static Rounding createRounding(final String expr, final DateTimeZone timeZone) { + private static Rounding createRounding(final String expr, final String timeZone) { DateTimeUnit timeUnit = DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(expr); final Rounding.Builder rounding; if (timeUnit != null) { @@ -247,7 +238,16 @@ private static Rounding createRounding(final String expr, final DateTimeZone tim } else { rounding = new Rounding.Builder(TimeValue.parseTimeValue(expr, "createRounding")); } - rounding.timeZone(timeZone); + rounding.timeZone(toDateTimeZone(timeZone)); return rounding.build(); } + + private static DateTimeZone toDateTimeZone(final String timezone) { + try { + return DateTimeZone.forOffsetHours(Integer.parseInt(timezone)); + } catch (NumberFormatException e) { + return DateTimeZone.forID(timezone); + } + } + } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java index 0698880fbe657..d892eb550a17a 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/ConfigTestHelpers.java @@ -71,13 +71,7 @@ public static DateHistogramGroupConfig randomDateHistogramGroupConfig(final Rand final String field = randomField(random); final DateHistogramInterval interval = randomInterval(); final DateHistogramInterval delay = random.nextBoolean() ? randomInterval() : null; - // TODO extend this to full set of ZoneId's once we move away from DateTime - // can't do it now because some ZoneIds are not compatible with DateTime - final String timezone = random.nextBoolean() - ? (random.nextBoolean() - ? randomDateTimeZone().toString() - : "Z") - : null; + final String timezone = random.nextBoolean() ? randomDateTimeZone().toString() : null; return new DateHistogramGroupConfig(field, interval, delay, timezone); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfigSerializingTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfigSerializingTests.java index 380181797a256..415e1a00a60cf 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfigSerializingTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfigSerializingTests.java @@ -5,7 +5,6 @@ */ package org.elasticsearch.xpack.core.rollup.job; -import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.fieldcaps.FieldCapabilities; import org.elasticsearch.common.io.stream.BytesStreamOutput; @@ -149,7 +148,6 @@ public void testBwcSerialization() throws IOException { final DateHistogramGroupConfig reference = ConfigTestHelpers.randomDateHistogramGroupConfig(random()); final BytesStreamOutput out = new BytesStreamOutput(); - out.setVersion(Version.V_6_4_0); reference.writeTo(out); // previous way to deserialize a DateHistogramGroupConfig diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java index 0eea707ea220e..17f6c64339756 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java @@ -5,7 +5,6 @@ */ package org.elasticsearch.xpack.rollup; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.rounding.DateTimeUnit; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.search.aggregations.AggregationBuilder; @@ -98,11 +97,14 @@ private static void checkDateHisto(DateHistogramAggregationBuilder source, List< for (Map agg : fieldCaps.getAggs()) { if (agg.get(RollupField.AGG).equals(DateHistogramAggregationBuilder.NAME)) { DateHistogramInterval interval = new DateHistogramInterval((String)agg.get(RollupField.INTERVAL)); - String tz = source.timeZone() == null - ? DateHistogramGroupConfig.DEFAULT_TIMEZONE.toString() - : source.timeZone().toString(); + + ZoneId thisTimezone = ZoneId.of(((String)agg.get(DateHistogramGroupConfig.TIME_ZONE)), ZoneId.SHORT_IDS); + ZoneId sourceTimeZone = source.timeZone() == null + ? ZoneId.of(DateHistogramGroupConfig.DEFAULT_TIMEZONE) + : ZoneId.of(source.timeZone().toString(), ZoneId.SHORT_IDS); + // Ensure we are working on the same timezone - if (timezoneRulesCompatible(agg, tz) == false) { + if (thisTimezone.getRules().equals(sourceTimeZone.getRules()) == false) { continue; } if (source.dateHistogramInterval() != null) { @@ -140,15 +142,6 @@ private static void checkDateHisto(DateHistogramAggregationBuilder source, List< } } - public static boolean timezoneRulesCompatible(Map caps, String targetTZ) { - ZoneId thisTimezone = ZoneId.of((String)caps.get(DateHistogramGroupConfig.TIME_ZONE), ZoneId.SHORT_IDS); - ZoneId sourceTimeZone = Strings.isNullOrEmpty(targetTZ) - ? DateHistogramGroupConfig.DEFAULT_TIMEZONE - : ZoneId.of(targetTZ, ZoneId.SHORT_IDS); - - return thisTimezone.getRules().equals(sourceTimeZone.getRules()); - } - private static boolean isCalendarInterval(DateHistogramInterval interval) { return DateHistogramAggregationBuilder.DATE_FIELD_UNITS.containsKey(interval.toString()); } diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java index 1739519a6d9a5..76c6beef33704 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java @@ -339,8 +339,9 @@ private static String rewriteFieldName(Set jobCaps, String type = (String)agg.get(RollupField.AGG); // If the cap is for a date_histo, and the query is a range, the timezones need to match - if (type.equals(DateHistogramAggregationBuilder.NAME) && Strings.isNullOrEmpty(timeZone) == false) { - boolean matchingTZ = RollupJobIdentifierUtils.timezoneRulesCompatible(agg, timeZone); + if (type.equals(DateHistogramAggregationBuilder.NAME) && timeZone != null) { + boolean matchingTZ = ((String)agg.get(DateHistogramGroupConfig.TIME_ZONE)) + .equalsIgnoreCase(timeZone); if (matchingTZ == false) { incompatibleTimeZones.add((String)agg.get(DateHistogramGroupConfig.TIME_ZONE)); } diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java index 0337cede31460..ee29e56a33169 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java @@ -7,7 +7,6 @@ import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchResponse; -import org.elasticsearch.common.time.DateUtils; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.RangeQueryBuilder; @@ -43,7 +42,6 @@ import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; import org.joda.time.DateTimeZone; -import java.time.ZoneId; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -216,7 +214,7 @@ public static List> createValueSourceBuilders(fi final DateHistogramValuesSourceBuilder dateHistogramBuilder = new DateHistogramValuesSourceBuilder(dateHistogramName); dateHistogramBuilder.dateHistogramInterval(dateHistogram.getInterval()); dateHistogramBuilder.field(dateHistogramField); - dateHistogramBuilder.timeZone(DateUtils.zoneIdToDateTimeZone(ZoneId.of(dateHistogram.getTimeZone()))); + dateHistogramBuilder.timeZone(toDateTimeZone(dateHistogram.getTimeZone())); return Collections.singletonList(dateHistogramBuilder); } diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java index 8ac2bcc303d16..8a71604a11e0f 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java @@ -70,17 +70,17 @@ public void testEmptyDateHistoInterval() { public void testNullTimeZone() { DateHistogramGroupConfig config = new DateHistogramGroupConfig("foo", DateHistogramInterval.HOUR, null, null); - assertThat(config.getTimeZone(), equalTo("Z")); + assertThat(config.getTimeZone(), equalTo(DateTimeZone.UTC.getID())); } public void testEmptyTimeZone() { DateHistogramGroupConfig config = new DateHistogramGroupConfig("foo", DateHistogramInterval.HOUR, null, ""); - assertThat(config.getTimeZone(), equalTo("Z")); + assertThat(config.getTimeZone(), equalTo(DateTimeZone.UTC.getID())); } public void testDefaultTimeZone() { DateHistogramGroupConfig config = new DateHistogramGroupConfig("foo", DateHistogramInterval.HOUR); - assertThat(config.getTimeZone(), equalTo("Z")); + assertThat(config.getTimeZone(), equalTo(DateTimeZone.UTC.getID())); } public void testUnkownTimeZone() { diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java index ac1fced098d08..b87b1f3761fdb 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java @@ -115,7 +115,7 @@ public void testSimpleDateHisto() throws Exception { "the_histo.date_histogram.timestamp", 3, "the_histo.date_histogram.interval", "1ms", "the_histo.date_histogram._count", 2, - "the_histo.date_histogram.time_zone", "Z", + "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), "_rollup.id", job.getId() ) )); @@ -128,7 +128,7 @@ public void testSimpleDateHisto() throws Exception { "the_histo.date_histogram.timestamp", 7, "the_histo.date_histogram.interval", "1ms", "the_histo.date_histogram._count", 1, - "the_histo.date_histogram.time_zone", "Z", + "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), "_rollup.id", job.getId() ) )); @@ -178,7 +178,7 @@ public void testDateHistoAndMetrics() throws Exception { "counter.min.value", 10.0, "counter.max.value", 20.0, "counter.sum.value", 50.0, - "the_histo.date_histogram.time_zone", "Z", + "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), "_rollup.id", job.getId() ) )); @@ -196,7 +196,7 @@ public void testDateHistoAndMetrics() throws Exception { "counter.min.value", 32.0, "counter.max.value", 55.0, "counter.sum.value", 141.0, - "the_histo.date_histogram.time_zone", "Z", + "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), "_rollup.id", job.getId() ) )); @@ -214,7 +214,7 @@ public void testDateHistoAndMetrics() throws Exception { "counter.min.value", 55.0, "counter.max.value", 80.0, "counter.sum.value", 275.0, - "the_histo.date_histogram.time_zone", "Z", + "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), "_rollup.id", job.getId() ) )); @@ -232,7 +232,7 @@ public void testDateHistoAndMetrics() throws Exception { "counter.min.value", 80.0, "counter.max.value", 100.0, "counter.sum.value", 270.0, - "the_histo.date_histogram.time_zone", "Z", + "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), "_rollup.id", job.getId() ) )); @@ -250,7 +250,7 @@ public void testDateHistoAndMetrics() throws Exception { "counter.min.value", 120.0, "counter.max.value", 200.0, "counter.sum.value", 440.0, - "the_histo.date_histogram.time_zone", "Z", + "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), "_rollup.id", job.getId() ) )); @@ -291,7 +291,7 @@ public void testSimpleDateHistoWithDelay() throws Exception { "the_histo.date_histogram.timestamp", rounding.round(now - TimeValue.timeValueHours(5).getMillis()), "the_histo.date_histogram.interval", "1m", "the_histo.date_histogram._count", 2, - "the_histo.date_histogram.time_zone", "Z", + "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), "_rollup.id", job.getId() ) )); @@ -304,7 +304,7 @@ public void testSimpleDateHistoWithDelay() throws Exception { "the_histo.date_histogram.timestamp", rounding.round(now - TimeValue.timeValueMinutes(75).getMillis()), "the_histo.date_histogram.interval", "1m", "the_histo.date_histogram._count", 2, - "the_histo.date_histogram.time_zone", "Z", + "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), "_rollup.id", job.getId() ) )); @@ -317,7 +317,7 @@ public void testSimpleDateHistoWithDelay() throws Exception { "the_histo.date_histogram.timestamp", rounding.round(now - TimeValue.timeValueMinutes(61).getMillis()), "the_histo.date_histogram.interval", "1m", "the_histo.date_histogram._count", 1, - "the_histo.date_histogram.time_zone", "Z", + "the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(), "_rollup.id", job.getId() ) )); diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/delete_job.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/delete_job.yml index 64b5515d4e09f..2208502b3e4cf 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/delete_job.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/delete_job.yml @@ -57,7 +57,7 @@ setup: date_histogram: interval: "1h" field: "the_field" - time_zone: "Z" + time_zone: "UTC" metrics: - field: "value_field" metrics: @@ -110,7 +110,7 @@ setup: date_histogram: interval: "1h" field: "the_field" - time_zone: "Z" + time_zone: "UTC" metrics: - field: "value_field" metrics: @@ -163,7 +163,7 @@ setup: date_histogram: interval: "1h" field: "the_field" - time_zone: "Z" + time_zone: "UTC" metrics: - field: "value_field" metrics: diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_jobs.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_jobs.yml index e1682278c3392..3e03ac924ec89 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_jobs.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_jobs.yml @@ -58,7 +58,7 @@ setup: date_histogram: interval: "1h" field: "the_field" - time_zone: "Z" + time_zone: "UTC" metrics: - field: "value_field" metrics: @@ -175,7 +175,7 @@ setup: date_histogram: interval: "1h" field: "the_field" - time_zone: "Z" + time_zone: "UTC" metrics: - field: "value_field" metrics: @@ -201,7 +201,7 @@ setup: date_histogram: interval: "1h" field: "the_field" - time_zone: "Z" + time_zone: "UTC" metrics: - field: "value_field" metrics: diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_caps.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_caps.yml index 78bab755137c4..f39cfc6ca13a5 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_caps.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_caps.yml @@ -78,7 +78,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -125,7 +125,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -137,7 +137,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -210,7 +210,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -222,7 +222,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -237,7 +237,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_index_caps.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_index_caps.yml index 536e9ec823358..38b7303ecb3ae 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_index_caps.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/get_rollup_index_caps.yml @@ -78,7 +78,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -125,7 +125,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -137,7 +137,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -185,7 +185,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -258,7 +258,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -270,7 +270,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -284,7 +284,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -361,7 +361,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -373,7 +373,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -387,7 +387,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" @@ -460,7 +460,7 @@ setup: the_field: - agg: "date_histogram" interval: "1h" - time_zone: "Z" + time_zone: "UTC" value_field: - agg: "min" - agg: "max" 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 af2284dcd5c08..7f3f0347ec0df 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 @@ -58,7 +58,7 @@ setup: date_histogram: interval: "1h" field: "the_field" - time_zone: "Z" + time_zone: "UTC" metrics: - field: "value_field" metrics: diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml index 0378f3aa0f92e..5e867e8c5c3e3 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml @@ -213,7 +213,7 @@ setup: --- -"Search with Metric - UTC timezone": +"Search with Metric": - do: xpack.rollup.rollup_search: @@ -1306,288 +1306,3 @@ setup: - match: { aggregations.date_histogram#histo.buckets.3.max#the_max.value: 4 } ---- -"Old style UTC TZ": - - - do: - indices.create: - index: tz_rollup - body: - settings: - number_of_shards: 1 - number_of_replicas: 0 - mappings: - _doc: - properties: - partition.terms.value: - type: keyword - partition.terms._count: - type: long - timestamp.date_histogram.time_zone: - type: keyword - timestamp.date_histogram.interval: - type: keyword - timestamp.date_histogram.timestamp: - type: date - timestamp.date_histogram._count: - type: long - price.max.value: - type: double - _rollup.id: - type: keyword - _rollup.version: - type: long - _meta: - _rollup: - sensor: - cron: "* * * * * ?" - rollup_index: "tz_rollup" - index_pattern: "tz" - timeout: "20s" - page_size: 1000 - groups: - date_histogram: - field: "timestamp" - interval: "1h" - time_zone: "UTC" - terms: - fields: - - "partition" - id: tz - metrics: - - field: "price" - metrics: - - max - - - do: - headers: - Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser - bulk: - refresh: true - body: - - index: - _index: "tz_rollup" - _type: "_doc" - - timestamp.date_histogram.timestamp: "2016-12-31T22:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" - timestamp.date_histogram.time_zone: "UTC" - timestamp.date_histogram._count: 1 - partition.terms.value: "a" - partition.terms._count: 1 - price.max.value: 1 - "_rollup.id": "tz" - "_rollup.version": 2 - - index: - _index: "tz_rollup" - _type: "_doc" - - timestamp.date_histogram.timestamp: "2016-12-31T23:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" - timestamp.date_histogram.time_zone: "UTC" - timestamp.date_histogram._count: 2 - partition.terms.value: "b" - partition.terms._count: 2 - price.max.value: 2 - "_rollup.id": "tz" - "_rollup.version": 2 - - index: - _index: "tz_rollup" - _type: "_doc" - - timestamp.date_histogram.timestamp: "2017-01-01T00:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" - timestamp.date_histogram.time_zone: "UTC" - timestamp.date_histogram._count: 10 - partition.terms.value: "a" - partition.terms._count: 10 - price.max.value: 3 - "_rollup.id": "tz" - "_rollup.version": 2 - - index: - _index: "tz_rollup" - _type: "_doc" - - timestamp.date_histogram.timestamp: "2017-01-01T01:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" - timestamp.date_histogram.time_zone: "UTC" - timestamp.date_histogram._count: 10 - partition.terms.value: "b" - partition.terms._count: 10 - price.max.value: 4 - "_rollup.id": "tz" - "_rollup.version": 2 - - index: - _index: "tz_rollup" - _type: "_doc" - - timestamp.date_histogram.timestamp: "2017-01-01T01:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" - timestamp.date_histogram.time_zone: "UTC" - timestamp.date_histogram._count: 10 - partition.terms.value: "a" - partition.terms._count: 10 - price.max.value: 3 - "_rollup.id": "tz" - "_rollup.version": 2 - - do: - xpack.rollup.rollup_search: - index: "tz_rollup" - body: - size: 0 - aggs: - histo: - date_histogram: - field: "timestamp" - interval: "1h" - time_zone: "UTC" - - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } - - match: { aggregations.histo.buckets.0.doc_count: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } - - match: { aggregations.histo.buckets.1.doc_count: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } - - match: { aggregations.histo.buckets.2.doc_count: 10 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } - - match: { aggregations.histo.buckets.3.doc_count: 20 } - ---- -"New style UTC TZ": - - - do: - indices.create: - index: tz_rollup - body: - settings: - number_of_shards: 1 - number_of_replicas: 0 - mappings: - _doc: - properties: - partition.terms.value: - type: keyword - partition.terms._count: - type: long - timestamp.date_histogram.time_zone: - type: keyword - timestamp.date_histogram.interval: - type: keyword - timestamp.date_histogram.timestamp: - type: date - timestamp.date_histogram._count: - type: long - price.max.value: - type: double - _rollup.id: - type: keyword - _rollup.version: - type: long - _meta: - _rollup: - sensor: - cron: "* * * * * ?" - rollup_index: "tz_rollup" - index_pattern: "tz" - timeout: "20s" - page_size: 1000 - groups: - date_histogram: - field: "timestamp" - interval: "1h" - time_zone: "Z" - terms: - fields: - - "partition" - id: tz - metrics: - - field: "price" - metrics: - - max - - - do: - headers: - Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser - bulk: - refresh: true - body: - - index: - _index: "tz_rollup" - _type: "_doc" - - timestamp.date_histogram.timestamp: "2016-12-31T22:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" - timestamp.date_histogram.time_zone: "Z" - timestamp.date_histogram._count: 1 - partition.terms.value: "a" - partition.terms._count: 1 - price.max.value: 1 - "_rollup.id": "tz" - "_rollup.version": 2 - - - index: - _index: "tz_rollup" - _type: "_doc" - - timestamp.date_histogram.timestamp: "2016-12-31T23:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" - timestamp.date_histogram.time_zone: "Z" - timestamp.date_histogram._count: 2 - partition.terms.value: "b" - partition.terms._count: 2 - price.max.value: 2 - "_rollup.id": "tz" - "_rollup.version": 2 - - - index: - _index: "tz_rollup" - _type: "_doc" - - timestamp.date_histogram.timestamp: "2017-01-01T00:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" - timestamp.date_histogram.time_zone: "Z" - timestamp.date_histogram._count: 10 - partition.terms.value: "a" - partition.terms._count: 10 - price.max.value: 3 - "_rollup.id": "tz" - "_rollup.version": 2 - - index: - _index: "tz_rollup" - _type: "_doc" - - timestamp.date_histogram.timestamp: "2017-01-01T01:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" - timestamp.date_histogram.time_zone: "Z" - timestamp.date_histogram._count: 10 - partition.terms.value: "b" - partition.terms._count: 10 - price.max.value: 4 - "_rollup.id": "tz" - "_rollup.version": 2 - - - index: - _index: "tz_rollup" - _type: "_doc" - - timestamp.date_histogram.timestamp: "2017-01-01T01:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" - timestamp.date_histogram.time_zone: "Z" - timestamp.date_histogram._count: 10 - partition.terms.value: "a" - partition.terms._count: 10 - price.max.value: 3 - "_rollup.id": "tz" - "_rollup.version": 2 - - - do: - xpack.rollup.rollup_search: - index: "tz_rollup" - body: - size: 0 - aggs: - histo: - date_histogram: - field: "timestamp" - interval: "1h" - time_zone: "UTC" - - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } - - match: { aggregations.histo.buckets.0.doc_count: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } - - match: { aggregations.histo.buckets.1.doc_count: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } - - match: { aggregations.histo.buckets.2.doc_count: 10 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } - - match: { aggregations.histo.buckets.3.doc_count: 20 } - - diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/security_tests.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/security_tests.yml index 21131f3ec049f..c42ab9d06f6ce 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/security_tests.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/security_tests.yml @@ -171,7 +171,7 @@ teardown: hits.hits.0._id: "foo$VxMkzTqILshClbtbFi4-rQ" - match: hits.hits.0._source: - timestamp.date_histogram.time_zone: "Z" + timestamp.date_histogram.time_zone: "UTC" timestamp.date_histogram.timestamp: 0 value_field.max.value: 1232.0 _rollup.version: 2 @@ -334,7 +334,7 @@ teardown: hits.hits.0._id: "foo$VxMkzTqILshClbtbFi4-rQ" - match: hits.hits.0._source: - timestamp.date_histogram.time_zone: "Z" + timestamp.date_histogram.time_zone: "UTC" timestamp.date_histogram.timestamp: 0 value_field.max.value: 1232.0 _rollup.version: 2 From 72a1bed199d5e1bc128aa5f804dd277bcf6b4890 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Thu, 10 Jan 2019 14:03:19 -0500 Subject: [PATCH 11/18] Fix rule matching --- .../xpack/core/rollup/job/DateHistogramGroupConfig.java | 9 +++++---- .../xpack/rollup/RollupJobIdentifierUtils.java | 4 ++-- .../xpack/rollup/action/TransportRollupSearchAction.java | 5 +++-- .../elasticsearch/xpack/rollup/config/ConfigTests.java | 6 +++--- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java index 73297cf8f32a8..a4b2b9e1223d6 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java @@ -26,6 +26,7 @@ import java.io.IOException; import java.time.ZoneId; +import java.time.ZoneOffset; import java.util.Map; import java.util.Objects; @@ -55,6 +56,7 @@ public class DateHistogramGroupConfig implements Writeable, ToXContentObject { public static final String TIME_ZONE = "time_zone"; public static final String DELAY = "delay"; public static final String DEFAULT_TIMEZONE = "UTC"; + public static final ZoneId DEFAULT_ZONEID_TIMEZONE = ZoneOffset.UTC; private static final ConstructingObjectParser PARSER; static { PARSER = new ConstructingObjectParser<>(NAME, a -> @@ -104,8 +106,7 @@ public DateHistogramGroupConfig(final String field, this.interval = interval; this.field = field; this.delay = delay; - this.timeZone = ZoneId.of((timeZone != null && timeZone.isEmpty() == false) - ? timeZone : DEFAULT_TIMEZONE, ZoneId.SHORT_IDS).toString(); + this.timeZone = (timeZone != null && timeZone.isEmpty() == false) ? timeZone : DEFAULT_TIMEZONE; // validate interval createRounding(this.interval.toString(), this.timeZone); @@ -213,12 +214,12 @@ public boolean equals(final Object other) { return Objects.equals(interval, that.interval) && Objects.equals(field, that.field) && Objects.equals(delay, that.delay) - && Objects.equals(timeZone, that.timeZone); + && ZoneId.of(timeZone, ZoneId.SHORT_IDS).getRules().equals(ZoneId.of(that.timeZone, ZoneId.SHORT_IDS).getRules()); } @Override public int hashCode() { - return Objects.hash(interval, field, delay, timeZone); + return Objects.hash(interval, field, delay, ZoneId.of(timeZone)); } @Override diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java index 17f6c64339756..f850397989253 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java @@ -98,9 +98,9 @@ private static void checkDateHisto(DateHistogramAggregationBuilder source, List< if (agg.get(RollupField.AGG).equals(DateHistogramAggregationBuilder.NAME)) { DateHistogramInterval interval = new DateHistogramInterval((String)agg.get(RollupField.INTERVAL)); - ZoneId thisTimezone = ZoneId.of(((String)agg.get(DateHistogramGroupConfig.TIME_ZONE)), ZoneId.SHORT_IDS); + ZoneId thisTimezone = ZoneId.of(((String) agg.get(DateHistogramGroupConfig.TIME_ZONE)), ZoneId.SHORT_IDS); ZoneId sourceTimeZone = source.timeZone() == null - ? ZoneId.of(DateHistogramGroupConfig.DEFAULT_TIMEZONE) + ? DateHistogramGroupConfig.DEFAULT_ZONEID_TIMEZONE : ZoneId.of(source.timeZone().toString(), ZoneId.SHORT_IDS); // Ensure we are working on the same timezone diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java index 76c6beef33704..9e8dff56df8c9 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java @@ -64,6 +64,7 @@ import org.joda.time.DateTimeZone; import java.io.IOException; +import java.time.ZoneId; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -340,8 +341,8 @@ private static String rewriteFieldName(Set jobCaps, // If the cap is for a date_histo, and the query is a range, the timezones need to match if (type.equals(DateHistogramAggregationBuilder.NAME) && timeZone != null) { - boolean matchingTZ = ((String)agg.get(DateHistogramGroupConfig.TIME_ZONE)) - .equalsIgnoreCase(timeZone); + boolean matchingTZ = ZoneId.of((String)agg.get(DateHistogramGroupConfig.TIME_ZONE), ZoneId.SHORT_IDS) + .getRules().equals(ZoneId.of(timeZone, ZoneId.SHORT_IDS).getRules()); if (matchingTZ == false) { incompatibleTimeZones.add((String)agg.get(DateHistogramGroupConfig.TIME_ZONE)); } diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java index 8a71604a11e0f..3f9b6fc042d57 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/config/ConfigTests.java @@ -13,8 +13,8 @@ import org.elasticsearch.xpack.core.rollup.job.MetricConfig; import org.elasticsearch.xpack.core.rollup.job.RollupJob; import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; +import org.joda.time.DateTimeZone; -import java.time.zone.ZoneRulesException; import java.util.HashMap; import java.util.Map; @@ -84,9 +84,9 @@ public void testDefaultTimeZone() { } public void testUnkownTimeZone() { - ZoneRulesException e = expectThrows(ZoneRulesException.class, + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new DateHistogramGroupConfig("foo", DateHistogramInterval.HOUR, null, "FOO")); - assertThat(e.getMessage(), equalTo("Unknown time-zone ID: FOO")); + assertThat(e.getMessage(), equalTo("The datetime zone id 'FOO' is not recognised")); } public void testObsoleteTimeZone() { From 94abbebffda14286034ca1a96d73e89cdcaa3efa Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Thu, 10 Jan 2019 14:30:38 -0500 Subject: [PATCH 12/18] Add test for TZ validation in action --- .../action/TransportPutRollupJobAction.java | 17 ++++++++------ .../action/PutJobStateMachineTests.java | 22 +++++++++++++++++++ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java index 6ec8f65ae411d..db6c1c5ddeaa7 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java @@ -95,13 +95,7 @@ protected void masterOperation(PutRollupJobAction.Request request, ClusterState } XPackPlugin.checkReadyForXPackCustomMetadata(clusterState); - - String timeZone = request.getConfig().getGroupConfig().getDateHistogram().getTimeZone(); - String modernTZ = DateUtils.DEPRECATED_LONG_TIMEZONES.get(timeZone); - if (modernTZ != null) { - deprecationLogger.deprecated("Creating Rollup job [" + request.getConfig().getId() + "] with timezone [" - + timeZone + "], but [" + timeZone + "] has been deprecated by the IANA. Use [" + modernTZ +"] instead."); - } + checkForDeprecatedTZ(request); FieldCapabilitiesRequest fieldCapsRequest = new FieldCapabilitiesRequest() .indices(request.getConfig().getIndexPattern()) @@ -127,6 +121,15 @@ public void onFailure(Exception e) { }); } + static void checkForDeprecatedTZ(PutRollupJobAction.Request request) { + String timeZone = request.getConfig().getGroupConfig().getDateHistogram().getTimeZone(); + String modernTZ = DateUtils.DEPRECATED_LONG_TIMEZONES.get(timeZone); + if (modernTZ != null) { + deprecationLogger.deprecated("Creating Rollup job [" + request.getConfig().getId() + "] with timezone [" + + timeZone + "], but [" + timeZone + "] has been deprecated by the IANA. Use [" + modernTZ +"] instead."); + } + } + private static RollupJob createRollupJob(RollupJobConfig config, ThreadPool threadPool) { // ensure we only filter for the allowed headers Map filteredHeaders = threadPool.getThreadContext().getHeaders().entrySet().stream() diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java index 3d346456ea98d..3f49609953ea9 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java @@ -23,9 +23,13 @@ import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.persistent.PersistentTasksCustomMetaData; import org.elasticsearch.persistent.PersistentTasksService; +import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers; import org.elasticsearch.xpack.core.rollup.RollupField; +import org.elasticsearch.xpack.core.rollup.action.PutRollupJobAction; +import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig; +import org.elasticsearch.xpack.core.rollup.job.GroupConfig; import org.elasticsearch.xpack.core.rollup.job.RollupJob; import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig; import org.elasticsearch.xpack.rollup.Rollup; @@ -424,4 +428,22 @@ public void testStartTask() { verify(tasksService).sendStartRequest(eq(job.getConfig().getId()), eq(RollupField.TASK_NAME), eq(job), any()); verify(tasksService).waitForPersistentTaskCondition(eq(job.getConfig().getId()), any(), any(), any()); } + + public void testDeprecatedTimeZone() { + GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h"), null, "Japan")); + RollupJobConfig config = new RollupJobConfig("foo", randomAlphaOfLength(5), "rollup", ConfigTestHelpers.randomCron(), + 100, groupConfig, Collections.emptyList(), null); + PutRollupJobAction.Request request = new PutRollupJobAction.Request(config); + TransportPutRollupJobAction.checkForDeprecatedTZ(request); + assertWarnings("Creating Rollup job [foo] with timezone [Japan], but [Japan] has been deprecated by the IANA. " + + "Use [Asia/Tokyo] instead."); + } + + public void testTimeZone() { + GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h"), null, "EST")); + RollupJobConfig config = new RollupJobConfig("foo", randomAlphaOfLength(5), "rollup", ConfigTestHelpers.randomCron(), + 100, groupConfig, Collections.emptyList(), null); + PutRollupJobAction.Request request = new PutRollupJobAction.Request(config); + TransportPutRollupJobAction.checkForDeprecatedTZ(request); + } } From feba7e5c721ca7f07cd21326a710ba296205b796 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Fri, 11 Jan 2019 11:02:37 -0500 Subject: [PATCH 13/18] Yml test version updates --- .../resources/rest-api-spec/test/rollup/rollup_search.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml index 5e867e8c5c3e3..29e0261eb05aa 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml @@ -907,8 +907,8 @@ setup: --- "Obsolete Timezone": - skip: - version: " - 6.5.99" - reason: "IANA TZ deprecations in 6.6" + version: " - 6.6.99" + reason: "IANA TZ deprecations in 6.7" features: "warnings" - do: indices.create: @@ -1088,7 +1088,9 @@ setup: --- "Obsolete BWC Timezone": - + - skip: + version: " - 6.6.99" + reason: "IANA TZ deprecations in 6.7" - do: indices.create: index: tz_rollup From de22b2f3fa31218f7e2f9153db2bc74977d8bd95 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Tue, 5 Feb 2019 16:00:12 -0500 Subject: [PATCH 14/18] Remove last traces of Joda --- .../rollup/job/DateHistogramGroupConfig.java | 12 +-- .../xpack/rollup/RollupRequestTranslator.java | 5 +- .../action/TransportRollupSearchAction.java | 10 +-- .../xpack/rollup/job/RollupIndexer.java | 9 -- .../rollup/RollupJobIdentifierUtilTests.java | 9 +- .../rollup/action/SearchActionTests.java | 16 +++- .../xpack/rollup/job/IndexerUtilsTests.java | 84 +++++++++++++++++++ .../test/rollup/rollup_search.yml | 58 ++++++------- 8 files changed, 141 insertions(+), 62 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java index e4a83a8fcbd9b..c9fe0c644a86b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java @@ -21,7 +21,6 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; -import org.joda.time.DateTimeZone; import java.io.IOException; import java.time.ZoneId; @@ -238,16 +237,7 @@ private static Rounding createRounding(final String expr, final String timeZone) } else { rounding = new Rounding.Builder(TimeValue.parseTimeValue(expr, "createRounding")); } - rounding.timeZone(ZoneId.of(timeZone)); + rounding.timeZone(ZoneId.of(timeZone, ZoneId.SHORT_IDS)); return rounding.build(); } - - private static DateTimeZone toDateTimeZone(final String timezone) { - try { - return DateTimeZone.forOffsetHours(Integer.parseInt(timezone)); - } catch (NumberFormatException e) { - return DateTimeZone.forID(timezone); - } - } - } diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupRequestTranslator.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupRequestTranslator.java index 41f679f49086e..5982aaab1190a 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupRequestTranslator.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupRequestTranslator.java @@ -19,8 +19,9 @@ import org.elasticsearch.search.aggregations.metrics.SumAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder; import org.elasticsearch.xpack.core.rollup.RollupField; -import org.joda.time.DateTimeZone; +import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig; +import java.time.ZoneId; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -204,7 +205,7 @@ private static List translateDateHistogram(DateHistogramAggr rolledDateHisto.interval(source.interval()); } - DateTimeZone timezone = source.timeZone() == null ? DateTimeZone.UTC : source.timeZone(); + ZoneId timezone = source.timeZone() == null ? DateHistogramGroupConfig.DEFAULT_ZONEID_TIMEZONE : source.timeZone(); rolledDateHisto.timeZone(timezone); rolledDateHisto.offset(source.offset()); if (source.extendedBounds() != null) { diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java index 9e8dff56df8c9..dbdf36e586a6e 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java @@ -61,7 +61,6 @@ import org.elasticsearch.xpack.rollup.RollupJobIdentifierUtils; import org.elasticsearch.xpack.rollup.RollupRequestTranslator; import org.elasticsearch.xpack.rollup.RollupResponseTranslator; -import org.joda.time.DateTimeZone; import java.io.IOException; import java.time.ZoneId; @@ -291,17 +290,16 @@ static QueryBuilder rewriteQuery(QueryBuilder builder, Set jobCap String fieldName = range.fieldName(); // Many range queries don't include the timezone because the default is UTC, but the query // builder will return null so we need to set it here - String timeZone = range.timeZone() == null ? DateTimeZone.UTC.toString() : range.timeZone(); + String timeZone = range.timeZone() == null ? DateHistogramGroupConfig.DEFAULT_ZONEID_TIMEZONE.toString() : range.timeZone(); String rewrittenFieldName = rewriteFieldName(jobCaps, RangeQueryBuilder.NAME, fieldName, timeZone); RangeQueryBuilder rewritten = new RangeQueryBuilder(rewrittenFieldName) .from(range.from()) .to(range.to()) .includeLower(range.includeLower()) - .includeUpper(range.includeUpper()); - if (range.timeZone() != null) { - rewritten.timeZone(range.timeZone()); - } + .includeUpper(range.includeUpper()) + .timeZone(timeZone); + if (range.format() != null) { rewritten.format(range.format()); } diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java index 1d5f9093a29df..d0e4c471218a8 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java @@ -40,7 +40,6 @@ import org.elasticsearch.xpack.core.rollup.job.RollupJob; import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig; import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; -import org.joda.time.DateTimeZone; import java.time.ZoneId; import java.util.ArrayList; @@ -291,13 +290,5 @@ static List createAggregationBuilders(final List bestCaps = RollupJobIdentifierUtils.findBestJobs(builder, caps); assertThat(bestCaps.size(), equalTo(1)); builder = new DateHistogramAggregationBuilder("foo").field("foo") .dateHistogramInterval(job.getGroupConfig().getDateHistogram().getInterval()) - .timeZone(DateTimeZone.forID("America/Edmonton")); + .timeZone(ZoneId.of("America/Edmonton")); bestCaps = RollupJobIdentifierUtils.findBestJobs(builder, caps); assertThat(bestCaps.size(), equalTo(1)); @@ -694,14 +695,14 @@ public void testObsoleteTimezone() { builder = new DateHistogramAggregationBuilder("foo").field("foo") .dateHistogramInterval(job.getGroupConfig().getDateHistogram().getInterval()) - .timeZone(DateTimeZone.forID("Canada/Mountain")); + .timeZone(ZoneId.of("Canada/Mountain")); bestCaps = RollupJobIdentifierUtils.findBestJobs(builder, caps); assertThat(bestCaps.size(), equalTo(1)); builder = new DateHistogramAggregationBuilder("foo").field("foo") .dateHistogramInterval(job.getGroupConfig().getDateHistogram().getInterval()) - .timeZone(DateTimeZone.forID("America/Edmonton")); + .timeZone(ZoneId.of("America/Edmonton")); bestCaps = RollupJobIdentifierUtils.findBestJobs(builder, caps); assertThat(bestCaps.size(), equalTo(1)); diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java index 0032b5a88a563..c9e25f8eb056e 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java @@ -118,7 +118,7 @@ public void testBadQuery() { assertThat(e.getMessage(), equalTo("Unsupported Query in search request: [match_phrase]")); } - public void testRange() { + public void testRangeTimezoneUTC() { final GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h"))); final RollupJobConfig config = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10, groupConfig, emptyList(), null); RollupJobCaps cap = new RollupJobCaps(config); @@ -127,6 +127,19 @@ public void testRange() { QueryBuilder rewritten = TransportRollupSearchAction.rewriteQuery(new RangeQueryBuilder("foo").gt(1).timeZone("UTC"), caps); assertThat(rewritten, instanceOf(RangeQueryBuilder.class)); assertThat(((RangeQueryBuilder)rewritten).fieldName(), equalTo("foo.date_histogram.timestamp")); + assertThat(((RangeQueryBuilder)rewritten).timeZone(), equalTo("UTC")); + } + + public void testRangeTimezoneZ() { + final GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h"))); + final RollupJobConfig config = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10, groupConfig, emptyList(), null); + RollupJobCaps cap = new RollupJobCaps(config); + Set caps = new HashSet<>(); + caps.add(cap); + QueryBuilder rewritten = TransportRollupSearchAction.rewriteQuery(new RangeQueryBuilder("foo").gt(1).timeZone("Z"), caps); + assertThat(rewritten, instanceOf(RangeQueryBuilder.class)); + assertThat(((RangeQueryBuilder)rewritten).fieldName(), equalTo("foo.date_histogram.timestamp")); + assertThat(((RangeQueryBuilder)rewritten).timeZone(), equalTo("Z")); } public void testRangeNullTimeZone() { @@ -138,6 +151,7 @@ public void testRangeNullTimeZone() { QueryBuilder rewritten = TransportRollupSearchAction.rewriteQuery(new RangeQueryBuilder("foo").gt(1), caps); assertThat(rewritten, instanceOf(RangeQueryBuilder.class)); assertThat(((RangeQueryBuilder)rewritten).fieldName(), equalTo("foo.date_histogram.timestamp")); + assertThat(((RangeQueryBuilder)rewritten).timeZone(), equalTo("Z")); } public void testRangeWrongTZ() { diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/IndexerUtilsTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/IndexerUtilsTests.java index cbf85e84b16c3..38b90328a8743 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/IndexerUtilsTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/IndexerUtilsTests.java @@ -47,6 +47,7 @@ import org.mockito.stubbing.Answer; import java.io.IOException; +import java.time.ZoneId; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -561,6 +562,89 @@ public void testMissingBuckets() throws IOException { } } + public void testTimezone() throws IOException { + String indexName = randomAlphaOfLengthBetween(1, 10); + RollupIndexerJobStats stats = new RollupIndexerJobStats(0, 0, 0, 0, 0, 0, 0, 0, 0, 0); + + String timestampField = "the_histo"; + String valueField = "the_avg"; + + Directory directory = newDirectory(); + RandomIndexWriter indexWriter = new RandomIndexWriter(random(), directory); + + { + Document document = new Document(); + long timestamp = 1443659400000L; // 2015-10-01T00:30:00Z + document.add(new SortedNumericDocValuesField(timestampField, timestamp)); + document.add(new LongPoint(timestampField, timestamp)); + document.add(new SortedNumericDocValuesField(valueField, randomIntBetween(1, 100))); + indexWriter.addDocument(document); + } + { + Document document = new Document(); + long timestamp = 1443663000000L; // 2015-10-01T01:30:00Z + document.add(new SortedNumericDocValuesField(timestampField, timestamp)); + document.add(new LongPoint(timestampField, timestamp)); + document.add(new SortedNumericDocValuesField(valueField, randomIntBetween(1, 100))); + indexWriter.addDocument(document); + } + indexWriter.close(); + + IndexReader indexReader = DirectoryReader.open(directory); + IndexSearcher indexSearcher = newIndexSearcher(indexReader); + + DateFieldMapper.Builder builder = new DateFieldMapper.Builder(timestampField); + DateFieldMapper.DateFieldType timestampFieldType = builder.fieldType(); + timestampFieldType.setHasDocValues(true); + timestampFieldType.setName(timestampField); + + MappedFieldType valueFieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.LONG); + valueFieldType.setName(valueField); + valueFieldType.setHasDocValues(true); + valueFieldType.setName(valueField); + + // Setup the composite agg + DateHistogramValuesSourceBuilder dateHisto + = new DateHistogramValuesSourceBuilder("the_histo." + DateHistogramAggregationBuilder.NAME) + .field(timestampField) + .dateHistogramInterval(new DateHistogramInterval("1d")) + .timeZone(ZoneId.of("-01:00", ZoneId.SHORT_IDS)); // adds a timezone so that we aren't on default UTC + + CompositeAggregationBuilder compositeBuilder = new CompositeAggregationBuilder(RollupIndexer.AGGREGATION_NAME, + singletonList(dateHisto)); + + MetricConfig metricConfig = new MetricConfig(valueField, singletonList("max")); + List metricAgg = createAggregationBuilders(singletonList(metricConfig)); + metricAgg.forEach(compositeBuilder::subAggregation); + + Aggregator aggregator = createAggregator(compositeBuilder, indexSearcher, timestampFieldType, valueFieldType); + aggregator.preCollection(); + indexSearcher.search(new MatchAllDocsQuery(), aggregator); + aggregator.postCollection(); + CompositeAggregation composite = (CompositeAggregation) aggregator.buildAggregation(0L); + indexReader.close(); + directory.close(); + + final GroupConfig groupConfig = randomGroupConfig(random()); + List docs = IndexerUtils.processBuckets(composite, indexName, stats, groupConfig, "foo", randomBoolean()); + + assertThat(docs.size(), equalTo(2)); + + Map map = docs.get(0).sourceAsMap(); + assertNotNull(map.get(valueField + "." + MaxAggregationBuilder.NAME + "." + RollupField.VALUE)); + assertThat(map.get("the_histo." + DateHistogramAggregationBuilder.NAME + "." + RollupField.COUNT_FIELD), equalTo(1)); + assertThat(map.get("the_histo." + DateHistogramAggregationBuilder.NAME + "." + RollupField.TIMESTAMP), + equalTo(1443574800000L)); // 2015-09-30T00:00:00.000-01:00 + + map = docs.get(1).sourceAsMap(); + assertNotNull(map.get(valueField + "." + MaxAggregationBuilder.NAME + "." + RollupField.VALUE)); + assertThat(map.get("the_histo." + DateHistogramAggregationBuilder.NAME + "." + RollupField.COUNT_FIELD), equalTo(1)); + assertThat(map.get("the_histo." + DateHistogramAggregationBuilder.NAME + "." + RollupField.TIMESTAMP), + equalTo(1443661200000L)); // 2015-10-01T00:00:00.000-01:00 + + + } + interface Mock { List getBuckets(); } diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml index 29e0261eb05aa..132df9aec180b 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml @@ -142,7 +142,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "UTC" + time_zone: "Z" - length: { aggregations.histo.buckets: 4 } - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } @@ -167,7 +167,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "UTC" + time_zone: "Z" format: "yyyy-MM-dd" - length: { aggregations.histo.buckets: 4 } @@ -225,7 +225,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "UTC" + time_zone: "Z" aggs: the_max: max: @@ -261,7 +261,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "UTC" + time_zone: "Z" aggs: the_max: max: @@ -408,7 +408,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "UTC" + time_zone: "Z" aggs: the_max: max: @@ -559,7 +559,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "UTC" + time_zone: "Z" aggs: the_max: max: @@ -708,7 +708,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "UTC" + time_zone: "Z" aggs: the_max: max: @@ -740,7 +740,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "UTC" + time_zone: "Z" - length: { aggregations.histo.buckets: 4 } - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } @@ -829,7 +829,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "UTC" + time_zone: "Z" - length: { aggregations.histo.buckets: 4 } - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } @@ -1043,16 +1043,16 @@ setup: field: "price" - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000-07:00" } + - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000America/Edmonton" } - match: { aggregations.histo.buckets.0.doc_count: 1 } - match: { aggregations.histo.buckets.0.the_max.value: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000-07:00" } + - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000America/Edmonton" } - match: { aggregations.histo.buckets.1.doc_count: 2 } - match: { aggregations.histo.buckets.1.the_max.value: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000-07:00" } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000America/Edmonton" } - match: { aggregations.histo.buckets.2.doc_count: 10 } - match: { aggregations.histo.buckets.2.the_max.value: 3 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000-07:00" } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000America/Edmonton" } - match: { aggregations.histo.buckets.3.doc_count: 20 } - match: { aggregations.histo.buckets.3.the_max.value: 4 } @@ -1073,16 +1073,16 @@ setup: field: "price" - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000-07:00" } + - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000Canada/Mountain" } - match: { aggregations.histo.buckets.0.doc_count: 1 } - match: { aggregations.histo.buckets.0.the_max.value: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000-07:00" } + - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000Canada/Mountain" } - match: { aggregations.histo.buckets.1.doc_count: 2 } - match: { aggregations.histo.buckets.1.the_max.value: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000-07:00" } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000Canada/Mountain" } - match: { aggregations.histo.buckets.2.doc_count: 10 } - match: { aggregations.histo.buckets.2.the_max.value: 3 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000-07:00" } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000Canada/Mountain" } - match: { aggregations.histo.buckets.3.doc_count: 20 } - match: { aggregations.histo.buckets.3.the_max.value: 4 } @@ -1230,16 +1230,16 @@ setup: field: "price" - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000-07:00" } + - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000America/Edmonton" } - match: { aggregations.histo.buckets.0.doc_count: 1 } - match: { aggregations.histo.buckets.0.the_max.value: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000-07:00" } + - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000America/Edmonton" } - match: { aggregations.histo.buckets.1.doc_count: 2 } - match: { aggregations.histo.buckets.1.the_max.value: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000-07:00" } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000America/Edmonton" } - match: { aggregations.histo.buckets.2.doc_count: 10 } - match: { aggregations.histo.buckets.2.the_max.value: 3 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000-07:00" } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000America/Edmonton" } - match: { aggregations.histo.buckets.3.doc_count: 20 } - match: { aggregations.histo.buckets.3.the_max.value: 4 } @@ -1260,16 +1260,16 @@ setup: field: "price" - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000-07:00" } + - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000Canada/Mountain" } - match: { aggregations.histo.buckets.0.doc_count: 1 } - match: { aggregations.histo.buckets.0.the_max.value: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000-07:00" } + - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000Canada/Mountain" } - match: { aggregations.histo.buckets.1.doc_count: 2 } - match: { aggregations.histo.buckets.1.the_max.value: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000-07:00" } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000Canada/Mountain" } - match: { aggregations.histo.buckets.2.doc_count: 10 } - match: { aggregations.histo.buckets.2.the_max.value: 3 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000-07:00" } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000Canada/Mountain" } - match: { aggregations.histo.buckets.3.doc_count: 20 } - match: { aggregations.histo.buckets.3.the_max.value: 4 } @@ -1294,16 +1294,16 @@ setup: max: field: "price" - - match: { aggregations.date_histogram#histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } + - match: { aggregations.date_histogram#histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000UTC" } - match: { aggregations.date_histogram#histo.buckets.0.doc_count: 1 } - match: { aggregations.date_histogram#histo.buckets.0.max#the_max.value: 1 } - - match: { aggregations.date_histogram#histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } + - match: { aggregations.date_histogram#histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000UTC" } - match: { aggregations.date_histogram#histo.buckets.1.doc_count: 2 } - match: { aggregations.date_histogram#histo.buckets.1.max#the_max.value: 2 } - - match: { aggregations.date_histogram#histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } + - match: { aggregations.date_histogram#histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000UTC" } - match: { aggregations.date_histogram#histo.buckets.2.doc_count: 10 } - match: { aggregations.date_histogram#histo.buckets.2.max#the_max.value: 3 } - - match: { aggregations.date_histogram#histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } + - match: { aggregations.date_histogram#histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000UTC" } - match: { aggregations.date_histogram#histo.buckets.3.doc_count: 20 } - match: { aggregations.date_histogram#histo.buckets.3.max#the_max.value: 4 } From b88091bcac700756d2c1319510f71b44034753e6 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Wed, 6 Feb 2019 22:36:51 -0500 Subject: [PATCH 15/18] Do not set timezone on rollup request histo We will address this in a separate bugfix pr --- .../xpack/rollup/RollupRequestTranslator.java | 4 -- .../test/rollup/rollup_search.yml | 40 +++++++++---------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupRequestTranslator.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupRequestTranslator.java index 5982aaab1190a..84bc3b00c5365 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupRequestTranslator.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupRequestTranslator.java @@ -19,9 +19,7 @@ import org.elasticsearch.search.aggregations.metrics.SumAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder; import org.elasticsearch.xpack.core.rollup.RollupField; -import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig; -import java.time.ZoneId; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -205,8 +203,6 @@ private static List translateDateHistogram(DateHistogramAggr rolledDateHisto.interval(source.interval()); } - ZoneId timezone = source.timeZone() == null ? DateHistogramGroupConfig.DEFAULT_ZONEID_TIMEZONE : source.timeZone(); - rolledDateHisto.timeZone(timezone); rolledDateHisto.offset(source.offset()); if (source.extendedBounds() != null) { rolledDateHisto.extendedBounds(source.extendedBounds()); diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml index 132df9aec180b..cf136999306ed 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml @@ -1043,16 +1043,16 @@ setup: field: "price" - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000America/Edmonton" } + - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } - match: { aggregations.histo.buckets.0.doc_count: 1 } - match: { aggregations.histo.buckets.0.the_max.value: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000America/Edmonton" } + - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } - match: { aggregations.histo.buckets.1.doc_count: 2 } - match: { aggregations.histo.buckets.1.the_max.value: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000America/Edmonton" } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } - match: { aggregations.histo.buckets.2.doc_count: 10 } - match: { aggregations.histo.buckets.2.the_max.value: 3 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000America/Edmonton" } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } - match: { aggregations.histo.buckets.3.doc_count: 20 } - match: { aggregations.histo.buckets.3.the_max.value: 4 } @@ -1073,16 +1073,16 @@ setup: field: "price" - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000Canada/Mountain" } + - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } - match: { aggregations.histo.buckets.0.doc_count: 1 } - match: { aggregations.histo.buckets.0.the_max.value: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000Canada/Mountain" } + - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } - match: { aggregations.histo.buckets.1.doc_count: 2 } - match: { aggregations.histo.buckets.1.the_max.value: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000Canada/Mountain" } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } - match: { aggregations.histo.buckets.2.doc_count: 10 } - match: { aggregations.histo.buckets.2.the_max.value: 3 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000Canada/Mountain" } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } - match: { aggregations.histo.buckets.3.doc_count: 20 } - match: { aggregations.histo.buckets.3.the_max.value: 4 } @@ -1230,16 +1230,16 @@ setup: field: "price" - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000America/Edmonton" } + - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } - match: { aggregations.histo.buckets.0.doc_count: 1 } - match: { aggregations.histo.buckets.0.the_max.value: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000America/Edmonton" } + - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } - match: { aggregations.histo.buckets.1.doc_count: 2 } - match: { aggregations.histo.buckets.1.the_max.value: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000America/Edmonton" } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } - match: { aggregations.histo.buckets.2.doc_count: 10 } - match: { aggregations.histo.buckets.2.the_max.value: 3 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000America/Edmonton" } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } - match: { aggregations.histo.buckets.3.doc_count: 20 } - match: { aggregations.histo.buckets.3.the_max.value: 4 } @@ -1260,16 +1260,16 @@ setup: field: "price" - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2016-12-31T22:00:00.000Canada/Mountain" } + - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } - match: { aggregations.histo.buckets.0.doc_count: 1 } - match: { aggregations.histo.buckets.0.the_max.value: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2016-12-31T23:00:00.000Canada/Mountain" } + - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } - match: { aggregations.histo.buckets.1.doc_count: 2 } - match: { aggregations.histo.buckets.1.the_max.value: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T00:00:00.000Canada/Mountain" } + - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } - match: { aggregations.histo.buckets.2.doc_count: 10 } - match: { aggregations.histo.buckets.2.the_max.value: 3 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T01:00:00.000Canada/Mountain" } + - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } - match: { aggregations.histo.buckets.3.doc_count: 20 } - match: { aggregations.histo.buckets.3.the_max.value: 4 } @@ -1294,16 +1294,16 @@ setup: max: field: "price" - - match: { aggregations.date_histogram#histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000UTC" } + - match: { aggregations.date_histogram#histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } - match: { aggregations.date_histogram#histo.buckets.0.doc_count: 1 } - match: { aggregations.date_histogram#histo.buckets.0.max#the_max.value: 1 } - - match: { aggregations.date_histogram#histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000UTC" } + - match: { aggregations.date_histogram#histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } - match: { aggregations.date_histogram#histo.buckets.1.doc_count: 2 } - match: { aggregations.date_histogram#histo.buckets.1.max#the_max.value: 2 } - - match: { aggregations.date_histogram#histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000UTC" } + - match: { aggregations.date_histogram#histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } - match: { aggregations.date_histogram#histo.buckets.2.doc_count: 10 } - match: { aggregations.date_histogram#histo.buckets.2.max#the_max.value: 3 } - - match: { aggregations.date_histogram#histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000UTC" } + - match: { aggregations.date_histogram#histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } - match: { aggregations.date_histogram#histo.buckets.3.doc_count: 20 } - match: { aggregations.date_histogram#histo.buckets.3.max#the_max.value: 4 } From fbda1674be1e4bf81082873dcdeb501d310989da Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Mon, 15 Apr 2019 13:02:33 -0400 Subject: [PATCH 16/18] checkstyle --- .../xpack/rollup/action/TransportRollupSearchAction.java | 3 --- .../xpack/rollup/RollupRequestTranslationTests.java | 2 -- 2 files changed, 5 deletions(-) diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java index 827feb63272d4..414a0d08ef35a 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java @@ -56,17 +56,14 @@ import org.elasticsearch.xpack.core.rollup.RollupField; import org.elasticsearch.xpack.core.rollup.action.RollupJobCaps; import org.elasticsearch.xpack.core.rollup.action.RollupSearchAction; -import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig; import org.elasticsearch.xpack.rollup.Rollup; import org.elasticsearch.xpack.rollup.RollupJobIdentifierUtils; import org.elasticsearch.xpack.rollup.RollupRequestTranslator; import org.elasticsearch.xpack.rollup.RollupResponseTranslator; import java.io.IOException; -import java.time.ZoneId; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Objects; diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupRequestTranslationTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupRequestTranslationTests.java index 7b6dcd54edd88..db58115489d2a 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupRequestTranslationTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupRequestTranslationTests.java @@ -9,8 +9,6 @@ import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.index.query.TermQueryBuilder; import org.elasticsearch.search.SearchModule; import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; From 8822afdc2fd1b5ec75b1356a3ee8159a8bb46e23 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Mon, 15 Apr 2019 17:22:21 -0400 Subject: [PATCH 17/18] Fix tests, make more realistic --- .../test/rollup/rollup_search.yml | 247 +++++++----------- 1 file changed, 90 insertions(+), 157 deletions(-) diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml index b717f6d4fadd5..9397110220fe3 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml @@ -884,28 +884,27 @@ setup: --- "Obsolete Timezone": - skip: - version: " - 6.6.99" - reason: "IANA TZ deprecations in 6.7" + version: " - 7.0.99" + reason: "IANA TZ deprecations in 7.1" features: "warnings" - do: indices.create: index: tz body: mappings: - _doc: - properties: - timestamp: - type: date - partition: - type: keyword - price: - type: integer + properties: + timestamp: + type: date + partition: + type: keyword + price: + type: integer - do: headers: Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser warnings: - "Creating Rollup job [tz] with timezone [Canada/Mountain], but [Canada/Mountain] has been deprecated by the IANA. Use [America/Edmonton] instead." - xpack.rollup.put_job: + rollup.put_job: id: tz body: > { @@ -916,7 +915,7 @@ setup: "groups" : { "date_histogram": { "field": "timestamp", - "interval": "1h", + "interval": "5m", "time_zone": "Canada/Mountain" }, "terms": { @@ -940,8 +939,8 @@ setup: - index: _index: "tz_rollup" _type: "_doc" - - timestamp.date_histogram.timestamp: "2017-01-01T05:00:00Z" - timestamp.date_histogram.interval: "1h" + - timestamp.date_histogram.timestamp: 1531221000000 + timestamp.date_histogram.interval: "5m" timestamp.date_histogram.time_zone: "America/Edmonton" timestamp.date_histogram._count: 1 partition.terms.value: "a" @@ -953,8 +952,8 @@ setup: - index: _index: "tz_rollup" _type: "_doc" - - timestamp.date_histogram.timestamp: "2017-01-01T06:00:00Z" - timestamp.date_histogram.interval: "1h" + - timestamp.date_histogram.timestamp: 1531221300000 + timestamp.date_histogram.interval: "5m" timestamp.date_histogram.time_zone: "America/Edmonton" timestamp.date_histogram._count: 2 partition.terms.value: "b" @@ -966,34 +965,8 @@ setup: - index: _index: "tz_rollup" _type: "_doc" - - timestamp.date_histogram.timestamp: "2017-01-01T07:00:00Z" - timestamp.date_histogram.interval: "1h" - timestamp.date_histogram.time_zone: "America/Edmonton" - timestamp.date_histogram._count: 10 - partition.terms.value: "a" - partition.terms._count: 10 - price.max.value: 3 - "_rollup.id": "tz" - "_rollup.version": 2 - - - index: - _index: "tz_rollup" - _type: "_doc" - - timestamp.date_histogram.timestamp: "2017-01-01T08:00:00Z" - timestamp.date_histogram.interval: "1h" - timestamp.date_histogram.time_zone: "America/Edmonton" - timestamp.date_histogram._count: 10 - partition.terms.value: "b" - partition.terms._count: 10 - price.max.value: 4 - "_rollup.id": "tz" - "_rollup.version": 2 - - - index: - _index: "tz_rollup" - _type: "_doc" - - timestamp.date_histogram.timestamp: "2017-01-01T08:00:00Z" - timestamp.date_histogram.interval: "1h" + - timestamp.date_histogram.timestamp: 1531221600000 + timestamp.date_histogram.interval: "5m" timestamp.date_histogram.time_zone: "America/Edmonton" timestamp.date_histogram._count: 10 partition.terms.value: "a" @@ -1002,9 +975,8 @@ setup: "_rollup.id": "tz" "_rollup.version": 2 - - do: - xpack.rollup.rollup_search: + rollup.rollup_search: index: "tz_rollup" body: size: 0 @@ -1012,29 +984,26 @@ setup: histo: date_histogram: field: "timestamp" - interval: "1h" + interval: "5m" time_zone: "America/Edmonton" aggs: the_max: max: field: "price" - - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } + - length: { aggregations.histo.buckets: 3 } + - match: { aggregations.histo.buckets.0.key_as_string: "2018-07-10T05:10:00.000-06:00" } - match: { aggregations.histo.buckets.0.doc_count: 1 } - match: { aggregations.histo.buckets.0.the_max.value: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } + - match: { aggregations.histo.buckets.1.key_as_string: "2018-07-10T05:15:00.000-06:00" } - match: { aggregations.histo.buckets.1.doc_count: 2 } - match: { aggregations.histo.buckets.1.the_max.value: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } + - match: { aggregations.histo.buckets.2.key_as_string: "2018-07-10T05:20:00.000-06:00" } - match: { aggregations.histo.buckets.2.doc_count: 10 } - match: { aggregations.histo.buckets.2.the_max.value: 3 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } - - match: { aggregations.histo.buckets.3.doc_count: 20 } - - match: { aggregations.histo.buckets.3.the_max.value: 4 } - do: - xpack.rollup.rollup_search: + rollup.rollup_search: index: "tz_rollup" body: size: 0 @@ -1042,32 +1011,29 @@ setup: histo: date_histogram: field: "timestamp" - interval: "1h" + interval: "5m" time_zone: "Canada/Mountain" aggs: the_max: max: field: "price" - - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } + - length: { aggregations.histo.buckets: 3 } + - match: { aggregations.histo.buckets.0.key_as_string: "2018-07-10T05:10:00.000-06:00" } - match: { aggregations.histo.buckets.0.doc_count: 1 } - match: { aggregations.histo.buckets.0.the_max.value: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } + - match: { aggregations.histo.buckets.1.key_as_string: "2018-07-10T05:15:00.000-06:00" } - match: { aggregations.histo.buckets.1.doc_count: 2 } - match: { aggregations.histo.buckets.1.the_max.value: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } + - match: { aggregations.histo.buckets.2.key_as_string: "2018-07-10T05:20:00.000-06:00" } - match: { aggregations.histo.buckets.2.doc_count: 10 } - match: { aggregations.histo.buckets.2.the_max.value: 3 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } - - match: { aggregations.histo.buckets.3.doc_count: 20 } - - match: { aggregations.histo.buckets.3.the_max.value: 4 } --- "Obsolete BWC Timezone": - skip: - version: " - 6.6.99" - reason: "IANA TZ deprecations in 6.7" + version: " - 7.0.99" + reason: "IANA TZ deprecations in 7.1" - do: indices.create: index: tz_rollup @@ -1076,47 +1042,46 @@ setup: number_of_shards: 1 number_of_replicas: 0 mappings: - _doc: - properties: - partition.terms.value: - type: keyword - partition.terms._count: - type: long - timestamp.date_histogram.time_zone: - type: keyword - timestamp.date_histogram.interval: - type: keyword - timestamp.date_histogram.timestamp: - type: date - timestamp.date_histogram._count: - type: long - price.max.value: - type: double - _rollup.id: - type: keyword - _rollup.version: - type: long - _meta: - _rollup: - sensor: - cron: "* * * * * ?" - rollup_index: "tz_rollup" - index_pattern: "tz" - timeout: "20s" - page_size: 1000 - groups: - date_histogram: - field: "timestamp" - interval: "1h" - time_zone: "Canada/Mountain" - terms: - fields: - - "partition" - id: tz - metrics: - - field: "price" - metrics: - - max + properties: + partition.terms.value: + type: keyword + partition.terms._count: + type: long + timestamp.date_histogram.time_zone: + type: keyword + timestamp.date_histogram.interval: + type: keyword + timestamp.date_histogram.timestamp: + type: date + timestamp.date_histogram._count: + type: long + price.max.value: + type: double + _rollup.id: + type: keyword + _rollup.version: + type: long + _meta: + _rollup: + sensor: + cron: "* * * * * ?" + rollup_index: "tz_rollup" + index_pattern: "tz" + timeout: "20s" + page_size: 1000 + groups: + date_histogram: + field: "timestamp" + interval: "5m" + time_zone: "Canada/Mountain" + terms: + fields: + - "partition" + id: tz + metrics: + - field: "price" + metrics: + - max - do: headers: @@ -1127,8 +1092,8 @@ setup: - index: _index: "tz_rollup" _type: "_doc" - - timestamp.date_histogram.timestamp: "2016-12-31T22:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" + - timestamp.date_histogram.timestamp: 1531221000000 + timestamp.date_histogram.interval: "5m" timestamp.date_histogram.time_zone: "Canada/Mountain" timestamp.date_histogram._count: 1 partition.terms.value: "a" @@ -1140,8 +1105,8 @@ setup: - index: _index: "tz_rollup" _type: "_doc" - - timestamp.date_histogram.timestamp: "2016-12-31T23:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" + - timestamp.date_histogram.timestamp: 1531221300000 + timestamp.date_histogram.interval: "5m" timestamp.date_histogram.time_zone: "Canada/Mountain" timestamp.date_histogram._count: 2 partition.terms.value: "b" @@ -1153,34 +1118,8 @@ setup: - index: _index: "tz_rollup" _type: "_doc" - - timestamp.date_histogram.timestamp: "2017-01-01T00:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" - timestamp.date_histogram.time_zone: "Canada/Mountain" - timestamp.date_histogram._count: 10 - partition.terms.value: "a" - partition.terms._count: 10 - price.max.value: 3 - "_rollup.id": "tz" - "_rollup.version": 2 - - - index: - _index: "tz_rollup" - _type: "_doc" - - timestamp.date_histogram.timestamp: "2017-01-01T01:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" - timestamp.date_histogram.time_zone: "Canada/Mountain" - timestamp.date_histogram._count: 10 - partition.terms.value: "b" - partition.terms._count: 10 - price.max.value: 4 - "_rollup.id": "tz" - "_rollup.version": 2 - - - index: - _index: "tz_rollup" - _type: "_doc" - - timestamp.date_histogram.timestamp: "2017-01-01T01:00:00.000-07:00" - timestamp.date_histogram.interval: "1h" + - timestamp.date_histogram.timestamp: 1531221600000 + timestamp.date_histogram.interval: "5m" timestamp.date_histogram.time_zone: "Canada/Mountain" timestamp.date_histogram._count: 10 partition.terms.value: "a" @@ -1189,9 +1128,8 @@ setup: "_rollup.id": "tz" "_rollup.version": 2 - - do: - xpack.rollup.rollup_search: + rollup.rollup_search: index: "tz_rollup" body: size: 0 @@ -1199,29 +1137,27 @@ setup: histo: date_histogram: field: "timestamp" - interval: "1h" + interval: "5m" time_zone: "America/Edmonton" aggs: the_max: max: field: "price" - - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } + - length: { aggregations.histo.buckets: 3 } + - match: { aggregations.histo.buckets.0.key_as_string: "2018-07-10T05:10:00.000-06:00" } - match: { aggregations.histo.buckets.0.doc_count: 1 } - match: { aggregations.histo.buckets.0.the_max.value: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } + - match: { aggregations.histo.buckets.1.key_as_string: "2018-07-10T05:15:00.000-06:00" } - match: { aggregations.histo.buckets.1.doc_count: 2 } - match: { aggregations.histo.buckets.1.the_max.value: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } + - match: { aggregations.histo.buckets.2.key_as_string: "2018-07-10T05:20:00.000-06:00" } - match: { aggregations.histo.buckets.2.doc_count: 10 } - match: { aggregations.histo.buckets.2.the_max.value: 3 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } - - match: { aggregations.histo.buckets.3.doc_count: 20 } - - match: { aggregations.histo.buckets.3.the_max.value: 4 } + - do: - xpack.rollup.rollup_search: + rollup.rollup_search: index: "tz_rollup" body: size: 0 @@ -1229,26 +1165,23 @@ setup: histo: date_histogram: field: "timestamp" - interval: "1h" + interval: "5m" time_zone: "Canada/Mountain" aggs: the_max: max: field: "price" - - length: { aggregations.histo.buckets: 4 } - - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } + - length: { aggregations.histo.buckets: 3 } + - match: { aggregations.histo.buckets.0.key_as_string: "2018-07-10T05:10:00.000-06:00" } - match: { aggregations.histo.buckets.0.doc_count: 1 } - match: { aggregations.histo.buckets.0.the_max.value: 1 } - - match: { aggregations.histo.buckets.1.key_as_string: "2017-01-01T06:00:00.000Z" } + - match: { aggregations.histo.buckets.1.key_as_string: "2018-07-10T05:15:00.000-06:00" } - match: { aggregations.histo.buckets.1.doc_count: 2 } - match: { aggregations.histo.buckets.1.the_max.value: 2 } - - match: { aggregations.histo.buckets.2.key_as_string: "2017-01-01T07:00:00.000Z" } + - match: { aggregations.histo.buckets.2.key_as_string: "2018-07-10T05:20:00.000-06:00" } - match: { aggregations.histo.buckets.2.doc_count: 10 } - match: { aggregations.histo.buckets.2.the_max.value: 3 } - - match: { aggregations.histo.buckets.3.key_as_string: "2017-01-01T08:00:00.000Z" } - - match: { aggregations.histo.buckets.3.doc_count: 20 } - - match: { aggregations.histo.buckets.3.the_max.value: 4 } --- From a9f2911dc1e1884dc4addc64d7559827e5c61de4 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Mon, 15 Apr 2019 17:48:04 -0400 Subject: [PATCH 18/18] Remove unnecessary changes --- .../xpack/rollup/action/SearchActionTests.java | 12 ------------ .../test/rollup/rollup_search.yml | 18 +++++++++--------- 2 files changed, 9 insertions(+), 21 deletions(-) diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java index 8e408f0b66829..a795edca83ed3 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/SearchActionTests.java @@ -130,18 +130,6 @@ public void testRangeTimezoneUTC() { assertThat(((RangeQueryBuilder)rewritten).timeZone(), equalTo("UTC")); } - public void testRangeTimezoneZ() { - final GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h"))); - final RollupJobConfig config = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10, groupConfig, emptyList(), null); - RollupJobCaps cap = new RollupJobCaps(config); - Set caps = new HashSet<>(); - caps.add(cap); - QueryBuilder rewritten = TransportRollupSearchAction.rewriteQuery(new RangeQueryBuilder("foo").gt(1).timeZone("Z"), caps); - assertThat(rewritten, instanceOf(RangeQueryBuilder.class)); - assertThat(((RangeQueryBuilder)rewritten).fieldName(), equalTo("foo.date_histogram.timestamp")); - assertThat(((RangeQueryBuilder)rewritten).timeZone(), equalTo("Z")); - } - public void testRangeNullTimeZone() { final GroupConfig groupConfig = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h"), null, null)); final RollupJobConfig config = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10, groupConfig, emptyList(), null); diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml index 9397110220fe3..be9c9f4a41e1c 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml @@ -136,7 +136,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "Z" + time_zone: "UTC" - length: { aggregations.histo.buckets: 4 } - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } @@ -161,7 +161,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "Z" + time_zone: "UTC" format: "yyyy-MM-dd" - length: { aggregations.histo.buckets: 4 } @@ -219,7 +219,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "Z" + time_zone: "UTC" aggs: the_max: max: @@ -255,7 +255,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "Z" + time_zone: "UTC" aggs: the_max: max: @@ -397,7 +397,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "Z" + time_zone: "UTC" aggs: the_max: max: @@ -543,7 +543,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "Z" + time_zone: "UTC" aggs: the_max: max: @@ -687,7 +687,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "Z" + time_zone: "UTC" aggs: the_max: max: @@ -719,7 +719,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "Z" + time_zone: "UTC" - length: { aggregations.histo.buckets: 4 } - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" } @@ -807,7 +807,7 @@ setup: date_histogram: field: "timestamp" interval: "1h" - time_zone: "Z" + time_zone: "UTC" - length: { aggregations.histo.buckets: 4 } - match: { aggregations.histo.buckets.0.key_as_string: "2017-01-01T05:00:00.000Z" }