identify desktop is paired in the metrics event #17892

vinistevam · 2023-02-24T11:40:48Z

Explanation

Adds the metrics code needed to support tracking usage of the MetaMask Desktop app.

The aim of this PR is to identify if the desktop app is paired with the extension on MetaMetrics to track key operations such as swaps, snaps, transfers, and interaction with dapps. It also includes desktop-related events.

Changes include:

metametrics: included desktop_enabled in traits to identify whenever the user pairs/unpairs the extension and the desktop app.
UI changes to emit desktop-related events whenever a button is clicked
- Included a new event category DESKTOP

Manual Testing Steps

Setup Segment following: https://github.com/MetaMask/metamask-extension/blob/develop/development/README.md#segment
Build the extension for desktop build type - yarn start --build-type desktop
Navigate to settings->experimental
Click on enable desktop button
Once the desktop is paired, perform a transfer (any key operation)
Check on the segment if identify was triggered with the desktop_enabled: true
Example:

analytics.identify({
  userId: '0x',
  traits: {
    desktop_enabled: true,
  }
});

Pre-merge author checklist

I've clearly explained:
- What problem this PR is solving
- How this problem was solved
- How reviewers can test my changes
Sufficient automated test coverage has been added

Pre-merge reviewer checklist

Manual testing (e.g. pull and build branch, run in browser, test code being changed)
PR is linked to the appropriate GitHub issue
IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

github-actions · 2023-02-24T11:41:02Z

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.

vinistevam · 2023-02-24T11:55:02Z

I have read the CLA Document and I hereby sign the CLA

metamaskbot · 2023-02-24T12:30:37Z

Builds ready [e5d59e5]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (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
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1823 ± 85 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	103	184	134	20	10
		domContentLoaded	1512	2124	1803	177	85
		load	1529	2124	1823	178	85
		domInteractive	1512	2124	1803	177	85

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

background: 0 bytes
ui: 0 bytes
common: 18 bytes

brad-decker · 2023-02-24T14:16:04Z

@worldlyjohn have you been consulted on these changes?

brad-decker · 2023-02-24T14:17:22Z

app/scripts/controllers/metametrics.js

+        ///: BEGIN:ONLY_INCLUDE_IN(desktop)
+        desktop_paired: this.getDesktopEnabled(),
+        ///: END:ONLY_INCLUDE_IN


desktop_paired seems more like a user trait then a property we want on every event. @worldlyjohn thoughts?

Thanks for reviewing my PR 🙇
If more appropriate I can move it to user traits.

brad-decker · 2023-02-24T14:18:18Z

ui/pages/desktop-error/render-desktop-error.js

+    if (typeof trackEvent === 'function') {
+      trackEvent({
+        category: EVENT.CATEGORIES.ERROR,
+        event: `Error: ${text}`,
+      });


Why are we tracking errors in segment?

The goal here was to measure the errors while using the desktop app as a companion but you are right, It is already collected by Sentry and if necessary in Segment, it can be tracked by using the trackPage method. I will remove it and push the changes.

brad-decker · 2023-02-24T14:19:35Z

ui/pages/desktop-error/render-desktop-error.js

+            if (onClick) {
+              onClick();
+              if (typeof trackEvent === 'function') {
+                trackEvent({
+                  category: EVENT.CATEGORIES.DESKTOP,
+                  event: `${text} Button Clicked`,
+                });
+              }


These types of dynamic event names are not acceptable....They end up creating variadic events that make it very hard to derive meaning from. @worldlyjohn will advise you on if we want this event or not and if so how to add properties to cover the use cases.

One quick update on this case. I pushed some changes so that instead of using the text it uses the id of the button to determine the event name. In case, more properties need to be added it's possible to extend this function to also include specific properties for each event.

id also not sufficient as it's easy for an element id to change and unintended consequence is the event is now changed too. usually when you want to do this, the best practice is a more generic event name and using this value as a property (though the id field should probably be validated that it matches one element in a predefined set)

metamaskbot · 2023-02-26T18:04:06Z

Builds ready [f436b74]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (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
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1510 ± 50 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	97	137	113	11	6
		domContentLoaded	1244	1624	1476	87	42
		load	1244	1701	1510	104	50
		domInteractive	1244	1624	1476	87	42

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

background: 0 bytes
ui: 0 bytes
common: 18 bytes

metamaskbot · 2023-02-27T18:11:31Z

Builds ready [0a8eacb]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (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
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1479 ± 35 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	92	138	112	14	7
		domContentLoaded	1236	1549	1458	68	32
		load	1314	1634	1479	72	35
		domInteractive	1236	1549	1458	68	32

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

background: 0 bytes
ui: 0 bytes
common: 18 bytes

metamaskbot · 2023-02-28T11:54:23Z

Builds ready [c7a1773]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (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
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1748 ± 79 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	102	199	128	25	12
		domContentLoaded	1479	2127	1718	172	83
		load	1570	2127	1748	165	79
		domInteractive	1479	2127	1718	172	83

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

background: 0 bytes
ui: 0 bytes
common: 18 bytes

metamaskbot · 2023-02-28T15:04:10Z

Builds ready [3dee418]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (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
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1618 ± 51 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	99	154	118	16	7
		domContentLoaded	1454	1774	1582	92	44
		load	1454	1895	1618	106	51
		domInteractive	1454	1774	1582	92	44

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

background: 0 bytes
ui: 0 bytes
common: 52 bytes

worldlyjohn · 2023-02-28T19:02:51Z

ui/pages/desktop-error/render-desktop-error.js

@@ -61,10 +74,57 @@ export function renderDesktopError({
    );
  };

+  const getEventNameById = (id) => {


none of these events follow our naming conventions defined here: https://www.notion.so/Naming-Conventions-95bf27517033451888053f7ab654d327

worldlyjohn · 2023-02-28T19:04:24Z

ui/pages/desktop-error/render-desktop-error.js

+              if (typeof trackEvent === 'function') {
+                trackEvent({
+                  category: EVENT.CATEGORIES.DESKTOP,
+                  event: `${getEventNameById(id)} Button Clicked`,


using dynamic event names is frowned upon as it will create N new tables. can this be a single event name and use dynamic id (if truely necessary) as a property instead? (though even that is less than ideal, better if you run this through a defined list of possible values)

🙏 Great thanks!
I changed it to a single event Desktop Button Clicked and also included an additional parameter in the renderCTA that will be the value of the property to identify the CTA performed by the user.

worldlyjohn · 2023-02-28T19:06:48Z

ui/components/app/desktop-enable-button/desktop-enable-button.component.js

@@ -40,6 +43,10 @@ export default function DesktopEnableButton() {
    if (desktopEnabled) {
      await disableDesktop();
      setDesktopEnabled(false);
+      trackEvent({
+        category: EVENT.CATEGORIES.DESKTOP,
+        event: `Disable Desktop Button Clicked`,


instead of separate events for enable/disabled, these should be properties of the same event. another option is that this is actually a setting change.

either of these works:

event name: Desktop Button Clicked properties: {enabled: true|false}

or

event name: Settings Updated properties: {desktop_enabled: true|false}

I've opted for the settings change as you suggested:

event name: Settings Updated properties: {desktop_enabled: true|false}

metamaskbot · 2023-03-01T12:48:29Z

Builds ready [380f8b2]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (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
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1667 ± 54 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	101	211	130	23	11
		domContentLoaded	1472	1871	1651	107	51
		load	1472	1872	1667	112	54
		domInteractive	1472	1871	1651	107	51

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

background: 0 bytes
ui: 0 bytes
common: 52 bytes

metamaskbot · 2023-03-02T05:56:33Z

Builds ready [e98a6ee]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (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
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1501 ± 42 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	94	204	121	26	12
		domContentLoaded	1344	1660	1489	88	42
		load	1344	1660	1501	87	42
		domInteractive	1344	1660	1489	88	42

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

background: 0 bytes
ui: 0 bytes
common: 52 bytes

worldlyjohn · 2023-03-02T18:21:06Z

works for me!

vinistevam force-pushed the fix/549-add-metrics-mmd-paired branch from e396ade to e5d59e5 Compare February 24, 2023 11:48

vinistevam marked this pull request as ready for review February 24, 2023 14:11

vinistevam requested a review from a team as a code owner February 24, 2023 14:11

vinistevam requested a review from chloeYue February 24, 2023 14:11

brad-decker suggested changes Feb 24, 2023

View reviewed changes

vinistevam force-pushed the fix/549-add-metrics-mmd-paired branch from 0a8eacb to c7a1773 Compare February 28, 2023 11:30

worldlyjohn reviewed Feb 28, 2023

View reviewed changes

vinistevam force-pushed the fix/549-add-metrics-mmd-paired branch from dd78de7 to 380f8b2 Compare March 1, 2023 12:21

vinistevam force-pushed the fix/549-add-metrics-mmd-paired branch from 380f8b2 to ca1be4e Compare March 1, 2023 15:43

vinistevam added 6 commits March 1, 2023 18:51

fix: added property to identify desktop is paired

c725012

fix: add desktop related events

fc648ba

fix: remove track error

9cc859a

fix: use event name by id

fe7f866

fix: use traits to identify desktop is paired

2289bde

fix: use single event name property to identify

e98a6ee

vinistevam force-pushed the fix/549-add-metrics-mmd-paired branch from ca1be4e to e98a6ee Compare March 1, 2023 18:51

brad-decker approved these changes Mar 2, 2023

View reviewed changes

cryptotavares approved these changes Mar 2, 2023

View reviewed changes

brad-decker merged commit e07ec9d into develop Mar 2, 2023

brad-decker deleted the fix/549-add-metrics-mmd-paired branch March 2, 2023 20:55

github-actions bot locked and limited conversation to collaborators Mar 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

identify desktop is paired in the metrics event #17892

identify desktop is paired in the metrics event #17892

vinistevam commented Feb 24, 2023 •

edited

Loading

github-actions bot commented Feb 24, 2023 •

edited

Loading

vinistevam commented Feb 24, 2023

metamaskbot commented Feb 24, 2023

brad-decker commented Feb 24, 2023

brad-decker Feb 24, 2023

vinistevam Feb 26, 2023

brad-decker Feb 24, 2023

vinistevam Feb 26, 2023

brad-decker Feb 24, 2023

vinistevam Feb 28, 2023

worldlyjohn Feb 28, 2023

metamaskbot commented Feb 26, 2023

metamaskbot commented Feb 27, 2023

metamaskbot commented Feb 28, 2023

metamaskbot commented Feb 28, 2023

worldlyjohn Feb 28, 2023

worldlyjohn Feb 28, 2023

vinistevam Mar 1, 2023

worldlyjohn Feb 28, 2023

vinistevam Mar 1, 2023

metamaskbot commented Mar 1, 2023

metamaskbot commented Mar 2, 2023

worldlyjohn commented Mar 2, 2023

identify desktop is paired in the metrics event #17892

identify desktop is paired in the metrics event #17892

Conversation

vinistevam commented Feb 24, 2023 • edited Loading

Explanation

Manual Testing Steps

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Feb 24, 2023 • edited Loading

vinistevam commented Feb 24, 2023

metamaskbot commented Feb 24, 2023

brad-decker commented Feb 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metamaskbot commented Feb 26, 2023

metamaskbot commented Feb 27, 2023

metamaskbot commented Feb 28, 2023

metamaskbot commented Feb 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metamaskbot commented Mar 1, 2023

metamaskbot commented Mar 2, 2023

worldlyjohn commented Mar 2, 2023

vinistevam commented Feb 24, 2023 •

edited

Loading

github-actions bot commented Feb 24, 2023 •

edited

Loading