-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ref(utils): Clean up timestamp calculation code #10069
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,26 +1,6 @@ | ||
import { dynamicRequire, isNodeEnv } from './node'; | ||
import { getGlobalObject } from './worldwide'; | ||
import { GLOBAL_OBJ } from './worldwide'; | ||
|
||
// eslint-disable-next-line deprecation/deprecation | ||
const WINDOW = getGlobalObject<Window>(); | ||
|
||
/** | ||
* An object that can return the current timestamp in seconds since the UNIX epoch. | ||
*/ | ||
interface TimestampSource { | ||
nowSeconds(): number; | ||
} | ||
|
||
/** | ||
* A TimestampSource implementation for environments that do not support the Performance Web API natively. | ||
* | ||
* Note that this TimestampSource does not use a monotonic clock. A call to `nowSeconds` may return a timestamp earlier | ||
* than a previously returned value. We do not try to emulate a monotonic behavior in order to facilitate debugging. It | ||
* is more obvious to explain "why does my span have negative duration" than "why my spans have zero duration". | ||
*/ | ||
const dateTimestampSource: TimestampSource = { | ||
nowSeconds: () => Date.now() / 1000, | ||
}; | ||
const ONE_SECOND_IN_MS = 1000; | ||
|
||
/** | ||
* A partial definition of the [Performance Web API]{@link https://developer.mozilla.org/en-US/docs/Web/API/Performance} | ||
|
@@ -37,89 +17,56 @@ interface Performance { | |
now(): number; | ||
} | ||
|
||
/** | ||
* Returns a timestamp in seconds since the UNIX epoch using the Date API. | ||
* | ||
* TODO(v8): Return type should be rounded. | ||
*/ | ||
export function dateTimestampInSeconds(): number { | ||
return Date.now() / ONE_SECOND_IN_MS; | ||
} | ||
|
||
/** | ||
* Returns a wrapper around the native Performance API browser implementation, or undefined for browsers that do not | ||
* support the API. | ||
* | ||
* Wrapping the native API works around differences in behavior from different browsers. | ||
*/ | ||
function getBrowserPerformance(): Performance | undefined { | ||
const { performance } = WINDOW; | ||
function createUnixTimestampInSecondsFunc(): () => number { | ||
const { performance } = GLOBAL_OBJ as typeof GLOBAL_OBJ & { performance?: Performance }; | ||
if (!performance || !performance.now) { | ||
return undefined; | ||
return dateTimestampInSeconds; | ||
} | ||
|
||
// Replace performance.timeOrigin with our own timeOrigin based on Date.now(). | ||
// | ||
// This is a partial workaround for browsers reporting performance.timeOrigin such that performance.timeOrigin + | ||
// performance.now() gives a date arbitrarily in the past. | ||
// | ||
// Additionally, computing timeOrigin in this way fills the gap for browsers where performance.timeOrigin is | ||
// undefined. | ||
// | ||
// The assumption that performance.timeOrigin + performance.now() ~= Date.now() is flawed, but we depend on it to | ||
// interact with data coming out of performance entries. | ||
// | ||
// Note that despite recommendations against it in the spec, browsers implement the Performance API with a clock that | ||
// might stop when the computer is asleep (and perhaps under other circumstances). Such behavior causes | ||
// performance.timeOrigin + performance.now() to have an arbitrary skew over Date.now(). In laptop computers, we have | ||
// observed skews that can be as long as days, weeks or months. | ||
// | ||
// See https://github.com/getsentry/sentry-javascript/issues/2590. | ||
// Some browser and environments don't have a timeOrigin, so we fallback to | ||
// using Date.now() to compute the starting time. | ||
const approxStartingTimeOrigin = Date.now() - performance.now(); | ||
const timeOrigin = performance.timeOrigin == undefined ? approxStartingTimeOrigin : 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. | ||
// | ||
// BUG: despite our best intentions, this workaround has its limitations. It mostly addresses timings of pageload | ||
// transactions, but ignores the skew built up over time that can aversely affect timestamps of navigation | ||
// transactions of long-lived web pages. | ||
const timeOrigin = Date.now() - performance.now(); | ||
|
||
return { | ||
now: () => performance.now(), | ||
timeOrigin, | ||
// 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 | ||
return () => { | ||
return (timeOrigin + performance.now()) / ONE_SECOND_IN_MS; | ||
}; | ||
} | ||
|
||
/** | ||
* Returns the native Performance API implementation from Node.js. Returns undefined in old Node.js versions that don't | ||
* implement the API. | ||
*/ | ||
function getNodePerformance(): Performance | undefined { | ||
try { | ||
const perfHooks = dynamicRequire(module, 'perf_hooks') as { performance: Performance }; | ||
return perfHooks.performance; | ||
} catch (_) { | ||
return undefined; | ||
} | ||
} | ||
|
||
/** | ||
* The Performance API implementation for the current platform, if available. | ||
*/ | ||
const platformPerformance: Performance | undefined = isNodeEnv() ? getNodePerformance() : getBrowserPerformance(); | ||
|
||
const timestampSource: TimestampSource = | ||
platformPerformance === undefined | ||
? dateTimestampSource | ||
: { | ||
nowSeconds: () => (platformPerformance.timeOrigin + platformPerformance.now()) / 1000, | ||
}; | ||
|
||
/** | ||
* Returns a timestamp in seconds since the UNIX epoch using the Date API. | ||
*/ | ||
export const dateTimestampInSeconds: () => number = dateTimestampSource.nowSeconds.bind(dateTimestampSource); | ||
|
||
/** | ||
* Returns a timestamp in seconds since the UNIX epoch using either the Performance or Date APIs, depending on the | ||
* availability of the Performance API. | ||
* | ||
* See `usingPerformanceAPI` to test whether the Performance API is used. | ||
* | ||
* 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. | ||
*/ | ||
export const timestampInSeconds: () => number = timestampSource.nowSeconds.bind(timestampSource); | ||
export const timestampInSeconds = createUnixTimestampInSecondsFunc(); | ||
|
||
/** | ||
* Re-exported with an old name for backwards-compatibility. | ||
|
@@ -129,11 +76,6 @@ export const timestampInSeconds: () => number = timestampSource.nowSeconds.bind( | |
*/ | ||
export const timestampWithMs = timestampInSeconds; | ||
|
||
/** | ||
* A boolean that is true when timestampInSeconds uses the Performance API to produce monotonic timestamps. | ||
*/ | ||
export const usingPerformanceAPI = platformPerformance !== undefined; | ||
|
||
/** | ||
* Internal helper to store what is the source of browserPerformanceTimeOrigin below. For debugging only. | ||
*/ | ||
|
@@ -148,7 +90,7 @@ export const browserPerformanceTimeOrigin = ((): number | undefined => { | |
// performance.timing.navigationStart, which results in poor results in performance data. We only treat time origin | ||
// data as reliable if they are within a reasonable threshold of the current time. | ||
|
||
const { performance } = WINDOW; | ||
const { performance } = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window; | ||
if (!performance || !performance.now) { | ||
_browserPerformanceTimeOriginMode = 'none'; | ||
return undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally what we do with |
||
|
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this a new type instead of this type cast?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine to keep esp because it's only being used here.