From 9cd70b8f2e6f130811d87c1978f2067bfdbb41db Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 26 Mar 2026 11:36:29 +0100 Subject: [PATCH 1/7] feat: add Sentry traces for Assets Health dashboard Add comprehensive tracing across AssetsController and balance selector to power an Assets Health dashboard in Sentry. Traces cover data source timing, errors, full fetch pipeline, update pipeline, subscription failures, state size (once on app start), and aggregated balance selector performance. --- .../src/AssetsController.test.ts | 16 +- .../assets-controller/src/AssetsController.ts | 158 +++++++++++++++--- .../src/selectors/balance.ts | 31 ++++ 3 files changed, 183 insertions(+), 22 deletions(-) diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index bfcee994d8f..0e8f4209da2 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -1222,8 +1222,13 @@ describe('AssetsController', () => { // Allow #start() -> getAssets() to resolve so the callback runs await new Promise((resolve) => setTimeout(resolve, 100)); - expect(traceMock).toHaveBeenCalledTimes(1); - const [request] = traceMock.mock.calls[0]; + const firstInitFetchCalls = traceMock.mock.calls.filter( + (call) => + (call[0] as TraceRequest).name === + 'AssetsController First Init Fetch', + ); + expect(firstInitFetchCalls).toHaveLength(1); + const [request] = firstInitFetchCalls[0]; expect(request).toMatchObject({ name: 'AssetsController First Init Fetch', data: expect.objectContaining({ @@ -1271,7 +1276,12 @@ describe('AssetsController', () => { messenger.publish('KeyringController:unlock'); await new Promise((resolve) => setTimeout(resolve, 100)); - expect(traceMock).toHaveBeenCalledTimes(1); + const firstInitFetchCalls = traceMock.mock.calls.filter( + (call) => + (call[0] as TraceRequest).name === + 'AssetsController First Init Fetch', + ); + expect(firstInitFetchCalls).toHaveLength(1); }, ); }); diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 7a8164c8a22..7600fc226c2 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -509,6 +509,72 @@ export class AssetsController extends BaseController< /** Whether we have already reported first init fetch for this session (reset on #stop). */ #firstInitFetchReported = false; + /** Whether we have already reported state size for this session (reset on #stop). */ + #stateSizeReported = false; + + /** + * Fire-and-forget trace helper. Swallows errors so telemetry never breaks the controller. + * + * @param name - Trace / span name visible in Sentry. + * @param data - Key-value pairs attached as span data. + * @param tags - Key-value pairs used for Sentry filtering. + */ + #emitTrace( + name: string, + data: Record, + tags: Record = { + controller: 'AssetsController', + }, + ): void { + if (!this.#trace) { + return; + } + try { + this.#trace({ name, data, tags }, () => undefined).catch(() => { + // Telemetry failure must not break. + }); + } catch { + // Telemetry failure must not break. + } + } + + /** + * Emit a state-size trace once on app start (first state update after unlock). + */ + #emitStateSizeTrace(): void { + if (!this.#trace || this.#stateSizeReported) { + return; + } + this.#stateSizeReported = true; + + const { + assetsBalance: balances, + customAssets, + assetsInfo, + assetsPrice, + } = this.state; + + let balanceEntries = 0; + for (const acct of Object.values(balances)) { + balanceEntries += Object.keys(acct).length; + } + + let customAssetEntries = 0; + for (const ids of Object.values(customAssets)) { + if (Array.isArray(ids)) { + customAssetEntries += ids.length; + } + } + + this.#emitTrace('Assets State Size', { + balance_entries: balanceEntries, + balance_accounts: Object.keys(balances).length, + metadata_entries: Object.keys(assetsInfo).length, + price_entries: Object.keys(assetsPrice).length, + custom_asset_entries: customAssetEntries, + }); + } + /** Whether the client (UI) is open. Combined with #keyringUnlocked for #updateActive. */ #uiOpen = false; @@ -934,8 +1000,9 @@ export class AssetsController extends BaseController< }) as Middleware, ); + const middlewareErrors: string[] = []; const chain = wrapped.reduceRight( - (next, middleware) => + (next, middleware, index) => async ( ctx, ): Promise<{ @@ -946,6 +1013,8 @@ export class AssetsController extends BaseController< try { return await middleware(ctx, next); } catch (error) { + const sourceName = names[index] ?? `middleware_${index}`; + middlewareErrors.push(sourceName); console.error('[AssetsController] Middleware failed:', error); return next(ctx); } @@ -973,6 +1042,28 @@ export class AssetsController extends BaseController< durationByDataSource[key] = ms; } } + + // Emit per-source timing traces for the Assets Health dashboard + for (const [sourceName, durationMs] of Object.entries( + durationByDataSource, + )) { + this.#emitTrace('Assets Data Source Timing', { + source: sourceName, + duration_ms: durationMs, + chain_count: request.chainIds.length, + account_count: request.accountsWithSupportedChains.length, + }); + } + + // Emit error traces for failed middlewares + if (middlewareErrors.length > 0) { + this.#emitTrace('Assets Data Source Error', { + failed_sources: middlewareErrors.join(','), + error_count: middlewareErrors.length, + chain_count: request.chainIds.length, + }); + } + return { response: result.response, durationByDataSource }; } @@ -1047,25 +1138,34 @@ export class AssetsController extends BaseController< const updateMode = options?.updateMode ?? (isPartialChainFetch ? 'merge' : 'full'); await this.#updateState({ ...response, updateMode }); - if (this.#trace && !this.#firstInitFetchReported) { + + const durationMs = Date.now() - startTime; + + // Emit trace for every full fetch (Assets Health dashboard) + this.#emitTrace('Assets Full Fetch', { + duration_ms: durationMs, + chain_count: chainIds.length, + account_count: accounts.length, + basic_functionality: this.#isBasicFunctionality(), + asset_count: response.assetsBalance + ? Object.values(response.assetsBalance).reduce( + (sum, acct) => sum + Object.keys(acct).length, + 0, + ) + : 0, + price_count: response.assetsPrice + ? Object.keys(response.assetsPrice).length + : 0, + ...durationByDataSource, + }); + + if (!this.#firstInitFetchReported) { this.#firstInitFetchReported = true; - const durationMs = Date.now() - startTime; - try { - await this.#trace( - { - name: 'AssetsController First Init Fetch', - data: { - duration_ms: durationMs, - chain_ids: JSON.stringify(chainIds), - ...durationByDataSource, - }, - tags: { controller: 'AssetsController' }, - }, - () => undefined, - ); - } catch { - // Telemetry failure must not break. - } + this.#emitTrace('AssetsController First Init Fetch', { + duration_ms: durationMs, + chain_ids: JSON.stringify(chainIds), + ...durationByDataSource, + }); } } @@ -1669,6 +1769,9 @@ export class AssetsController extends BaseController< } }); + // Emit state size trace (throttled to avoid JSON.stringify on every update) + this.#emitStateSizeTrace(); + // Calculate changed prices const changedPriceAssets: string[] = normalizedResponse.assetsPrice ? Object.keys(normalizedResponse.assetsPrice).filter( @@ -1879,6 +1982,7 @@ export class AssetsController extends BaseController< }); this.#firstInitFetchReported = false; + this.#stateSizeReported = false; // Stop price subscription first (uses direct messenger call) this.unsubscribeAssetsPrice(); @@ -2099,6 +2203,10 @@ export class AssetsController extends BaseController< `[AssetsController] Failed to subscribe to '${sourceId}':`, error, ); + this.#emitTrace('Assets Subscription Error', { + source: sourceId, + error_message: String(error), + }); }); // Track subscription @@ -2293,6 +2401,7 @@ export class AssetsController extends BaseController< sourceId: string, request?: DataRequest, ): Promise { + const updateStart = Date.now(); log('Assets updated from data source', { sourceId, hasBalance: Boolean(response.assetsBalance), @@ -2318,6 +2427,17 @@ export class AssetsController extends BaseController< ); await this.#updateState(enrichedResponse); + + this.#emitTrace('Assets Update Pipeline', { + source: sourceId, + duration_ms: Date.now() - updateStart, + has_balance: Boolean(response.assetsBalance), + has_price: Boolean(response.assetsPrice), + has_metadata: Boolean(enrichedResponse.assetsInfo), + balance_account_count: response.assetsBalance + ? Object.keys(response.assetsBalance).length + : 0, + }); } // ============================================================================ diff --git a/packages/assets-controller/src/selectors/balance.ts b/packages/assets-controller/src/selectors/balance.ts index ed0933c0e07..1a7b734ea39 100644 --- a/packages/assets-controller/src/selectors/balance.ts +++ b/packages/assets-controller/src/selectors/balance.ts @@ -1,5 +1,6 @@ import type { AccountTreeControllerState } from '@metamask/account-tree-controller'; import { toHex } from '@metamask/controller-utils'; +import type { TraceCallback } from '@metamask/controller-utils'; import type { InternalAccount } from '@metamask/keyring-internal-api'; import type { CaipChainId, Hex } from '@metamask/utils'; import { @@ -394,7 +395,9 @@ export function getAggregatedBalanceForAccount( accountTreeState?: AccountTreeControllerState, internalAccountsOrAccountIds?: InternalAccount[] | AccountId[], accountsById?: AccountsById, + trace?: TraceCallback, ): AggregatedBalanceForAccount { + const startTime = trace ? performance.now() : 0; const { assetsBalance, assetsInfo, assetPreferences, assetsPrice } = state; const metadata = (assetsInfo ?? {}) as Record; @@ -468,6 +471,34 @@ export function getAggregatedBalanceForAccount( } } + if (trace) { + const durationMs = performance.now() - startTime; + const uniqueNetworks = new Set(); + for (const assetId of merged.keys()) { + const info = getAssetInfo(assetInfoCache, assetId); + uniqueNetworks.add(info.chainId); + } + try { + trace( + { + name: 'Aggregated Balance Selector', + data: { + duration_ms: durationMs, + asset_count: merged.size, + network_count: uniqueNetworks.size, + account_count: accountsToAggregate.length, + }, + tags: { controller: 'AssetsController' }, + }, + () => undefined, + ).catch(() => { + // Telemetry failure must not break. + }); + } catch { + // Telemetry failure must not break. + } + } + if (hasPrices) { const pricePercentChange1d = totalBalanceInFiat > 0 ? weightedNumerator / totalBalanceInFiat : 0; From dbd93752fc344fa4240255b4440da89632da49e7 Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 26 Mar 2026 11:39:32 +0100 Subject: [PATCH 2/7] docs: update changelog for assets health dashboard traces --- packages/assets-controller/CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index f625b06fecd..00840804d12 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add Sentry traces for Assets Health dashboard ([#8310](https://github.com/MetaMask/core/pull/8310)) + - `Assets Data Source Timing` — per-source latency for each middleware in the fetch pipeline + - `Assets Data Source Error` — tracks middleware failures with source names and error counts + - `Assets Full Fetch` — end-to-end fetch timing with asset/price/chain/account counts + - `Assets Update Pipeline` — enrichment pipeline timing for pushed data source updates + - `Assets Subscription Error` — subscription failure tracking per data source + - `Assets State Size` — entry counts for balances, metadata, prices, and custom assets (once on app start) + - `Aggregated Balance Selector` — balance selector computation time with asset/network/account counts + - Add optional `trace` parameter to `getAggregatedBalanceForAccount` selector + ### Changed - Bump `@metamask/transaction-controller` from `^63.1.0` to `^63.2.0` ([#8301](https://github.com/MetaMask/core/pull/8301)) From 55f19615eb3a6af3e68c51e4a343874b2942dcfb Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 26 Mar 2026 12:07:16 +0100 Subject: [PATCH 3/7] refactor: extract trace names into named constants Replace hardcoded trace name strings with named constants (TRACE_FIRST_INIT_FETCH, TRACE_FULL_FETCH, TRACE_DATA_SOURCE_TIMING, etc.) so they are easy to find and refactor. --- .../assets-controller/src/AssetsController.ts | 25 +++++++++++++------ .../src/selectors/balance.ts | 7 +++++- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 7600fc226c2..eb16b36d1ef 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -159,6 +159,17 @@ const MESSENGER_EXPOSED_METHODS = [ /** Default polling interval hint for data sources (30 seconds) */ const DEFAULT_POLLING_INTERVAL_MS = 30_000; +// ============================================================================ +// TRACE NAMES — used in Sentry spans (search these strings in Discover) +// ============================================================================ +const TRACE_FIRST_INIT_FETCH = 'AssetsController First Init Fetch'; +const TRACE_FULL_FETCH = 'Assets Full Fetch'; +const TRACE_DATA_SOURCE_TIMING = 'Assets Data Source Timing'; +const TRACE_DATA_SOURCE_ERROR = 'Assets Data Source Error'; +const TRACE_UPDATE_PIPELINE = 'Assets Update Pipeline'; +const TRACE_SUBSCRIPTION_ERROR = 'Assets Subscription Error'; +const TRACE_STATE_SIZE = 'Assets State Size'; + const log = createModuleLogger(projectLogger, CONTROLLER_NAME); // ============================================================================ @@ -566,7 +577,7 @@ export class AssetsController extends BaseController< } } - this.#emitTrace('Assets State Size', { + this.#emitTrace(TRACE_STATE_SIZE, { balance_entries: balanceEntries, balance_accounts: Object.keys(balances).length, metadata_entries: Object.keys(assetsInfo).length, @@ -1047,7 +1058,7 @@ export class AssetsController extends BaseController< for (const [sourceName, durationMs] of Object.entries( durationByDataSource, )) { - this.#emitTrace('Assets Data Source Timing', { + this.#emitTrace(TRACE_DATA_SOURCE_TIMING, { source: sourceName, duration_ms: durationMs, chain_count: request.chainIds.length, @@ -1057,7 +1068,7 @@ export class AssetsController extends BaseController< // Emit error traces for failed middlewares if (middlewareErrors.length > 0) { - this.#emitTrace('Assets Data Source Error', { + this.#emitTrace(TRACE_DATA_SOURCE_ERROR, { failed_sources: middlewareErrors.join(','), error_count: middlewareErrors.length, chain_count: request.chainIds.length, @@ -1142,7 +1153,7 @@ export class AssetsController extends BaseController< const durationMs = Date.now() - startTime; // Emit trace for every full fetch (Assets Health dashboard) - this.#emitTrace('Assets Full Fetch', { + this.#emitTrace(TRACE_FULL_FETCH, { duration_ms: durationMs, chain_count: chainIds.length, account_count: accounts.length, @@ -1161,7 +1172,7 @@ export class AssetsController extends BaseController< if (!this.#firstInitFetchReported) { this.#firstInitFetchReported = true; - this.#emitTrace('AssetsController First Init Fetch', { + this.#emitTrace(TRACE_FIRST_INIT_FETCH, { duration_ms: durationMs, chain_ids: JSON.stringify(chainIds), ...durationByDataSource, @@ -2203,7 +2214,7 @@ export class AssetsController extends BaseController< `[AssetsController] Failed to subscribe to '${sourceId}':`, error, ); - this.#emitTrace('Assets Subscription Error', { + this.#emitTrace(TRACE_SUBSCRIPTION_ERROR, { source: sourceId, error_message: String(error), }); @@ -2428,7 +2439,7 @@ export class AssetsController extends BaseController< await this.#updateState(enrichedResponse); - this.#emitTrace('Assets Update Pipeline', { + this.#emitTrace(TRACE_UPDATE_PIPELINE, { source: sourceId, duration_ms: Date.now() - updateStart, has_balance: Boolean(response.assetsBalance), diff --git a/packages/assets-controller/src/selectors/balance.ts b/packages/assets-controller/src/selectors/balance.ts index 1a7b734ea39..d53acc65d8e 100644 --- a/packages/assets-controller/src/selectors/balance.ts +++ b/packages/assets-controller/src/selectors/balance.ts @@ -20,6 +20,11 @@ import type { Caip19AssetId, } from '../types'; +// ============================================================================ +// TRACE NAMES — used in Sentry spans (search these strings in Discover) +// ============================================================================ +const TRACE_AGGREGATED_BALANCE_SELECTOR = 'Aggregated Balance Selector'; + export type EnabledNetworkMap = | Record> | undefined; @@ -481,7 +486,7 @@ export function getAggregatedBalanceForAccount( try { trace( { - name: 'Aggregated Balance Selector', + name: TRACE_AGGREGATED_BALANCE_SELECTOR, data: { duration_ms: durationMs, asset_count: merged.size, From c2c2745ffe1ea3a7e8d5c4d597f53b96eb3e872d Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 26 Mar 2026 14:29:37 +0100 Subject: [PATCH 4/7] fix: add op --- .../assets-controller/src/AssetsController.ts | 155 +++++++++++------- .../src/selectors/balance.ts | 1 + packages/controller-utils/src/types.ts | 6 + 3 files changed, 107 insertions(+), 55 deletions(-) diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index eb16b36d1ef..61f8298c898 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -528,11 +528,13 @@ export class AssetsController extends BaseController< * * @param name - Trace / span name visible in Sentry. * @param data - Key-value pairs attached as span data. + * @param op - Sentry operation category (e.g. 'controller.fetch'). * @param tags - Key-value pairs used for Sentry filtering. */ #emitTrace( name: string, data: Record, + op: string = 'controller', tags: Record = { controller: 'AssetsController', }, @@ -541,7 +543,7 @@ export class AssetsController extends BaseController< return; } try { - this.#trace({ name, data, tags }, () => undefined).catch(() => { + this.#trace({ name, op, data, tags }, () => undefined).catch(() => { // Telemetry failure must not break. }); } catch { @@ -565,9 +567,22 @@ export class AssetsController extends BaseController< assetsPrice, } = this.state; + // Count balance entries and collect unique asset IDs / chain IDs in one pass. let balanceEntries = 0; + const uniqueAssets = new Set(); + const uniqueNetworks = new Set(); + for (const acct of Object.values(balances)) { - balanceEntries += Object.keys(acct).length; + const assetIds = Object.keys(acct); + balanceEntries += assetIds.length; + for (const assetId of assetIds) { + uniqueAssets.add(assetId); + // CAIP-19 format: "eip155:1/slip44:60" — chainId is everything before "/" + const slash = assetId.indexOf('/'); + if (slash > 0) { + uniqueNetworks.add(assetId.slice(0, slash)); + } + } } let customAssetEntries = 0; @@ -577,13 +592,19 @@ export class AssetsController extends BaseController< } } - this.#emitTrace(TRACE_STATE_SIZE, { - balance_entries: balanceEntries, - balance_accounts: Object.keys(balances).length, - metadata_entries: Object.keys(assetsInfo).length, - price_entries: Object.keys(assetsPrice).length, - custom_asset_entries: customAssetEntries, - }); + this.#emitTrace( + TRACE_STATE_SIZE, + { + balance_entries: balanceEntries, + balance_accounts: Object.keys(balances).length, + unique_asset_count: uniqueAssets.size, + network_count: uniqueNetworks.size, + metadata_entries: Object.keys(assetsInfo).length, + price_entries: Object.keys(assetsPrice).length, + custom_asset_entries: customAssetEntries, + }, + 'controller.state', + ); } /** Whether the client (UI) is open. Combined with #keyringUnlocked for #updateActive. */ @@ -1058,21 +1079,29 @@ export class AssetsController extends BaseController< for (const [sourceName, durationMs] of Object.entries( durationByDataSource, )) { - this.#emitTrace(TRACE_DATA_SOURCE_TIMING, { - source: sourceName, - duration_ms: durationMs, - chain_count: request.chainIds.length, - account_count: request.accountsWithSupportedChains.length, - }); + this.#emitTrace( + TRACE_DATA_SOURCE_TIMING, + { + source: sourceName, + duration_ms: durationMs, + chain_count: request.chainIds.length, + account_count: request.accountsWithSupportedChains.length, + }, + 'controller.datasource', + ); } // Emit error traces for failed middlewares if (middlewareErrors.length > 0) { - this.#emitTrace(TRACE_DATA_SOURCE_ERROR, { - failed_sources: middlewareErrors.join(','), - error_count: middlewareErrors.length, - chain_count: request.chainIds.length, - }); + this.#emitTrace( + TRACE_DATA_SOURCE_ERROR, + { + failed_sources: middlewareErrors.join(','), + error_count: middlewareErrors.length, + chain_count: request.chainIds.length, + }, + 'controller.error', + ); } return { response: result.response, durationByDataSource }; @@ -1153,30 +1182,38 @@ export class AssetsController extends BaseController< const durationMs = Date.now() - startTime; // Emit trace for every full fetch (Assets Health dashboard) - this.#emitTrace(TRACE_FULL_FETCH, { - duration_ms: durationMs, - chain_count: chainIds.length, - account_count: accounts.length, - basic_functionality: this.#isBasicFunctionality(), - asset_count: response.assetsBalance - ? Object.values(response.assetsBalance).reduce( - (sum, acct) => sum + Object.keys(acct).length, - 0, - ) - : 0, - price_count: response.assetsPrice - ? Object.keys(response.assetsPrice).length - : 0, - ...durationByDataSource, - }); + this.#emitTrace( + TRACE_FULL_FETCH, + { + duration_ms: durationMs, + chain_count: chainIds.length, + account_count: accounts.length, + basic_functionality: this.#isBasicFunctionality(), + asset_count: response.assetsBalance + ? Object.values(response.assetsBalance).reduce( + (sum, acct) => sum + Object.keys(acct).length, + 0, + ) + : 0, + price_count: response.assetsPrice + ? Object.keys(response.assetsPrice).length + : 0, + ...durationByDataSource, + }, + 'controller.fetch', + ); if (!this.#firstInitFetchReported) { this.#firstInitFetchReported = true; - this.#emitTrace(TRACE_FIRST_INIT_FETCH, { - duration_ms: durationMs, - chain_ids: JSON.stringify(chainIds), - ...durationByDataSource, - }); + this.#emitTrace( + TRACE_FIRST_INIT_FETCH, + { + duration_ms: durationMs, + chain_ids: JSON.stringify(chainIds), + ...durationByDataSource, + }, + 'controller.fetch', + ); } } @@ -2214,10 +2251,14 @@ export class AssetsController extends BaseController< `[AssetsController] Failed to subscribe to '${sourceId}':`, error, ); - this.#emitTrace(TRACE_SUBSCRIPTION_ERROR, { - source: sourceId, - error_message: String(error), - }); + this.#emitTrace( + TRACE_SUBSCRIPTION_ERROR, + { + source: sourceId, + error_message: String(error), + }, + 'controller.error', + ); }); // Track subscription @@ -2439,16 +2480,20 @@ export class AssetsController extends BaseController< await this.#updateState(enrichedResponse); - this.#emitTrace(TRACE_UPDATE_PIPELINE, { - source: sourceId, - duration_ms: Date.now() - updateStart, - has_balance: Boolean(response.assetsBalance), - has_price: Boolean(response.assetsPrice), - has_metadata: Boolean(enrichedResponse.assetsInfo), - balance_account_count: response.assetsBalance - ? Object.keys(response.assetsBalance).length - : 0, - }); + this.#emitTrace( + TRACE_UPDATE_PIPELINE, + { + source: sourceId, + duration_ms: Date.now() - updateStart, + has_balance: Boolean(response.assetsBalance), + has_price: Boolean(response.assetsPrice), + has_metadata: Boolean(enrichedResponse.assetsInfo), + balance_account_count: response.assetsBalance + ? Object.keys(response.assetsBalance).length + : 0, + }, + 'controller.update', + ); } // ============================================================================ diff --git a/packages/assets-controller/src/selectors/balance.ts b/packages/assets-controller/src/selectors/balance.ts index d53acc65d8e..4c361d90a5f 100644 --- a/packages/assets-controller/src/selectors/balance.ts +++ b/packages/assets-controller/src/selectors/balance.ts @@ -487,6 +487,7 @@ export function getAggregatedBalanceForAccount( trace( { name: TRACE_AGGREGATED_BALANCE_SELECTOR, + op: 'ui.selector.aggregated-balance', data: { duration_ms: durationMs, asset_count: merged.size, diff --git a/packages/controller-utils/src/types.ts b/packages/controller-utils/src/types.ts index 42453a22c85..fb20f0097b9 100644 --- a/packages/controller-utils/src/types.ts +++ b/packages/controller-utils/src/types.ts @@ -234,6 +234,12 @@ export type TraceRequest = { */ id?: string; + /** + * Custom operation name for the trace span (e.g. 'function', 'ui.selector', + * 'controller.fetch'). When omitted the consumer may default to 'custom'. + */ + op?: string; + /** Trace context in which to execute the operation. */ parentContext?: TraceContext; From c60f71a8d4e16d2ea86bbeca5e81f827993c9a35 Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 26 Mar 2026 14:30:04 +0100 Subject: [PATCH 5/7] fix: rename op --- packages/assets-controller/src/AssetsController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 61f8298c898..bd73ede8540 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -534,7 +534,7 @@ export class AssetsController extends BaseController< #emitTrace( name: string, data: Record, - op: string = 'controller', + op: string = 'assets-controller', tags: Record = { controller: 'AssetsController', }, From 55f73f6ce9559023370290e9ef3bd1b33ae55455 Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 26 Mar 2026 14:47:33 +0100 Subject: [PATCH 6/7] fix: clean up --- packages/assets-controller/CHANGELOG.md | 14 +- .../src/AssetsController.test.ts | 6 +- .../assets-controller/src/AssetsController.ts | 156 +++++++----------- .../src/selectors/balance.ts | 3 +- packages/controller-utils/src/types.ts | 6 - 5 files changed, 74 insertions(+), 111 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 00840804d12..394dca12a06 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -10,13 +10,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add Sentry traces for Assets Health dashboard ([#8310](https://github.com/MetaMask/core/pull/8310)) - - `Assets Data Source Timing` — per-source latency for each middleware in the fetch pipeline - - `Assets Data Source Error` — tracks middleware failures with source names and error counts - - `Assets Full Fetch` — end-to-end fetch timing with asset/price/chain/account counts - - `Assets Update Pipeline` — enrichment pipeline timing for pushed data source updates - - `Assets Subscription Error` — subscription failure tracking per data source - - `Assets State Size` — entry counts for balances, metadata, prices, and custom assets (once on app start) - - `Aggregated Balance Selector` — balance selector computation time with asset/network/account counts + - `AssetsDataSourceTiming` — per-source latency for each middleware in the fetch pipeline + - `AssetsDataSourceError` — tracks middleware failures with source names and error counts + - `AssetsFullFetch` — end-to-end fetch timing with asset/price/chain/account counts + - `AssetsUpdatePipeline` — enrichment pipeline timing for pushed data source updates + - `AssetsSubscriptionError` — subscription failure tracking per data source + - `AssetsStateSize` — entry counts for balances, metadata, prices, custom assets, unique assets, and network count (once on app start) + - `AggregatedBalanceSelector` — balance selector computation time with asset/network/account counts - Add optional `trace` parameter to `getAggregatedBalanceForAccount` selector ### Changed diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 0e8f4209da2..23cd788cb4d 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -1225,12 +1225,12 @@ describe('AssetsController', () => { const firstInitFetchCalls = traceMock.mock.calls.filter( (call) => (call[0] as TraceRequest).name === - 'AssetsController First Init Fetch', + 'AssetsControllerFirstInitFetch', ); expect(firstInitFetchCalls).toHaveLength(1); const [request] = firstInitFetchCalls[0]; expect(request).toMatchObject({ - name: 'AssetsController First Init Fetch', + name: 'AssetsControllerFirstInitFetch', data: expect.objectContaining({ duration_ms: expect.any(Number), chain_ids: expect.any(String), @@ -1279,7 +1279,7 @@ describe('AssetsController', () => { const firstInitFetchCalls = traceMock.mock.calls.filter( (call) => (call[0] as TraceRequest).name === - 'AssetsController First Init Fetch', + 'AssetsControllerFirstInitFetch', ); expect(firstInitFetchCalls).toHaveLength(1); }, diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index bd73ede8540..22d6eff4a16 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -162,13 +162,13 @@ const DEFAULT_POLLING_INTERVAL_MS = 30_000; // ============================================================================ // TRACE NAMES — used in Sentry spans (search these strings in Discover) // ============================================================================ -const TRACE_FIRST_INIT_FETCH = 'AssetsController First Init Fetch'; -const TRACE_FULL_FETCH = 'Assets Full Fetch'; -const TRACE_DATA_SOURCE_TIMING = 'Assets Data Source Timing'; -const TRACE_DATA_SOURCE_ERROR = 'Assets Data Source Error'; -const TRACE_UPDATE_PIPELINE = 'Assets Update Pipeline'; -const TRACE_SUBSCRIPTION_ERROR = 'Assets Subscription Error'; -const TRACE_STATE_SIZE = 'Assets State Size'; +const TRACE_FIRST_INIT_FETCH = 'AssetsControllerFirstInitFetch'; +const TRACE_FULL_FETCH = 'AssetsFullFetch'; +const TRACE_DATA_SOURCE_TIMING = 'AssetsDataSourceTiming'; +const TRACE_DATA_SOURCE_ERROR = 'AssetsDataSourceError'; +const TRACE_UPDATE_PIPELINE = 'AssetsUpdatePipeline'; +const TRACE_SUBSCRIPTION_ERROR = 'AssetsSubscriptionError'; +const TRACE_STATE_SIZE = 'AssetsStateSize'; const log = createModuleLogger(projectLogger, CONTROLLER_NAME); @@ -528,13 +528,11 @@ export class AssetsController extends BaseController< * * @param name - Trace / span name visible in Sentry. * @param data - Key-value pairs attached as span data. - * @param op - Sentry operation category (e.g. 'controller.fetch'). * @param tags - Key-value pairs used for Sentry filtering. */ #emitTrace( name: string, data: Record, - op: string = 'assets-controller', tags: Record = { controller: 'AssetsController', }, @@ -543,7 +541,7 @@ export class AssetsController extends BaseController< return; } try { - this.#trace({ name, op, data, tags }, () => undefined).catch(() => { + this.#trace({ name, data, tags }, () => undefined).catch(() => { // Telemetry failure must not break. }); } catch { @@ -592,19 +590,15 @@ export class AssetsController extends BaseController< } } - this.#emitTrace( - TRACE_STATE_SIZE, - { - balance_entries: balanceEntries, - balance_accounts: Object.keys(balances).length, - unique_asset_count: uniqueAssets.size, - network_count: uniqueNetworks.size, - metadata_entries: Object.keys(assetsInfo).length, - price_entries: Object.keys(assetsPrice).length, - custom_asset_entries: customAssetEntries, - }, - 'controller.state', - ); + this.#emitTrace(TRACE_STATE_SIZE, { + balance_entries: balanceEntries, + balance_accounts: Object.keys(balances).length, + unique_asset_count: uniqueAssets.size, + network_count: uniqueNetworks.size, + metadata_entries: Object.keys(assetsInfo).length, + price_entries: Object.keys(assetsPrice).length, + custom_asset_entries: customAssetEntries, + }); } /** Whether the client (UI) is open. Combined with #keyringUnlocked for #updateActive. */ @@ -1079,29 +1073,21 @@ export class AssetsController extends BaseController< for (const [sourceName, durationMs] of Object.entries( durationByDataSource, )) { - this.#emitTrace( - TRACE_DATA_SOURCE_TIMING, - { - source: sourceName, - duration_ms: durationMs, - chain_count: request.chainIds.length, - account_count: request.accountsWithSupportedChains.length, - }, - 'controller.datasource', - ); + this.#emitTrace(TRACE_DATA_SOURCE_TIMING, { + source: sourceName, + duration_ms: durationMs, + chain_count: request.chainIds.length, + account_count: request.accountsWithSupportedChains.length, + }); } // Emit error traces for failed middlewares if (middlewareErrors.length > 0) { - this.#emitTrace( - TRACE_DATA_SOURCE_ERROR, - { - failed_sources: middlewareErrors.join(','), - error_count: middlewareErrors.length, - chain_count: request.chainIds.length, - }, - 'controller.error', - ); + this.#emitTrace(TRACE_DATA_SOURCE_ERROR, { + failed_sources: middlewareErrors.join(','), + error_count: middlewareErrors.length, + chain_count: request.chainIds.length, + }); } return { response: result.response, durationByDataSource }; @@ -1182,38 +1168,30 @@ export class AssetsController extends BaseController< const durationMs = Date.now() - startTime; // Emit trace for every full fetch (Assets Health dashboard) - this.#emitTrace( - TRACE_FULL_FETCH, - { - duration_ms: durationMs, - chain_count: chainIds.length, - account_count: accounts.length, - basic_functionality: this.#isBasicFunctionality(), - asset_count: response.assetsBalance - ? Object.values(response.assetsBalance).reduce( - (sum, acct) => sum + Object.keys(acct).length, - 0, - ) - : 0, - price_count: response.assetsPrice - ? Object.keys(response.assetsPrice).length - : 0, - ...durationByDataSource, - }, - 'controller.fetch', - ); + this.#emitTrace(TRACE_FULL_FETCH, { + duration_ms: durationMs, + chain_count: chainIds.length, + account_count: accounts.length, + basic_functionality: this.#isBasicFunctionality(), + asset_count: response.assetsBalance + ? Object.values(response.assetsBalance).reduce( + (sum, acct) => sum + Object.keys(acct).length, + 0, + ) + : 0, + price_count: response.assetsPrice + ? Object.keys(response.assetsPrice).length + : 0, + ...durationByDataSource, + }); if (!this.#firstInitFetchReported) { this.#firstInitFetchReported = true; - this.#emitTrace( - TRACE_FIRST_INIT_FETCH, - { - duration_ms: durationMs, - chain_ids: JSON.stringify(chainIds), - ...durationByDataSource, - }, - 'controller.fetch', - ); + this.#emitTrace(TRACE_FIRST_INIT_FETCH, { + duration_ms: durationMs, + chain_ids: JSON.stringify(chainIds), + ...durationByDataSource, + }); } } @@ -2251,14 +2229,10 @@ export class AssetsController extends BaseController< `[AssetsController] Failed to subscribe to '${sourceId}':`, error, ); - this.#emitTrace( - TRACE_SUBSCRIPTION_ERROR, - { - source: sourceId, - error_message: String(error), - }, - 'controller.error', - ); + this.#emitTrace(TRACE_SUBSCRIPTION_ERROR, { + source: sourceId, + error_message: String(error), + }); }); // Track subscription @@ -2480,20 +2454,16 @@ export class AssetsController extends BaseController< await this.#updateState(enrichedResponse); - this.#emitTrace( - TRACE_UPDATE_PIPELINE, - { - source: sourceId, - duration_ms: Date.now() - updateStart, - has_balance: Boolean(response.assetsBalance), - has_price: Boolean(response.assetsPrice), - has_metadata: Boolean(enrichedResponse.assetsInfo), - balance_account_count: response.assetsBalance - ? Object.keys(response.assetsBalance).length - : 0, - }, - 'controller.update', - ); + this.#emitTrace(TRACE_UPDATE_PIPELINE, { + source: sourceId, + duration_ms: Date.now() - updateStart, + has_balance: Boolean(response.assetsBalance), + has_price: Boolean(response.assetsPrice), + has_metadata: Boolean(enrichedResponse.assetsInfo), + balance_account_count: response.assetsBalance + ? Object.keys(response.assetsBalance).length + : 0, + }); } // ============================================================================ diff --git a/packages/assets-controller/src/selectors/balance.ts b/packages/assets-controller/src/selectors/balance.ts index 4c361d90a5f..ba781c04af0 100644 --- a/packages/assets-controller/src/selectors/balance.ts +++ b/packages/assets-controller/src/selectors/balance.ts @@ -23,7 +23,7 @@ import type { // ============================================================================ // TRACE NAMES — used in Sentry spans (search these strings in Discover) // ============================================================================ -const TRACE_AGGREGATED_BALANCE_SELECTOR = 'Aggregated Balance Selector'; +const TRACE_AGGREGATED_BALANCE_SELECTOR = 'AggregatedBalanceSelector'; export type EnabledNetworkMap = | Record> @@ -487,7 +487,6 @@ export function getAggregatedBalanceForAccount( trace( { name: TRACE_AGGREGATED_BALANCE_SELECTOR, - op: 'ui.selector.aggregated-balance', data: { duration_ms: durationMs, asset_count: merged.size, diff --git a/packages/controller-utils/src/types.ts b/packages/controller-utils/src/types.ts index fb20f0097b9..42453a22c85 100644 --- a/packages/controller-utils/src/types.ts +++ b/packages/controller-utils/src/types.ts @@ -234,12 +234,6 @@ export type TraceRequest = { */ id?: string; - /** - * Custom operation name for the trace span (e.g. 'function', 'ui.selector', - * 'controller.fetch'). When omitted the consumer may default to 'custom'. - */ - op?: string; - /** Trace context in which to execute the operation. */ parentContext?: TraceContext; From 8427f8732369a2c81dff95143e2f1c926fb4bffc Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 26 Mar 2026 15:50:52 +0100 Subject: [PATCH 7/7] fix: fix PR comments --- .../assets-controller/src/AssetsController.ts | 20 +++++-------- .../src/selectors/balance.ts | 30 ++++++++----------- 2 files changed, 21 insertions(+), 29 deletions(-) diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 22d6eff4a16..2a32590437c 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -540,13 +540,9 @@ export class AssetsController extends BaseController< if (!this.#trace) { return; } - try { - this.#trace({ name, data, tags }, () => undefined).catch(() => { - // Telemetry failure must not break. - }); - } catch { + this.#trace({ name, data, tags }, () => undefined).catch(() => { // Telemetry failure must not break. - } + }); } /** @@ -1017,11 +1013,11 @@ export class AssetsController extends BaseController< response: DataResponse; getAssetsState: () => AssetsControllerStateInternal; }> => { - const start = Date.now(); + const start = performance.now(); try { return await middleware(ctx, next); } finally { - inclusive[i] = Date.now() - start; + inclusive[i] = performance.now() - start; } }) as Middleware, ); @@ -1125,7 +1121,7 @@ export class AssetsController extends BaseController< } if (options?.forceUpdate) { - const startTime = Date.now(); + const startTime = performance.now(); const request = this.#buildDataRequest(accounts, chainIds, { assetTypes, dataTypes, @@ -1165,7 +1161,7 @@ export class AssetsController extends BaseController< options?.updateMode ?? (isPartialChainFetch ? 'merge' : 'full'); await this.#updateState({ ...response, updateMode }); - const durationMs = Date.now() - startTime; + const durationMs = performance.now() - startTime; // Emit trace for every full fetch (Assets Health dashboard) this.#emitTrace(TRACE_FULL_FETCH, { @@ -2427,7 +2423,7 @@ export class AssetsController extends BaseController< sourceId: string, request?: DataRequest, ): Promise { - const updateStart = Date.now(); + const updateStart = performance.now(); log('Assets updated from data source', { sourceId, hasBalance: Boolean(response.assetsBalance), @@ -2456,7 +2452,7 @@ export class AssetsController extends BaseController< this.#emitTrace(TRACE_UPDATE_PIPELINE, { source: sourceId, - duration_ms: Date.now() - updateStart, + duration_ms: performance.now() - updateStart, has_balance: Boolean(response.assetsBalance), has_price: Boolean(response.assetsPrice), has_metadata: Boolean(enrichedResponse.assetsInfo), diff --git a/packages/assets-controller/src/selectors/balance.ts b/packages/assets-controller/src/selectors/balance.ts index ba781c04af0..1e686e5c75c 100644 --- a/packages/assets-controller/src/selectors/balance.ts +++ b/packages/assets-controller/src/selectors/balance.ts @@ -483,25 +483,21 @@ export function getAggregatedBalanceForAccount( const info = getAssetInfo(assetInfoCache, assetId); uniqueNetworks.add(info.chainId); } - try { - trace( - { - name: TRACE_AGGREGATED_BALANCE_SELECTOR, - data: { - duration_ms: durationMs, - asset_count: merged.size, - network_count: uniqueNetworks.size, - account_count: accountsToAggregate.length, - }, - tags: { controller: 'AssetsController' }, + trace( + { + name: TRACE_AGGREGATED_BALANCE_SELECTOR, + data: { + duration_ms: durationMs, + asset_count: merged.size, + network_count: uniqueNetworks.size, + account_count: accountsToAggregate.length, }, - () => undefined, - ).catch(() => { - // Telemetry failure must not break. - }); - } catch { + tags: { controller: 'AssetsController' }, + }, + () => undefined, + ).catch(() => { // Telemetry failure must not break. - } + }); } if (hasPrices) {