Configure Sentry for Mobile #21682
Configure Sentry for Mobile #21682
Conversation
Jenkins BuildsClick to see older builds (120)
|
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. |
|
Indeed this bit fails for status-mobile : This can be fixed as an additional step in the build stage. |
siddarthkay
force-pushed
the
ilmotta/add-sentry-support
branch
3 times, most recently
from
77da7ff to
9cff936
Compare
|
@ilmotta : can you check sentry now? I've added an ugly fix to ensure version exists for status-go. |
|
Hey @siddarthkay, I get an error: After changing |
|
Oh @ilmotta : I see this issue too now, Maybe at the time of testing there would have been something cached at my end. |
referenced issue: #21706 Signed-off-by: markoburcul <[email protected]>
siddarthkay
force-pushed
the
ilmotta/add-sentry-support
branch
8 times, most recently
from
1696c95 to
d4b269d
Compare
|
should be fixed now : |
|
related status-go fix : status-im/status-go#6276 |
siddarthkay
force-pushed
the
ilmotta/add-sentry-support
branch
from
d4b269d to
04b0faa
Compare
siddarthkay
force-pushed
the
ilmotta/add-sentry-support
branch
from
04b0faa to
15bf611
Compare
✔️ status-mobile/prs/android/PR-21682#42 🔹 ~8 min 11 sec 🔹 04b0faa 🔹 📦 android package |
|
I tested printing version and the derivation logs show status-go version properly |
|
Many thanks @siddarthkay. Tested the recent changes from this PR and it worked (I didn't test the release build yet). Just need to confirm one thing with you @igor-sirotin. In the screenshot below, the context version is correct. The release value is 
|
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