From a83ebd2dfad6566ae677c64bd30c470608640b50 Mon Sep 17 00:00:00 2001 From: Nathan L Smith Date: Thu, 21 Jan 2021 08:24:08 -0600 Subject: [PATCH 1/8] Round start and end values When getting the start and end times, use the d3 time scale `ticks` function to round the start and end times. Example from a query: Before: ```json { "range": { "@timestamp": { "gte": 1611262874814, "lte": 1611263774814, "format": "epoch_millis" } } }, ``` After: ```json { "range": { "@timestamp": { "gte": 1611263040000, "lte": 1611263880000, "format": "epoch_millis" } } }, ``` The `ticks` function makes it so the amount of rounding is proportional to the size of the time range, so shorter time ranges will be rounded less. Also fix a bug where invalid ranges in the query string were not handled correctly. Fixes #84530. --- .../url_params_context/helpers.test.ts | 59 ++++++++++++------- .../context/url_params_context/helpers.ts | 29 ++++++--- .../url_params_context.test.tsx | 31 +++++----- .../url_params_context/url_params_context.tsx | 6 +- 4 files changed, 76 insertions(+), 49 deletions(-) diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts index 587cb172eeab7..1f5d87f2e5502 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts @@ -9,28 +9,6 @@ import moment from 'moment-timezone'; import * as helpers from './helpers'; describe('url_params_context helpers', () => { - describe('getParsedDate', () => { - describe('given undefined', () => { - it('returns undefined', () => { - expect(helpers.getParsedDate(undefined)).toBeUndefined(); - }); - }); - - describe('given a parsable date', () => { - it('returns the parsed date', () => { - expect(helpers.getParsedDate('1970-01-01T00:00:00.000Z')).toEqual( - '1970-01-01T00:00:00.000Z' - ); - }); - }); - - describe('given a non-parsable date', () => { - it('returns null', () => { - expect(helpers.getParsedDate('nope')).toEqual(null); - }); - }); - }); - describe('getDateRange', () => { describe('when rangeFrom and rangeTo are not changed', () => { it('returns the previous state', () => { @@ -52,6 +30,43 @@ describe('url_params_context helpers', () => { }); }); + describe('when rangeFrom or rangeTo are falsy', () => { + it('returns the previous state', () => { + jest.spyOn(console, 'warn').mockImplementationOnce(() => {}); + expect( + helpers.getDateRange({ + state: { + start: '1972-01-01T00:00:00.000Z', + end: '1973-01-01T00:00:00.000Z', + }, + rangeFrom: '', + rangeTo: 'now', + }) + ).toEqual({ + start: '1972-01-01T00:00:00.000Z', + end: '1973-01-01T00:00:00.000Z', + }); + }); + }); + + describe('when the start or end are invalid', () => { + it('returns the previous state', () => { + expect( + helpers.getDateRange({ + state: { + start: '1972-01-01T00:00:00.000Z', + end: '1973-01-01T00:00:00.000Z', + }, + rangeFrom: 'nope', + rangeTo: 'now', + }) + ).toEqual({ + start: '1972-01-01T00:00:00.000Z', + end: '1973-01-01T00:00:00.000Z', + }); + }); + }); + describe('when rangeFrom or rangeTo have changed', () => { it('returns new state', () => { jest.spyOn(datemath, 'parse').mockReturnValue(moment(0).utc()); diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts index bff2ef5deb86c..e887c5ceee668 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts @@ -4,15 +4,16 @@ * you may not use this file except in compliance with the Elastic License. */ -import { compact, pickBy } from 'lodash'; import datemath from '@elastic/datemath'; +import { scaleTime } from 'd3-scale'; +import { compact, pickBy } from 'lodash'; import { IUrlParams } from './types'; -export function getParsedDate(rawDate?: string, opts = {}) { +function getParsedDate(rawDate?: string, options = {}) { if (rawDate) { - const parsed = datemath.parse(rawDate, opts); - if (parsed) { - return parsed.toISOString(); + const parsed = datemath.parse(rawDate, options); + if (parsed && parsed.isValid()) { + return parsed.toDate(); } } } @@ -26,13 +27,27 @@ export function getDateRange({ rangeFrom?: string; rangeTo?: string; }) { + // If the previous state had the same range, just return that instead of calculating a new range. if (state.rangeFrom === rangeFrom && state.rangeTo === rangeTo) { return { start: state.start, end: state.end }; } + const start = getParsedDate(rangeFrom); + const end = getParsedDate(rangeTo, { roundUp: true }); + + // `getParsedDate` will return undefined for invalid or empty dates. We return + // the previous state if either date is undefined. + if (!start || !end) { + return { start: state.start, end: state.end }; + } + + // Calculate ticks for the time ranges to produce nicely rounded values + const ticks = scaleTime().domain([start, end]).ticks(); + + // Return the first and last tick values. return { - start: getParsedDate(rangeFrom), - end: getParsedDate(rangeTo, { roundUp: true }), + start: ticks[0].toISOString(), + end: ticks[ticks.length - 1].toISOString(), }; } diff --git a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx index 6b57039678e0a..bd7b6617dfd20 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx +++ b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx @@ -4,15 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -import * as React from 'react'; -import { UrlParamsContext, UrlParamsProvider } from './url_params_context'; +import { waitFor } from '@testing-library/react'; import { mount } from 'enzyme'; -import { Location, History } from 'history'; -import { MemoryRouter, Router } from 'react-router-dom'; +import { History, Location } from 'history'; import moment from 'moment-timezone'; +import * as React from 'react'; +import { MemoryRouter, Router } from 'react-router-dom'; import { IUrlParams } from './types'; -import { getParsedDate } from './helpers'; -import { waitFor } from '@testing-library/react'; +import { UrlParamsContext, UrlParamsProvider } from './url_params_context'; function mountParams(location: Location) { return mount( @@ -50,8 +49,8 @@ describe('UrlParamsContext', () => { const wrapper = mountParams(location); const params = getDataFromOutput(wrapper); - expect(params.start).toEqual('2010-03-15T12:00:00.000Z'); - expect(params.end).toEqual('2010-04-10T12:00:00.000Z'); + expect(params.start).toEqual('2010-03-17T05:00:00.000Z'); + expect(params.end).toEqual('2010-04-09T05:00:00.000Z'); }); it('should update param values if location has changed', () => { @@ -66,8 +65,8 @@ describe('UrlParamsContext', () => { // force an update wrapper.setProps({ abc: 123 }); const params = getDataFromOutput(wrapper); - expect(params.start).toEqual('2009-03-15T12:00:00.000Z'); - expect(params.end).toEqual('2009-04-10T12:00:00.000Z'); + expect(params.start).toEqual('2009-03-17T05:00:00.000Z'); + expect(params.end).toEqual('2009-04-09T05:00:00.000Z'); }); it('should parse relative time ranges on mount', () => { @@ -81,8 +80,8 @@ describe('UrlParamsContext', () => { // force an update wrapper.setProps({ abc: 123 }); const params = getDataFromOutput(wrapper); - expect(params.start).toEqual(getParsedDate('now-1d/d')); - expect(params.end).toEqual(getParsedDate('now-1d/d', { roundUp: true })); + expect(new Date(params.start).getTime()).not.toBeNaN(); + expect(new Date(params.end).getTime()).not.toBeNaN(); }); it('should refresh the time range with new values', async () => { @@ -130,8 +129,8 @@ describe('UrlParamsContext', () => { expect(calls.length).toBe(2); const params = getDataFromOutput(wrapper); - expect(params.start).toEqual('2005-09-20T12:00:00.000Z'); - expect(params.end).toEqual('2005-10-21T12:00:00.000Z'); + expect(params.start).toEqual('2005-09-21T05:00:00.000Z'); + expect(params.end).toEqual('2005-10-21T05:00:00.000Z'); }); it('should refresh the time range with new values if time range is relative', async () => { @@ -177,7 +176,7 @@ describe('UrlParamsContext', () => { await waitFor(() => {}); const params = getDataFromOutput(wrapper); - expect(params.start).toEqual('2000-06-14T00:00:00.000Z'); - expect(params.end).toEqual('2000-06-14T23:59:59.999Z'); + expect(params.start).toEqual('2000-06-14T02:00:00.000Z'); + expect(params.end).toEqual('2000-06-14T23:00:00.000Z'); }); }); diff --git a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx index 0a3f8459ff002..55a0887b6b6ac 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx +++ b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx @@ -14,12 +14,11 @@ import React, { import { withRouter } from 'react-router-dom'; import { uniqueId, mapValues } from 'lodash'; import { IUrlParams } from './types'; -import { getParsedDate } from './helpers'; +import { getDateRange } from './helpers'; import { resolveUrlParams } from './resolve_url_params'; import { UIFilters } from '../../../typings/ui_filters'; import { localUIFilterNames, - // eslint-disable-next-line @kbn/eslint/no-restricted-paths } from '../../../server/lib/ui_filters/local_ui_filters/config'; import { pickKeys } from '../../../common/utils/pick_keys'; @@ -79,8 +78,7 @@ const UrlParamsProvider: React.ComponentClass<{}> = withRouter( (timeRange: TimeRange) => { refUrlParams.current = { ...refUrlParams.current, - start: getParsedDate(timeRange.rangeFrom), - end: getParsedDate(timeRange.rangeTo, { roundUp: true }), + ...getDateRange({ state: {}, ...timeRange }), }; forceUpdate(uniqueId()); From 0c6021d6423ee3890cc45fd27d1771ad3e3299b5 Mon Sep 17 00:00:00 2001 From: Nathan L Smith Date: Mon, 25 Jan 2021 15:12:34 -0600 Subject: [PATCH 2/8] fixes --- .../context/url_params_context/helpers.ts | 4 ++-- .../url_params_context.test.tsx | 20 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts index e887c5ceee668..10cd9a9c02201 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts @@ -5,7 +5,7 @@ */ import datemath from '@elastic/datemath'; -import { scaleTime } from 'd3-scale'; +import { scaleUtc } from 'd3-scale'; import { compact, pickBy } from 'lodash'; import { IUrlParams } from './types'; @@ -42,7 +42,7 @@ export function getDateRange({ } // Calculate ticks for the time ranges to produce nicely rounded values - const ticks = scaleTime().domain([start, end]).ticks(); + const ticks = scaleUtc().domain([start, end]).nice().ticks(); // Return the first and last tick values. return { diff --git a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx index bd7b6617dfd20..b66314e4e4cc5 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx +++ b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx @@ -32,11 +32,11 @@ function getDataFromOutput(wrapper: ReturnType) { } describe('UrlParamsContext', () => { - beforeEach(() => { + beforeAll(() => { moment.tz.setDefault('Etc/GMT'); }); - afterEach(() => { + afterAll(() => { moment.tz.setDefault(''); }); @@ -49,8 +49,8 @@ describe('UrlParamsContext', () => { const wrapper = mountParams(location); const params = getDataFromOutput(wrapper); - expect(params.start).toEqual('2010-03-17T05:00:00.000Z'); - expect(params.end).toEqual('2010-04-09T05:00:00.000Z'); + expect(params.start).toEqual('2010-03-15T00:00:00.000Z'); + expect(params.end).toEqual('2010-04-11T00:00:00.000Z'); }); it('should update param values if location has changed', () => { @@ -65,8 +65,8 @@ describe('UrlParamsContext', () => { // force an update wrapper.setProps({ abc: 123 }); const params = getDataFromOutput(wrapper); - expect(params.start).toEqual('2009-03-17T05:00:00.000Z'); - expect(params.end).toEqual('2009-04-09T05:00:00.000Z'); + expect(params.start).toEqual('2009-03-15T00:00:00.000Z'); + expect(params.end).toEqual('2009-04-11T00:00:00.000Z'); }); it('should parse relative time ranges on mount', () => { @@ -129,8 +129,8 @@ describe('UrlParamsContext', () => { expect(calls.length).toBe(2); const params = getDataFromOutput(wrapper); - expect(params.start).toEqual('2005-09-21T05:00:00.000Z'); - expect(params.end).toEqual('2005-10-21T05:00:00.000Z'); + expect(params.start).toEqual('2005-09-19T00:00:00.000Z'); + expect(params.end).toEqual('2005-10-23T00:00:00.000Z'); }); it('should refresh the time range with new values if time range is relative', async () => { @@ -176,7 +176,7 @@ describe('UrlParamsContext', () => { await waitFor(() => {}); const params = getDataFromOutput(wrapper); - expect(params.start).toEqual('2000-06-14T02:00:00.000Z'); - expect(params.end).toEqual('2000-06-14T23:00:00.000Z'); + expect(params.start).toEqual('2000-06-14T00:00:00.000Z'); + expect(params.end).toEqual('2000-06-15T00:00:00.000Z'); }); }); From ba1a4b783aaaf1f6b119de488242af7809501d38 Mon Sep 17 00:00:00 2001 From: Nathan L Smith Date: Tue, 26 Jan 2021 14:07:40 -0600 Subject: [PATCH 3/8] test fix --- .../context/url_params_context/helpers.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts index 1f5d87f2e5502..52df1e0054505 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts @@ -10,6 +10,21 @@ import * as helpers from './helpers'; describe('url_params_context helpers', () => { describe('getDateRange', () => { + describe('with non-rounded dates', () => { + it('rounds the values', () => { + expect( + helpers.getDateRange({ + state: {}, + rangeFrom: '1970-01-05T01:22:33.234Z', + rangeTo: '1971-01-10T10:11:12.123Z', + }) + ).toEqual({ + start: '1970-01-01T00:00:00.000Z', + end: '1971-02-01T00:00:00.000Z', + }); + }); + }); + describe('when rangeFrom and rangeTo are not changed', () => { it('returns the previous state', () => { expect( @@ -32,7 +47,9 @@ describe('url_params_context helpers', () => { describe('when rangeFrom or rangeTo are falsy', () => { it('returns the previous state', () => { + // Disable console warning about not receiving a valid date for rangeFrom jest.spyOn(console, 'warn').mockImplementationOnce(() => {}); + expect( helpers.getDateRange({ state: { From 11dec6e1bff1ad2155a05f65c68fbc610c7f13b3 Mon Sep 17 00:00:00 2001 From: Nathan L Smith Date: Tue, 26 Jan 2021 14:50:51 -0600 Subject: [PATCH 4/8] add test --- .../url_params_context/url_params_context.test.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx index b66314e4e4cc5..5c00672cd3dd1 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx +++ b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx @@ -75,13 +75,20 @@ describe('UrlParamsContext', () => { search: '?rangeFrom=now-1d%2Fd&rangeTo=now-1d%2Fd&transactionId=UPDATED', } as Location; + const nowSpy = jest.spyOn(Date, 'now').mockReturnValue(0); + const wrapper = mountParams(location); // force an update wrapper.setProps({ abc: 123 }); const params = getDataFromOutput(wrapper); - expect(new Date(params.start).getTime()).not.toBeNaN(); - expect(new Date(params.end).getTime()).not.toBeNaN(); + + expect([params.start, params.end]).toEqual([ + '1969-12-31T00:00:00.000Z', + '1970-01-01T00:00:00.000Z', + ]); + + nowSpy.mockRestore(); }); it('should refresh the time range with new values', async () => { From 6b448d24c9b8ecef0da427e5e7d0f1e939c27271 Mon Sep 17 00:00:00 2001 From: Nathan L Smith Date: Tue, 26 Jan 2021 16:21:31 -0600 Subject: [PATCH 5/8] only round lower bound, fix tests --- .../url_params_context/helpers.test.ts | 4 +-- .../context/url_params_context/helpers.ts | 6 ++-- .../url_params_context.test.tsx | 30 +++++++++++++------ 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts index 52df1e0054505..7bfc8c629378d 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts @@ -11,7 +11,7 @@ import * as helpers from './helpers'; describe('url_params_context helpers', () => { describe('getDateRange', () => { describe('with non-rounded dates', () => { - it('rounds the values', () => { + it('rounds the lower value', () => { expect( helpers.getDateRange({ state: {}, @@ -20,7 +20,7 @@ describe('url_params_context helpers', () => { }) ).toEqual({ start: '1970-01-01T00:00:00.000Z', - end: '1971-02-01T00:00:00.000Z', + end: '1971-01-10T10:11:12.123Z', }); }); }); diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts index 10cd9a9c02201..2cef665df005d 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts @@ -41,13 +41,15 @@ export function getDateRange({ return { start: state.start, end: state.end }; } - // Calculate ticks for the time ranges to produce nicely rounded values + // Calculate ticks for the time ranges to produce nicely rounded values. We + // only use lower value since the upper end is not likely to matter because the + // cache is constancly cleared when there are writes to the shard. const ticks = scaleUtc().domain([start, end]).nice().ticks(); // Return the first and last tick values. return { start: ticks[0].toISOString(), - end: ticks[ticks.length - 1].toISOString(), + end: end.toISOString(), }; } diff --git a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx index 5c00672cd3dd1..240bb27c81f5f 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx +++ b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx @@ -49,8 +49,11 @@ describe('UrlParamsContext', () => { const wrapper = mountParams(location); const params = getDataFromOutput(wrapper); - expect(params.start).toEqual('2010-03-15T00:00:00.000Z'); - expect(params.end).toEqual('2010-04-11T00:00:00.000Z'); + + expect([params.start, params.end]).toEqual([ + '2010-03-15T00:00:00.000Z', + '2010-04-10T12:00:00.000Z', + ]); }); it('should update param values if location has changed', () => { @@ -65,8 +68,11 @@ describe('UrlParamsContext', () => { // force an update wrapper.setProps({ abc: 123 }); const params = getDataFromOutput(wrapper); - expect(params.start).toEqual('2009-03-15T00:00:00.000Z'); - expect(params.end).toEqual('2009-04-11T00:00:00.000Z'); + + expect([params.start, params.end]).toEqual([ + '2009-03-15T00:00:00.000Z', + '2009-04-10T12:00:00.000Z', + ]); }); it('should parse relative time ranges on mount', () => { @@ -85,7 +91,7 @@ describe('UrlParamsContext', () => { expect([params.start, params.end]).toEqual([ '1969-12-31T00:00:00.000Z', - '1970-01-01T00:00:00.000Z', + '1969-12-31T23:59:59.999Z', ]); nowSpy.mockRestore(); @@ -136,8 +142,11 @@ describe('UrlParamsContext', () => { expect(calls.length).toBe(2); const params = getDataFromOutput(wrapper); - expect(params.start).toEqual('2005-09-19T00:00:00.000Z'); - expect(params.end).toEqual('2005-10-23T00:00:00.000Z'); + + expect([params.start, params.end]).toEqual([ + '2005-09-19T00:00:00.000Z', + '2005-10-21T12:00:00.000Z', + ]); }); it('should refresh the time range with new values if time range is relative', async () => { @@ -183,7 +192,10 @@ describe('UrlParamsContext', () => { await waitFor(() => {}); const params = getDataFromOutput(wrapper); - expect(params.start).toEqual('2000-06-14T00:00:00.000Z'); - expect(params.end).toEqual('2000-06-15T00:00:00.000Z'); + + expect([params.start, params.end]).toEqual([ + '2000-06-14T00:00:00.000Z', + '2000-06-14T23:59:59.999Z', + ]); }); }); From f7394c62660ec4c7f21118ecc7cd80a158e55b74 Mon Sep 17 00:00:00 2001 From: Nathan L Smith Date: Thu, 28 Jan 2021 00:20:06 -0600 Subject: [PATCH 6/8] range id and more tests --- .../shared/DatePicker/date_picker.test.tsx | 4 +- .../components/shared/DatePicker/index.tsx | 3 +- .../url_params_context/helpers.test.ts | 51 +++++++++++++++---- .../context/url_params_context/helpers.ts | 6 +-- .../mock_url_params_context_provider.tsx | 4 +- .../url_params_context.test.tsx | 10 ++-- .../url_params_context/url_params_context.tsx | 15 ++++-- .../plugins/apm/public/hooks/use_fetcher.tsx | 3 ++ 8 files changed, 71 insertions(+), 25 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/DatePicker/date_picker.test.tsx b/x-pack/plugins/apm/public/components/shared/DatePicker/date_picker.test.tsx index 222c27cc7ed6d..afe510f732dae 100644 --- a/x-pack/plugins/apm/public/components/shared/DatePicker/date_picker.test.tsx +++ b/x-pack/plugins/apm/public/components/shared/DatePicker/date_picker.test.tsx @@ -31,8 +31,10 @@ function MockUrlParamsProvider({ return ( {}, + rangeId: 0, refreshTimeRange: mockRefreshTimeRange, + urlParams, uiFilters: useUiFilters(urlParams), }} children={children} diff --git a/x-pack/plugins/apm/public/components/shared/DatePicker/index.tsx b/x-pack/plugins/apm/public/components/shared/DatePicker/index.tsx index f847ce0b6e96f..4c2ff6dbcc02d 100644 --- a/x-pack/plugins/apm/public/components/shared/DatePicker/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/DatePicker/index.tsx @@ -35,7 +35,7 @@ export function DatePicker() { }) ); - const { urlParams, refreshTimeRange } = useUrlParams(); + const { incrementRangeId, urlParams, refreshTimeRange } = useUrlParams(); function updateUrl(nextQuery: { rangeFrom?: string; @@ -111,6 +111,7 @@ export function DatePicker() { onTimeChange={onTimeChange} onRefresh={({ start, end }) => { clearCache(); + incrementRangeId(); refreshTimeRange({ rangeFrom: start, rangeTo: end }); }} onRefreshChange={onRefreshChange} diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts index 7bfc8c629378d..5f3c8842ad549 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts @@ -11,16 +11,47 @@ import * as helpers from './helpers'; describe('url_params_context helpers', () => { describe('getDateRange', () => { describe('with non-rounded dates', () => { - it('rounds the lower value', () => { - expect( - helpers.getDateRange({ - state: {}, - rangeFrom: '1970-01-05T01:22:33.234Z', - rangeTo: '1971-01-10T10:11:12.123Z', - }) - ).toEqual({ - start: '1970-01-01T00:00:00.000Z', - end: '1971-01-10T10:11:12.123Z', + describe('one minute', () => { + it('rounds the values', () => { + expect( + helpers.getDateRange({ + state: {}, + rangeFrom: '2021-01-28T05:47:52.134Z', + rangeTo: '2021-01-28T05:48:55.304Z', + }) + ).toEqual({ + start: '2021-01-28T05:47:50.000Z', + end: '2021-01-28T05:49:00.000Z', + }); + }); + }); + describe('one day', () => { + it('rounds the values', () => { + expect( + helpers.getDateRange({ + state: {}, + rangeFrom: '2021-01-27T05:46:07.377Z', + rangeTo: '2021-01-28T05:46:13.367Z', + }) + ).toEqual({ + start: '2021-01-27T03:00:00.000Z', + end: '2021-01-28T06:00:00.000Z', + }); + }); + }); + + describe('one year', () => { + it('rounds the values', () => { + expect( + helpers.getDateRange({ + state: {}, + rangeFrom: '2020-01-28T05:52:36.290Z', + rangeTo: '2021-01-28T05:52:39.741Z', + }) + ).toEqual({ + start: '2020-01-01T00:00:00.000Z', + end: '2021-02-01T00:00:00.000Z', + }); }); }); }); diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts index 2cef665df005d..0be11d440aecb 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts @@ -41,15 +41,13 @@ export function getDateRange({ return { start: state.start, end: state.end }; } - // Calculate ticks for the time ranges to produce nicely rounded values. We - // only use lower value since the upper end is not likely to matter because the - // cache is constancly cleared when there are writes to the shard. + // Calculate ticks for the time ranges to produce nicely rounded values. const ticks = scaleUtc().domain([start, end]).nice().ticks(); // Return the first and last tick values. return { start: ticks[0].toISOString(), - end: end.toISOString(), + end: ticks[ticks.length - 1].toISOString(), }; } diff --git a/x-pack/plugins/apm/public/context/url_params_context/mock_url_params_context_provider.tsx b/x-pack/plugins/apm/public/context/url_params_context/mock_url_params_context_provider.tsx index b593cbd57a9a9..b9a46304b93f9 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/mock_url_params_context_provider.tsx +++ b/x-pack/plugins/apm/public/context/url_params_context/mock_url_params_context_provider.tsx @@ -30,8 +30,10 @@ export function MockUrlParamsContextProvider({ return ( {}, + rangeId: 0, refreshTimeRange, + urlParams, uiFilters: useUiFilters(urlParams), }} children={children} diff --git a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx index 240bb27c81f5f..cc5c2af9f6de8 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx +++ b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx @@ -52,7 +52,7 @@ describe('UrlParamsContext', () => { expect([params.start, params.end]).toEqual([ '2010-03-15T00:00:00.000Z', - '2010-04-10T12:00:00.000Z', + '2010-04-11T00:00:00.000Z', ]); }); @@ -71,7 +71,7 @@ describe('UrlParamsContext', () => { expect([params.start, params.end]).toEqual([ '2009-03-15T00:00:00.000Z', - '2009-04-10T12:00:00.000Z', + '2009-04-11T00:00:00.000Z', ]); }); @@ -91,7 +91,7 @@ describe('UrlParamsContext', () => { expect([params.start, params.end]).toEqual([ '1969-12-31T00:00:00.000Z', - '1969-12-31T23:59:59.999Z', + '1970-01-01T00:00:00.000Z', ]); nowSpy.mockRestore(); @@ -145,7 +145,7 @@ describe('UrlParamsContext', () => { expect([params.start, params.end]).toEqual([ '2005-09-19T00:00:00.000Z', - '2005-10-21T12:00:00.000Z', + '2005-10-23T00:00:00.000Z', ]); }); @@ -195,7 +195,7 @@ describe('UrlParamsContext', () => { expect([params.start, params.end]).toEqual([ '2000-06-14T00:00:00.000Z', - '2000-06-14T23:59:59.999Z', + '2000-06-15T00:00:00.000Z', ]); }); }); diff --git a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx index 55a0887b6b6ac..31d28c39519d0 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx +++ b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx @@ -48,9 +48,11 @@ function useUiFilters(params: IUrlParams): UIFilters { const defaultRefresh = (_time: TimeRange) => {}; const UrlParamsContext = createContext({ - urlParams: {} as IUrlParams, + incrementRangeId: () => {}, + rangeId: 0, refreshTimeRange: defaultRefresh, uiFilters: {} as UIFilters, + urlParams: {} as IUrlParams, }); const UrlParamsProvider: React.ComponentClass<{}> = withRouter( @@ -59,6 +61,9 @@ const UrlParamsProvider: React.ComponentClass<{}> = withRouter( const { start, end, rangeFrom, rangeTo } = refUrlParams.current; + // Counter to force an update in useFetcher when the refresh button is clicked. + const [rangeId, setRangeId] = useState(0); + const [, forceUpdate] = useState(''); const urlParams = useMemo( @@ -74,6 +79,8 @@ const UrlParamsProvider: React.ComponentClass<{}> = withRouter( refUrlParams.current = urlParams; + const incrementRangeId = () => setRangeId((prevRangeId) => prevRangeId + 1); + const refreshTimeRange = useCallback( (timeRange: TimeRange) => { refUrlParams.current = { @@ -90,11 +97,13 @@ const UrlParamsProvider: React.ComponentClass<{}> = withRouter( const contextValue = useMemo(() => { return { - urlParams, + incrementRangeId, + rangeId, refreshTimeRange, + urlParams, uiFilters, }; - }, [urlParams, refreshTimeRange, uiFilters]); + }, [rangeId, refreshTimeRange, uiFilters, urlParams]); return ( diff --git a/x-pack/plugins/apm/public/hooks/use_fetcher.tsx b/x-pack/plugins/apm/public/hooks/use_fetcher.tsx index 8174f06e06b8b..12bbdb7e0bad0 100644 --- a/x-pack/plugins/apm/public/hooks/use_fetcher.tsx +++ b/x-pack/plugins/apm/public/hooks/use_fetcher.tsx @@ -10,6 +10,7 @@ import { IHttpFetchError } from 'src/core/public'; import { toMountPoint } from '../../../../../src/plugins/kibana_react/public'; import { APMClient, callApmApi } from '../services/rest/createCallApmApi'; import { useApmPluginContext } from '../context/apm_plugin/use_apm_plugin_context'; +import { useUrlParams } from '../context/url_params_context/use_url_params'; export enum FETCH_STATUS { LOADING = 'loading', @@ -49,6 +50,7 @@ export function useFetcher( status: FETCH_STATUS.NOT_INITIATED, }); const [counter, setCounter] = useState(0); + const { rangeId } = useUrlParams(); useEffect(() => { let didCancel = false; @@ -132,6 +134,7 @@ export function useFetcher( }, [ counter, preservePreviousData, + rangeId, showToastOnError, ...fnDeps, /* eslint-enable react-hooks/exhaustive-deps */ From f10a3fabc89f0a64ea8191586b5f17910931143a Mon Sep 17 00:00:00 2001 From: Nathan L Smith Date: Thu, 28 Jan 2021 08:36:04 -0600 Subject: [PATCH 7/8] fix test --- .../components/app/TraceLink/trace_link.test.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/apm/public/components/app/TraceLink/trace_link.test.tsx b/x-pack/plugins/apm/public/components/app/TraceLink/trace_link.test.tsx index c07e00ef387c9..72db5e1dbdb54 100644 --- a/x-pack/plugins/apm/public/components/app/TraceLink/trace_link.test.tsx +++ b/x-pack/plugins/apm/public/components/app/TraceLink/trace_link.test.tsx @@ -60,12 +60,14 @@ describe('TraceLink', () => { describe('when no transaction is found', () => { it('renders a trace page', () => { jest.spyOn(urlParamsHooks, 'useUrlParams').mockReturnValue({ + incrementRangeId: () => {}, + rangeId: 0, + refreshTimeRange: jest.fn(), + uiFilters: {}, urlParams: { rangeFrom: 'now-24h', rangeTo: 'now', }, - refreshTimeRange: jest.fn(), - uiFilters: {}, }); jest.spyOn(hooks, 'useFetcher').mockReturnValue({ data: { transaction: undefined }, @@ -87,12 +89,14 @@ describe('TraceLink', () => { describe('transaction page', () => { beforeAll(() => { jest.spyOn(urlParamsHooks, 'useUrlParams').mockReturnValue({ + incrementRangeId: () => {}, + rangeId: 0, + refreshTimeRange: jest.fn(), + uiFilters: {}, urlParams: { rangeFrom: 'now-24h', rangeTo: 'now', }, - refreshTimeRange: jest.fn(), - uiFilters: {}, }); }); From 378c08f980fb2100c17ba4e9ecd291f5afbef44c Mon Sep 17 00:00:00 2001 From: Nathan L Smith Date: Mon, 1 Feb 2021 19:01:36 -0600 Subject: [PATCH 8/8] Don't expose increment --- .../app/TraceLink/trace_link.test.tsx | 2 - .../shared/DatePicker/date_picker.test.tsx | 1 - .../components/shared/DatePicker/index.tsx | 3 +- .../mock_url_params_context_provider.tsx | 1 - .../url_params_context/url_params_context.tsx | 45 +++++++------------ 5 files changed, 18 insertions(+), 34 deletions(-) diff --git a/x-pack/plugins/apm/public/components/app/TraceLink/trace_link.test.tsx b/x-pack/plugins/apm/public/components/app/TraceLink/trace_link.test.tsx index 72db5e1dbdb54..a33be140d9a36 100644 --- a/x-pack/plugins/apm/public/components/app/TraceLink/trace_link.test.tsx +++ b/x-pack/plugins/apm/public/components/app/TraceLink/trace_link.test.tsx @@ -60,7 +60,6 @@ describe('TraceLink', () => { describe('when no transaction is found', () => { it('renders a trace page', () => { jest.spyOn(urlParamsHooks, 'useUrlParams').mockReturnValue({ - incrementRangeId: () => {}, rangeId: 0, refreshTimeRange: jest.fn(), uiFilters: {}, @@ -89,7 +88,6 @@ describe('TraceLink', () => { describe('transaction page', () => { beforeAll(() => { jest.spyOn(urlParamsHooks, 'useUrlParams').mockReturnValue({ - incrementRangeId: () => {}, rangeId: 0, refreshTimeRange: jest.fn(), uiFilters: {}, diff --git a/x-pack/plugins/apm/public/components/shared/DatePicker/date_picker.test.tsx b/x-pack/plugins/apm/public/components/shared/DatePicker/date_picker.test.tsx index afe510f732dae..1bf3cee32f286 100644 --- a/x-pack/plugins/apm/public/components/shared/DatePicker/date_picker.test.tsx +++ b/x-pack/plugins/apm/public/components/shared/DatePicker/date_picker.test.tsx @@ -31,7 +31,6 @@ function MockUrlParamsProvider({ return ( {}, rangeId: 0, refreshTimeRange: mockRefreshTimeRange, urlParams, diff --git a/x-pack/plugins/apm/public/components/shared/DatePicker/index.tsx b/x-pack/plugins/apm/public/components/shared/DatePicker/index.tsx index 4c2ff6dbcc02d..f847ce0b6e96f 100644 --- a/x-pack/plugins/apm/public/components/shared/DatePicker/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/DatePicker/index.tsx @@ -35,7 +35,7 @@ export function DatePicker() { }) ); - const { incrementRangeId, urlParams, refreshTimeRange } = useUrlParams(); + const { urlParams, refreshTimeRange } = useUrlParams(); function updateUrl(nextQuery: { rangeFrom?: string; @@ -111,7 +111,6 @@ export function DatePicker() { onTimeChange={onTimeChange} onRefresh={({ start, end }) => { clearCache(); - incrementRangeId(); refreshTimeRange({ rangeFrom: start, rangeTo: end }); }} onRefreshChange={onRefreshChange} diff --git a/x-pack/plugins/apm/public/context/url_params_context/mock_url_params_context_provider.tsx b/x-pack/plugins/apm/public/context/url_params_context/mock_url_params_context_provider.tsx index b9a46304b93f9..1e546599ee8a3 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/mock_url_params_context_provider.tsx +++ b/x-pack/plugins/apm/public/context/url_params_context/mock_url_params_context_provider.tsx @@ -30,7 +30,6 @@ export function MockUrlParamsContextProvider({ return ( {}, rangeId: 0, refreshTimeRange, urlParams, diff --git a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx index 31d28c39519d0..f66a45db261a7 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx +++ b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx @@ -4,27 +4,25 @@ * you may not use this file except in compliance with the Elastic License. */ +import { mapValues } from 'lodash'; import React, { createContext, - useMemo, useCallback, + useMemo, useRef, useState, } from 'react'; import { withRouter } from 'react-router-dom'; -import { uniqueId, mapValues } from 'lodash'; -import { IUrlParams } from './types'; -import { getDateRange } from './helpers'; -import { resolveUrlParams } from './resolve_url_params'; -import { UIFilters } from '../../../typings/ui_filters'; -import { - localUIFilterNames, - // eslint-disable-next-line @kbn/eslint/no-restricted-paths -} from '../../../server/lib/ui_filters/local_ui_filters/config'; +import { ENVIRONMENT_ALL } from '../../../common/environment_filter_values'; +import { LocalUIFilterName } from '../../../common/ui_filter'; import { pickKeys } from '../../../common/utils/pick_keys'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { localUIFilterNames } from '../../../server/lib/ui_filters/local_ui_filters/config'; +import { UIFilters } from '../../../typings/ui_filters'; import { useDeepObjectIdentity } from '../../hooks/useDeepObjectIdentity'; -import { LocalUIFilterName } from '../../../common/ui_filter'; -import { ENVIRONMENT_ALL } from '../../../common/environment_filter_values'; +import { getDateRange } from './helpers'; +import { resolveUrlParams } from './resolve_url_params'; +import { IUrlParams } from './types'; interface TimeRange { rangeFrom: string; @@ -48,7 +46,6 @@ function useUiFilters(params: IUrlParams): UIFilters { const defaultRefresh = (_time: TimeRange) => {}; const UrlParamsContext = createContext({ - incrementRangeId: () => {}, rangeId: 0, refreshTimeRange: defaultRefresh, uiFilters: {} as UIFilters, @@ -64,8 +61,6 @@ const UrlParamsProvider: React.ComponentClass<{}> = withRouter( // Counter to force an update in useFetcher when the refresh button is clicked. const [rangeId, setRangeId] = useState(0); - const [, forceUpdate] = useState(''); - const urlParams = useMemo( () => resolveUrlParams(location, { @@ -79,25 +74,19 @@ const UrlParamsProvider: React.ComponentClass<{}> = withRouter( refUrlParams.current = urlParams; - const incrementRangeId = () => setRangeId((prevRangeId) => prevRangeId + 1); - - const refreshTimeRange = useCallback( - (timeRange: TimeRange) => { - refUrlParams.current = { - ...refUrlParams.current, - ...getDateRange({ state: {}, ...timeRange }), - }; + const refreshTimeRange = useCallback((timeRange: TimeRange) => { + refUrlParams.current = { + ...refUrlParams.current, + ...getDateRange({ state: {}, ...timeRange }), + }; - forceUpdate(uniqueId()); - }, - [forceUpdate] - ); + setRangeId((prevRangeId) => prevRangeId + 1); + }, []); const uiFilters = useUiFilters(urlParams); const contextValue = useMemo(() => { return { - incrementRangeId, rangeId, refreshTimeRange, urlParams,