Repository navigation
fix(core): Compute span end time from performance.now() duration - #24903
Merged
Lms24 merged 6 commits intoOct 7, 2026
Conversation
Lms24
added this pull request to stack #24904
September 30, 2026 15:21
Contributor
size-limit report 📦
|
Lms24
force-pushed
the
lms/fix-core-span-end-monotonic-duration
branch
from
October 2, 2026 09:44
d35bfbf to
9e95d55
Compare
Lms24
marked this pull request as ready for review
October 2, 2026 10:13
Lms24
force-pushed
the
lms/fix-core-span-end-monotonic-duration
branch
from
October 2, 2026 10:18
264fffd to
779d31b
Compare
Lms24
force-pushed
the
lms/fix-core-span-end-monotonic-duration
branch
from
October 2, 2026 14:13
779d31b to
8b5f7d7
Compare
JPeer264
approved these changes
Oct 5, 2026
| /** | ||
| * Returns `performance.now()` in milliseconds, or `undefined` if the Performance API is unavailable. | ||
| */ | ||
| export function safePerformanceNow(): number | undefined { |
Member
There was a problem hiding this comment.
q: Should we make an oxlint rule to not use performance.now? For server runtimes it wouldn't be necessary, but for core and browser it might make sense
Member
Author
There was a problem hiding this comment.
For server runtimes it wouldn't be necessary
it's necessary specifically for server. The reason for the withRandomSafeContext wrapper is that NextJS cached components break when calling random functions without this special context hack.
Member
Author
There was a problem hiding this comment.
RE lint rule: Could make sense, but I'd do it separately. Also I think we already thought about this when we introduced withRandomSafeContext. Will check with Charly and Awad who implemented this originally.
Member
Author
There was a problem hiding this comment.
Oh, turns out we already have this rule: no-unsafe-random-apis. However, it's missing some syntax variations at the moment. Will open a separate PR.
logaretm
approved these changes
Oct 5, 2026
logaretm
left a comment
logaretm
left a comment
Member
There was a problem hiding this comment.
LGTM! I think we talked about this, it's either we get correct timings or accurate durations. So, I think this is the right decision here.
Lms24
force-pushed
the
lms/fix-core-span-end-monotonic-duration
branch
2 times, most recently
from
October 6, 2026 10:01
f3ec383 to
b3c446c
Compare
Lms24
force-pushed
the
lms/fix-core-span-end-monotonic-duration
branch
3 times, most recently
from
October 6, 2026 17:24
5489794 to
b573c7c
Compare
Lms24
force-pushed
the
lms/fix-core-span-end-monotonic-duration
branch
from
October 7, 2026 08:34
b573c7c to
8e8641b
Compare
Lms24
force-pushed
the
lms/fix-core-span-end-monotonic-duration
branch
from
October 7, 2026 09:23
1dc236f to
50ac6a9
Compare
Spans that run while the time origin is reset (e.g. after the device slept) got the drift added to their duration, which could even be negative when the wall clock jumped backwards. Now `span.end()` without a timestamp adds the `performance.now()` time since the span started to the start time, like OpenTelemetry does. Spans with an explicit start time keep using `timestampInSeconds()` for the end. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Use loose null checks when deciding whether to compute the span end time from the performance.now() duration, and move the misplaced _onSpanEnded doc comment back to its method. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Lms24
force-pushed
the
lms/fix-core-span-end-monotonic-duration
branch
from
October 7, 2026 11:58
50ac6a9 to
54b7759
Compare
Lms24
added a commit
that referenced
this pull request
Oct 7, 2026
This PR fixes* clock drift that occurs when devices go to sleep. After devices sleep for a few seconds, the browser's monotonic clock (accessed via `performance.now()`) stops counting. Once the sleep stops, the monotonic clock resumes right where it left off before the sleep, causing significant discrepancies between the actual time (wall clock) and the browser's monotonic clock time. Or in other words, there are two ways to get an absolute timestamp: - Using the monotonic clock via `peformance.timeOrigin + performance.now()` - has high, sub-millisecond precision and a guarantee that there are no time jumps or adjustments. But suffers from the sleep problem described above - Using the wall clock via `Date.now()` - lower millisecond precision, is known to be corrected (NTP or via users) and can even cause back jumps. But no sleep problem. With this fix, we make the following adjustments, to kinda get the best of both worlds: - Every `timestampInSecondsCall` checks the monotonic against the wall clock. If a threshold of difference is enocuntered, it corrects the `timeOrigin` so that we can keep using the precise monotonic clock, but anchor its relative time to a corrected time origin. - On every time origin correction, we remember the previous origin (up to 30 corrections). Needed for performance entries - Makes `browserPerformanceTimeOrigin` a time origin corrected helper function, where you pass in a relative time that comes from monotonic clock relative timestamps and it returns the time origin that was most accurate at that relative time point. - All spans and other telemetry we create from browsers `PerformanceEntry` objects carry relative times which are completely sleep-drift uncorrected. We use `browserPerformanceTimeOrigin` to return a corrected time stamp so that we can create an absolute timestamp and bring the performance entries to their respective actual time. ### FAQ If you think this sounds complicated, I agree. So let me answer the most obvious questions, because I asked myself these a lot, too: **Why not just always use `Date.now()` and avoid the complicated click drift detection and correction logic?** The main issue with this is that we still need to rely on relative monotonic clock timestamps for performance entries. We cannot correct them just with `Date.now()` alone but we still need to anchor them. So either we rely on the original `window.timeOrigin` and therefore have all performance entry telemetry happen much "earlier" than its surrounding telemetry relying on `Date.now()`, or we keep the time origin correction logic. But even in the second case (where we already pay the tax for drift detection and correction), telemetry would still have two different anchors: `Date.now()` for regular telemetry and the corrected time origin + monotonic time for performance entry telemetry. Another reason is that the monotonic clock gives us sun-millisecond precision while the wall clock stops at a millisecond resolution. **Why the reduction from a drift detection threshold of 5 minutes to just one second?** Because we keep everything centered around detection and origin offset correction, these timestamps need to be as accurate as possible. On phones, short frequent sleeps are very likely to happen. A lot of sleeps can accumulate until that 5 minutes threshold is reached. So it's really important we make these timestamps as accurate as possible. Fwiw, I reproduced this locally and the drift is already noticeable after just a few seconds of sleep. The 5 minutes threshold was arguably far too big beforehand. **Is there precedence for all of this stuff?** Somewhat, but I'd argue we're doing a bit more: OTel uses a mixture of `Date.now()` and `performance.now`: - Spans start at `Date.now()` and at start time, they record a `performance.now()` timestamp. On span end, they also take `performance.now()`, compute the diff and convert that to an end timestamp based on the start timestamp + diff. This is neat because it guarantees monotony in span durations. I stole this in #24903 In other places, OTel relies purely on Date.now(), and for performance entries, they simply take uncorrected timestamps. So our fix is more complete. \* **So... we're good now?** Well, not perfectly and we never will. With this choice we make another commitment (which we already did previously in less obvious cases) to `Date.now()`. This value can drift as well, just not for sleeps: - NTP adjustments: Can make hard correction of the wall clock, or make the clock tick just a bit faster or slower until the device wall clock synced with the network time. - User adjustments: Users can adjust the device time at any time into any direction. I think we'll have to live with both and I'm not particularly worried about them. My main objective is getting rid of the sleep drift. Fixes #2590 Supersedes #22488, #22585, #23067, #23068 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Lms24
added a commit
that referenced
this pull request
Oct 7, 2026
Effect spans used Effect's own clock (a fixed origin plus a monotonic clock) for their start, end and event times. #23054 makes Sentry correct its clock for drift (e.g. after the device slept), so Effect spans could end up shifted by the drift relative to the Sentry spans around them. With this PR, an Effect span now starts on Sentry's clock, and its end and event times are converted by their offset from the span's start. This is the same per-span offset that OpenTelemetry's SDK uses. We also do this for our spans now, see #24903 for details. The PR keeps Effect's durations and any end time passed explicitly via `span.end(time, exit)`. With `withTracerTiming(false)`, Effect passes `0`, and we now use Sentry's clock instead of setting 1970 timestamps. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After Spans #23054, spans that run while the time origin is corrected (e.g. after the device slept) get the drift added to their duration. However, If the wall clock jumps backwards, the duration can even be negative. Also, adding sleep time to spans is arguably unexpected and unprecedented in comparison to OpenTelemetry.
This PR changes s
span.end()without a explicit timestamp to compute the end as start time +performance.now()time since the start, like OpenTelemetry does. Start times still usetimestampInSeconds(), so real-time started spans and spans from performance entries stay on the same timeline.Tradeoff: when a correction happens while a span runs, its end is now based on the old time origin. So children or errors recorded after the correction can appear after the span ended. Spans with an explicit start time keep using
timestampInSeconds()for the end.