chore(runway): cherry-pick chore: add advanced charts traces cp-7.76.0#29792
Merged
Conversation
#29497) ## **Description** Adds Sentry performance traces for token-overview advanced chart visibility (time from series change until skeleton clears), with separate trace types for dashboards to distinguish initial load / asset or currency changes from time-range-only updates. Also fixes a bug in metrics consent detection during early Sentry initialization. ## Changes ### Advanced Chart Performance Tracing **AdvancedChart component:** - Added optional `onSkeletonHidden` callback that fires once when loading/layout settles and the chart is ready - Guarded by refs to reset when series key or HTML content changes **PriceAdvanced component:** - Starts `trace()` when `ohlcvSeriesKey` changes - Ends trace via `endTrace()` when: - Skeleton hides (chart ready) - Chart error occurs - Trace is superseded by a newer series - Falls back to legacy chart - `getAdvancedChartVisibilityTraceRequest()` selects appropriate `TraceName` and `TraceOperation` based on what changed: - Same asset + currency with only range change → `TokenOverviewAdvancedChartTimeRangeVisible` trace - Otherwise → `TokenOverviewAdvancedChartInitialVisible` trace **trace.ts:** - Added new `TraceName` values: - `TokenOverviewAdvancedChartInitialVisible` - for initial load or asset/currency changes - `TokenOverviewAdvancedChartTimeRangeVisible` - for time range selector changes only - Added new `TraceOperation` values: - `token_overview.advanced_chart` - initial load or asset/currency change - `token_overview.advanced_chart_time_range` - time range change only ### Metrics Consent Fix **Problem:** The original implementation attempted to read from `analytics.isEnabled()` during Sentry initialization in `index.js:45`. However, this occurs before Engine and Redux are initialized, causing `analytics.isEnabled()` to incorrectly return `false` even for opted-in users (it checks in-memory state that doesn't exist yet). **Solution:** `hasMetricsConsent()` now reads directly from AnalyticsController's persisted state in FilesystemStorage (`persist:AnalyticsController`) before falling back to the legacy `METRICS_OPT_IN` storage key. This approach: - Works before Engine/Redux initialization (FilesystemStorage is always available) - Reads the actual persisted opt-in value from disk (`{"optedIn": true/false}`) - Follows the same pattern as `ControllerStorage.getAllPersistedState()` in `persistConfig/index.ts` - Avoids dependency on deprecated `METRICS_OPT_IN` as the primary source **MetaMetricsAndDataCollectionSection:** - Calls `updateCachedConsent()` when user toggles analytics participation - Ensures in-memory consent cache stays synchronized with AnalyticsController state ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Adds tracing for advanced charts ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-3129 ## **Manual testing steps** ```gherkin Feature: Advanced Chart Performance Tracing Scenario: user opens token details and chart loads successfully Given the user is on the home screen When user taps on a token to open token details Then a single "Starting sampled root span" log appears for "Token Overview Advanced Chart Initial Visible" And a single "Finishing" log appears for the same span ID And no duplicate "Token Overview" traces appear Scenario: user navigates back before chart finishes loading Given the user is on token details with the chart still loading (skeleton visible) When user quickly navigates back to the home screen Then a "Finishing" log appears for the "Token Overview Advanced Chart Initial Visible" span And the finish log appears after the navigation back (after "[UserInteraction]" log) Scenario: user navigates back after chart has loaded Given the user is on token details and the chart has fully loaded When user navigates back to the home screen Then no "Token Overview" trace logs appear during navigation back Scenario: user changes time range on the chart Given the user is on token details with the chart loaded on 1D range When user taps on the 1W time range selector Then a "Finishing" log appears for the previous "Token Overview Advanced Chart Initial Visible" span And a new "Starting sampled root span" log appears for "Token Overview Advanced Chart Time Range Visible" And a "Finishing" log appears for the new span when the chart reloads ``` ``` How to verify locally: 1. Enable Sentry trace logging by filtering console output for "Tracing" 2. Navigate to a token's details screen 3. Confirm you see exactly ONE "Starting sampled root span" for "Token Overview Advanced Chart Initial Visible" followed by ONE "Finishing" log for the same span ID 4. Navigate back — confirm no additional trace start/finish for that span 5. Re-enter the token, change time range — confirm the old trace ends with superseded and a new "Time Range Visible" trace starts and finishes ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** <!-- Every checklist item must be consciously assessed before marking this PR as "Ready for review". A checked box means you deliberately considered that responsibility, not that you literally performed every action listed. Unchecked boxes are ambiguous: they are not an implicit "N/A" and they are not a silent "skip". See `docs/readme/ready-for-review.md` for the full checklist semantics. --> - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. #### Performance checks (if applicable) - [ ] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [ ] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Adds new Sentry tracing lifecycle hooks around the token overview advanced chart and changes how metrics consent is determined during early app startup, which could affect whether performance data is captured or suppressed. > > **Overview** > Adds Sentry performance traces for token overview *advanced chart visibility*, starting a trace on `ohlcvSeriesKey` changes and ending it when the chart skeleton disappears, errors, is superseded by a newer request, falls back to the legacy chart, or unmounts (including `assetId` and truncated error details). > > Extends `AdvancedChart` with an `onSkeletonHidden` callback that fires once when the native skeleton overlay is removed (resetting on series/HTML reload), and adds new `TraceName`/`TraceOperation` constants to distinguish *initial/asset changes* vs *time-range-only* updates. > > Fixes early Sentry init consent detection by updating `hasMetricsConsent()` to read `persist:AnalyticsController` from `redux-persist-filesystem-storage` (fallback to legacy `METRICS_OPT_IN`), and keeps the in-memory consent cache in sync via `updateCachedConsent()` from settings; tests were updated/added for these behaviors. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 7ec6bbd. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Contributor
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Contributor
🔍 Smart E2E Test Selection⏭️ Smart E2E selection skipped - PR targets a release branch (release/*) All E2E tests pre-selected. |
|
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.
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.



Description
Adds Sentry performance traces for token-overview advanced chart
visibility (time from series change until skeleton clears), with
separate trace types for dashboards to distinguish initial load / asset
or currency changes from time-range-only updates.
Also fixes a bug in metrics consent detection during early Sentry
initialization.
Changes
Advanced Chart Performance Tracing
AdvancedChart component:
onSkeletonHiddencallback that fires once whenloading/layout settles and the chart is ready
PriceAdvanced component:
trace()whenohlcvSeriesKeychangesendTrace()when:getAdvancedChartVisibilityTraceRequest()selects appropriateTraceNameandTraceOperationbased on what changed:TokenOverviewAdvancedChartTimeRangeVisibletraceTokenOverviewAdvancedChartInitialVisibletracetrace.ts:
TraceNamevalues:TokenOverviewAdvancedChartInitialVisible- for initial load orasset/currency changes
TokenOverviewAdvancedChartTimeRangeVisible- for time range selectorchanges only
TraceOperationvalues:token_overview.advanced_chart- initial load or asset/currencychange
token_overview.advanced_chart_time_range- time range change onlyMetrics Consent Fix
Problem:
The original implementation attempted to read from
analytics.isEnabled()during Sentry initialization inindex.js:45.However, this occurs before Engine and Redux are initialized, causing
analytics.isEnabled()to incorrectly returnfalseeven for opted-inusers (it checks in-memory state that doesn't exist yet).
Solution:
hasMetricsConsent()now reads directly from AnalyticsController'spersisted state in FilesystemStorage (
persist:AnalyticsController)before falling back to the legacy
METRICS_OPT_INstorage key. Thisapproach:
available)
{"optedIn": true/false})ControllerStorage.getAllPersistedState()in
persistConfig/index.tsMETRICS_OPT_INas the primary sourceMetaMetricsAndDataCollectionSection:
updateCachedConsent()when user toggles analyticsparticipation
AnalyticsController state
Changelog
CHANGELOG entry: Adds tracing for advanced charts
Related issues
Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-3129
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Docs and MetaMask Mobile
Coding
Standards.
if applicable
guidelines).
Not required for external contributors.
Performance checks (if applicable)
SRPs
to import wallets with many accounts and tokens
performance metrics
trace()for usage andaddTokenfor an example
For performance guidelines and tooling, see the Performance
Guide.
Pre-merge reviewer checklist
app, test code being changed).
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
Note
Medium Risk
Adds new Sentry tracing lifecycle hooks around the token overview
advanced chart and changes how metrics consent is determined during
early app startup, which could affect whether performance data is
captured or suppressed.
Overview
Adds Sentry performance traces for token overview advanced chart
visibility, starting a trace on
ohlcvSeriesKeychanges and ending itwhen the chart skeleton disappears, errors, is superseded by a newer
request, falls back to the legacy chart, or unmounts (including
assetIdand truncated error details).Extends
AdvancedChartwith anonSkeletonHiddencallback that firesonce when the native skeleton overlay is removed (resetting on
series/HTML reload), and adds new
TraceName/TraceOperationconstantsto distinguish initial/asset changes vs time-range-only updates.
Fixes early Sentry init consent detection by updating
hasMetricsConsent()to readpersist:AnalyticsControllerfromredux-persist-filesystem-storage(fallback to legacyMETRICS_OPT_IN), and keeps the in-memory consent cache in sync viaupdateCachedConsent()from settings; tests were updated/added forthese behaviors.
Reviewed by Cursor Bugbot for commit
7ec6bbd. Bugbot is set up for automated
code reviews on this repo. Configure
here.