Skip to content

fix: sentry sessions (#26192) [cherry-pick]#26620

Merged
Gudahtt merged 1 commit intoVersion-v12.1.0from
cherry-pick/fix-sentry-sessions
Aug 22, 2024
Merged

fix: sentry sessions (#26192) [cherry-pick]#26620
Gudahtt merged 1 commit intoVersion-v12.1.0from
cherry-pick/fix-sentry-sessions

Conversation

@Gudahtt
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt commented Aug 22, 2024

Cherry-pick of #26192 for v12.1.0. Original description:

Description

Fix the automatic tracking of Sentry sessions, and refactor Sentry setup to simplify logic and improve logging.

Specifically:

  • 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.
    • Since the file already references process.env and global.stateHooks internally.
  • Flatten all the functions within setupSentry.js.
    • Since they no longer require a dynamic getState callback.
  • Log all Sentry messages via the debug package based on the DEBUG environment variable.
    • Create separate loggers for UI and background to differentiate logs.
    • e.g. DEBUG=metamask:sentry:* in .metamask.rc
  • Add additional log messages for easier debug.
    • Including all sent events and completed sessions if METAMASK_DEBUG is enabled.
  • Add mock SENTRY_DSN_DEV to Flask and MMI test builds.
  • Remove duplication in package scripts.

Open in GitHub Codespaces

Related issues

Fixes: #2555 #15691

Manual testing steps

Screenshots/Recordings

Before

After

Updated Logs

Pre-merge author checklist

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.

@socket-security
Copy link
Copy Markdown

socket-security bot commented Aug 22, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@sentry/types@8.20.0 None 0 329 kB sentry-bot

🚮 Removed packages: npm/@sentry/types@8.19.0)

View full report↗︎

@Gudahtt Gudahtt force-pushed the cherry-pick/fix-sentry-sessions branch from 64eb4c5 to 95ecf1e Compare August 22, 2024 16:54
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 22, 2024

Codecov Report

Attention: Patch coverage is 18.09524% with 86 lines in your changes missing coverage. Please review.

Project coverage is 69.82%. Comparing base (5d4bd9d) to head (95ecf1e).

Files Patch % Lines
app/scripts/lib/setupSentry.js 15.46% 82 Missing ⚠️
app/scripts/lib/sentry-filter-events.ts 0.00% 3 Missing ⚠️
app/scripts/sentry-install.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                        Coverage Diff                         @@
##           cherry-pick/update-to-sentry-8   #26620      +/-   ##
==================================================================
- Coverage                           69.83%   69.82%   -0.01%     
==================================================================
  Files                                1368     1368              
  Lines                               48653    48670      +17     
  Branches                            13434    13429       -5     
==================================================================
+ Hits                                33975    33981       +6     
- Misses                              14678    14689      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [95ecf1e]
Page Load Metrics (155 ± 169 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint711761072211
domContentLoaded9502484
load441690155352169
domInteractive9502484

Base automatically changed from cherry-pick/update-to-sentry-8 to Version-v12.1.0 August 22, 2024 18:53
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.
@Gudahtt Gudahtt force-pushed the cherry-pick/fix-sentry-sessions branch from 95ecf1e to eb89eaa Compare August 22, 2024 18:54
@Gudahtt Gudahtt marked this pull request as ready for review August 22, 2024 18:54
@Gudahtt Gudahtt requested review from a team and kumavis as code owners August 22, 2024 18:54
@hjetpoluru hjetpoluru self-requested a review August 22, 2024 19:04
@Gudahtt Gudahtt merged commit 04c5fc6 into Version-v12.1.0 Aug 22, 2024
@Gudahtt Gudahtt deleted the cherry-pick/fix-sentry-sessions branch August 22, 2024 19:35
@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2024
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [eb89eaa]
Page Load Metrics (74 ± 17 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint582891044722
domContentLoaded98224168
load42203743517
domInteractive98224168

@metamaskbot metamaskbot added the release-12.2.0 Issue or pull request that will be included in release 12.2.0 label Aug 28, 2024
@metamaskbot
Copy link
Copy Markdown
Collaborator

No release label on PR. Adding release label release-12.2.0 on PR, as PR was cherry-picked in branch 12.2.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-12.2.0 Issue or pull request that will be included in release 12.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants