Skip to content

Commit 914ebff

Browse files
authored
feat: accounts telemetry (#37375)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR adds tracing to the following actions: 1. Show Account List 2. Show Account Private Key List 3. Create Multichain Account 4. Discover Accounts 5. Show Account Address List 6. EVM Discover Accounts 7. Snap Discover Accounts 8. Select Account <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 9. What is the improvement/solution? --> [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/37375?quickstart=1) ## **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: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MUL-1186?atlOrigin=eyJpIjoiOTg3NjBhM2ZlYTAzNGI4Y2EyZGI4MjVlNjQzMjQ5ZWQiLCJwIjoiaiJ9 ## **Manual testing steps** In `.metamaskrc` add your own sentry code Example: ```SENTRY_DSN_DEV=<link>``` Test the different flows: - Open a the account list - Open the address list - Reveal the private key from the account details - Import a new srp - Create a new account ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** N/a ### **After** N/a ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/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-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds Sentry trace instrumentation for multichain account flows (list, addresses, private keys, creation, discovery) across background and UI, with comprehensive tests. > > - **Trace Library**: > - Add new `TraceName` and `TraceOperation` entries for accounts (`ShowAccountList`, `ShowAccountAddressList`, `ShowAccountPrivateKeyList`, `CreateMultichainAccount`, `DiscoverAccounts`, `EvmDiscoverAccounts`, `AccountList`, `AccountCreate`, `AccountUi`, `AccountDiscover`). > - **Background/Services**: > - Inject `trace` into `MultichainAccountService` config. > - Instrument `discoverAndCreateAccounts` and `_addAccountsWithBalance` with `trace`/`endTrace`, including nested `EvmDiscoverAccounts` span and guaranteed `finally` endings. > - **UI**: > - Add traces on key user actions: > - Account picker opens account list; end traces on `MultichainAccountList` mount. > - Account menu “Addresses” navigation. > - Address hover list “View all” via new `onViewAllClick` prop. > - Address list page ends `ShowAccountAddressList` on mount. > - Private key list traces around reveal flow and ends on reveal. > - Add account button traces around create flow. > - Account details “Networks” link traces before navigation. > - **Tests**: > - Extensive unit tests updated/added to assert trace start/end calls and navigation; minor mock fixes (e.g., `getPlatformInfo` return shape). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 834eaf1. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 85a6242 commit 914ebff

21 files changed

+772
-64
lines changed

app/scripts/controller-init/multichain/multichain-account-service-init.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ describe('MultichainAccountServiceInit', () => {
8484
messenger: requestMock.controllerMessenger,
8585
providers: expect.any(Array),
8686
providerConfigs: expect.any(Object),
87+
config: expect.any(Object),
8788
});
8889
});
8990
});

app/scripts/controller-init/multichain/multichain-account-service-init.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
///: BEGIN:ONLY_INCLUDE_IF(bitcoin)
2424
import { isMultichainFeatureEnabled } from '../../../../shared/lib/multichain-feature-flags';
2525
///: END:ONLY_INCLUDE_IF
26+
import { trace } from '../../../../shared/lib/trace';
2627

2728
/**
2829
* Initialize the multichain account service.
@@ -80,6 +81,10 @@ export const MultichainAccountServiceInit: ControllerInitFunction<
8081
providerConfigs: {
8182
[SOL_ACCOUNT_PROVIDER_NAME]: snapAccountProviderConfig,
8283
},
84+
config: {
85+
// @ts-expect-error Controller uses string for names rather than enum
86+
trace,
87+
},
8388
});
8489

8590
const preferencesState = initMessenger.call('PreferencesController:getState');

app/scripts/metamask-controller.js

Lines changed: 60 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4527,6 +4527,10 @@ export default class MetamaskController extends EventEmitter {
45274527
* @returns {Promise<Record<string, number>>} Discovered account counts by chain.
45284528
*/
45294529
async discoverAndCreateAccounts(id) {
4530+
trace({
4531+
name: TraceName.DiscoverAccounts,
4532+
op: TraceOperation.AccountDiscover,
4533+
});
45304534
try {
45314535
// If no keyring id is provided, we assume one keyring was added to the vault
45324536
const keyringIdToDiscover =
@@ -4556,6 +4560,10 @@ export default class MetamaskController extends EventEmitter {
45564560
Solana: 0,
45574561
Tron: 0,
45584562
};
4563+
} finally {
4564+
endTrace({
4565+
name: TraceName.DiscoverAccounts,
4566+
});
45594567
}
45604568
}
45614569

@@ -4949,6 +4957,10 @@ export default class MetamaskController extends EventEmitter {
49494957
* For the context, we do not need to import the Solana account if the onboarding flow has not completed yet during the social login import flow.
49504958
*/
49514959
async _addAccountsWithBalance(keyringId, shouldImportSolanaAccount = true) {
4960+
trace({
4961+
name: TraceName.DiscoverAccounts,
4962+
op: TraceOperation.AccountDiscover,
4963+
});
49524964
try {
49534965
// Scan accounts until we find an empty one
49544966
const chainId = this.#getGlobalChainId();
@@ -4970,41 +4982,54 @@ export default class MetamaskController extends EventEmitter {
49704982
);
49714983
let address = accounts[accounts.length - 1];
49724984

4973-
for (let count = accounts.length; ; count++) {
4974-
const balance = await this.getBalance(address, this.provider);
4975-
4976-
if (balance === '0x0') {
4977-
// This account has no balance, so check for tokens
4978-
await this.tokenDetectionController.detectTokens({
4979-
chainIds: [chainId],
4980-
selectedAddress: address,
4981-
});
4985+
trace({
4986+
name: TraceName.EvmDiscoverAccounts,
4987+
op: TraceOperation.AccountDiscover,
4988+
});
4989+
try {
4990+
for (let count = accounts.length; ; count++) {
4991+
const balance = await this.getBalance(address, this.provider);
4992+
4993+
if (balance === '0x0') {
4994+
// This account has no balance, so check for tokens
4995+
await this.tokenDetectionController.detectTokens({
4996+
chainIds: [chainId],
4997+
selectedAddress: address,
4998+
});
49824999

4983-
const tokens =
4984-
this.tokensController.state.allTokens?.[chainId]?.[address];
4985-
const detectedTokens =
4986-
this.tokensController.state.allDetectedTokens?.[chainId]?.[address];
5000+
const tokens =
5001+
this.tokensController.state.allTokens?.[chainId]?.[address];
5002+
const detectedTokens =
5003+
this.tokensController.state.allDetectedTokens?.[chainId]?.[
5004+
address
5005+
];
49875006

4988-
if (
4989-
(tokens?.length ?? 0) === 0 &&
4990-
(detectedTokens?.length ?? 0) === 0
4991-
) {
4992-
// This account has no balance or tokens
4993-
if (count !== 1) {
4994-
await this.removeAccount(address);
5007+
if (
5008+
(tokens?.length ?? 0) === 0 &&
5009+
(detectedTokens?.length ?? 0) === 0
5010+
) {
5011+
// This account has no balance or tokens
5012+
if (count !== 1) {
5013+
await this.removeAccount(address);
5014+
}
5015+
break;
49955016
}
4996-
break;
49975017
}
4998-
}
49995018

5000-
// This account has assets, so check the next one
5001-
address = await this.keyringController.withKeyring(
5002-
keyringSelector,
5003-
async ({ keyring }) => {
5004-
const [newAddress] = await keyring.addAccounts(1);
5005-
return newAddress;
5006-
},
5007-
);
5019+
// This account has assets, so check the next one
5020+
address = await this.keyringController.withKeyring(
5021+
keyringSelector,
5022+
async ({ keyring }) => {
5023+
const [newAddress] = await keyring.addAccounts(1);
5024+
return newAddress;
5025+
},
5026+
);
5027+
}
5028+
} finally {
5029+
endTrace({
5030+
name: TraceName.EvmDiscoverAccounts,
5031+
op: TraceOperation.AccountDiscover,
5032+
});
50085033
}
50095034

50105035
const discoveredAccounts = {
@@ -5081,6 +5106,11 @@ export default class MetamaskController extends EventEmitter {
50815106
Solana: 0,
50825107
Tron: 0,
50835108
};
5109+
} finally {
5110+
endTrace({
5111+
name: TraceName.DiscoverAccounts,
5112+
op: TraceOperation.AccountDiscover,
5113+
});
50845114
}
50855115
}
50865116

0 commit comments

Comments
 (0)