-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat: Add performance tracing infrastructure #11132
feat: Add performance tracing infrastructure #11132
Conversation
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@sentry/types@6.3.1 |
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.
Left some comments
app/components/Views/Settings/DeveloperOptions/DeveloperOptions.types.ts
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11132 +/- ##
==========================================
+ Coverage 52.75% 55.23% +2.47%
==========================================
Files 1534 1558 +24
Lines 36777 37227 +450
Branches 4335 4414 +79
==========================================
+ Hits 19403 20561 +1158
- Misses 16058 16194 +136
+ Partials 1316 472 -844 ☔ View full report in Codecov by Sentry. |
3481ff3
to
5e8c448
Compare
b1ba884
to
947a51f
Compare
Bitrise✅✅✅ Commit hash: 3cc85dc Note
|
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.
Nice, LGTM. Feel free to reach out for another approval once the conflicts are fixed.
Bitrise✅✅✅ Commit hash: 6b55a47 Note
|
Quality Gate passedIssues Measures |
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.
lgtm
perhaps like https://github.com/MetaMask/metamask-extension/blob/develop/development/README.md#sentry
we should add a Sentry section here https://github.com/MetaMask/metamask-mobile/blob/main/docs/readme/testing.md
with heading e.g. Generating a trace test
that mentions our new env vars, with the odd screenshot or two if it helps
Description
Add the
trace
,endTrace
methods, to be used anywhere in the mobile code to generate a Sentry traces. Implementation is almost same as we have in the extension. The only difference is in extensionwithIsolatedScope
, in mobilewithScope
is used because@sentry/browser
version mismatch in@sentry/react-native
. This is also noted in thetrace.ts
file.This adds an easily accessible development action menu where devs wants to test some functionality locally. Specifically this PR adds generating nested traces in Sentry.
MM_ENABLE_SETTINGS_PAGE_DEV_OPTIONS
: Needs to be set totrue
in order to showDeveloper Options
in the settings pageMM_SENTRY_DSN_DEV
: This is explicitly set fortest-metamask
project and the expected value is in the.js.env.example
Related issues
Fixes: https://github.com/MetaMask/mobile-planning/issues/1877
Manual testing steps
Developer Options
Generate Trace Test
https://metamask.sentry.io/traces/?project=273496&statsPeriod=15m
, in this link you will be able to see traces that have been send totest-metamask
in last 15 minutesDeveloper Test
trace is triggeredScreenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist