Repository navigation
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
This branch has not been deployed
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.