From c93ab9e84edfffbae7e63894f6c960262a8c299e Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Tue, 3 Jan 2023 23:53:25 +0800 Subject: [PATCH] fix(core): fix precision loss in numberToHrtime (#3480) --- CHANGELOG.md | 1 + packages/opentelemetry-core/src/common/time.ts | 16 ++++------------ .../test/common/time.test.ts | 18 +++++++++++++++++- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf293f3546..65b5e2e83e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/ * `telemetry.sdk.version` * fix(selenium-tests): updated webpack version for selenium test issue [#3456](https://github.com/open-telemetry/opentelemetry-js/issues/3456) @SaumyaBhushan * fix(sdk-metrics): fix duplicated registration of metrics for collectors [#3488](https://github.com/open-telemetry/opentelemetry-js/pull/3488) @legendecas +* fix(core): fix precision loss in numberToHrtime [#3480](https://github.com/open-telemetry/opentelemetry-js/pull/3480) @legendecas ### :books: (Refine Doc) diff --git a/packages/opentelemetry-core/src/common/time.ts b/packages/opentelemetry-core/src/common/time.ts index 64bc64d2d9..aa94aefec9 100644 --- a/packages/opentelemetry-core/src/common/time.ts +++ b/packages/opentelemetry-core/src/common/time.ts @@ -19,18 +19,12 @@ import { otperformance as performance } from '../platform'; import { TimeOriginLegacy } from './types'; const NANOSECOND_DIGITS = 9; +const NANOSECOND_DIGITS_IN_MILLIS = 6; +const MILLISECONDS_TO_NANOSECONDS = Math.pow(10, NANOSECOND_DIGITS_IN_MILLIS); const SECOND_TO_NANOSECONDS = Math.pow(10, NANOSECOND_DIGITS); /** - * Converts a number to HrTime, HrTime = [number, number]. - * The first number is UNIX Epoch time in seconds since 00:00:00 UTC on 1 January 1970. - * The second number represents the partial second elapsed since Unix Epoch time represented by first number in nanoseconds. - * For example, 2021-01-01T12:30:10.150Z in UNIX Epoch time in milliseconds is represented as 1609504210150. - * numberToHrtime calculates the first number by converting and truncating the Epoch time in milliseconds to seconds: - * HrTime[0] = Math.trunc(1609504210150 / 1000) = 1609504210. - * numberToHrtime calculates the second number by converting the digits after the decimal point of the subtraction, (1609504210150 / 1000) - HrTime[0], to nanoseconds: - * HrTime[1] = Number((1609504210.150 - HrTime[0]).toFixed(9)) * SECOND_TO_NANOSECONDS = 150000000. - * This is represented in HrTime format as [1609504210, 150000000]. + * Converts a number of milliseconds from epoch to HrTime([seconds, remainder in nanoseconds]). * @param epochMillis */ function numberToHrtime(epochMillis: number): api.HrTime { @@ -38,9 +32,7 @@ function numberToHrtime(epochMillis: number): api.HrTime { // Decimals only. const seconds = Math.trunc(epochSeconds); // Round sub-nanosecond accuracy to nanosecond. - const nanos = - Number((epochSeconds - seconds).toFixed(NANOSECOND_DIGITS)) * - SECOND_TO_NANOSECONDS; + const nanos = Math.round((epochMillis % 1000) * MILLISECONDS_TO_NANOSECONDS); return [seconds, nanos]; } diff --git a/packages/opentelemetry-core/test/common/time.test.ts b/packages/opentelemetry-core/test/common/time.test.ts index a8c284b9b0..d0b933e7a1 100644 --- a/packages/opentelemetry-core/test/common/time.test.ts +++ b/packages/opentelemetry-core/test/common/time.test.ts @@ -105,7 +105,7 @@ describe('time', () => { it('should convert Date hrTime', () => { const timeInput = new Date(1609297640313); const output = timeInputToHrTime(timeInput); - assert.deepStrictEqual(output, [1609297640, 312999964]); + assert.deepStrictEqual(output, [1609297640, 313000000]); }); it('should convert epoch milliseconds hrTime', () => { @@ -114,6 +114,22 @@ describe('time', () => { assert.deepStrictEqual(output[0], Math.trunc(timeInput / 1000)); }); + it('should convert arbitrary epoch milliseconds (with sub-millis precision) hrTime', () => { + sinon.stub(performance, 'timeOrigin').value(111.5); + const inputs = [ + // [ input, expected ] + [1609297640313, [1609297640, 313000000]], + // inevitable precision loss without decimal arithmetics. + [1609297640313.333, [1609297640, 313333008]], + // eslint-disable-next-line @typescript-eslint/no-loss-of-precision + [1609297640313.333333333, [1609297640, 313333252]], + ] as const; + for (const [idx, input] of inputs.entries()) { + const output = timeInputToHrTime(input[0]); + assert.deepStrictEqual(output, input[1], `input[${idx}]: ${input}`); + } + }); + it('should convert performance.now() hrTime', () => { sinon.stub(performance, 'timeOrigin').value(111.5);