feat(snap-keyring): more snap account metrics #23113

ccharly · 2024-02-22T09:36:04Z

Description

Requirements:

refactor(snap-keyring): refactor confirmation dialogs #23007

Add more MetaMetrics events related to snap account interactions (see scenarios below).

Scenarios

Settings:

Account Snap Experimental Toggle On

Account menu:

Add account Snap button clicked

Snap account creation:

Create account snap confirmation seen
Create account Snap Cancel button clicked
Create account Snap Create button clicked
Account created confirmation seen
Account created confirmation ok button clicked
Account Snap successfully created

Snap account removal:

Remove account snap confirmation seen
Remove account Snap Cancel button clicked
Remove account Snap Create button clicked
Account removed confirmation seen
Account removed confirmation ok button clicked
Account Snap successfully removed

Snap account transactions:

Please complete the transaction on the Snap spinner shown
Finish signing with account snap confirmation shown
Finish signing with account Snap URL clicked
Finish signing with account Snap Close button clicked
Finish signing with account Snap Go to site button clicked

Related issues

Fixes: https://github.com/MetaMask/accounts-planning/issues/247

Manual testing steps

Start the mock-segment.js server. If you also wish to see event's content, you can use this branch to run the server first: feature/mock-segment-verbose.

Alternatively, you can "cherry-pick" the changes without the actual commit like so:
git cherry-pick -n f938d6d18cd977ba53eb68668a2d475e26b8a131

Run the server:

# With the `feature/mock-segment-verbose` changes
node development/mock-segment.js -v

# Without
node development/mock-segment.js

Then:

Build and install the extension (as usual) and load it
Use the SSK dapp: https://metamask.github.io/snap-simple-keyring/latest/
Refers to those scenarios and "run" some of them manually using the SSK dapp and your extension

Screenshots/Recordings

snap-account-toggle.mov

add-snap-account-link.mov

create-snap-account-success.mov

create-snap-account-cancel.mov

delete-snap-account-success.mov

delete-snap-account-cancel.mov

transaction-spinner-and-go-to-site.mov

Pre-merge author checklist

Pre-merge reviewer checklist

I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
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.

github-actions · 2024-02-22T09:36:17Z

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.

codecov · 2024-02-22T18:22:51Z

Codecov Report

Attention: Patch coverage is 56.25000% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 68.66%. Comparing base (b110819) to head (1ab98e8).

Files	Patch %	Lines
app/scripts/lib/snap-keyring/snap-keyring.ts	4.76%	20 Missing ⚠️
...saction-base/confirm-transaction-base.component.js	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #23113      +/-   ##
===========================================
- Coverage    68.70%   68.66%   -0.05%     
===========================================
  Files         1105     1105              
  Lines        43275    43330      +55     
  Branches     11570    11581      +11     
===========================================
+ Hits         29731    29749      +18     
- Misses       13544    13581      +37

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

metamaskbot · 2024-02-22T18:28:20Z

Builds ready [70c85c9]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1736 ± 45 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	114	246	166	31	15
		domContentLoaded	10	70	24	15	7
		load	1552	1914	1736	94	45
		domInteractive	10	70	24	15	7

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 374 Bytes (0.01%)
ui: 8.93 KiB (0.13%)
common: 1.18 KiB (0.03%)

metamaskbot · 2024-02-26T09:46:11Z

Builds ready [b292bef]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1807 ± 50 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	138	442	195	83	40
		domContentLoaded	10	77	32	19	9
		load	1603	2100	1807	103	50
		domInteractive	10	77	32	19	9

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 1.49 KiB (0.04%)
ui: 8.99 KiB (0.13%)
common: 1.18 KiB (0.02%)

metamaskbot · 2024-02-26T10:18:14Z

Builds ready [46b6652]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1208 ± 460 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	68	411	137	80	38
		domContentLoaded	9	84	31	21	10
		load	54	2970	1208	959	460
		domInteractive	9	84	31	21	10

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 1.49 KiB (0.04%)
ui: 8.99 KiB (0.13%)
common: 1.18 KiB (0.02%)

ccharly · 2024-02-27T15:42:07Z

app/scripts/lib/snap-keyring/snap-keyring.ts

-                origin,
+
+              // User should now see the "Successfuly added account" page
+              trackSnapAccountEvent(


Note for the reviewers:

I thought of maybe augmenting the showSuccess function (or event deeper, to the component itself) to support those events, like having a:

events: { onLoad: MetaMetricsEventName.AddSnapAccountSuccessViewed, onSubmit: MetaMetricsEventName.AddSnapAccountSuccessClicked },

So that our component would take care "hooking" those actions and fire events automatically. But this requires a bit more effort to implement properly.

I'm also open for suggestion here, since having those metrics in the snap-keyring.ts feels a bit wrong to me.

WDYT?

I really like this idea and I agree that the implementation would scope creep this task a bit. If you strongly feel that it should be done first then I recommend opening a new PR for that change

Since I'd like to move on with this PR, I'll probably keep it that way for now, and I'll leave a TODO to improve that later on.

…rejection

metamaskbot · 2024-03-01T16:20:35Z

Builds ready [401d1bd]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1188 ± 525 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	101	518	167	89	43
		domContentLoaded	17	97	44	25	12
		load	62	2938	1188	1093	525
		domInteractive	17	97	44	25	12

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 1019 Bytes (0.03%)
ui: 4.33 KiB (0.06%)
common: 1.18 KiB (0.03%)

gantunesr

Review notes,

The event Account Snap Experimental Toggle On is missing
The event Add Snap Account Clicked is not being triggered
The event Add Snap Account Cancelled is being triggered twice
The event Remove Snap Account Cancelled is being triggered twice
The event Snap Account Transaction Finalize Viewed is being triggered twice
Confirm if Snap Account Transaction Finalize Redirect "Go To Site" Clicked is correct when the user clicks the “Go to site” button. The metrics document says that the event should be Snap Account Transaction Finalize Closed

@ccharly the code LGTM but some events were not 100% correct and others were triggered twice, that could be because of the confirmations screens and may require a separate fix since it occurs to other unrelated confirmations too

shared/constants/metametrics.ts

gantunesr · 2024-03-05T21:33:31Z

app/scripts/lib/snap-keyring/snap-keyring.ts

-                origin,
+
+              // User should now see the "Successfuly added account" page
+              trackSnapAccountEvent(


I really like this idea and I agree that the implementation would scope creep this task a bit. If you strongly feel that it should be done first then I recommend opening a new PR for that change

montelaidev · 2024-03-06T07:43:28Z

It seems like SnapAccountTransactionLoadingViewed isn't loading for me when its a personal sign or sign type data

montelaidev · 2024-03-06T07:45:33Z

The event Account Snap Experimental Toggle On is missing

The toggle events are showing up for me.

montelaidev · 2024-03-06T07:46:01Z

The event AddSnapAccountClicked doesn't look like it is being used.

montelaidev · 2024-03-06T07:48:17Z

Is this right? RemoveSnapAccountConfirmed doesn't get triggered if the snap account is removed from within the extension?

ccharly · 2024-03-06T12:25:29Z

The event Account Snap Experimental Toggle On is missing

@gantunesr Yes, it also works for me. Did you went to the experimental tab? Also, I'm using flask "flavored" build, IDK if this is important or no?

ccharly · 2024-03-06T12:26:55Z

The event AddSnapAccountClicked doesn't look like it is being used.

@montelaidev True! Actually I forgot to remove it, I used the same event than other account types AccountAddSelected (with the property 'account_type': 'snap')

ccharly · 2024-03-06T15:28:31Z

@montelaidev

Is this right? RemoveSnapAccountConfirmed doesn't get triggered if the snap account is removed from within the extension?

You're right about this one. Those new events are only linked to specific snap account screens (triggered externally - outside of the extension).

I'll double check if we need to hook that "path" too. Thanks!

metamaskbot · 2024-03-06T16:26:15Z

Builds ready [b83cdbc]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1328 ± 392 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	62	309	131	49	24
		domContentLoaded	9	147	35	30	14
		load	49	2052	1328	817	392
		domInteractive	9	147	35	30	14

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 916 Bytes (0.02%)
ui: 4.33 KiB (0.06%)
common: 1.12 KiB (0.02%)

gantunesr

LGTM!

metamaskbot · 2024-03-07T13:22:12Z

Builds ready [27723df]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1461 ± 494 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	81	436	166	77	37
		domContentLoaded	12	88	33	22	10
		load	68	3004	1461	1029	494
		domInteractive	12	88	33	22	10

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 924 Bytes (0.02%)
ui: 4.33 KiB (0.06%)
common: 1.12 KiB (0.02%)

metamaskbot · 2024-03-07T14:03:45Z

Builds ready [1ab98e8]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1081 ± 486 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	77	251	139	53	25
		domContentLoaded	12	76	32	19	9
		load	63	2742	1081	1012	486
		domInteractive	12	76	32	19	9

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 924 Bytes (0.02%)
ui: 4.33 KiB (0.06%)
common: 1.12 KiB (0.02%)

ccharly added good first issue Good for newcomers area-accounts Relating to how sensitive account data is managed and stored. area-snaps team-accounts-framework Accounts Framework team labels Feb 22, 2024

ccharly force-pushed the feature/247/more-snap-account-metrics branch 3 times, most recently from 8409680 to 74ee095 Compare February 22, 2024 17:15

ccharly removed the good first issue Good for newcomers label Feb 22, 2024

ccharly force-pushed the feature/247/more-snap-account-metrics branch 2 times, most recently from 3f4499f to 70c85c9 Compare February 22, 2024 18:06

ccharly force-pushed the feature/247/more-snap-account-metrics branch 4 times, most recently from 3a2a16a to b292bef Compare February 26, 2024 09:22

ccharly marked this pull request as ready for review February 26, 2024 09:40

ccharly requested a review from a team as a code owner February 26, 2024 09:40

ccharly force-pushed the feature/247/more-snap-account-metrics branch from b292bef to 46b6652 Compare February 26, 2024 09:55

ccharly commented Feb 27, 2024

View reviewed changes

ccharly force-pushed the feature/247/more-snap-account-metrics branch from 46b6652 to 923f5f1 Compare February 28, 2024 15:09

danroc changed the title ~~feat(snap-keyring): more snap account metrics~~ feat(snap-accounts): more snap account metrics Feb 29, 2024

danroc changed the title ~~feat(snap-accounts): more snap account metrics~~ feat(snap-keyring): more snap account metrics Feb 29, 2024

ccharly added 4 commits March 1, 2024 15:59

feat(shared): add MetaMetricsEventAccountType.Snap

9018409

feat(ui): forward trackEvent context to confirmation templates

7f20fa1

feat(metametrics): add event for snap account page selection

a9ea8dd

feat(metametrics): add events for snap account confirmation/creation/…

fa364f1

…rejection

feat(metametrics): add events during snap account removal

401d1bd

ccharly force-pushed the feature/247/more-snap-account-metrics branch 2 times, most recently from 43ba093 to 401d1bd Compare March 1, 2024 15:38

gantunesr reviewed Mar 5, 2024

View reviewed changes

ccharly added 2 commits March 6, 2024 13:45

fix(metametrics): update snap accounts wording: Cancelled -> Canceled

296e779

refactor(metametrics): remove unused AddSnapAccountClicked

c33324f

ccharly requested a review from kumavis as a code owner March 6, 2024 15:48

fix(snap-keyring): remove extraneous "canceled" events

b83cdbc

ccharly force-pushed the feature/247/more-snap-account-metrics branch from 7f39894 to b83cdbc Compare March 6, 2024 16:00

fix(snap-keyring): fix add/remove snap account event flow

19bba6e

gantunesr removed the request for review from kumavis March 6, 2024 16:18

refactor(snap-keyring): add some TODOs for dialog/metametrics

27723df

gantunesr approved these changes Mar 7, 2024

View reviewed changes

montelaidev approved these changes Mar 7, 2024

View reviewed changes

Merge branch 'develop' into feature/247/more-snap-account-metrics

1ab98e8

ccharly merged commit 3423aab into develop Mar 7, 2024

ccharly deleted the feature/247/more-snap-account-metrics branch March 7, 2024 14:04

github-actions bot locked and limited conversation to collaborators Mar 7, 2024

metamaskbot added the release-11.14.0 Issue or pull request that will be included in release 11.14.0 label Mar 7, 2024

feat(snap-keyring): more snap account metrics #23113

feat(snap-keyring): more snap account metrics #23113

Uh oh!

Conversation

ccharly commented Feb 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Requirements:

Scenarios

Related issues

Manual testing steps

Screenshots/Recordings

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Feb 22, 2024

Uh oh!

codecov bot commented Feb 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

metamaskbot commented Feb 22, 2024

Uh oh!

metamaskbot commented Feb 26, 2024

Uh oh!

metamaskbot commented Feb 26, 2024

Uh oh!

ccharly Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

gantunesr Mar 5, 2024

Choose a reason for hiding this comment

Uh oh!

ccharly Mar 6, 2024

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Mar 1, 2024

Uh oh!

gantunesr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gantunesr Mar 5, 2024

Choose a reason for hiding this comment

Uh oh!

montelaidev commented Mar 6, 2024

Uh oh!

montelaidev commented Mar 6, 2024

Uh oh!

montelaidev commented Mar 6, 2024

Uh oh!

montelaidev commented Mar 6, 2024

Uh oh!

ccharly commented Mar 6, 2024

Uh oh!

ccharly commented Mar 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ccharly commented Mar 6, 2024

Uh oh!

metamaskbot commented Mar 6, 2024

Uh oh!

gantunesr left a comment

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Mar 7, 2024

Uh oh!

metamaskbot commented Mar 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ccharly commented Feb 22, 2024 •

edited

Loading

codecov bot commented Feb 22, 2024 •

edited

Loading

gantunesr left a comment •

edited

Loading

ccharly commented Mar 6, 2024 •

edited

Loading