refactor: add performance tracing infrastructure

app/scripts/lib/setupSentry.js

@@ -7,11 +7,13 @@ import { filterEvents } from './sentry-filter-events';
 
 const projectLogger = createProjectLogger('sentry');
 
-const log = createModuleLogger(
+export const log = createModuleLogger(
   projectLogger,
   globalThis.document ? 'ui' : 'background',
 );
 
+const internalLog = createModuleLogger(log, 'internal');
+
 /* eslint-disable prefer-destructuring */
 // Destructuring breaks the inlining of the environment variables
 const METAMASK_BUILD_TYPE = process.env.METAMASK_BUILD_TYPE;
@@ -466,6 +468,7 @@ export default function setupSentry() {
 
   return {
     ...Sentry,
+    getMetaMetricsEnabled,
   };
 }
 
@@ -482,6 +485,7 @@ function getClientOptions() {
     integrations: [
       Sentry.dedupeIntegration(),
       Sentry.extraErrorDataIntegration(),
+      Sentry.browserTracingIntegration(),
       filterEvents({ getMetaMetricsEnabled, log }),
     ],
     release: RELEASE,
@@ -492,6 +496,7 @@ function getClientOptions() {
     // we can safely turn them off by setting the `sendClientReports` option to
     // `false`.
     sendClientReports: false,
+    tracesSampleRate: 0.01,
     transport: makeTransport,
   };
 }
@@ -648,6 +653,7 @@ function setSentryClient() {
     release,
   });
 
+  Sentry.registerSpanErrorInstrumentation();
   Sentry.init(clientOptions);
 
   addDebugListeners();
@@ -872,7 +878,7 @@ function integrateLogging() {
   for (const loggerType of ['log', 'error']) {
     logger[loggerType] = (...args) => {
       const message = args[0].replace(`Sentry Logger [${loggerType}]: `, '');
-      log(message, ...args.slice(1));
+      internalLog(message, ...args.slice(1));
     };
   }
 
@@ -887,18 +893,14 @@ function addDebugListeners() {
   const client = Sentry.getClient();
 
   client?.on('beforeEnvelope', (event) => {
-    const type = event?.[1]?.[0]?.[0]?.type;
-    const data = event?.[1]?.[0]?.[1] ?? {};
-
-    if (type !== 'session' || data.status !== 'exited') {
-      return;
+    if (isCompletedSessionEnvelope(event)) {
+      log('Completed session', event);
     }
-
-    log('Completed session', data);
   });
 
   client?.on('afterSendEvent', (event) => {
-    log('Event', event);
+    const type = getEventType(event);
+    log(type, event);
   });
 
   log('Added debug listeners');
@@ -915,3 +917,22 @@ function makeTransport(options) {
     return await fetch(...args);
   });
 }
+
+function isCompletedSessionEnvelope(envelope) {
+  const type = envelope?.[1]?.[0]?.[0]?.type;
+  const data = envelope?.[1]?.[0]?.[1] ?? {};
+
+  return type === 'session' && data.status === 'exited';
+}
+
+function getEventType(event) {
+  if (event.type === 'transaction') {
+    return 'Trace';
+  }
+
+  if (event.level === 'error') {
+    return 'Error';
+  }
+
+  return 'Event';
+}

development/README.md

@@ -55,39 +55,22 @@ You can inspect the requests in the `Network` tab of your browser's Developer To
 by filtering for `POST` requests to `/v1/batch`. The full url will be `http://localhost:9090/v1/batch`
 or `https://api.segment.io/v1/batch` respectively.
 
-## Sentry
+## Debugging Sentry
 
-### Debugging in Sentry
+1. Set `SENTRY_DSN_DEV`, or `SENTRY_DSN` if using a production build, in `.metamaskrc` to a suitable Sentry URL.
+  - The example value specified in `.metamaskrc.dist` uses the `test-metamask` project in the MetaMask account.
+  - Alternatively, create a free Sentry account with a new organization and project.
+  - The DSN is specified in: `Settings > Projects > [Project Name] > Client Keys (DSN)`.
 
-To debug in a production Sentry environment:
+2. To display Sentry logs, include `DEBUG=metamask:sentry:*` in `.metamaskrc`.
 
-- If you have not already got a Sentry account, you can create a free account on [Sentry](https://sentry.io/)
-- Create a New Sentry Organization
-    - If you already have an existing Sentry account and workspace, open the sidebar drop down menu, then click `Switch organization` followed by `Create a new organization`
-- Create a New Project
-- Copy the `Public Key` and `Project ID` from the Client Keys section under your projects Settings
-    - Select `Settings` in the sidebar menu, then select `Projects` in the secondary menu. Click your project then select `Client Keys (DSN)` from the secondary menu. Click the `Configure` button on the `Client Keys` page and copy your `Project Id` and `Public Key`
--   Add/replace the `SENTRY_DSN` and `SENTRY_DSN_DEV` variables in `.metamaskrc`
-    ```
-    SENTRY_DSN_DEV=https://{SENTRY_PUBLIC_KEY}@sentry.io/{SENTRY_PROJECT_ID}
-    SENTRY_DSN=https://{SENTRY_PUBLIC_KEY}@sentry.io/{SENTRY_PROJECT_ID}
-    ```
--   Build the project to the `./dist/` folder with `yarn dist`
+3. To display more verbose logs if not in a developer build, include `METAMASK_DEBUG=true` in `.metamaskrc`.
 
-Errors reported whilst using the extension will be displayed in Sentry's `Issues` page.
+4. Ensure metrics are enabled during onboarding or via `Settings > Security & privacy > Participate in MetaMetrics`.
 
-To debug in test build we need to comment out the below:- <br>
-- `setupSentry` function comment the return statement in the `app/scripts/lib
-/setupSentry.js`  https://github.com/MetaMask/metamask-extension/blob/e3c76ca699e94bacfc43793d28291fa5ddf06752/app/scripts/lib/setupSentry.js#L496
-- `setupStateHooks` function set the if condition to true in the `ui
-/index.js` https://github.com/MetaMask/metamask-extension/blob/e3c76ca699e94bacfc43793d28291fa5ddf06752/ui/index.js#L242
+5. To test Sentry via the developer options in the UI, include `ENABLE_SETTINGS_PAGE_DEV_OPTIONS=true` in `.metamaskrc`.
 
-How to trigger Sentry error:-
-1. Open the background console.
-2. Load the extension app and open the developer console.
-3. Toggle the `Participate in MetaMetrics` menu option to the `ON` position.
-4. Enter `window.stateHooks.throwTestBackgroundError()` into the developer console.
-5. There should now be requests sent to sentry in the background network tab.
+6. Alternatively, call `window.stateHooks.throwTestError()` or `window.stateHooks.throwTestBackgroundError()` via the UI console.
 
 ## Source Maps
 

shared/lib/trace.test.ts

@@ -0,0 +1,112 @@
+import { Span, startSpan, withIsolationScope } from '@sentry/browser';
+import { trace } from './trace';
+
+jest.mock('@sentry/browser', () => ({
+  withIsolationScope: jest.fn(),
+  startSpan: jest.fn(),
+}));
+
+const NAME_MOCK = 'testTransaction';
+const PARENT_CONTEXT_MOCK = {} as Span;
+
+const TAGS_MOCK = {
+  tag1: 'value1',
+  tag2: true,
+  tag3: 123,
+};
+
+const DATA_MOCK = {
+  data1: 'value1',
+  data2: true,
+  data3: 123,
+};
+
+function mockGetMetaMetricsEnabled(enabled: boolean) {
+  global.sentry = {
+    getMetaMetricsEnabled: () => Promise.resolve(enabled),
+  };
+}
+
+describe('Trace', () => {
+  const startSpanMock = jest.mocked(startSpan);
+  const withIsolationScopeMock = jest.mocked(withIsolationScope);
+  const setTagsMock = jest.fn();
+
+  beforeEach(() => {
+    jest.resetAllMocks();
+
+    startSpanMock.mockImplementation((_, fn) => fn({} as Span));
+
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    withIsolationScopeMock.mockImplementation((fn: any) =>
+      fn({ setTags: setTagsMock }),
+    );
+  });
+
+  describe('trace', () => {
+    // @ts-expect-error This function is missing from the Mocha type definitions
+    it.each([
+      ['enabled', true],
+      ['disabled', false],
+    ])(
+      'executes callback if Sentry is %s',
+      async (_: string, sentryEnabled: boolean) => {
+        let callbackExecuted = false;
+
+        mockGetMetaMetricsEnabled(sentryEnabled);
+
+        await trace({ name: NAME_MOCK }, async () => {
+          callbackExecuted = true;
+        });
+
+        expect(callbackExecuted).toBe(true);
+      },
+    );
+
+    // @ts-expect-error This function is missing from the Mocha type definitions
+    it.each([
+      ['enabled', true],
+      ['disabled', false],
+    ])(
+      'returns value from callback if Sentry is %s',
+      async (_: string, sentryEnabled: boolean) => {
+        mockGetMetaMetricsEnabled(sentryEnabled);
+
+        const result = await trace({ name: NAME_MOCK }, async () => {
+          return true;
+        });
+
+        expect(result).toBe(true);
+      },
+    );
+
+    it('invokes Sentry if enabled', async () => {
+      mockGetMetaMetricsEnabled(true);
+
+      await trace(
+        {
+          name: NAME_MOCK,
+          tags: TAGS_MOCK,
+          data: DATA_MOCK,
+          parentContext: PARENT_CONTEXT_MOCK,
+        },
+        async () => Promise.resolve(),
+      );
+
+      expect(withIsolationScopeMock).toHaveBeenCalledTimes(1);
+
+      expect(startSpanMock).toHaveBeenCalledTimes(1);
+      expect(startSpanMock).toHaveBeenCalledWith(
+        {
+          name: NAME_MOCK,
+          parentSpan: PARENT_CONTEXT_MOCK,
+          attributes: DATA_MOCK,
+        },
+        expect.any(Function),
+      );
+
+      expect(setTagsMock).toHaveBeenCalledTimes(1);
+      expect(setTagsMock).toHaveBeenCalledWith(TAGS_MOCK);
+    });
+  });
+});

shared/lib/trace.ts

@@ -0,0 +1,54 @@
+import * as Sentry from '@sentry/browser';
+import { Primitive } from '@sentry/types';
+import { createModuleLogger } from '@metamask/utils';
+import { log as sentryLogger } from '../../app/scripts/lib/setupSentry';
+
+const log = createModuleLogger(sentryLogger, 'trace');
+
+export type TraceRequest = {
+  data?: Record<string, number | string | boolean>;
+  name: string;
+  parentContext?: unknown;
+  tags?: Record<string, number | string | boolean>;
+};
+
+export async function trace<T>(
+  request: TraceRequest,
+  fn: (context?: unknown) => Promise<T>,
+): Promise<T> {
+  const { data: attributes, name, parentContext, tags } = request;
+  const parentSpan = (parentContext ?? null) as Sentry.Span | null;
+
+  const isSentryEnabled =
+    (await globalThis.sentry.getMetaMetricsEnabled()) as boolean;
+
+  const callback = async (span: Sentry.Span | null) => {
+    log('Starting trace', name, request);
+
+    const start = Date.now();
+    let error;
+
+    try {
+      return await fn(span);
+    } catch (currentError) {
+      error = currentError;
+      throw currentError;
+    } finally {
+      const end = Date.now();
+      const duration = end - start;
+
+      log('Finished trace', name, duration, { error, request });
+    }
+  };
+
+  if (!isSentryEnabled) {
+    log('Skipping Sentry trace as metrics disabled', name, request);
+    return callback(null);
+  }
+
+  return await Sentry.withIsolationScope(async (scope) => {
+    scope.setTags(tags as Record<string, Primitive>);
+
+    return await Sentry.startSpan({ name, parentSpan, attributes }, callback);
+  });
+}

types/global.d.ts

@@ -1,6 +1,10 @@
+// Many of the state hooks return untyped raw state.
+/* eslint-disable @typescript-eslint/no-explicit-any */
+
 // In order for variables to be considered on the global scope they must be
 // declared using var and not const or let, which is why this rule is disabled
 /* eslint-disable no-var */
+
 import * as Sentry from '@sentry/browser';
 import {
   Success,
@@ -224,13 +228,32 @@ declare class Chrome {
   runtime: Runtime;
 }
 
-type SentryObject = Sentry;
+type SentryObject = Sentry & {
+  getMetaMetricsEnabled: () => Promise<boolean>;
+};
 
 type HttpProvider = {
   host: string;
   timeout: number;
 };
 
+type StateHooks = {
+  getCleanAppState?: () => Promise<any>;
+  getLogs?: () => any[];
+  getMostRecentPersistedState?: () => any;
+  getPersistedState: () => Promise<any>;
+  getSentryAppState?: () => any;
+  getSentryState: () => {
+    browser: string;
+    version: string;
+    state?: any;
+    persistedState?: any;
+  };
+  metamaskGetState?: () => Promise<any>;
+  throwTestBackgroundError?: (msg?: string) => Promise<void>;
+  throwTestError?: (msg?: string) => void;
+};
+
 export declare global {
   var platform: Platform;
   // Sentry is undefined in dev, so use optional chaining
@@ -240,6 +263,8 @@ export declare global {
 
   var ethereumProvider: HttpProvider;
 
+  var stateHooks: StateHooks;
+
   namespace jest {
     // The interface is being used for declaration merging, which is an acceptable exception to this rule.
     // eslint-disable-next-line @typescript-eslint/consistent-type-definitions