Skip to content
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

fix: Add a wrapper around performance for React Native #2915

Merged
merged 2 commits into from
Sep 18, 2020

Conversation

jennmueng
Copy link
Member

Calling performance.now() in React Native seems to include some large offset even though it is relatively precise. This causes gigantic timestamp jumps in React Native, along with breadcrumbs being completely out of order.

This is solved by having a unique wrapper for performance on React Native. The initial offset is set at the start, and any subsequent calls to performance.now() will have the offset subtracted. This solution should work short of finding the source of this offset and submitting a PR to React Native; even if it does eventually get fixed, this solution should still work as the initial offset would then be 0 or close to 0.

https://sentry.slack.com/archives/C01354JB6ES/p1600273809119300

Related Issues:
#2590
getsentry/sentry-react-native#1004

Asana Task:
https://app.asana.com/0/1163978839534255/1193837000315289/f

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy this addressed the issue but sounds like a big in RN.
Is it the case? If so, is it tracked on their repo?

@jennmueng
Copy link
Member Author

Happy this addressed the issue but sounds like a big in RN.
Is it the case? If so, is it tracked on their repo?

From my investigation, no, I should make an issue there later today.

@github-actions
Copy link
Contributor

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 17.94 KB (+0.31% 🔺)
@sentry/browser - Webpack 18.79 KB (+0.33% 🔺)
@sentry/react - Webpack 18.79 KB (+0.33% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 23.73 KB (+0.23% 🔺)

Copy link
Contributor

@rhcarvalho rhcarvalho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jennmueng. This is fine to unblock RN. Long term (which I hope will be soon) I want to address #2590 and this patch should become unnecessary 🤞 .

@danmaas
Copy link

danmaas commented Sep 17, 2020

Thanks for this fix!

In our experience with deploying a React Native app on mobile devices in the real world, any attempt to find an absolute time reference has been very unreliable. So I don't mind that you are calculating and storing an offset.

@fedyk
Copy link

fedyk commented Sep 18, 2020

Hey, I am using Expo to react-native project. After today update I noticed an exception in misc.ts:

ReferenceError: Can't find variable: performance

getReactNativePerformanceWrapper
    misc.js:203:9
<anonymous>
    misc.js:214:25
<anonymous>
    misc.js:241:4
loadModuleImplementation
    require.js:321:11
<unknown>
    AppEntry.bundle?platform=ios&dev=true&minify=false&hot=false:136399:26
loadModuleImplementation
    require.js:321:11
<unknown>
    AppEntry.bundle?platform=ios&dev=true&minify=false&hot=false:136003:34
loadModuleImplementation
    require.js:321:11
<unknown>
    AppEntry.bundle?platform=ios&dev=true&minify=false&hot=false:135646:27
loadModuleImplementation
    require.js:321:11
<unknown>
    AppEntry.bundle?platform=ios&dev=true&minify=false&hot=false:135624:27
loadModuleImplementation
    require.js:321:11
<unknown>
    AppEntry.bundle?platform=ios&dev=true&minify=false&hot=false:134963:25
loadModuleImplementation
    require.js:321:11
<unknown>
    AppEntry.bundle?platform=ios&dev=true&minify=false&hot=false:134903:29
loadModuleImplementation
    require.js:321:11
<unknown>
    AppEntry.bundle?platform=ios&dev=true&minify=false&hot=false:134766:25
loadModuleImplementation
    require.js:321:11
<unknown>
    AppEntry.bundle?platform=ios&dev=true&minify=false&hot=false:134428:50
loadModuleImplementation
    require.js:321:11
<unknown>
    AppEntry.bundle?platform=ios&dev=true&minify=false&hot=false:134095:50
loadModuleImplementation
    require.js:321:11
<unknown>
    AppEntry.bundle?platform=ios&dev=true&minify=false&hot=false:134053:47
loadModuleImplementation
    require.js:321:11
<global>
    AppEntry.js:3
loadModuleImplementation
    require.js:321:11
guardedLoadModule
    require.js:201:44
global code
    AppEntry.bundle?platform=ios&dev=true&minify=false&hot=false:225462:3

Does anyone know if performance.now() is widely available in react-native?

@jennmueng
Copy link
Member Author

@fedyk It isn't available until >=0.63.0. My mistake and forgot to check that it exists before calling it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants