From 7b7c193551b8122612c788c4a732566ef019377e Mon Sep 17 00:00:00 2001 From: Giordano Ricci Date: Tue, 2 Nov 2021 16:18:39 +0000 Subject: [PATCH] Elasticsearch: Add time zone setting to Date Histogram aggregation (#40882) * TimeZonePicker: Allow specifying internal timezones to display * Elasticsearch: Add time zone setting to Date Histogram aggregation * rename time_zone to timeZone * Add tests * fix typo * Update packages/grafana-data/src/datetime/timezones.ts Co-authored-by: Chris Cowan Co-authored-by: Chris Cowan --- .../grafana-data/src/datetime/timezones.ts | 48 ++++++++++--------- .../DateTimePickers/TimeZonePicker.tsx | 4 +- pkg/tsdb/elasticsearch/client/models.go | 1 + pkg/tsdb/elasticsearch/time_series_query.go | 6 +++ .../elasticsearch/time_series_query_test.go | 48 ++++++++++++++++++- .../DateHistogramSettingsEditor.tsx | 14 +++++- .../BucketAggregationsEditor/aggregations.ts | 1 + .../BucketAggregationsEditor/utils.ts | 3 +- .../datasource/elasticsearch/query_builder.ts | 4 ++ .../elasticsearch/specs/query_builder.test.ts | 45 +++++++++++++++++ 10 files changed, 146 insertions(+), 28 deletions(-) diff --git a/packages/grafana-data/src/datetime/timezones.ts b/packages/grafana-data/src/datetime/timezones.ts index 2d89ff3d141..b1528a66aaa 100644 --- a/packages/grafana-data/src/datetime/timezones.ts +++ b/packages/grafana-data/src/datetime/timezones.ts @@ -48,11 +48,13 @@ export const getTimeZoneInfo = (zone: string, timestamp: number): TimeZoneInfo | return mapToInfo(zone, timestamp); }; -export const getTimeZones = memoize((includeInternal = false): TimeZone[] => { +export const getTimeZones = memoize((includeInternal: boolean | InternalTimeZones[] = false): TimeZone[] => { const initial: TimeZone[] = []; - if (includeInternal) { - initial.push.apply(initial, [InternalTimeZones.default, InternalTimeZones.localBrowserTime, InternalTimeZones.utc]); + if (includeInternal === true) { + initial.push(InternalTimeZones.default, InternalTimeZones.localBrowserTime, InternalTimeZones.utc); + } else if (includeInternal) { + initial.push(...includeInternal); } return moment.tz.names().reduce((zones: TimeZone[], zone: string) => { @@ -67,32 +69,34 @@ export const getTimeZones = memoize((includeInternal = false): TimeZone[] => { }, initial); }); -export const getTimeZoneGroups = memoize((includeInternal = false): GroupedTimeZones[] => { - const timeZones = getTimeZones(includeInternal); +export const getTimeZoneGroups = memoize( + (includeInternal: boolean | InternalTimeZones[] = false): GroupedTimeZones[] => { + const timeZones = getTimeZones(includeInternal); - const groups = timeZones.reduce((groups: Record, zone: TimeZone) => { - const delimiter = zone.indexOf('/'); + const groups = timeZones.reduce((groups: Record, zone: TimeZone) => { + const delimiter = zone.indexOf('/'); - if (delimiter === -1) { - const group = ''; + if (delimiter === -1) { + const group = ''; + groups[group] = groups[group] ?? []; + groups[group].push(zone); + + return groups; + } + + const group = zone.substr(0, delimiter); groups[group] = groups[group] ?? []; groups[group].push(zone); return groups; - } + }, {}); - const group = zone.substr(0, delimiter); - groups[group] = groups[group] ?? []; - groups[group].push(zone); - - return groups; - }, {}); - - return Object.keys(groups).map((name) => ({ - name, - zones: groups[name], - })); -}); + return Object.keys(groups).map((name) => ({ + name, + zones: groups[name], + })); + } +); const mapInternal = (zone: string, timestamp: number): TimeZoneInfo | undefined => { switch (zone) { diff --git a/packages/grafana-ui/src/components/DateTimePickers/TimeZonePicker.tsx b/packages/grafana-ui/src/components/DateTimePickers/TimeZonePicker.tsx index fb46150f984..8af52c661f2 100644 --- a/packages/grafana-ui/src/components/DateTimePickers/TimeZonePicker.tsx +++ b/packages/grafana-ui/src/components/DateTimePickers/TimeZonePicker.tsx @@ -20,7 +20,7 @@ export interface Props { width?: number; autoFocus?: boolean; onBlur?: () => void; - includeInternal?: boolean; + includeInternal?: boolean | InternalTimeZones[]; disabled?: boolean; } @@ -63,7 +63,7 @@ interface SelectableZoneGroup extends SelectableValue { options: SelectableZone[]; } -const useTimeZones = (includeInternal: boolean): SelectableZoneGroup[] => { +const useTimeZones = (includeInternal: boolean | InternalTimeZones[]): SelectableZoneGroup[] => { const now = Date.now(); const timeZoneGroups = getTimeZoneGroups(includeInternal).map((group: GroupedTimeZones) => { diff --git a/pkg/tsdb/elasticsearch/client/models.go b/pkg/tsdb/elasticsearch/client/models.go index 1c4c0fcc2a9..79039e224c7 100644 --- a/pkg/tsdb/elasticsearch/client/models.go +++ b/pkg/tsdb/elasticsearch/client/models.go @@ -244,6 +244,7 @@ type DateHistogramAgg struct { ExtendedBounds *ExtendedBounds `json:"extended_bounds"` Format string `json:"format"` Offset string `json:"offset,omitempty"` + TimeZone string `json:"time_zone,omitempty"` } // FiltersAggregation represents a filters aggregation diff --git a/pkg/tsdb/elasticsearch/time_series_query.go b/pkg/tsdb/elasticsearch/time_series_query.go index 345a52eed37..9561592893d 100644 --- a/pkg/tsdb/elasticsearch/time_series_query.go +++ b/pkg/tsdb/elasticsearch/time_series_query.go @@ -264,6 +264,12 @@ func addDateHistogramAgg(aggBuilder es.AggBuilder, bucketAgg *BucketAgg, timeFro a.Missing = &missing } + if timezone, err := bucketAgg.Settings.Get("timeZone").String(); err == nil { + if timezone != "utc" { + a.TimeZone = timezone + } + } + aggBuilder = b }) diff --git a/pkg/tsdb/elasticsearch/time_series_query_test.go b/pkg/tsdb/elasticsearch/time_series_query_test.go index e4a515d372c..cff33e0cee3 100644 --- a/pkg/tsdb/elasticsearch/time_series_query_test.go +++ b/pkg/tsdb/elasticsearch/time_series_query_test.go @@ -402,6 +402,52 @@ func TestExecuteTimeSeriesQuery(t *testing.T) { require.Equal(t, hAgg.Field, "@timestamp") require.Equal(t, hAgg.Interval, "$__interval") require.Equal(t, hAgg.MinDocCount, 2) + + t.Run("Should not include time_zone when timeZone is utc", func(t *testing.T) { + c := newFakeClient("7.0.0") + _, err := executeTsdbQuery(c, `{ + "timeField": "@timestamp", + "bucketAggs": [ + { + "id": "2", + "type": "date_histogram", + "field": "@timestamp", + "settings": { + "timeZone": "utc" + } + } + ], + "metrics": [{"type": "count", "id": "1" }] + }`, from, to, 15*time.Second) + require.NoError(t, err) + sr := c.multisearchRequests[0].Requests[0] + + dateHistogram := sr.Aggs[0].Aggregation.Aggregation.(*es.DateHistogramAgg) + require.Empty(t, dateHistogram.TimeZone) + }) + + t.Run("Should include time_zone when timeZone is not utc", func(t *testing.T) { + c := newFakeClient("7.0.0") + _, err := executeTsdbQuery(c, `{ + "timeField": "@timestamp", + "bucketAggs": [ + { + "id": "2", + "type": "date_histogram", + "field": "@timestamp", + "settings": { + "timeZone": "America/Los_Angeles" + } + } + ], + "metrics": [{"type": "count", "id": "1" }] + }`, from, to, 15*time.Second) + require.NoError(t, err) + sr := c.multisearchRequests[0].Requests[0] + + deteHistogram := sr.Aggs[0].Aggregation.Aggregation.(*es.DateHistogramAgg) + require.Equal(t, deteHistogram.TimeZone, "America/Los_Angeles") + }) }) t.Run("With histogram agg", func(t *testing.T) { @@ -1163,7 +1209,7 @@ func TestTimeSeriesQueryParser(t *testing.T) { "timeField": "@timestamp", "query": "@metric:cpu", "alias": "{{@hostname}} {{metric}}", - "interval": "10m", + "interval": "10m", "metrics": [ { "field": "@value", diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/SettingsEditor/DateHistogramSettingsEditor.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/SettingsEditor/DateHistogramSettingsEditor.tsx index 7b0fe750a8c..1eef6876005 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/SettingsEditor/DateHistogramSettingsEditor.tsx +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/SettingsEditor/DateHistogramSettingsEditor.tsx @@ -1,9 +1,9 @@ import React from 'react'; -import { InlineField, Input, Select } from '@grafana/ui'; +import { InlineField, Input, Select, TimeZonePicker } from '@grafana/ui'; import { DateHistogram } from '../aggregations'; import { bucketAggregationConfig } from '../utils'; import { useDispatch } from '../../../../hooks/useStatelessReducer'; -import { SelectableValue } from '@grafana/data'; +import { InternalTimeZones, SelectableValue } from '@grafana/data'; import { changeBucketAggregationSetting } from '../state/actions'; import { inlineFieldProps } from '.'; import { uniqueId } from 'lodash'; @@ -99,6 +99,16 @@ export const DateHistogramSettingsEditor = ({ bucketAgg }: Props) => { defaultValue={bucketAgg.settings?.offset || bucketAggregationConfig.date_histogram.defaultSettings?.offset} /> + + + { + dispatch(changeBucketAggregationSetting({ bucketAgg, settingName: 'timeZone', newValue: timeZone })); + }} + /> + ); }; diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/aggregations.ts b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/aggregations.ts index b0154b34305..5e230c26b19 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/aggregations.ts +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/aggregations.ts @@ -19,6 +19,7 @@ export interface DateHistogram extends BucketAggregationWithField { min_doc_count?: string; trimEdges?: string; offset?: string; + timeZone?: string; }; } diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/utils.ts b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/utils.ts index 02f0dfa0f68..56673b91236 100644 --- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/utils.ts +++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/utils.ts @@ -1,6 +1,6 @@ import { BucketsConfiguration } from '../../../types'; import { defaultFilter } from './SettingsEditor/FiltersSettingsEditor/utils'; -import { SelectableValue } from '@grafana/data'; +import { InternalTimeZones, SelectableValue } from '@grafana/data'; export const bucketAggregationConfig: BucketsConfiguration = { terms: { @@ -34,6 +34,7 @@ export const bucketAggregationConfig: BucketsConfiguration = { interval: 'auto', min_doc_count: '0', trimEdges: '0', + timeZone: InternalTimeZones.utc, }, }, histogram: { diff --git a/public/app/plugins/datasource/elasticsearch/query_builder.ts b/public/app/plugins/datasource/elasticsearch/query_builder.ts index 99811061911..a0581d648f8 100644 --- a/public/app/plugins/datasource/elasticsearch/query_builder.ts +++ b/public/app/plugins/datasource/elasticsearch/query_builder.ts @@ -1,3 +1,4 @@ +import { InternalTimeZones } from '@grafana/data'; import { gte, lt } from 'semver'; import { Filters, @@ -99,6 +100,9 @@ export class ElasticQueryBuilder { esAgg.min_doc_count = settings.min_doc_count || 0; esAgg.extended_bounds = { min: '$timeFrom', max: '$timeTo' }; esAgg.format = 'epoch_millis'; + if (settings.timeZone && settings.timeZone !== InternalTimeZones.utc) { + esAgg.time_zone = settings.timeZone; + } if (settings.offset !== '') { esAgg.offset = settings.offset; diff --git a/public/app/plugins/datasource/elasticsearch/specs/query_builder.test.ts b/public/app/plugins/datasource/elasticsearch/specs/query_builder.test.ts index 66a5fb40e15..fcc03afffa6 100644 --- a/public/app/plugins/datasource/elasticsearch/specs/query_builder.test.ts +++ b/public/app/plugins/datasource/elasticsearch/specs/query_builder.test.ts @@ -841,5 +841,50 @@ describe('ElasticQueryBuilder', () => { expect(serialDiff.lag).toBe(1); }); + + describe('date_histogram', () => { + it('should not include time_zone if not present in the query model', () => { + const query = builder.build({ + refId: 'A', + metrics: [{ type: 'count', id: '1' }], + timeField: '@timestamp', + bucketAggs: [{ type: 'date_histogram', field: '@timestamp', id: '2', settings: { min_doc_count: '1' } }], + }); + + expect(query.aggs['2'].date_histogram.time_zone).not.toBeDefined(); + }); + + it('should not include time_zone if "utc" in the query model', () => { + const query = builder.build({ + refId: 'A', + metrics: [{ type: 'count', id: '1' }], + timeField: '@timestamp', + bucketAggs: [ + { type: 'date_histogram', field: '@timestamp', id: '2', settings: { min_doc_count: '1', timeZone: 'utc' } }, + ], + }); + + expect(query.aggs['2'].date_histogram.time_zone).not.toBeDefined(); + }); + + it('should include time_zone if not "utc" in the query model', () => { + const expectedTimezone = 'America/Los_angeles'; + const query = builder.build({ + refId: 'A', + metrics: [{ type: 'count', id: '1' }], + timeField: '@timestamp', + bucketAggs: [ + { + type: 'date_histogram', + field: '@timestamp', + id: '2', + settings: { min_doc_count: '1', timeZone: expectedTimezone }, + }, + ], + }); + + expect(query.aggs['2'].date_histogram.time_zone).toBe(expectedTimezone); + }); + }); }); });