-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat(tracing): make long animation frames opt-out #13255
Conversation
a lot of failing tests due to an undefined PerformanceObserver? Wonder if we need to add a mockPerformanceObserver to all the tests or if there's a better solution |
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 took a look at the failing tests and looks like there are two classes of test fails:
- fails because
PerformanceObserver
is undefined in unit tests. I think you're right, this is likely because certain global APIs likePerformanceObserver
are missing in the test environment. But it surfaces a good point: We should guard referencingPerformanceObserver
because we should be defensive about it not being present (for example if people use the SDK in web workers). - fails in integration tests because long animation frame spans are added to transaction events where we don't expect them. I think here we should just check that what we receive is correct and adjust the tests as necessary.
size-limit report 📦
|
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.
Looks good to me! Thanks for adjusting the tests!
Enable long animation frames by default. If long animation frames are not supported by the browser, Sentry will fallback to using long tasks where enabled.