fix(core): Account for clock drift on every timestampInSeconds call

Author: Lms24

packages/core/src/utils/time.ts

@@ -3,6 +3,12 @@ import { GLOBAL_OBJ } from './worldwide';
 
 const ONE_SECOND_IN_MS = 1000;
 
+/**
+ * Maximum tolerated difference between the monotonic clock and the wall clock before we consider
+ * the monotonic clock's time origin stale.
+ */
+const CLOCK_DRIFT_THRESHOLD_MS = 300_000; // 5 minutes in milliseconds
+
 /**
  * A partial definition of the [Performance Web API]{@link https://developer.mozilla.org/en-US/docs/Web/API/Performance}
  * for accessing a high-resolution monotonic clock.
@@ -39,19 +45,32 @@ function createUnixTimestampInSecondsFunc(): () => number {
     return dateTimestampInSeconds;
   }
 
-  const timeOrigin = performance.timeOrigin;
-
   // performance.now() is a monotonic clock, which means it starts at 0 when the process begins. To get the current
   // wall clock time (actual UNIX timestamp), we need to add the starting time origin and the current time elapsed.
-  //
-  // TODO: This does not account for the case where the monotonic clock that powers performance.now() drifts from the
-  // wall clock time, which causes the returned timestamp to be inaccurate. We should investigate how to detect and
-  // correct for this.
-  // See: https://github.com/getsentry/sentry-javascript/issues/2590
-  // See: https://github.com/mdn/content/issues/4713
-  // See: https://dev.to/noamr/when-a-millisecond-is-not-a-millisecond-3h6
+  let timeOrigin = performance.timeOrigin;
+
   return () => {
-    return (timeOrigin + withRandomSafeContext(() => performance.now())) / ONE_SECOND_IN_MS;
+    return withRandomSafeContext(() => {
+      const performanceNow = performance.now();
+      const dateNow = Date.now();
+
+      // `timeOrigin + performance.now()` only equals wall clock time for as long as both clocks advance in lockstep.
+      // performance.now() stops advancing while the device is asleep, so it under-counts elapsed wall time; conversely
+      // the wall clock itself can be stepped by Network Time Protocol (NTP) or the user. Either way the two drift apart
+      // by arbitrary amounts. Re-deriving the origin restores absolute accuracy while still taking elapsed time from
+      // the monotonic clock, so durations keep sub-millisecond precision and cannot run backwards.
+      // Timestamps taken before a correction are measured against a different origin than those taken after it, so a
+      // span that starts before one and ends after it absorbs the drift into its duration. Spans that lie entirely on
+      // one side of a correction are unaffected.
+      // See: https://github.com/getsentry/sentry-javascript/issues/2590
+      // See: https://github.com/mdn/content/issues/4713
+      // See: https://dev.to/noamr/when-a-millisecond-is-not-a-millisecond-3h6
+      if (Math.abs(timeOrigin + performanceNow - dateNow) > CLOCK_DRIFT_THRESHOLD_MS) {
+        timeOrigin = dateNow - performanceNow;
+      }
+
+      return (timeOrigin + performanceNow) / ONE_SECOND_IN_MS;
+    });
   };
 }
 
@@ -61,10 +80,12 @@ let _cachedTimestampInSeconds: (() => number) | undefined;
  * Returns a timestamp in seconds since the UNIX epoch using either the Performance or Date APIs, depending on the
  * availability of the Performance API.
  *
- * BUG: Note that because of how browsers implement the Performance API, the clock might stop when the computer is
- * asleep. This creates a skew between `dateTimestampInSeconds` and `timestampInSeconds`. The
- * skew can grow to arbitrary amounts like days, weeks or months.
- * See https://github.com/getsentry/sentry-javascript/issues/2590.
+ * Because the Performance API's clock and the wall clock can drift apart (the former stops while the computer is
+ * asleep, the latter can be stepped by NTP or the user), the time origin they are combined against is re-derived from
+ * `Date.now()` whenever the two disagree by more than {@link CLOCK_DRIFT_THRESHOLD_MS}. Two timestamps taken on either
+ * side of such a correction are skewed relative to each other by the amount of drift, so a span that starts before a
+ * correction and ends after it reports the wall clock time elapsed rather than the time the monotonic clock was
+ * running. See https://github.com/getsentry/sentry-javascript/issues/2590.
  */
 export function timestampInSeconds(): number {
   // We store this in a closure so that we don't have to create a new function every time this is called.
@@ -92,14 +113,13 @@ function getBrowserTimeOrigin(): number | undefined {
     return undefined;
   }
 
-  const threshold = 300_000; // 5 minutes in milliseconds
   const performanceNow = withRandomSafeContext(() => performance.now());
   const dateNow = safeDateNow();
 
   const timeOrigin = performance.timeOrigin;
   if (typeof timeOrigin === 'number') {
     const timeOriginDelta = Math.abs(timeOrigin + performanceNow - dateNow);
-    if (timeOriginDelta < threshold) {
+    if (timeOriginDelta < CLOCK_DRIFT_THRESHOLD_MS) {
       return timeOrigin;
     }
   }

packages/core/test/lib/utils/time.test.ts

@@ -1,4 +1,4 @@
-import { describe, expect, it, vi } from 'vitest';
+import { afterEach, describe, expect, it, vi } from 'vitest';
 
 async function getFreshPerformanceTimeOrigin() {
   // Adding the query param with the date, forces a fresh import each time this is called
@@ -7,8 +7,179 @@ async function getFreshPerformanceTimeOrigin() {
   return timeModule.browserPerformanceTimeOrigin();
 }
 
+let freshImportCounter = 0;
+
+async function getFreshTimestampInSeconds(): Promise<() => number> {
+  // A counter rather than `Date.now()`: these tests run under fake timers, which freeze the wall clock and would
+  // otherwise hand out a cached module.
+  const timeModule = await import(`../../../src/utils/time?update=${freshImportCounter++}`);
+  return timeModule.timestampInSeconds;
+}
+
 const RELIABLE_THRESHOLD_MS = 300_000;
 
+describe('timestampInSeconds', () => {
+  afterEach(() => {
+    vi.useRealTimers();
+    vi.unstubAllGlobals();
+  });
+
+  it('derives the timestamp from `performance.timeOrigin` and `performance.now()`', async () => {
+    const currentTimeMs = 1767778040866;
+    const timeSincePageloadMs = 1_234.56789;
+
+    vi.useFakeTimers();
+    vi.setSystemTime(new Date(currentTimeMs));
+    vi.stubGlobal('performance', {
+      timeOrigin: currentTimeMs - timeSincePageloadMs,
+      now: () => timeSincePageloadMs,
+    });
+
+    const timestampInSeconds = await getFreshTimestampInSeconds();
+
+    expect(timestampInSeconds()).toBe(currentTimeMs / 1000);
+  });
+
+  it('falls back to `Date.now()` if the performance API is unavailable', async () => {
+    const currentTimeMs = 1767778040866;
+
+    vi.useFakeTimers();
+    vi.setSystemTime(new Date(currentTimeMs));
+    vi.stubGlobal('performance', undefined);
+
+    const timestampInSeconds = await getFreshTimestampInSeconds();
+
+    expect(timestampInSeconds()).toBe(currentTimeMs / 1000);
+  });
+
+  it('keeps using `performance.timeOrigin` while the clocks agree', async () => {
+    const currentTimeMs = 1767778040866;
+    // Below the drift threshold, so the (inaccurate) time origin must be preserved.
+    const timeOriginSkewMs = RELIABLE_THRESHOLD_MS - 2_000;
+
+    let timeSincePageloadMs = 1_000;
+
+    vi.useFakeTimers();
+    vi.setSystemTime(new Date(currentTimeMs));
+    vi.stubGlobal('performance', {
+      timeOrigin: currentTimeMs - timeSincePageloadMs + timeOriginSkewMs,
+      now: () => timeSincePageloadMs,
+    });
+
+    const timestampInSeconds = await getFreshTimestampInSeconds();
+
+    expect(timestampInSeconds()).toBe((currentTimeMs + timeOriginSkewMs) / 1000);
+
+    timeSincePageloadMs = 5_000;
+    vi.setSystemTime(new Date(currentTimeMs + 4_000));
+
+    expect(timestampInSeconds()).toBe((currentTimeMs + 4_000 + timeOriginSkewMs) / 1000);
+  });
+
+  it('re-derives the time origin once the monotonic clock drifts from the wall clock', async () => {
+    const currentTimeMs = 1767778040866;
+    const timeSincePageloadMs = 1_000;
+
+    // The monotonic clock pauses during sleep, so the wall clock advances much further than it does.
+    const sleepDurationMs = RELIABLE_THRESHOLD_MS + 60_000;
+
+    vi.useFakeTimers();
+    vi.setSystemTime(new Date(currentTimeMs));
+    vi.stubGlobal('performance', {
+      timeOrigin: currentTimeMs - timeSincePageloadMs,
+      now: () => timeSincePageloadMs,
+    });
+
+    const timestampInSeconds = await getFreshTimestampInSeconds();
+
+    expect(timestampInSeconds()).toBe(currentTimeMs / 1000);
+
+    vi.setSystemTime(new Date(currentTimeMs + sleepDurationMs));
+
+    expect(timestampInSeconds()).toBe((currentTimeMs + sleepDurationMs) / 1000);
+  });
+
+  it('keeps deriving elapsed time from the monotonic clock after re-deriving the time origin', async () => {
+    const currentTimeMs = 1767778040866;
+    const sleepDurationMs = RELIABLE_THRESHOLD_MS + 60_000;
+
+    let timeSincePageloadMs = 1_000;
+
+    vi.useFakeTimers();
+    vi.setSystemTime(new Date(currentTimeMs));
+    vi.stubGlobal('performance', {
+      timeOrigin: currentTimeMs - timeSincePageloadMs,
+      now: () => timeSincePageloadMs,
+    });
+
+    const timestampInSeconds = await getFreshTimestampInSeconds();
+
+    timestampInSeconds();
+    vi.setSystemTime(new Date(currentTimeMs + sleepDurationMs));
+    const afterCorrection = timestampInSeconds();
+
+    // `Date.now()` deliberately stays put while the monotonic clock advances sub-millisecond, proving the elapsed
+    // time comes from `performance.now()` rather than from the coarser wall clock.
+    timeSincePageloadMs += 0.25;
+    expect(timestampInSeconds()).toBeCloseTo(afterCorrection + 0.25 / 1000, 10);
+  });
+
+  it('does not re-derive the time origin repeatedly once the clocks agree again', async () => {
+    const currentTimeMs = 1767778040866;
+    const sleepDurationMs = RELIABLE_THRESHOLD_MS + 60_000;
+
+    let timeSincePageloadMs = 1_000;
+
+    vi.useFakeTimers();
+    vi.setSystemTime(new Date(currentTimeMs));
+    vi.stubGlobal('performance', {
+      timeOrigin: currentTimeMs - timeSincePageloadMs,
+      now: () => timeSincePageloadMs,
+    });
+
+    const timestampInSeconds = await getFreshTimestampInSeconds();
+
+    timestampInSeconds();
+    vi.setSystemTime(new Date(currentTimeMs + sleepDurationMs));
+    timestampInSeconds();
+
+    // Advance both clocks in lockstep: the re-derived time origin must stay valid, so timestamps track the wall clock
+    // exactly rather than oscillating between the two sources.
+    for (let i = 1; i <= 3; i++) {
+      timeSincePageloadMs += 1_000;
+      vi.setSystemTime(new Date(currentTimeMs + sleepDurationMs + i * 1_000));
+      expect(timestampInSeconds()).toBe((currentTimeMs + sleepDurationMs + i * 1_000) / 1000);
+    }
+  });
+
+  it('produces monotonically increasing timestamps when the wall clock steps backwards', async () => {
+    const currentTimeMs = 1767778040866;
+
+    let timeSincePageloadMs = 1_000;
+
+    vi.useFakeTimers();
+    vi.setSystemTime(new Date(currentTimeMs));
+    vi.stubGlobal('performance', {
+      timeOrigin: currentTimeMs - timeSincePageloadMs,
+      now: () => timeSincePageloadMs,
+    });
+
+    const timestampInSeconds = await getFreshTimestampInSeconds();
+
+    const before = timestampInSeconds();
+
+    // A backwards wall clock step (NTP correction, user changing the clock) beyond the threshold.
+    vi.setSystemTime(new Date(currentTimeMs - RELIABLE_THRESHOLD_MS - 60_000));
+    timeSincePageloadMs += 1_000;
+    const afterStep = timestampInSeconds();
+
+    // The correction itself moves the timestamp backwards, but elapsed time afterwards is still monotonic.
+    timeSincePageloadMs += 1_000;
+    expect(timestampInSeconds()).toBeGreaterThan(afterStep);
+    expect(before).toBeGreaterThan(afterStep);
+  });
+});
+
 describe('browserPerformanceTimeOrigin', () => {
   it('returns `performance.timeOrigin` if it is available and reliable', async () => {
     const timeOrigin = await getFreshPerformanceTimeOrigin();