Introduce crashlogging / Sentry
#109
Conversation
📲 You can test the changes from this Pull Request in Gravatar Android by scanning the QR code below to install the corresponding build.
|
It doesn't seem to be needed in the current configuration
There was a problem hiding this comment.
Pull Request Overview
This PR integrates Sentry crash logging into the Gravatar Android app through an internal crashlogging wrapper module. The integration includes automatic crash reporting, user context tracking, and build-time source mapping for production releases.
Key changes:
- Introduces a new
crashloggingmodule with Gravatar-specific crash logging configuration - Adds Sentry Gradle Plugin for mapping file uploads and source context in CI builds
- Updates dependency management to support the crash logging integration
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
crashlogging/ |
New module implementing Sentry integration with user data and locale providers |
app/ |
Integrates crash logging module and initializes Sentry in the application |
build-logic/convention/ |
Adds Sentry Gradle Plugin configuration for automated mapping uploads |
gradle/libs.versions.toml |
Updates dependency versions and adds Sentry-related libraries |
loginUi/build.gradle.kts |
Refactors secrets properties function name for consistency |
userComponent/build.gradle.kts |
Changes Gravatar core dependency from implementation to api |
settings.gradle.kts |
Includes the new crashlogging module |
crashlogging/src/main/java/com/gravatar/crashlogging/ContextBasedLocaleProvider.kt
Show resolved
Hide resolved
build-logic/convention/src/main/kotlin/GravatarAndroidApplicationConventionPlugin.kt
Show resolved
Hide resolved
build-logic/convention/src/main/kotlin/GravatarAndroidApplicationConventionPlugin.kt
Show resolved
Hide resolved
|
Requesting review from multiple people here to speed up the process (I start AFK the day after tomorrow). 1 review will be completely fine 👍 Thanks! |
|
|
||
| internal class GravatarCrashLoggingDataProvider( | ||
| localeProvider: LocaleProvider, | ||
| userRepository: UserRepository |
There was a problem hiding this comment.
I'm aware that this can circular DI dependency if crashlogging was about to be used in userComponent module. It's possible to address it one way or another, but until it won't be needed, I think there's no point to make this setup more complex now.
There was a problem hiding this comment.
Although not ideal and I would imagine pushing the new profile (or rather CrashLoggingUser) when updated outside of the module rather than depending on the :userComponent I think this is okay for now.
if crashlogging was about to be used in userComponent module.
I assume that fatal crashes are reported automatically. So that would only be needed if we wanted to report non-fatal ones, correct?
There was a problem hiding this comment.
Although not ideal and I would imagine pushing the new profile
I get you, but I also like the reactiveness of this solution - it'd be great if code related to user wouldn't have to remember about pushing a new profile to things like "crash logging".
I assume that fatal crashes are reported automatically. So that would only be needed if we wanted to report non-fatal ones, correct?
True, you're correct! 👍
There was a problem hiding this comment.
👋 @wzieba !
I have reviewed and tested this PR as per the instructions, everything works as expected, nicely done introducing Sentry here! 💯 x 💯 ^ 💯
I've tested the overall solution building the release build on CLI using this command below, and everything is working exactly as expected (give-or-take the mappings upload, see question).
CI=true SENTRY_AUTH_TOKEN=<SENTRY_AUTH_TOKEN> ./gradlew assembleRelease
ProGuard Deobfuscation + Source Context: ✅
User Data: ✅
Sentry ProGuard Mapping Upload: ✅ (ish, , see question)
I have left one question (❓), few suggestions (💡) and one minor (🔍) comments for you to consider. I am going to approve this PR anyway, since none is blocking. I am NOT going to merge this PR yet to give you some time to apply any of my suggestions. However, feel free to ignore them and merge the PR yourself.
build-logic/convention/src/main/kotlin/GravatarAndroidApplicationConventionPlugin.kt
Show resolved
Hide resolved
crashlogging/src/main/java/com/gravatar/crashlogging/GravatarCrashLoggingDataProvider.kt
Outdated
Show resolved
Hide resolved
crashlogging/src/main/java/com/gravatar/crashlogging/GravatarCrashLoggingDataProvider.kt
Outdated
Show resolved
Hide resolved
crashlogging/src/main/java/com/gravatar/crashlogging/GravatarCrashLoggingDataProvider.kt
Outdated
Show resolved
Hide resolved
|
|
||
| internal class GravatarCrashLoggingDataProvider( | ||
| localeProvider: LocaleProvider, | ||
| userRepository: UserRepository |
There was a problem hiding this comment.
Although not ideal and I would imagine pushing the new profile (or rather CrashLoggingUser) when updated outside of the module rather than depending on the :userComponent I think this is okay for now.
if crashlogging was about to be used in userComponent module.
I assume that fatal crashes are reported automatically. So that would only be needed if we wanted to report non-fatal ones, correct?
This reverts commit f672f18.
|
Thanks for the update @wzieba , another ✅ from me! |
Closes: AINFRA-954
Description
This PR introduces integration of Sentry via our internal
crashloggingwrapper. In addition, it introduces Sentry Gradle Plugin to send mapping files and source context.Screenshots
Proguard deobfuscation + source context works fine
Providing user data works fine as well
Testing Steps
Install the prototype build and observe Sentry dashboard for incoming events (there's a test error reported on app start).
🚨 Before merge 🚨
git revert f672f18