Skip to content

Commit

Permalink
fix: sentry sessions (#26192)
Browse files Browse the repository at this point in the history
Remove all dynamic `autoSessionTracking` enablement.
Add a custom Sentry transport to prevent any requests reaching Sentry if metrics are disabled.
Only consider metrics enabled when preference is set and onboarding is complete.
Enable Sentry if `METAMASK_ENVIRONMENT` is not `production` and `SENTRY_DSN_DEV` is specified.
Remove the arguments to the `setupSentry` function.
Flatten all the functions within `setupSentry.js`.
Log all Sentry messages via the `debug` package based on the `DEBUG` environment variable.
Add additional log messages for easier debug.
Add mock `SENTRY_DSN_DEV` to Flask and MMI test builds.
Remove duplication in package scripts.
  • Loading branch information
matthewwalsh0 authored and Gudahtt committed Aug 22, 2024
1 parent 5d4bd9d commit 95ecf1e
Show file tree
Hide file tree
Showing 13 changed files with 249 additions and 292 deletions.
7 changes: 6 additions & 1 deletion .metamaskrc.dist
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ BLOCKAID_PUBLIC_KEY=
; Set this to true to enable the snap path for the Firefox WebDriver (Linux)
; FIREFOX_SNAP=

ENABLE_CONFIRMATION_REDESIGN=
; Enable the redesigned confirmations still in development, without needing to toggle the developer setting.
; ENABLE_CONFIRMATION_REDESIGN=

; Enables the Settings Page - Developer Options
; ENABLE_SETTINGS_PAGE_DEV_OPTIONS=true

; The endpoint used to submit errors and tracing data to Sentry.
; The below is the `test-metamask` project.
; SENTRY_DSN_DEV=https://[email protected]/273496
17 changes: 8 additions & 9 deletions app/scripts/controllers/metametrics.js
Original file line number Diff line number Diff line change
Expand Up @@ -454,16 +454,15 @@ export default class MetaMetricsController {
* if not set
*/
async setParticipateInMetaMetrics(participateInMetaMetrics) {
let { metaMetricsId } = this.state;
if (participateInMetaMetrics && !metaMetricsId) {
// We also need to start sentry automatic session tracking at this point
await globalThis.sentry?.startSession();
metaMetricsId = this.generateMetaMetricsId();
} else if (participateInMetaMetrics === false) {
// We also need to stop sentry automatic session tracking at this point
await globalThis.sentry?.endSession();
}
const { metaMetricsId: existingMetaMetricsId } = this.state;

const metaMetricsId =
participateInMetaMetrics && !existingMetaMetricsId
? this.generateMetaMetricsId()
: existingMetaMetricsId;

this.store.updateState({ participateInMetaMetrics, metaMetricsId });

if (participateInMetaMetrics) {
this.trackEventsAfterMetricsOptIn();
this.clearEventsAfterMetricsOptIn();
Expand Down
7 changes: 1 addition & 6 deletions app/scripts/controllers/metametrics.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,7 @@ function getMetaMetricsController({
describe('MetaMetricsController', function () {
const now = new Date();
beforeEach(function () {
globalThis.sentry = {
startSession: jest.fn(),
endSession: jest.fn(),
};
globalThis.sentry = {};
jest.useFakeTimers().setSystemTime(now.getTime());
jest.spyOn(Utils, 'generateRandomId').mockReturnValue('DUMMY_RANDOM_ID');
});
Expand Down Expand Up @@ -316,7 +313,6 @@ describe('MetaMetricsController', function () {
metaMetricsController.state.participateInMetaMetrics,
).toStrictEqual(null);
await metaMetricsController.setParticipateInMetaMetrics(true);
expect(globalThis.sentry.startSession).toHaveBeenCalledTimes(1);
expect(
metaMetricsController.state.participateInMetaMetrics,
).toStrictEqual(true);
Expand All @@ -339,7 +335,6 @@ describe('MetaMetricsController', function () {
it('should not nullify the metaMetricsId when set to false', async function () {
const metaMetricsController = getMetaMetricsController();
await metaMetricsController.setParticipateInMetaMetrics(false);
expect(globalThis.sentry.endSession).toHaveBeenCalledTimes(1);
expect(metaMetricsController.state.metaMetricsId).toStrictEqual(
TEST_META_METRICS_ID,
);
Expand Down
15 changes: 12 additions & 3 deletions app/scripts/lib/sentry-filter-events.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Event as SentryEvent, Integration } from '@sentry/types';
import { logger } from '@sentry/utils';

const NAME = 'FilterEvents';

Expand All @@ -8,17 +7,27 @@ const NAME = 'FilterEvents';
*
* @param options - Options bag.
* @param options.getMetaMetricsEnabled - Function that returns whether MetaMetrics is enabled.
* @param options.log - Function to log messages.
*/
export function filterEvents({
getMetaMetricsEnabled,
log,
}: {
getMetaMetricsEnabled: () => Promise<boolean>;
log: (message: string) => void;
}): Integration {
return {
name: NAME,
processEvent: async (event: SentryEvent) => {
if (!(await getMetaMetricsEnabled())) {
logger.warn(`Event dropped due to MetaMetrics setting.`);
// This integration is required in addition to the custom transport as it provides an
// asynchronous context which we may need in order to read the persisted state from the
// store, so it can later be added to the event via the `beforeSend` overload.
// It also provides a more native solution for discarding events, but any
// session requests will always be handled by the custom transport.
const metricsEnabled = await getMetaMetricsEnabled();

if (!metricsEnabled) {
log('Event dropped as metrics disabled');
return null;
}

Expand Down
Loading

0 comments on commit 95ecf1e

Please sign in to comment.