Configure Sentry for Mobile #21682
Configure Sentry for Mobile #21682
Conversation
Jenkins BuildsClick to see older builds (198)
|
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
from
c639f62 to
f5ca9f9
Compare
siddarthkay
force-pushed
the
ilmotta/add-sentry-support
branch
from
f5ca9f9 to
ef4cd42
Compare
Jenkins does show that this credential has not been used anywhere. |
siddarthkay
force-pushed
the
ilmotta/add-sentry-support
branch
from
83e5b68 to
fe8bbaf
Compare
@ilmotta : should be fixed from CI side now. |
siddarthkay
force-pushed
the
ilmotta/add-sentry-support
branch
2 times, most recently
from
71042ee to
c717d0d
Compare
ilmotta
force-pushed
the
ilmotta/add-sentry-support
branch
2 times, most recently
from
7362310 to
a081ac4
Compare
ilmotta
force-pushed
the
ilmotta/add-sentry-support
branch
from
e0581a8 to
74aa578
Compare
|
Hi @siddarthkay. Below are the results and we unfortunately still have the incorrect env var value set in PR builds. Release build results ✅I tested the following cases, all successful:
PR build results ⛔I tested the same scenarios as the release build, but Jenkins or some script in Jenkins is injecting the incorrect env var value @siddarthkay. The env var 
|
|
@ilmotta : I assumed we don't need I saw in description of this issue: #21706 
Incase PR builds also need to have |
Why would you need to use Sentry in PR builds? If this is something temporary to test how integration works, we can create a separate Sentry project and I would put credentials from those in the |
|
@siddarthkay @markoburcul we don't necessarily need PR builds integrated with Sentry, that would be fine for sure. But as I shared above, the env var has its value set to the string Probably the best is to unset the var, or at least use an actual empty string would fix the issue. |
referenced issue: #21706 Signed-off-by: markoburcul <[email protected]>
siddarthkay
force-pushed
the
ilmotta/add-sentry-support
branch
from
74aa578 to
a1e2e2c
Compare
siddarthkay
force-pushed
the
ilmotta/add-sentry-support
branch
from
40f145a to
586c8cb
Compare
This was because when you rebased you may have undone my changes to I have checked PR build now on iOS and this is what i see.
|
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