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

Add initial Sentry setup for crashes and perf tracking #7141

Merged
merged 10 commits into from
Oct 5, 2022

Conversation

amitkma
Copy link
Contributor

@amitkma amitkma commented Sep 15, 2022

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

  • Integrates basic Sentry setup to monitor crashes and unhandled exceptions.

Motivation and context

Closes #7076

Tests

  • It is hard to test because you have to manually throw an exception in code to test it. Otherwise it's difficult to produce a crash 🙃.

Tested devices

  • Physical
  • Emulator
  • OS version(s): Android 11 and 12

Checklist

@amitkma amitkma added the A-Telemetry Telemetry / analytics to understand usage label Sep 15, 2022
@amitkma amitkma force-pushed the feature/amit/sentry-integration branch 2 times, most recently from 7150916 to 72c03ee Compare September 15, 2022 15:56
@amitkma amitkma force-pushed the feature/amit/sentry-integration branch from 72c03ee to e315bc2 Compare September 16, 2022 10:11
@amitkma amitkma marked this pull request as ready for review September 16, 2022 10:47
@bmarty
Copy link
Member

bmarty commented Sep 16, 2022

It is hard to test because you have to manually throw an exception in code to test it. Otherwise it's difficult to produce a crash 🙃.

We have a hidden feature to crash the app for this specific purpose:

  • enable developer mode
  • restart the app
  • click on the green setting button which is now displayed
  • click on the button "CRASH THE APP"

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Nice work, thanks!
Just a small remark, else LGTM!

@amitkma
Copy link
Contributor Author

amitkma commented Sep 19, 2022

It is hard to test because you have to manually throw an exception in code to test it. Otherwise it's difficult to produce a crash 🙃.

We have a hidden feature to crash the app for this specific purpose:

  • enable developer mode
  • restart the app
  • click on the green setting button which is now displayed
  • click on the button "CRASH THE APP"

Thank you @bmarty, I wasn't aware of it.

@ElementBot
Copy link

ElementBot commented Sep 20, 2022

Warnings
⚠️

vector/src/main/AndroidManifest.xml#L206 - Consider splitting data tag into multiple tags with individual attributes to avoid confusion

⚠️

vector/src/main/AndroidManifest.xml#L206 - Consider splitting data tag into multiple tags with individual attributes to avoid confusion

⚠️

vector/src/main/AndroidManifest.xml#L209 - Consider splitting data tag into multiple tags with individual attributes to avoid confusion

⚠️

vector/src/main/AndroidManifest.xml#L209 - Consider splitting data tag into multiple tags with individual attributes to avoid confusion

⚠️

vector/src/main/AndroidManifest.xml#L263 - Expecting android:screenOrientation="unspecified" or "fullSensor" for this activity so the user can use the application in any orientation and provide a great experience on Chrome OS devices

⚠️

vector/src/main/AndroidManifest.xml#L263 - Expecting android:screenOrientation="unspecified" or "fullSensor" for this activity so the user can use the application in any orientation and provide a great experience on Chrome OS devices

⚠️

vector/src/main/AndroidManifest.xml#L272 - Attribute supportsPictureInPicture is only used in API level 24 and higher (current min is 21)

⚠️

vector/src/main/AndroidManifest.xml#L272 - Attribute supportsPictureInPicture is only used in API level 24 and higher (current min is 21)

⚠️

vector/src/main/AndroidManifest.xml#L279 - Attribute supportsPictureInPicture is only used in API level 24 and higher (current min is 21)

⚠️

vector/src/main/AndroidManifest.xml#L279 - Attribute supportsPictureInPicture is only used in API level 24 and higher (current min is 21)

⚠️

vector/src/main/AndroidManifest.xml#L285 - Attribute supportsPictureInPicture is only used in API level 24 and higher (current min is 21)

⚠️

vector/src/main/AndroidManifest.xml#L285 - Attribute supportsPictureInPicture is only used in API level 24 and higher (current min is 21)

⚠️

vector/src/main/AndroidManifest.xml#L404 - Exported receiver does not require permission

⚠️

vector/src/main/AndroidManifest.xml#L404 - Exported receiver does not require permission

Generated by 🚫 dangerJS against d118bdf

@sonarcloud
Copy link

sonarcloud bot commented Sep 27, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

58.3% 58.3% Coverage
0.0% 0.0% Duplication

@bmarty
Copy link
Member

bmarty commented Oct 5, 2022

Hello @amitkma once approved, as a PR submitter, please feel free to merge your PR :) Thanks!

@amitkma amitkma merged commit aad2eed into develop Oct 5, 2022
@amitkma amitkma deleted the feature/amit/sentry-integration branch October 5, 2022 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Telemetry Telemetry / analytics to understand usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sentry Integration (if consent provided)
4 participants