Skip to content

Commit

Permalink
Round start and end values (#89030)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
smith committed Feb 2, 2021
1 parent 33bf590 commit f317316
Show file tree
Hide file tree
Showing 8 changed files with 176 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,13 @@ describe('TraceLink', () => {
describe('when no transaction is found', () => {
it('renders a trace page', () => {
jest.spyOn(urlParamsHooks, 'useUrlParams').mockReturnValue({
rangeId: 0,
refreshTimeRange: jest.fn(),
uiFilters: {},
urlParams: {
rangeFrom: 'now-24h',
rangeTo: 'now',
},
refreshTimeRange: jest.fn(),
uiFilters: {},
});
jest.spyOn(hooks, 'useFetcher').mockReturnValue({
data: { transaction: undefined },
Expand All @@ -87,12 +88,13 @@ describe('TraceLink', () => {
describe('transaction page', () => {
beforeAll(() => {
jest.spyOn(urlParamsHooks, 'useUrlParams').mockReturnValue({
rangeId: 0,
refreshTimeRange: jest.fn(),
uiFilters: {},
urlParams: {
rangeFrom: 'now-24h',
rangeTo: 'now',
},
refreshTimeRange: jest.fn(),
uiFilters: {},
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ function MockUrlParamsProvider({
return (
<UrlParamsContext.Provider
value={{
urlParams,
rangeId: 0,
refreshTimeRange: mockRefreshTimeRange,
urlParams,
uiFilters: useUiFilters(urlParams),
}}
children={children}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,53 @@ 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('getDateRange', () => {
describe('with non-rounded dates', () => {
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('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('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('given a non-parsable date', () => {
it('returns null', () => {
expect(helpers.getParsedDate('nope')).toEqual(null);
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',
});
});
});
});
});

describe('getDateRange', () => {
describe('when rangeFrom and rangeTo are not changed', () => {
it('returns the previous state', () => {
expect(
Expand All @@ -52,6 +76,45 @@ 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: {
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());
Expand Down
29 changes: 22 additions & 7 deletions x-pack/plugins/apm/public/context/url_params_context/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 { scaleUtc } 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();
}
}
}
Expand All @@ -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 = scaleUtc().domain([start, end]).nice().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(),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ export function MockUrlParamsContextProvider({
return (
<UrlParamsContext.Provider
value={{
urlParams,
rangeId: 0,
refreshTimeRange,
urlParams,
uiFilters: useUiFilters(urlParams),
}}
children={children}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -33,11 +32,11 @@ function getDataFromOutput(wrapper: ReturnType<typeof mount>) {
}

describe('UrlParamsContext', () => {
beforeEach(() => {
beforeAll(() => {
moment.tz.setDefault('Etc/GMT');
});

afterEach(() => {
afterAll(() => {
moment.tz.setDefault('');
});

Expand All @@ -50,8 +49,11 @@ 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, params.end]).toEqual([
'2010-03-15T00:00:00.000Z',
'2010-04-11T00:00:00.000Z',
]);
});

it('should update param values if location has changed', () => {
Expand All @@ -66,8 +68,11 @@ 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, params.end]).toEqual([
'2009-03-15T00:00:00.000Z',
'2009-04-11T00:00:00.000Z',
]);
});

it('should parse relative time ranges on mount', () => {
Expand All @@ -76,13 +81,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(params.start).toEqual(getParsedDate('now-1d/d'));
expect(params.end).toEqual(getParsedDate('now-1d/d', { roundUp: true }));

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 () => {
Expand Down Expand Up @@ -130,8 +142,11 @@ 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, params.end]).toEqual([
'2005-09-19T00:00:00.000Z',
'2005-10-23T00:00:00.000Z',
]);
});

it('should refresh the time range with new values if time range is relative', async () => {
Expand Down Expand Up @@ -177,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-14T23:59:59.999Z');

expect([params.start, params.end]).toEqual([
'2000-06-14T00:00:00.000Z',
'2000-06-15T00:00:00.000Z',
]);
});
});
Loading

0 comments on commit f317316

Please sign in to comment.