Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

Gudahtt
Copy link
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.

Copy link

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↗︎

Copy link

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
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 kumavis and a team 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
64 of 68 checks passed
@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
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
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 team-wallet-framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants