perf: add trace for UI startup (#26636) [cherry-pick]#26925
Merged
Gudahtt merged 5 commits intoVersion-v12.2.0from Sep 6, 2024
Merged
perf: add trace for UI startup (#26636) [cherry-pick]#26925Gudahtt merged 5 commits intoVersion-v12.2.0from
Gudahtt merged 5 commits intoVersion-v12.2.0from
Conversation
6654704 to
baa689c
Compare
Collaborator
Builds ready [e1e4021]
Page Load Metrics (1681 ± 56 ms)
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## Version-v12.2.0 #26925 +/- ##
===================================================
- Coverage 70.49% 70.49% -0.00%
===================================================
Files 1400 1400
Lines 49611 49687 +76
Branches 13647 13655 +8
===================================================
+ Hits 34971 35024 +53
- Misses 14640 14663 +23 ☔ View full report in Codecov by Sentry. |
matthewwalsh0
requested changes
Sep 6, 2024
ui/pages/confirmations/confirm-transaction/confirm-transaction.component.js
Outdated
Show resolved
Hide resolved
Member
There was a problem hiding this comment.
Shouldn't impact the build as it's developer only, though it applies the new TraceName enum.
Member
Author
There was a problem hiding this comment.
Easier to leave this change in I think, to avoid a type error. The trace utility requires the enum to be used. We could adjust it to remove that requirement, but doesn't seem useful to do so
Contributor
|
I have tested this and it works as expected. (But I did not test transactions, as per Matthew's comments above) |
Refactor `trace` utility function to support no callback argument and only start a trace to be manually ended later. Add `endTrace` utility function. Add `endBackgroundTrace` action to contribute to background metrics from the UI. Always trace if in developer build or `METAMASK_DEBUG` is set. Pass `trace` callback and `traceContext` to `TransactionController`.
Support use of `trace` utility function in non-asynchronous contexts. Remove `getMetaMetricsEnabled` check. Generate default ID if omitted when starting and ending traces.
1d34547 to
19daa9f
Compare
Member
Author
|
I have re-tested this locally with the unnecessary code removed, and it seems to work correctly still |
Contributor
|
Just retested as well. LGTM |
danjm
approved these changes
Sep 6, 2024
Collaborator
Builds ready [19daa9f]
Page Load Metrics (1742 ± 103 ms)
|
matthewwalsh0
approved these changes
Sep 6, 2024
Collaborator
|
No release label on PR. Adding release label release-12.2.0 on PR, as PR was cherry-picked in branch 12.2.0. |
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
This is a cherry-pick of #26332 and #26636 for v12.2.0, with the TransactionController update removed from #26332.
Follow these steps to manually test:
metamaskprojectSENTRY_DSN_DEVin your.metamaskrcfileYou should see the UI traces in the Sentry performance dashboard on your personal account.
This was cherry-picked to get the benefits described in #26636 (#26332 was partially included only out of necessity). The original description of #26636 is below:
Description
Record a
UI StartupSentry trace to monitor startup times of the UI pages.In addition:
traceutility function in non-asynchronous contexts.getMetaMetricsEnabledcheck as this is already done in the custom transport before submission.Related issues
Fixes: #25724
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist