Configure Sentry for Mobile #21682
Configure Sentry for Mobile #21682
Conversation
Jenkins BuildsClick to see older builds (72)
|
ilmotta
force-pushed
the
ilmotta/add-sentry-support
branch
from
ce51060 to
3fc9ea1
Compare
ilmotta
force-pushed
the
ilmotta/add-sentry-support
branch
2 times, most recently
from
4d06118 to
dd61539
Compare
ilmotta
changed the title
ilmotta
force-pushed
the
ilmotta/add-sentry-support
branch
from
dd61539 to
86d99cd
Compare
|
@siddarthkay @markoburcul this PR should be ready now to be used for testing the full integration with Sentry. In theory, the only thing needed is to set the value of
Edit: after this PR is tested successfully I will remove the cherry-picked commit and disable the option to force crash from any build. |
|
@ilmotta : regarding point 3, ah yeah we don't need that in @markoburcul 's commit we pass those vars to status-go which is where it matters. |
|
@ilmotta did you test this with the changes from my PR? |
Sure I did, I cherry picked the single commit from https://github.com/status-im/status-mobile/pull/21766/commits. Local dev builds work, I just tested again https://sentry.infra.status.im/organizations/sentry/issues/142/?project=7&query=is%3Aunresolved&referrer=issue-stream&statsPeriod=14d&stream_index=0 One problem for sure is that running I pushed a commit just now updating |
|
From the screenshot below, we see Sentry DSN is not set in the latest PR build. 
|
Sentry DSN will be set only on release builds. Can you run manual pipeline with BUILD_TYPE=release from your branch? |
Makes sense for now 👍🏼 Hopefully not too far in the future away we can set-up a Sentry project to work with non-release builds, because that can be quite helpful as well. @markoburcul the manual job is failing https://ci.infra.status.im/job/status-mobile/job/manual/. I ran it twice with the correct parameters https://ci.infra.status.im/job/status-mobile/job/manual/340/parameters/, but could you take a look please? The iOS step is failing, then we can't get the up-to-date artifact to test. |
As @siddarthkay said: |
ilmotta
force-pushed
the
ilmotta/add-sentry-support
branch
2 times, most recently
from
98318b5 to
a2f986a
Compare
|
@markoburcul the The manual release build in Jenkins was successful this time https://ci.infra.status.im/job/status-mobile/job/manual/342/. Thanks for the help After trying out the manual release APK generated from Jenkins, you can see in the screenshots below that the Sentry DSN isn't set and that the app is using the expected commit hash used by the Jenkins build. 
|
I've updated my PR, I think it should work now.. |
ilmotta
force-pushed
the
ilmotta/add-sentry-support
branch
from
a2f986a to
52cbdf2
Compare
@markoburcul no luck yet. I cherry picked commit b62ae9e, but the env var is still not available. |
@ilmotta Actually it was the intention to keep it only for release builds for now. |
ilmotta
force-pushed
the
ilmotta/add-sentry-support
branch
3 times, most recently
from
bf616e1 to
5f4710b
Compare
ilmotta
changed the title
ilmotta
force-pushed
the
ilmotta/add-sentry-support
branch
from
5f4710b to
0f334a7
Compare
|
PR is ready to be reviewed again @siddarthkay @markoburcul. I removed all temporary workarounds I did to be able to test release builds from the CI, meaning the Sentry option to force crash the app is NOT available in release builds. I think it's better if we close PR #21766 and merge only this one because this one I tested the actual integration with Sentry. Example from an error reported now to Sentry from a release built from the CI: 
@churik do you want this PR to go through QA? I ask this because I already tested this PR with a release builds from https://ci.infra.status.im/job/status-mobile/job/manual/ and with the feature flag enabled so that the crash button was accessible. After pressing on the crash button Sentry receives the error event as expected. I also tested that when the user disables analytics tracking we are not sending errors to Sentry. |
|
@ilmotta @siddarthkay looks like 
|
modules/react-native-status/android/src/main/java/im/status/ethereum/module/StatusModule.kt
Outdated
Show resolved
Hide resolved
referenced issue: #21706 Signed-off-by: markoburcul <[email protected]>
ilmotta
force-pushed
the
ilmotta/add-sentry-support
branch
2 times, most recently
from
0c0c236 to
7bdbc7c
Compare
ilmotta
force-pushed
the
ilmotta/add-sentry-support
branch
from
7f8ffcf to
6039a40
Compare
|
If I understand correctly, the iOS builds that appeared in TestFlight were intended for testing these changes. However, by mistake, several QAs installed that build during release testing, and both noted that messaging was slowed down in this build compared to the release version. So, please hold off on pushing this. Let’s not include it in 2.32. |
|
Heads up, based on conversations with @churik and @igor-sirotin, although this PR shouldn't affect the app's behavior in any way, we believe it's better to integrate Sentry in a follow-up release, e.g. 2.32.1. |
Fixes: #21656
Summary
In this PR we integrate Sentry and implement a feature flagged feature (disabled by default) to force crash the app using the function
mobile/status.go#IntendedPanic()provided by status-go.PR is waiting for #21706 to be resolved for this one to be taken out of draft.Review notes
Documentation about Sentry in status-go: https://github.com/status-im/status-go/blob/3466ac2661bb19cd0eaa27761551de3b4d31b393/internal/sentry/README.md#L60
Areas that may be impacted
Whenever a panic happens in status-go and usage data collection is enabled we will send error data to Sentry. In theory, nothing should change for the user, therefore nothing should be visibly different in the app's behavior.
Steps to test
Before following the steps below, the CI must be configured with the appropriate
SENTRY_DSN_STATUS_GOvalue. You can also use a free tier cloud instance to do your own testing in case you want to play around with Sentry.Settings > Feature flagsand enableapp-monitoring > intentional-crashSettings > Advanced> and press onForce crash immediatelyUsually you should see the error in the Sentry dashboard in a matter of seconds.
status: ready