From 7e62ff28237f170a0476ee69478943c563b518c9 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Wed, 17 Apr 2019 13:33:51 -0400 Subject: [PATCH] [Rollup] Validate timezones based on rules not string comparision (#36237) 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. --- .../elasticsearch/common/time/DateUtils.java | 121 +++++++ .../rollup/job/DateHistogramGroupConfig.java | 10 +- .../RollupDataExtractorFactory.java | 4 +- .../rollup/RollupJobIdentifierUtils.java | 9 +- .../xpack/rollup/RollupRequestTranslator.java | 52 +-- .../action/TransportPutRollupJobAction.java | 15 + .../action/TransportRollupSearchAction.java | 4 +- .../xpack/rollup/job/RollupIndexer.java | 9 - .../rollup/RollupJobIdentifierUtilTests.java | 46 +++ .../rollup/RollupRequestTranslationTests.java | 114 +------ .../action/PutJobStateMachineTests.java | 22 ++ .../rollup/action/SearchActionTests.java | 4 +- .../xpack/rollup/config/ConfigTests.java | 5 + .../xpack/rollup/job/IndexerUtilsTests.java | 84 +++++ .../test/rollup/rollup_search.yml | 304 ++++++++++++++++++ 15 files changed, 645 insertions(+), 158 deletions(-) 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 910934a8f50..e6bf6a65105 100644 --- a/server/src/main/java/org/elasticsearch/common/time/DateUtils.java +++ b/server/src/main/java/org/elasticsearch/common/time/DateUtils.java @@ -66,6 +66,127 @@ public class DateUtils { 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/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 f4fee8acc3d..c9fe0c644a8 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 @@ -24,6 +24,7 @@ import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInter import java.io.IOException; import java.time.ZoneId; +import java.time.ZoneOffset; import java.util.Map; import java.util.Objects; @@ -52,7 +53,8 @@ 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"; + public static final ZoneId DEFAULT_ZONEID_TIMEZONE = ZoneOffset.UTC; private static final ConstructingObjectParser PARSER; static { PARSER = new ConstructingObjectParser<>(NAME, a -> @@ -210,12 +212,12 @@ public class DateHistogramGroupConfig implements Writeable, ToXContentObject { 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 @@ -235,7 +237,7 @@ public class DateHistogramGroupConfig implements Writeable, ToXContentObject { } 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(); } } 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 4971ad83879..8264d3e15fd 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 @@ -23,6 +23,8 @@ import org.elasticsearch.xpack.core.rollup.action.RollupJobCaps.RollupFieldCaps; 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; @@ -124,7 +126,7 @@ public class RollupDataExtractorFactory implements DataExtractorFactory { 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 { 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 59141d2a83a..95b5069edcf 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 @@ -17,6 +17,7 @@ import org.elasticsearch.xpack.core.rollup.RollupField; import org.elasticsearch.xpack.core.rollup.action.RollupJobCaps; import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig; +import java.time.ZoneId; import java.util.ArrayList; import java.util.Comparator; import java.util.HashSet; @@ -96,11 +97,13 @@ public class RollupJobIdentifierUtils { 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 ? "UTC" : source.timeZone().toString(); + ZoneId thisTimezone = ZoneId.of(((String) agg.get(DateHistogramGroupConfig.TIME_ZONE)), ZoneId.SHORT_IDS); + ZoneId sourceTimeZone = source.timeZone() == null + ? DateHistogramGroupConfig.DEFAULT_ZONEID_TIMEZONE + : ZoneId.of(source.timeZone().toString(), ZoneId.SHORT_IDS); // Ensure we are working on the same timezone - if (thisTimezone.equalsIgnoreCase(sourceTimeZone) == false) { + if (thisTimezone.getRules().equals(sourceTimeZone.getRules()) == 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 45462681198..7cf8f8d1293 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.BytesStreamOutput; 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; @@ -22,8 +20,8 @@ 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.time.ZoneId; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -47,7 +45,7 @@ import java.util.function.Supplier; * } * * - * 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 +114,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 +189,13 @@ public class RollupRequestTranslator { *
  • 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,13 +205,9 @@ public class RollupRequestTranslator { 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)); + ZoneId timeZone = source.timeZone() == null ? DateHistogramGroupConfig.DEFAULT_ZONEID_TIMEZONE : source.timeZone(); + rolledDateHisto.timeZone(timeZone); - if (source.timeZone() != null) { - rolledDateHisto.timeZone(source.timeZone()); - } rolledDateHisto.offset(source.offset()); if (source.extendedBounds() != null) { rolledDateHisto.extendedBounds(source.extendedBounds()); @@ -248,14 +229,13 @@ public class RollupRequestTranslator { * 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()); @@ -328,10 +308,9 @@ public class RollupRequestTranslator { * */ 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)); @@ -359,8 +338,6 @@ public class RollupRequestTranslator { * 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 @@ -371,15 +348,14 @@ public class RollupRequestTranslator { * @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/TransportPutRollupJobAction.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportPutRollupJobAction.java index cb04f5554b4..db6c1c5ddea 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.metadata.MappingMetaData; 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 filteredHeaders = threadPool.getThreadContext().getHeaders().entrySet().stream() 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 e85a92c0613..414a0d08ef3 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 @@ -173,10 +173,12 @@ public class TransportRollupSearchAction extends TransportAction 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/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 e051e912c48..daa888562e9 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 @@ -41,7 +41,6 @@ import org.elasticsearch.xpack.core.rollup.job.RollupIndexerJobStats; 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; @@ -311,13 +310,5 @@ public abstract class RollupIndexer extends AsyncTwoPhaseIndexer caps = singletonSet(cap); + + DateHistogramAggregationBuilder builder = new DateHistogramAggregationBuilder("foo").field("foo") + .dateHistogramInterval(job.getGroupConfig().getDateHistogram().getInterval()) + .timeZone(ZoneId.of("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(ZoneId.of("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(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(ZoneId.of("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 f691d10d20d..db58115489d 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 @@ package org.elasticsearch.xpack.rollup; 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; @@ -33,7 +31,6 @@ import org.junit.Before; import java.io.IOException; import java.time.ZoneId; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.function.Function; @@ -65,9 +62,8 @@ public class RollupRequestTranslationTests extends ESTestCase { .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); @@ -93,22 +89,6 @@ public class RollupRequestTranslationTests extends ESTestCase { 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() { @@ -118,9 +98,8 @@ public class RollupRequestTranslationTests extends ESTestCase { .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); @@ -133,7 +112,6 @@ public class RollupRequestTranslationTests extends ESTestCase { public void testSimpleMetric() { int i = ESTestCase.randomIntBetween(0, 2); List translated = new ArrayList<>(); - List filterConditions = new ArrayList<>(); Class clazz = null; String fieldName = null; @@ -141,17 +119,17 @@ public class RollupRequestTranslationTests extends ESTestCase { 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"; } @@ -160,14 +138,12 @@ public class RollupRequestTranslationTests extends ESTestCase { 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.")); } @@ -178,9 +154,8 @@ public class RollupRequestTranslationTests extends ESTestCase { .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); @@ -206,20 +181,6 @@ public class RollupRequestTranslationTests extends ESTestCase { 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() { @@ -228,9 +189,8 @@ public class RollupRequestTranslationTests extends ESTestCase { .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); @@ -256,20 +216,6 @@ public class RollupRequestTranslationTests extends ESTestCase { 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 testDateHistoWithTimezone() { @@ -278,9 +224,8 @@ public class RollupRequestTranslationTests extends ESTestCase { histo.interval(86400000) .field("foo") .timeZone(timeZone); - 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); @@ -288,25 +233,11 @@ public class RollupRequestTranslationTests extends ESTestCase { assertThat(translatedHisto.interval(), equalTo(86400000L)); assertThat(translatedHisto.field(), equalTo("foo.date_histogram.timestamp")); assertThat(translatedHisto.timeZone(), equalTo(timeZone)); - 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(timeZone.toString())); - } 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() @@ -319,8 +250,6 @@ public class RollupRequestTranslationTests extends ESTestCase { 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 { @@ -329,9 +258,8 @@ public class RollupRequestTranslationTests extends ESTestCase { 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); @@ -356,8 +284,6 @@ public class RollupRequestTranslationTests extends ESTestCase { 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() { @@ -368,9 +294,8 @@ public class RollupRequestTranslationTests extends ESTestCase { .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); @@ -396,18 +321,6 @@ public class RollupRequestTranslationTests extends ESTestCase { 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() { @@ -415,10 +328,9 @@ public class RollupRequestTranslationTests extends ESTestCase { 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/action/PutJobStateMachineTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/action/PutJobStateMachineTests.java index 3d346456ea9..3f49609953e 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.cluster.metadata.MappingMetaData; 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 class PutJobStateMachineTests extends ESTestCase { 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); + } } 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 5a851d17e5e..a795edca83e 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 class SearchActionTests extends ESTestCase { 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,7 @@ public class SearchActionTests extends ESTestCase { 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 testRangeNullTimeZone() { @@ -138,6 +139,7 @@ public class SearchActionTests extends ESTestCase { QueryBuilder rewritten = TransportRollupSearchAction.rewriteQuery(new RangeQueryBuilder("foo").gt(1), caps); assertThat(rewritten, instanceOf(RangeQueryBuilder.class)); assertThat(((RangeQueryBuilder)rewritten).fieldName(), equalTo("foo.date_histogram.timestamp")); + assertNull(((RangeQueryBuilder)rewritten).timeZone()); } public void testRangeDifferentTZ() { 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 9f8796f4c95..2f0612a65d2 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 @@ -90,6 +90,11 @@ public class ConfigTests extends ESTestCase { assertThat(e.getMessage(), equalTo("Unknown time-zone ID: FOO")); } + public void testObsoleteTimeZone() { + DateHistogramGroupConfig config = new DateHistogramGroupConfig("foo", DateHistogramInterval.HOUR, null, "Canada/Mountain"); + assertThat(config.getTimeZone(), equalTo("Canada/Mountain")); + } + 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/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 cbf85e84b16..38b90328a87 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.joda.time.DateTime; 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 class IndexerUtilsTests extends AggregatorTestCase { } } + 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 a7765dfc15f..be9c9f4a41e 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 @@ -881,6 +881,308 @@ setup: interval: "1h" time_zone: "UTC" +--- +"Obsolete Timezone": + - skip: + version: " - 7.0.99" + reason: "IANA TZ deprecations in 7.1" + features: "warnings" + - do: + indices.create: + index: tz + body: + mappings: + 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." + rollup.put_job: + id: tz + body: > + { + "index_pattern": "tz", + "rollup_index": "tz_rollup", + "cron": "*/30 * * * * ?", + "page_size" :10, + "groups" : { + "date_histogram": { + "field": "timestamp", + "interval": "5m", + "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: 1531221000000 + timestamp.date_histogram.interval: "5m" + 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: 1531221300000 + timestamp.date_histogram.interval: "5m" + 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: 1531221600000 + timestamp.date_histogram.interval: "5m" + 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: + rollup.rollup_search: + index: "tz_rollup" + body: + size: 0 + aggs: + histo: + date_histogram: + field: "timestamp" + interval: "5m" + time_zone: "America/Edmonton" + aggs: + the_max: + max: + field: "price" + + - 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: "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: "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 } + + - do: + rollup.rollup_search: + index: "tz_rollup" + body: + size: 0 + aggs: + histo: + date_histogram: + field: "timestamp" + interval: "5m" + time_zone: "Canada/Mountain" + aggs: + the_max: + max: + field: "price" + + - 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: "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: "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 } + +--- +"Obsolete BWC Timezone": + - skip: + version: " - 7.0.99" + reason: "IANA TZ deprecations in 7.1" + - do: + indices.create: + index: tz_rollup + body: + settings: + number_of_shards: 1 + number_of_replicas: 0 + mappings: + 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: + 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: 1531221000000 + timestamp.date_histogram.interval: "5m" + 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: 1531221300000 + timestamp.date_histogram.interval: "5m" + 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: 1531221600000 + timestamp.date_histogram.interval: "5m" + 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: + rollup.rollup_search: + index: "tz_rollup" + body: + size: 0 + aggs: + histo: + date_histogram: + field: "timestamp" + interval: "5m" + time_zone: "America/Edmonton" + aggs: + the_max: + max: + field: "price" + + - 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: "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: "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 } + + + - do: + rollup.rollup_search: + index: "tz_rollup" + body: + size: 0 + aggs: + histo: + date_histogram: + field: "timestamp" + interval: "5m" + time_zone: "Canada/Mountain" + aggs: + the_max: + max: + field: "price" + + - 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: "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: "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 } + --- "Search with typed_keys": @@ -914,3 +1216,5 @@ setup: - 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 } + +