Set sentry autoTrackSessions default (#20132)

* Set sentry autoTrackSessions default

* endSession....

* fixup

* updated comment

* prevent breaking devmode

* remove changes to beforeSend

* remove additional usage of sinon

app/scripts/controllers/metametrics.js

@@ -425,8 +425,12 @@ export default class MetaMetricsController {
   setParticipateInMetaMetrics(participateInMetaMetrics) {
     let { metaMetricsId } = this.state;
     if (participateInMetaMetrics && !metaMetricsId) {
+      // We also need to start sentry automatic session tracking at this point
+      globalThis.sentry?.startSession();
       metaMetricsId = this.generateMetaMetricsId();
     } else if (participateInMetaMetrics === false) {
+      // We also need to stop sentry automatic session tracking at this point
+      globalThis.sentry?.endSession();
       metaMetricsId = null;
     }
     this.store.updateState({ participateInMetaMetrics, metaMetricsId });

app/scripts/controllers/metametrics.test.js

@@ -146,6 +146,14 @@ describe('MetaMetricsController', function () {
   const now = new Date();
   let clock;
   beforeEach(function () {
+    globalThis.sentry = {
+      startSession: sinon.fake(() => {
+        /** NOOP */
+      }),
+      endSession: sinon.fake(() => {
+        /** NOOP */
+      }),
+    };
     clock = sinon.useFakeTimers(now.getTime());
     sinon.stub(Utils, 'generateRandomId').returns('DUMMY_RANDOM_ID');
   });
@@ -312,6 +320,7 @@ describe('MetaMetricsController', function () {
       });
       assert.equal(metaMetricsController.state.participateInMetaMetrics, null);
       metaMetricsController.setParticipateInMetaMetrics(true);
+      assert.ok(globalThis.sentry.startSession.calledOnce);
       assert.equal(metaMetricsController.state.participateInMetaMetrics, true);
       metaMetricsController.setParticipateInMetaMetrics(false);
       assert.equal(metaMetricsController.state.participateInMetaMetrics, false);
@@ -328,6 +337,7 @@ describe('MetaMetricsController', function () {
     it('should nullify the metaMetricsId when set to false', function () {
       const metaMetricsController = getMetaMetricsController();
       metaMetricsController.setParticipateInMetaMetrics(false);
+      assert.ok(globalThis.sentry.endSession.calledOnce);
       assert.equal(metaMetricsController.state.metaMetricsId, null);
     });
   });

app/scripts/lib/setupSentry.js

@@ -133,8 +133,45 @@ export default function setupSentry({ release, getState }) {
   Sentry.init({
     dsn: sentryTarget,
     debug: METAMASK_DEBUG,
+    /**
+     * autoSessionTracking defaults to true and operates by sending a session
+     * packet to sentry. This session packet does not appear to be filtered out
+     * via our beforeSend or FilterEvents integration. To avoid sending a
+     * request before we have the state tree and can validate the users
+     * preferences, we initiate this to false. Later, in startSession and
+     * endSession we modify this option and start the session or end the
+     * session manually.
+     *
+     * In sentry-install we call toggleSession after the page loads and state
+     * is available, this handles initiating the session for a user who has
+     * opted into MetaMetrics. This script is ran in both the background and UI
+     * so it should be effective at starting the session in both places.
+     *
+     * In the MetaMetricsController the session is manually started or stopped
+     * when the user opts in or out of MetaMetrics. This occurs in the
+     * setParticipateInMetaMetrics function which is exposed to the UI via the
+     * MetaMaskController.
+     *
+     * In actions.ts, after sending the updated participateInMetaMetrics flag
+     * to the background, we call toggleSession to ensure sentry is kept in
+     * sync with the user's preference.
+     *
+     * Types for the global Sentry object, and the new methods added as part of
+     * this effort were added to global.d.ts in the types folder.
+     */
+    autoSessionTracking: false,
     environment,
     integrations: [
+      /**
+       * Filtering of events must happen in this FilterEvents custom
+       * integration instead of in the beforeSend handler because the Dedupe
+       * integration is unaware of the beforeSend functionality. If an event is
+       * queued in the sentry context, additional events of the same name will
+       * be filtered out by Dedupe even if the original event was not sent due
+       * to the beforeSend method returning null.
+       *
+       * @see https://github.com/MetaMask/metamask-extension/pull/15677
+       */
       new FilterEvents({ getMetaMetricsEnabled }),
       new Dedupe(),
       new ExtraErrorData(),
@@ -144,7 +181,64 @@ export default function setupSentry({ release, getState }) {
     beforeBreadcrumb: beforeBreadcrumb(getState),
   });
 
-  return Sentry;
+  /**
+   * As long as a reference to the Sentry Hub can be found, and the user has
+   * opted into MetaMetrics, change the autoSessionTracking option and start
+   * a new sentry session.
+   */
+  const startSession = () => {
+    const hub = Sentry.getCurrentHub?.();
+    const options = hub.getClient?.().getOptions?.() ?? {};
+    if (hub && getMetaMetricsEnabled() === true) {
+      options.autoSessionTracking = true;
+      hub.startSession();
+    }
+  };
+
+  /**
+   * As long as a reference to the Sentry Hub can be found, and the user has
+   * opted out of MetaMetrics, change the autoSessionTracking option and end
+   * the current sentry session.
+   */
+  const endSession = () => {
+    const hub = Sentry.getCurrentHub?.();
+    const options = hub.getClient?.().getOptions?.() ?? {};
+    if (hub && getMetaMetricsEnabled() === false) {
+      options.autoSessionTracking = false;
+      hub.endSession();
+    }
+  };
+
+  /**
+   * Call the appropriate method (either startSession or endSession) depending
+   * on the state of metaMetrics optin and the state of autoSessionTracking on
+   * the Sentry client.
+   */
+  const toggleSession = () => {
+    const hub = Sentry.getCurrentHub?.();
+    const options = hub.getClient?.().getOptions?.() ?? {
+      autoSessionTracking: false,
+    };
+    const isMetaMetricsEnabled = getMetaMetricsEnabled();
+    if (
+      isMetaMetricsEnabled === true &&
+      options.autoSessionTracking === false
+    ) {
+      startSession();
+    } else if (
+      isMetaMetricsEnabled === false &&
+      options.autoSessionTracking === true
+    ) {
+      endSession();
+    }
+  };
+
+  return {
+    ...Sentry,
+    startSession,
+    endSession,
+    toggleSession,
+  };
 }
 
 /**

app/scripts/sentry-install.js

@@ -8,3 +8,22 @@ global.sentry = setupSentry({
   release: process.env.METAMASK_VERSION,
   getState: () => global.stateHooks?.getSentryState?.() || {},
 });
+
+/**
+ * As soon as state is available via getSentryState we can call the
+ * toggleSession method added to sentry in setupSentry to start automatic
+ * session tracking.
+ */
+function waitForStateHooks() {
+  if (global.stateHooks.getSentryState) {
+    // sentry is not defined in dev mode, so if sentry doesn't exist at this
+    // point it means that we are in dev mode and do not need to toggle the
+    // session. Using optional chaining is sufficient to prevent the error in
+    // development.
+    global.sentry?.toggleSession();
+  } else {
+    setTimeout(waitForStateHooks, 100);
+  }
+}
+
+waitForStateHooks();

test/e2e/tests/errors.spec.js

@@ -23,6 +23,36 @@ describe('Sentry errors', function () {
       },
     ],
   };
+  it('should NOT send error events when participateInMetaMetrics is false', async function () {
+    await withFixtures(
+      {
+        fixtures: new FixtureBuilder()
+          .withMetaMetricsController({
+            metaMetricsId: null,
+            participateInMetaMetrics: false,
+          })
+          .build(),
+        ganacheOptions,
+        title: this.test.title,
+        failOnConsoleError: false,
+        testSpecificMock: mockSentry,
+      },
+      async ({ driver, mockedEndpoint }) => {
+        await driver.navigate();
+        await driver.fill('#password', 'correct horse battery staple');
+        await driver.press('#password', driver.Key.ENTER);
+        // Trigger error
+        driver.executeScript('window.stateHooks.throwTestError()');
+        driver.delay(3000);
+        // Wait for Sentry request
+        const isPending = await mockedEndpoint.isPending();
+        assert.ok(
+          isPending,
+          'A request to sentry was sent when it should not have been',
+        );
+      },
+    );
+  });
   it('should send error events', async function () {
     await withFixtures(
       {

types/global.d.ts

@@ -1,15 +1,31 @@
 // 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';
 
 declare class Platform {
   openTab: (opts: { url: string }) => void;
 
   closeCurrentWindow: () => void;
 }
 
+declare class SentryObject extends Sentry {
+  // Verifies that the user has opted into metrics and then updates the sentry
+  // instance to track sessions and begins the session.
+  startSession: () => void;
+
+  // Verifies that the user has opted out of metrics and then updates the
+  // sentry instance to NOT track sessions and ends the current session.
+  endSession: () => void;
+
+  // Calls either startSession or endSession based on optin status
+  toggleSession: () => void;
+}
+
 export declare global {
   var platform: Platform;
+  // Sentry is undefined in dev, so use optional chaining
+  var sentry: SentryObject | undefined;
 
   namespace jest {
     interface Matchers<R> {

ui/index.js

@@ -195,9 +195,11 @@ function setupDebuggingHelpers(store) {
   /**
    * The following stateHook is a method intended to throw an error, used in
    * our E2E test to ensure that errors are attempted to be sent to sentry.
+   *
+   * @param {string} [msg] - The error message to throw, defaults to 'Test Error'
    */
-  window.stateHooks.throwTestError = async function () {
-    const error = new Error('Test Error');
+  window.stateHooks.throwTestError = async function (msg = 'Test Error') {
+    const error = new Error(msg);
     error.name = 'TestError';
     throw error;
   };

ui/store/actions.test.js

@@ -1619,6 +1619,32 @@ describe('Actions', () => {
     });
   });
 
+  describe('#setParticipateInMetaMetrics', () => {
+    beforeAll(() => {
+      window.sentry = {
+        toggleSession: jest.fn(),
+        endSession: jest.fn(),
+      };
+    });
+    it('sets participateInMetaMetrics to true', async () => {
+      const store = mockStore();
+      const setParticipateInMetaMetricsStub = jest.fn((_, cb) => cb());
+
+      background.getApi.returns({
+        setParticipateInMetaMetrics: setParticipateInMetaMetricsStub,
+      });
+
+      _setBackgroundConnection(background.getApi());
+
+      await store.dispatch(actions.setParticipateInMetaMetrics(true));
+      expect(setParticipateInMetaMetricsStub).toHaveBeenCalledWith(
+        true,
+        expect.anything(),
+      );
+      expect(window.sentry.toggleSession).toHaveBeenCalled();
+    });
+  });
+
   describe('#setUseBlockie', () => {
     afterEach(() => {
       sinon.restore();

ui/store/actions.ts

@@ -1570,6 +1570,7 @@ export function updateMetamaskState(
         });
       }
     });
+
     // Also emit an event for the selected account changing, either due to a
     // property update or if the entire account changes.
     if (isEqual(oldSelectedAccount, newSelectedAccount) === false) {
@@ -2883,6 +2884,12 @@ export function setParticipateInMetaMetrics(
             reject(err);
             return;
           }
+          /**
+           * We need to inform sentry that the user's optin preference may have
+           * changed. The logic to determine which way to toggle is in the
+           * toggleSession handler in setupSentry.js.
+           */
+          window.sentry?.toggleSession();
 
           dispatch({
             type: actionConstants.SET_PARTICIPATE_IN_METAMETRICS,