mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 09:48:58 -04:00
[Lens] Fix timezone used in normalize by unit (#154472)
This commit fixes an issue introduced in
https://github.com/elastic/kibana/pull/142741 where the time bounds were
computed in an expression `time_scale` that can be run on both the
server and the client.
Computing time-related information on each side (server/client), means
that we need to align temporarily with the client timezone to make an
effective calculation.
This temporary alignment had a bug when executed on the client: the
the first time it gets completed, the restored timezone was the wrong
timezone, in particular, the function used to get the current configured
timezone `moment().zoneName()` return abbreviated zone names and in some
cases also non-unique abbreviations (see
https://momentjs.com/timezone/docs/#/using-timezones/formatting/) making
the restoration a bit difficult and problematic.
The fix instead did the following:
- replace the `moment().zoneName()` with `moment.defaultZone?.name` even
this is not typed, this property is exposed by moment [since years
now](2448cdcbe1/moment-timezone.js (L603)
)
and is the only way to get the `defaultZone` configured through the
`setDefault`.
- replace the try/catch/finally block with a more readable
implementation: using a "safe" implementation to update the timezone in
moment we can get rid of the try/catch and we can make it more linear.
fix #154309
The unit test was firstly tested with the old implementation (and it was
failing because the `zoneName` was returning `EDT` which is not a valid
IANA timezone and the timezone were set to `undefined`. With the new
implementation, it returns the specified timezone correctly. I haven't
tested the function itself because I don't know the internal details and
this is also out of the scope of the fix.
This commit is contained in:
parent
79c493c69a
commit
ed4d49b59c
2 changed files with 37 additions and 21 deletions
|
@ -14,6 +14,7 @@ import { functionWrapper } from '@kbn/expressions-plugin/common/expression_funct
|
|||
|
||||
import { getTimeScale } from './time_scale';
|
||||
import type { TimeScaleArgs } from './types';
|
||||
import { getTimeBounds } from './time_scale_fn';
|
||||
|
||||
describe('time_scale', () => {
|
||||
let timeScaleWrapped: (
|
||||
|
@ -503,4 +504,19 @@ describe('time_scale', () => {
|
|||
// should resolve now without another async dependency
|
||||
expect(timeScaleResolved).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('getTimeBounds should not alter the default moment timezone', () => {
|
||||
// configuring an exotic timezone
|
||||
moment.tz.setDefault('Pacific/Honolulu');
|
||||
// @ts-ignore
|
||||
expect(moment.defaultZone?.name).toBe('Pacific/Honolulu');
|
||||
|
||||
getTimeBounds(
|
||||
{ from: '2023-04-01T00:00:00.000+02:00', to: '2023-04-02T00:00:00.000+02:00' },
|
||||
'Europe/Lisbon',
|
||||
() => new Date('2023-04-01T00:00:00.000Z')
|
||||
);
|
||||
// @ts-ignore
|
||||
expect(moment.defaultZone?.name).toBe('Pacific/Honolulu');
|
||||
});
|
||||
});
|
||||
|
|
|
@ -24,31 +24,31 @@ const unitInMs: Record<TimeScaleUnit, number> = {
|
|||
d: 1000 * 60 * 60 * 24,
|
||||
};
|
||||
|
||||
// the datemath plugin always parses dates by using the current default moment time zone.
|
||||
// to use the configured time zone, we are temporary switching it just for the calculation.
|
||||
function safelySetTimeZone(timeZone: string) {
|
||||
const zone = moment.tz.zone(timeZone);
|
||||
if (zone) moment.tz.setDefault(zone.name);
|
||||
}
|
||||
|
||||
// The code between this call and the reset in the finally block is not allowed to get async,
|
||||
// otherwise the timezone setting can leak out of this function.
|
||||
const withChangedTimeZone = <TReturnedValue = unknown>(
|
||||
timeZone: string | undefined,
|
||||
action: () => TReturnedValue
|
||||
): TReturnedValue => {
|
||||
/**
|
||||
* This function can be called both from server side and from the client side. Each of them could have
|
||||
* a different configured timezone. To be sure the time bounds are computed relative to the same passed timezone,
|
||||
* temporarily switch the default moment timezone to the one passed, and switch it back after the calculation is done.
|
||||
*/
|
||||
export function getTimeBounds(timeRange: TimeRange, timeZone?: string, getForceNow?: () => Date) {
|
||||
if (timeZone) {
|
||||
const defaultTimezone = moment().zoneName();
|
||||
try {
|
||||
moment.tz.setDefault(timeZone);
|
||||
return action();
|
||||
} finally {
|
||||
// reset default moment timezone
|
||||
moment.tz.setDefault(defaultTimezone);
|
||||
}
|
||||
// the `defaultZone` property is injected by moment.timezone.
|
||||
// If is not available is it fine to keep undefined because calling setDefault() will automatically reset to default
|
||||
// https://github.com/moment/moment-timezone/blob/2448cdcbe15875bc22ddfbc184794d0a6b568b90/moment-timezone.js#L603
|
||||
// @ts-expect-error because is not part of the exposed types unfortunately
|
||||
const currentDefaultTimeZone = moment.defaultZone?.name;
|
||||
safelySetTimeZone(timeZone);
|
||||
const timeBounds = calculateBounds(timeRange, { forceNow: getForceNow?.() });
|
||||
safelySetTimeZone(currentDefaultTimeZone);
|
||||
return timeBounds;
|
||||
} else {
|
||||
return action();
|
||||
return calculateBounds(timeRange, { forceNow: getForceNow?.() });
|
||||
}
|
||||
};
|
||||
|
||||
const getTimeBounds = (timeRange: TimeRange, timeZone?: string, getForceNow?: () => Date) =>
|
||||
withChangedTimeZone(timeZone, () => calculateBounds(timeRange, { forceNow: getForceNow?.() }));
|
||||
}
|
||||
|
||||
export const timeScaleFn =
|
||||
(
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue