feat: tag swap/bridge Sentry traces

oxlint-suppressions.json

@@ -1782,11 +1782,14 @@
     "typescript/no-unsafe-assignment": {
       "count": 22
     },
+    "typescript/no-unsafe-call": {
+      "count": 1
+    },
     "typescript/no-unsafe-member-access": {
       "count": 2
     },
     "typescript/no-unsafe-return": {
-      "count": 3
+      "count": 4
     }
   },
   "packages/bridge-controller/src/bridge-controller.test.ts": {
@@ -2023,13 +2026,13 @@
       "count": 9
     },
     "typescript/no-unsafe-call": {
-      "count": 2
+      "count": 3
     },
     "typescript/no-unsafe-member-access": {
       "count": 11
     },
     "typescript/no-unsafe-return": {
-      "count": 4
+      "count": 5
     }
   },
   "packages/bridge-status-controller/src/bridge-status-controller.intent-manager.test.ts": {
@@ -2079,13 +2082,13 @@
       "count": 12
     },
     "typescript/no-unsafe-call": {
-      "count": 5
+      "count": 1
     },
     "typescript/no-unsafe-member-access": {
       "count": 13
     },
     "typescript/no-unsafe-return": {
-      "count": 7
+      "count": 3
     }
   },
   "packages/bridge-status-controller/src/bridge-status-controller.ts": {
@@ -3648,11 +3651,6 @@
       "count": 1
     }
   },
-  "packages/mfa-recovery-controller/jest.config.cjs": {
-    "import/unambiguous": {
-      "count": 1
-    }
-  },
   "packages/money-account-api-data-service/jest.config.cjs": {
     "import/unambiguous": {
       "count": 1

packages/bridge-controller/CHANGELOG.md

@@ -7,8 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Export trace-related constants and types to ensure client requests include required fields ([#10893](https://github.com/MetaMask/core/pull/10893))
+  - `SwapFeatureTag` and `BRIDGE_FEATURE_ID_TO_FEATURE_TAG` for supported features
+  - `SwapRelatedTraceName` filters Swap, Bridge and Batch Sell traces from client-defined `TraceName`
+  - `SwapTraceRequest` defines required request fields for swap trace calls
+  - `SwapTraceFunction` is the type-safe shared trace interface that works for mobile, extension and controller consumers
+- Populate the `tags` field with the active `feature` when tracing swap/bridge operations ([#10893](https://github.com/MetaMask/core/pull/10893))
+
 ### Changed
 
+- **BREAKING:** Require `traceFn` when initializing the BridgeController ([#10893](https://github.com/MetaMask/core/pull/10893))
 - Bump `@metamask/keyring-api` from `^24.1.0` to `^24.3.0` ([#10792](https://github.com/MetaMask/core/pull/10792), [#10844](https://github.com/MetaMask/core/pull/10844))
 - Bump `@metamask/transaction-controller` from `^72.2.0` to `^72.2.1` ([#10817](https://github.com/MetaMask/core/pull/10817))
 - Bump `@metamask/accounts-controller` from `^40.0.0` to `^40.1.0` ([#10851](https://github.com/MetaMask/core/pull/10851))

packages/bridge-controller/src/bridge-controller.sse.test.ts

@@ -143,6 +143,7 @@ type WithControllerOptions = {
   options?: Partial<ConstructorParameters<typeof BridgeController>[0]>;
 };
 
+let mockTraceFn: jest.Mock;
 async function withController<ReturnValue>(
   ...args:
     | [WithControllerCallback<ReturnValue>]
@@ -221,6 +222,7 @@ async function withController<ReturnValue>(
     fetchFn: mockFetchFn,
     trackMetaMetricsFn,
     clientVersion: '13.8.0',
+    traceFn: mockTraceFn,
     ...options,
   });
 
@@ -257,6 +259,7 @@ describe('BridgeController SSE', function () {
     jest.clearAllMocks();
     jest.clearAllTimers();
     jest.resetAllMocks();
+    mockTraceFn = jest.fn(async (_request, fn) => await fn?.());
   });
 
   describe('quote tracing', () => {
@@ -400,6 +403,40 @@ describe('BridgeController SSE', function () {
         }),
       );
     });
+
+    it('swallows throws from a synchronous traceFn', async () => {
+      const traceFn = jest.fn(() => {
+        throw new Error('trace failed');
+      });
+
+      await withController(
+        {
+          options: {
+            traceFn,
+          },
+        },
+        async ({ controller, rootMessenger }) => {
+          mockFetchFn.mockImplementationOnce(async () =>
+            mockSseEventSource([mockBridgeQuotesErc20Erc20V1[0]]),
+          );
+          await rootMessenger.call(
+            'BridgeController:updateBridgeQuoteRequestParams',
+            quoteRequest,
+            metricsContext,
+          );
+          jest.advanceTimersByTime(1000);
+          await advanceToNthTimerThenFlush();
+          jest.advanceTimersByTime(11000);
+          await flushPromises();
+
+          expect(traceFn).toHaveBeenCalled();
+          expect(controller.state.quotesLoadingStatus).toBe(
+            RequestStatus.FETCHED,
+          );
+          expect(controller.state.quoteFetchError).toBeNull();
+        },
+      );
+    });
   });
 
   it('should trigger quote polling if request is valid', async function () {
@@ -507,6 +544,57 @@ describe('BridgeController SSE', function () {
         expect(getLayer1GasFeeMock).toHaveBeenCalledTimes(2);
         // eslint-disable-next-line jest/no-restricted-matchers
         expect(trackMetaMetricsFn.mock.calls).toMatchSnapshot();
+        expect(mockTraceFn.mock.calls).toMatchObject([
+          [
+            {
+              data: {
+                request_id: 'test-uuid-1234',
+                result: 'success',
+                srcChainId: 'eip155:1',
+                swap_type: 'crosschain',
+              },
+              name: 'Quote Provider First Result',
+              tags: {
+                feature: 'swap',
+              },
+            },
+            expect.any(Function),
+          ],
+          [
+            {
+              data: {
+                destChainId: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp',
+                feature_id: 'unified_swap_bridge',
+                provider: 'socket_celercircle',
+                request_id: 'test-uuid-1234',
+                result: 'success',
+                srcChainId: 'eip155:1',
+                swap_type: 'crosschain',
+              },
+              name: 'Quote Provider First Result',
+              tags: {
+                feature: 'swap',
+              },
+            },
+            expect.any(Function),
+          ],
+          [
+            {
+              data: {
+                destChainId: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp',
+                feature_id: 'unified_swap_bridge',
+                request_id: 'test-uuid-1234',
+                result: 'success',
+                srcChainId: 'eip155:1',
+              },
+              name: 'Bridge Quotes Fetched',
+              tags: {
+                feature: 'swap',
+              },
+            },
+            expect.any(Function),
+          ],
+        ]);
       },
     );
   });

packages/bridge-controller/src/bridge-controller.ts

@@ -3,7 +3,6 @@ import { BigNumber } from '@ethersproject/bignumber';
 import { Contract } from '@ethersproject/contracts';
 import { Web3Provider } from '@ethersproject/providers';
 import type { StateMetadata } from '@metamask/base-controller';
-import type { TraceCallback, TraceRequest } from '@metamask/controller-utils';
 import type { InternalAccount } from '@metamask/keyring-internal-api';
 import { abiERC20 } from '@metamask/metamask-eth-abis';
 import { StaticIntervalPollingController } from '@metamask/polling-controller';
@@ -22,7 +21,11 @@ import {
 } from './constants/bridge.js';
 import { CHAIN_IDS } from './constants/chains.js';
 import { SWAPS_CONTRACT_ADDRESSES } from './constants/swaps.js';
-import { TraceName } from './constants/traces.js';
+import {
+  BRIDGE_FEATURE_ID_TO_FEATURE_TAG,
+  TraceName,
+} from './constants/traces.js';
+import type { SwapTraceFunction } from './index.js';
 import { RequestStatus } from './types.js';
 import type {
   L1GasFees,
@@ -33,6 +36,7 @@ import type {
   BridgeControllerMessenger,
   FetchFunction,
   InputPrimaryDenomination,
+  SwapTraceRequest,
 } from './types.js';
 import { getAssetIdsForToken, toExchangeRates } from './utils/assets.js';
 import { hasSufficientBalance } from './utils/balance.js';
@@ -277,7 +281,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
     properties: CrossChainSwapsEventProperties<EventName>,
   ) => void;
 
-  readonly #trace: TraceCallback;
+  readonly #trace: SwapTraceFunction<TraceName>;
 
   readonly #config: {
     customBridgeApiBaseUrl?: string;
@@ -316,7 +320,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
       eventName: EventName,
       properties: CrossChainSwapsEventProperties<EventName>,
     ) => void;
-    traceFn?: TraceCallback;
+    traceFn: SwapTraceFunction<TraceName>;
     /**
      * When provided, called to determine whether to use AssetsController for exchange rates.
      * When true, rates are read from AssetsController:getExchangeRatesForBridge instead of
@@ -343,7 +347,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
     this.#fetchFn = fetchFn;
     this.#trackMetaMetricsFn = trackMetaMetricsFn;
     this.#config = config ?? {};
-    this.#trace = traceFn ?? (((_request, fn) => fn?.()) as TraceCallback);
+    this.#trace = traceFn;
     this.#getUseAssetsControllerForRates =
       getUseAssetsControllerForRates ?? (() => false);
 
@@ -858,7 +862,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
 
     const [firstQuoteRequest] = quoteRequests;
     let traceResult: QuoteTraceResult = 'error';
-    let traceName = TraceName.BatchSellQuotesFetched;
+    let traceName: TraceName = TraceName.BatchSellQuotesFetched;
     if (!isBatchSellRequest) {
       traceName = isCrossChain(
         firstQuoteRequest.srcChainId,
@@ -868,9 +872,19 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
         : TraceName.SwapQuotesFetched;
     }
     const tracedProviders = new Set<string>();
-    const traceWithoutImpact = async (request: TraceRequest): Promise<void> => {
+    const traceWithoutImpact = (
+      request: Omit<SwapTraceRequest<TraceName>, 'tags'>,
+    ) => {
       try {
-        await this.#trace(request, () => undefined);
+        this.#trace(
+          {
+            ...request,
+            tags: {
+              feature: BRIDGE_FEATURE_ID_TO_FEATURE_TAG[context.feature_id],
+            },
+          },
+          () => undefined,
+        );
       } catch {
         // Telemetry failures must not affect quote fetching or state updates.
       }
@@ -884,7 +898,6 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
       }
       tracedProviders.add(provider);
       // Provider telemetry must not delay quote processing.
-      // eslint-disable-next-line @typescript-eslint/no-floating-promises
       traceWithoutImpact({
         name: TraceName.QuoteProviderFirstResult,
         startTime: quoteTraceStartTime,
@@ -990,7 +1003,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
         error,
       );
     } finally {
-      await traceWithoutImpact({
+      traceWithoutImpact({
         name: traceName,
         startTime: quoteTraceStartTime,
         data: {

packages/bridge-controller/src/constants/traces.ts

@@ -1,6 +1,53 @@
+import { FeatureId } from '../validators/feature-flags.js';
+
+/**
+ * Sentry trace names published by the BridgeController
+ */
 export enum TraceName {
   BatchSellQuotesFetched = 'Batch Sell Quotes Fetched',
   BridgeQuotesFetched = 'Bridge Quotes Fetched',
   QuoteProviderFirstResult = 'Quote Provider First Result',
   SwapQuotesFetched = 'Swap Quotes Fetched',
+  SwapQuoteFetch = 'Swap Quote Fetch',
+  SwapTokenSearch = 'Swap Token Search',
+  SwapPopularTokensFetch = 'Swap Popular Tokens Fetch',
+  SwapViewLoaded = 'Swap View Loaded',
+  BridgeBalancesUpdated = 'Bridge Balances Updated',
 }
+
+/**
+ * Sentry tags used to identify swap/bridge features
+ */
+export enum TraceFeatureTag {
+  SWAP = 'swap',
+  BATCH_SELL = 'batch_sell',
+  QUICK_BUY = 'quick_buy',
+  PERPS = 'perps',
+  DAPP_SWAP = 'dapp_swap',
+  LIMIT_ORDER = 'limit_order',
+  RECURRING_ORDER = 'recurring_order',
+}
+
+export enum TraceOperation {
+  BridgeScreenPerformance = 'bridge.screen.performance',
+  BridgeDataFetch = 'bridge.data_fetch',
+}
+
+/**
+ * Map of FeatureId to Sentry Trace Feature Tag
+ */
+export const BRIDGE_FEATURE_ID_TO_FEATURE_TAG: Record<
+  FeatureId,
+  TraceFeatureTag
+> = {
+  [FeatureId.UNIFIED_SWAP_BRIDGE]: TraceFeatureTag.SWAP,
+  [FeatureId.BATCH_SELL]: TraceFeatureTag.BATCH_SELL,
+  [FeatureId.PERPS]: TraceFeatureTag.PERPS,
+  [FeatureId.QUICK_BUY_FOLLOW_TRADING]: TraceFeatureTag.QUICK_BUY,
+  [FeatureId.QUICK_BUY_EXPLORE]: TraceFeatureTag.QUICK_BUY,
+  [FeatureId.QUICK_BUY_TOKEN_DETAILS]: TraceFeatureTag.QUICK_BUY,
+  [FeatureId.DAPP_SWAP]: TraceFeatureTag.DAPP_SWAP,
+  [FeatureId.LIMIT_ORDER]: TraceFeatureTag.LIMIT_ORDER,
+  [FeatureId.RECURRING_ORDER]: TraceFeatureTag.RECURRING_ORDER,
+  [FeatureId.UNKNOWN]: TraceFeatureTag.SWAP,
+};

packages/bridge-controller/src/index.ts

@@ -76,6 +76,9 @@ export type {
   BridgeControllerGetStateAction,
   BridgeControllerStateChangeEvent,
   DeepPartial,
+  SwapTraceRequest,
+  SwapTraceFunction,
+  SwapRelatedTraceName,
 } from './types.js';
 
 export {
@@ -271,6 +274,11 @@ export {
   selectTokenWarnings,
 } from './selectors.js';
 
+export {
+  BRIDGE_FEATURE_ID_TO_FEATURE_TAG,
+  TraceFeatureTag as SwapFeatureTag,
+} from './constants/traces.js';
+
 export { DEFAULT_FEATURE_FLAG_CONFIG } from './constants/bridge.js';
 
 export { getBridgeFeatureFlags } from './utils/feature-flags.js';