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

Bump sentry version and enable performance metrics #6015

Merged
merged 3 commits into from
Apr 3, 2023

Conversation

Fatxx
Copy link
Contributor

@Fatxx Fatxx commented Mar 22, 2023

Development & PR Process

  1. Follow MetaMask Mobile Coding Standards
  2. Add release-xx label to identify the PR slated for a upcoming release (will be used in release discussion)
  3. Add needs-dev-review label when work is completed
  4. Add needs-qa label when dev review is completed
  5. Add QA Passed label when QA has signed off

Description

This PR aims to add support for new Sentry Performance monitoring metrics.

Gotchas:

  • The screenshot presented below is from a personal project since we are already on the limit of transactions in our Sentry I had to take this route in order to test the functionality;
  • Migration of native code doesn't require any changes on our codebase;

Screenshots/Recordings
Screen Shot 2023-03-22 at 09 22 28
Issue

Progresses #???

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@Fatxx Fatxx requested a review from a team as a code owner March 22, 2023 10:01
@github-actions
Copy link
Contributor

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.

@Fatxx Fatxx added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Mar 22, 2023
@socket-security
Copy link

socket-security bot commented Mar 22, 2023

New dependency changes detected. Learn more about Socket for GitHub ↗︎


👍 No new dependency issues detected in pull request

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

Ignoring: @sentry/cli@1.75.0

Pull request alert summary
Issue Status
Install scripts ✅ 0 issues
Native code ✅ 0 issues
Bin script shell injection ✅ 0 issues
Unresolved require ✅ 0 issues
Invalid package.json ✅ 0 issues
HTTP dependency ✅ 0 issues
Git dependency ✅ 0 issues
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues

📊 Modified Dependency Overview:

⬆️ Updated Package Version Diff Capability Access +/- Transitive Count Publisher
@sentry/react-native@3.0.3 2.4.2...3.0.3 network, filesystem, shell, environment +10/-18 sentry-bot

@Fatxx
Copy link
Contributor Author

Fatxx commented Mar 22, 2023

@SocketSecurity @sentry/cli@1.75.0

Copy link
Contributor

@tommasini tommasini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just please look at what is failing on the pipeline

app/util/sentryUtils.js Show resolved Hide resolved
@Fatxx
Copy link
Contributor Author

Fatxx commented Mar 23, 2023

This needs further change in sample rate

Copy link
Contributor

@NicolasMassart NicolasMassart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C: prevent including unrelated formatting changes in the PR.
The wdio folder is excluded from linter so this will not make your PR fail if you don't apply prettier to it.

@sethkfman
Copy link
Contributor

@SocketSecurity ignore @sentry/cli@1.75.0

@sethkfman sethkfman added the No QA Needed/E2E Only Apply this label when your PR does not need any QA effort. label Mar 24, 2023
Copy link
Contributor

@sethkfman sethkfman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, fix the yarn.lock conflicts and it can be merged

@sethkfman sethkfman removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Apr 3, 2023
@Fatxx Fatxx merged commit e609da3 into main Apr 3, 2023
@Fatxx Fatxx deleted the feat/enable-sentry-performance branch April 3, 2023 22:39
@github-actions github-actions bot locked and limited conversation to collaborators Apr 3, 2023
@sethkfman sethkfman added the release-6.4.0 PR for release 6.4.0 label Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
No QA Needed/E2E Only Apply this label when your PR does not need any QA effort. release-6.4.0 PR for release 6.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants