Skip to content

Commit

Permalink
[7.x] [APM] Correct timezone formatting (#48355) (#48915)
Browse files Browse the repository at this point in the history
* Don't set the timezone anywhere in APM since it's already set in autoload

For the chart X-axes:

* Create nice ticks for the configured timezone (ie at 1w, 1d, 12hrs interval) by offsetting the xMin/xMax
* Explicitly pass those tick values to the x-axis
* When formatting, use scaleUtc to format everything as UTC, and offset the time again with the configured timezone.

Fixes #47832
Fixes #48355
  • Loading branch information
smith authored Oct 22, 2019
1 parent ff0ebd5 commit 6e4f665
Show file tree
Hide file tree
Showing 9 changed files with 186 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@ import {
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import { memoize, padLeft, range } from 'lodash';
import { padLeft, range } from 'lodash';
import moment from 'moment-timezone';
import React, { Component } from 'react';
import styled from 'styled-components';
import { LegacyCoreStart } from 'src/core/public';
import { KibanaCoreContext } from '../../../../../../observability/public';
import { IUrlParams } from '../../../../context/UrlParamsContext/types';
import { KibanaLink } from '../../../shared/Links/KibanaLink';
Expand All @@ -39,12 +38,6 @@ import { ElasticDocsLink } from '../../../shared/Links/ElasticDocsLink';

type ScheduleKey = keyof Schedule;

const getUserTimezone = memoize((core: LegacyCoreStart): string => {
return core.uiSettings.get('dateFormat:tz') === 'Browser'
? moment.tz.guess()
: core.uiSettings.get('dateFormat:tz');
});

const SmallInput = styled.div`
.euiFormRow {
max-width: 85px;
Expand Down Expand Up @@ -284,17 +277,13 @@ export class WatcherFlyout extends Component<
return null;
}

const core = this.context;
const userTimezoneSetting = getUserTimezone(core);
const dailyTime = this.state.daily;
const inputTime = `${dailyTime}Z`; // Add tz to make into UTC
const inputFormat = 'HH:mmZ'; // Parse as 24 hour w. tz
const dailyTimeFormatted = moment(inputTime, inputFormat)
.tz(userTimezoneSetting)
.format('HH:mm'); // Format as 24h
const dailyTime12HourFormatted = moment(inputTime, inputFormat)
.tz(userTimezoneSetting)
.format('hh:mm A (z)'); // Format as 12h w. tz
const dailyTimeFormatted = moment(inputTime, inputFormat).format('HH:mm'); // Format as 24h
const dailyTime12HourFormatted = moment(inputTime, inputFormat).format(
'hh:mm A (z)'
); // Format as 12h w. tz

// Generate UTC hours for Daily Report select field
const intervalHours = range(24).map(i => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,16 @@ describe('TimestampTooltip', () => {
// mock Date.now
mockNow(1570737000000);

// hardcode timezone to avoid timezone issues on CI
jest
.spyOn(moment.tz, 'guess')
.mockImplementation(() => 'Europe/Copenhagen');
moment.tz.setDefault('Etc/GMT');
});

afterAll(() => moment.tz.setDefault(''));

it('should render component with relative time in body and absolute time in tooltip', () => {
expect(shallow(<TimestampTooltip time={timestamp} />))
.toMatchInlineSnapshot(`
<EuiToolTip
content="Oct 10th 2019, 17:06:40.123 (+0200 CEST)"
content="Oct 10th 2019, 15:06:40.123 (+0000 GMT)"
delay="regular"
position="top"
>
Expand All @@ -41,22 +40,22 @@ describe('TimestampTooltip', () => {
shallow(<TimestampTooltip time={timestamp} />)
.find('EuiToolTip')
.prop('content')
).toBe('Oct 10th 2019, 17:06:40.123 (+0200 CEST)');
).toBe('Oct 10th 2019, 15:06:40.123 (+0000 GMT)');
});

it('should format with precision in minutes', () => {
expect(
shallow(<TimestampTooltip time={timestamp} precision="minutes" />)
.find('EuiToolTip')
.prop('content')
).toBe('Oct 10th 2019, 17:06 (+0200 CEST)');
).toBe('Oct 10th 2019, 15:06 (+0000 GMT)');
});

it('should format with precision in days', () => {
expect(
shallow(<TimestampTooltip time={timestamp} precision="days" />)
.find('EuiToolTip')
.prop('content')
).toBe('Oct 10th 2019 (+0200 CEST)');
).toBe('Oct 10th 2019 (+0000 GMT)');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function getPreciseTime(precision: Props['precision']) {
}

export function TimestampTooltip({ time, precision = 'milliseconds' }: Props) {
const momentTime = moment.tz(time, moment.tz.guess());
const momentTime = moment(time);
const relativeTimeLabel = momentTime.fromNow();
const absoluteTimeLabel = momentTime.format(
`MMM Do YYYY${getPreciseTime(precision)} (ZZ zz)`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ import PropTypes from 'prop-types';
import React, { PureComponent } from 'react';
import { last } from 'lodash';
import { rgba } from 'polished';
import { scaleUtc } from 'd3-scale';

import StatusText from './StatusText';
import { SharedPlot } from './plotUtils';
import { i18n } from '@kbn/i18n';
import { isValidCoordinateValue } from '../../../../utils/isValidCoordinateValue';
import { getTimezoneOffsetInMs } from './getTimezoneOffsetInMs';

// undefined values are converted by react-vis into NaN when stacking
// see https://github.com/uber/react-vis/issues/1214
Expand All @@ -42,7 +44,7 @@ class StaticPlot extends PureComponent {
<LineSeries
getNull={getNull}
key={serie.title}
xType="time"
xType="time-utc"
curve={'curveMonotoneX'}
data={serie.data}
color={serie.color}
Expand All @@ -54,7 +56,7 @@ class StaticPlot extends PureComponent {
<AreaSeries
getNull={getNull}
key={serie.title}
xType="time"
xType="time-utc"
curve={'curveMonotoneX'}
data={serie.data}
color={serie.color}
Expand All @@ -78,7 +80,7 @@ class StaticPlot extends PureComponent {
<AreaSeries
getNull={getNull}
key={`${serie.title}-area`}
xType="time"
xType="time-utc"
curve={'curveMonotoneX'}
data={data}
color={serie.color}
Expand All @@ -90,7 +92,7 @@ class StaticPlot extends PureComponent {
<LineSeries
getNull={getNull}
key={`${serie.title}-line`}
xType="time"
xType="time-utc"
curve={'curveMonotoneX'}
data={data}
color={serie.color}
Expand All @@ -113,7 +115,7 @@ class StaticPlot extends PureComponent {
<VerticalRectSeries
getNull={getNull}
key={serie.title}
xType="time"
xType="time-utc"
curve={'curveMonotoneX'}
data={data}
color={serie.color}
Expand All @@ -126,7 +128,7 @@ class StaticPlot extends PureComponent {
<LineMarkSeries
getNull={getNull}
key={serie.title}
xType="time"
xType="time-utc"
curve={'curveMonotoneX'}
data={serie.data}
color={serie.color}
Expand All @@ -138,24 +140,42 @@ class StaticPlot extends PureComponent {
}
}

/**
* A tick format function that takes the timezone from Kibana's settings into
* account. Used if no tickFormatX prop is supplied.
*
* This produces the same results as the built-in formatter from D3, which is
* what react-vis uses, but shifts the timezone.
*/
tickFormatXTime = value => {
const xDomain = this.props.plotValues.x.domain();

const time = value.getTime();

return scaleUtc()
.domain(xDomain)
.tickFormat()(new Date(time - getTimezoneOffsetInMs(time)));
};

render() {
const {
width,
series,
tickFormatX,
tickFormatY,
plotValues,
noHits
} = this.props;
const { yTickValues } = plotValues;
const { width, series, tickFormatY, plotValues, noHits } = this.props;
const { xTickValues, yTickValues } = plotValues;

// approximate number of x-axis ticks based on the width of the plot. There should by approx 1 tick per 100px
// d3 will determine the exact number of ticks based on the selected range
const xTickTotal = Math.floor(width / 100);

const tickFormatX = this.props.tickFormatX || this.tickFormatXTime;

return (
<SharedPlot plotValues={plotValues}>
<XAxis tickSize={0} tickTotal={xTickTotal} tickFormat={tickFormatX} />
<XAxis
type="time-utc"
tickSize={0}
tickTotal={xTickTotal}
tickFormat={tickFormatX}
tickValues={xTickValues}
/>
{noHits ? (
<StatusText
marginLeft={30}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { getTimezoneOffsetInMs } from './getTimezoneOffsetInMs';
import moment from 'moment-timezone';

describe('getTimezoneOffsetInMs', () => {
describe('when no default timezone is set', () => {
it('guesses the timezone', () => {
const guess = jest.fn(() => 'Etc/UTC');
jest.spyOn(moment.tz, 'guess').mockImplementationOnce(guess);

getTimezoneOffsetInMs(Date.now());

expect(guess).toHaveBeenCalled();
});
});

describe('when a default timezone is set', () => {
let originalTimezone: moment.MomentZone | null;

beforeAll(() => {
// @ts-ignore moment types do not define defaultZone but it's there
originalTimezone = moment.defaultZone;
moment.tz.setDefault('America/Denver');
});

afterAll(() => {
moment.tz.setDefault(originalTimezone ? originalTimezone.name : '');
});

it('returns the time in milliseconds', () => {
expect(getTimezoneOffsetInMs(Date.now())).toEqual(21600000);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import moment from 'moment-timezone';

export function getTimezoneOffsetInMs(time: number) {
// @ts-ignore moment types don't define defaultZone but it's there
const zone = moment.defaultZone ? moment.defaultZone.name : moment.tz.guess();

// @ts-ignore
return moment.tz.zone(zone).parse(time) * 60000;
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import PropTypes from 'prop-types';
import React from 'react';

import { unit } from '../../../../style/variables';
import { getTimezoneOffsetInMs } from './getTimezoneOffsetInMs';

const XY_HEIGHT = unit * 16;
const XY_MARGIN = {
Expand Down Expand Up @@ -64,15 +65,30 @@ export function getPlotValues(
yMin = d3.min(flattenedCoordinates, d => d.y);
}

const [xMinZone, xMaxZone] = [xMin, xMax].map(x => {
return x - getTimezoneOffsetInMs(x);
});

const xScale = getXScale(xMin, xMax, width);
const yScale = getYScale(yMin, yMax);

const yMaxNice = yScale.domain()[1];
const yTickValues = [0, yMaxNice / 2, yMaxNice];

const xTickValues = d3.time.scale
.utc()
.domain([xMinZone, xMaxZone])
.range([0, width])
.ticks()
.map(x => {
const time = x.getTime();
return new Date(time + getTimezoneOffsetInMs(time));
});

return {
x: xScale,
y: yScale,
xTickValues,
yTickValues,
XY_MARGIN,
XY_HEIGHT: height || XY_HEIGHT,
Expand All @@ -90,7 +106,7 @@ export function SharedPlot({ plotValues, ...props }) {
dontCheckIfEmpty
height={height}
margin={margin}
xType="time"
xType="time-utc"
width={width}
xDomain={plotValues.x.domain()}
yDomain={plotValues.y.domain()}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

// @ts-ignore
import * as plotUtils from './plotUtils';

describe('plotUtils', () => {
describe('getPlotValues', () => {
describe('with empty arguments', () => {
it('returns plotvalues', () => {
expect(
plotUtils.getPlotValues([], [], { height: 1, width: 1 })
).toMatchObject({
XY_HEIGHT: 1,
XY_WIDTH: 1
});
});
});

describe('when yMin is given', () => {
it('uses the yMin in the scale', () => {
expect(
plotUtils
.getPlotValues([], [], { height: 1, width: 1, yMin: 100 })
.y.domain()[0]
).toEqual(100);
});

describe('when yMin is "min"', () => {
it('uses minimum y from the series', () => {
expect(
plotUtils
.getPlotValues(
[{ data: { x: 0, y: 200 } }, { data: { x: 0, y: 300 } }],
[],
{
height: 1,
width: 1,
yMin: 'min'
}
)
.y.domain()[0]
).toEqual(200);
});
});
});

describe('when yMax given', () => {
it('uses yMax', () => {
expect(
plotUtils
.getPlotValues([], [], {
height: 1,
width: 1,
yMax: 500
})
.y.domain()[1]
).toEqual(500);
});
});
});
});
Loading

0 comments on commit 6e4f665

Please sign in to comment.