metamask-extension
metamask-extension copied to clipboard
Fix sentry sessions
Description
The session count in our Sentry dashboard has decreased drastically since v11.1.0.
It seems that the only time the session count is incremented is when using captureException
and captureMessage
methods. This explains why the session count is much lower than previously but not zero. It also explains a crash rate close to but different than 100% — because captureMessage
is much less used than captureException on our repository.
Investigation Notes & Fix
The toggleSession()
function call inside setParticipateInMetaMetrics()
in the ui/store/actions.ts
file is redundant, as both onboarding/metametrics.js
(for onboarding) and ui/pages/settings/security-tab/security-tab.component.js
(for the toggling flow) call setParticipateInMetaMetrics()
on the MetaMetrics controller that starts or stops the session appropriately and updates the state directly. Moreover, by the time toggleSession()
is called inside setParticipateInMetaMetrics()
in the ui/store/actions.ts
file, the participateInMetaMetrics
property in the MetaMetrics controller state has not yet been updated. After removing that toggleSession()
function call, we can see that the state is changed on both flows by using copy(await window.stateHooks.getCleanAppState())
.
We also don’t need to use getMetaMetricsEnabled()
for startSession()
and endSession()
anymore because setParticipateInMetaMetrics()
is explicitly called with an argument that dictates whether to call the start session or end session methods. As such, I removed getMetaMetricsEnabled
from the conditions on startSession
and endSession
.
Using Hub.captureSession
and Hub.captureSession(true)
on start and end session respectively yields the right behaviour for the https://sentry.io/api/<project_id>/envelope/?sentry_key=<project secret>
requests. When starting a session the request payload includes "init": true
and when ending a session, the request payload includes "init": false
and a "duration"
property.
Finally, the sessions are still not captured on the sentry dashboard, unless the @sentry/browser
package is updated. This may be because we used the newer version for testing with sentry-example
, and two different package versions on the same project may confuse the dashboard internals. Upgrading the package to the latest version fixes this.
Summary of changes:
- Remove redundant
toggleSession()
function call fromsetParticipateInMetaMetrics
inui/store/actions.ts
. - Remove
toggleSession()
function definition fromapp/scripts/lib/setupSentry.js
because it’s not used anymore. - Remove usage of
getMetaMetricsEnabled()
insidestartSession()
andendSession()
inapp/scripts/lib/setupSentry.js
because repeated calls for both methods are not possible. - Update sentry browser package version from ^7.53.0 to ^7.81.1 so the sessions are logged on the sentry dashboard.
Manual testing steps
-
Create a new project on sentry as per the documentation
-
Remove the extension completely
-
Build the extension using
yarn dist
-
Install the extension from the
dist
folder -
Open the dev tools on the network tab for the UI and background processes.
-
Onboard opting into MetaMetrics
-
See the Sentry request with type “session” and “init” true.
-
You should now see the sessions on Releases > Adoption.
-
Reload the Extension on the Extension tab
-
Repeat Steps 5-8
-
Reload the Extension UI in the browser
-
Repeat Steps 5-8
-
When opting in and out of metametrics, Sentry session requests should be visible as well.
Pre-merge author checklist
- [ ] I’ve followed MetaMask Coding Standards.
- [ ] I've clearly explained what problem this PR is solving and how it is solved.
- [ ] I've linked related issues
- [ ] I've included manual testing steps
- [ ] I've included screenshots/recordings if applicable
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using JSDoc format if applicable
- [ ] I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
- [ ] I’ve properly set the pull request status:
- [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft".
Pre-merge reviewer checklist
- [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.
@metamaskbot update-policies
Policies updated