bugsnag / bugsnag-js-performance

Monitor the performance of your JavaScript (web and React Native) and see the results in your BugSnag dashboard.
https://docs.bugsnag.com/performance/integration-guides
MIT License
5 stars 3 forks source link

[PLAT-12625] Create new app start span wrapper #497

Closed gingerbenw closed 2 months ago

gingerbenw commented 2 months ago

Goal

Provide an alternative method to instrument app start spans aside from setWrapperComponentProvider

Design

Using a higher order component (HOC) will prevent the app from re-rendering when root props are updated from the native side

Changeset

Testing

github-actions[bot] commented 2 months ago

Browser bundle size

NPM build

Package
Before 196.14 kB
After 196.19 kB
± +48 bytes ⚠️

CDN build

Unminified Minfied Minified + gzipped
Before 98.89 kB 36.61 kB 11.07 kB
After 98.89 kB 36.62 kB 11.07 kB
± +8 bytes ⚠️ +8 bytes ⚠️ +7 bytes ⚠️

Code coverage

Ok File (✨=New File) Lines Branches Functions Statements
🔴 ✨ /home/runner/work/bugsnag-js-performance/bugsnag-js-performance/packages/platforms/react-native/lib/create-app-start-span.ts 100%
(+100%)
0%
(+0%)
100%
(+100%)
87.5%
(+87.5%)
🔴 ✨ /home/runner/work/bugsnag-js-performance/bugsnag-js-performance/packages/platforms/react-native/lib/use-end-span-on-mount.ts 40%
(+40%)
0%
(+0%)
0%
(+0%)
50%
(+50%)

Total:

Lines Branches Functions Statements
87.74%(-0.18%) 78.82%(-0.21%) 88.42%(-0.77%) 85.81%(-0.2%)

Generated against 2d964fc25036c3255f511d05b97b26d81ceb20ea on 4 September 2024 at 16:09:59 UTC

yousif-bugsnag commented 2 months ago

generally looks good to me - but I wonder if we should have some mechanism to prevent multiple app start spans from being created?

gingerbenw commented 2 months ago

I wonder if we should have some mechanism to prevent multiple app start spans from being created?

as in, if someone were to use the wrapper and the autoInstrumentAppStarts config option?

yousif-bugsnag commented 2 months ago

as in, if someone were to use the wrapper and the autoInstrumentAppStarts config option?

Yeah - perhaps that's something we just need to document clearly, but seems like it could happen fairly easily because that config option is true by default.

yousif-bugsnag commented 2 months ago

There's also nothing to stop someone wrapping multiple components using withInstrumentedAppStarts - they may even want to, for example an app that loads a login screen or goes straight to the main app screen depending on whether the user is authenticated might want to end the app start on whichever component is loaded first. But that could probably be a separate PR/future enhancement...

gingerbenw commented 2 months ago

@yousif-bugsnag added a simple check that only allows for a single app start span - create as many wrappers as you like 👍🏻