Use per-project Sentry configuration #4818
Conversation
Generated by 🚫 Danger |
There was a problem hiding this comment.
Pull request overview
This PR migrates Sentry configuration from a single sentry.properties file to per-project Gradle configuration, enabling separate Sentry projects for Android, Automotive, and Wear applications.
Key Changes
- Added Sentry configuration properties (auth token, org, and project names) to
dependencies.gradle.kts - Configured project-specific Sentry project names in each application module's build file
- Renamed
configureSentry()toapplyCommonSentryConfiguration()and added auth token and org configuration - Added "prototype" to ignored build types for Sentry uploads
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| dependencies.gradle.kts | Adds five new Sentry-related properties from secret properties: auth token, org, and three project names |
| build.gradle.kts | Renames Sentry configuration function, adds auth token and org properties, includes "prototype" in ignored build types, removes unused import |
| app/build.gradle.kts | Configures Android project-specific Sentry project name |
| automotive/build.gradle.kts | Configures Automotive project-specific Sentry project name |
| wear/build.gradle.kts | Configures Wear project-specific Sentry project name |
| set("sentryAuthToken", secretProperties.getProperty("sentryAuthToken", "")) | ||
| set("sentryOrg", secretProperties.getProperty("sentryOrg", "")) |
There was a problem hiding this comment.
If I'm not mistaken, those 2 properties as still in sentry.properties currently, not secret.properties.
- So either we keep the
sentry.propertiesfile as-is (and also in.configure) and let theSentryPluginExtensionautomatically read those (at least I'm guessing that's who reads those implicitly so far) fromsentry.properties, only overwriting theprojectNamevia Gradle (assuming setting asentry { … }section in thebuild.gradle.ktsfiles doesn't disableSentryPluginExtensionreading fromsentry.propertiesimplicitly, but instead adds on top of it so that the properties are merged?) - Or you plan to set those via Gradle as well like you do in your new
applyCommonSentryConfiguration, and want to move secret values fromsentry.propertiestosecrets.propertiesto have Gradle read from that.propertiesfile instead… which could make sense too… but in that case I'd expect this PR to contain changes for the.configure-files/secrets.properties.enc, deletion of the.configure-files/sentry.properties.encand removal of the entry forsentry.propertiesin the.configureJSON file to reflect that
PS: ICYMI, the convoluted/confusing way we currently use to manage/updates secrets in git repos with configure (and how to update the .enc files when you change secrets, etc) will very soon change to become way more simple and transparent (Internal ref: paaHJt-96q-p2). 🔜
There was a problem hiding this comment.
Thanks for bringing this up. This might have been unclear from the PR description but this is what I meant.
To address this, I propose configuring everything through the Gradle plugin. This change will need to be accompanied by an update to the
mobile-secretsrepo to add the necessary properties. I can do it once we agree on the approach and sync the modifications in both repositories.
I didn't want to update secrets in case the approach I'm proposing is incorrect. And yes, this approach means dropping sentry.properties.
There was a problem hiding this comment.
Ah, un-caffeinated me missed that part, thanks for highlighting it! Makes sense indeed.
If this PR is not extra-urgent and can wait a couple of days, maybe I can put PCAndroid as next on the list in my project to test our brand new way to manage in-repo secret files (AINFRA-1539).
Since that new approach to deal with in-repo secrets does not rely on .mobile-secrets nor .configure anymore, and instead transparently handles secret files via git's built-in filters feature, that means that any change you need to make on a secret files will then be done directly in the PCAndroid repo—and thus be part of the list of files changed in the PRs, with the contents of the secret file not being updated in main until the PR is merged there, like any other regular file, etc.
So once we adopt that new tool you wouldn't have the problem above in this PR anymore.
If you don't want to wait for me to migrate PCAndroid to the new tool and want to merge this PR earlier that's ok with me too, though in that case you'll indeed have to synchronize your merging of this PR with the push of changes to secret files in .mobile-secrets.
There was a problem hiding this comment.
No, I think it can wait. I'd be great to test using git filters for that. I'll keep it open for now but as a draft. Thanks!
0dfbdee to
4a0604d
Compare
|
Version |
FYI: The PR to migrate this repo to PS: Note that, timing-wise, I don't plan to merge it before mid-January (once I come back from AFK then the AI Workshop in NY) so that I can be around for support if needed and to help ease the adoption. |
| } | ||
|
|
||
| sentry { | ||
| projectName = project.findProperty("sentryWearProject")?.toString() |
There was a problem hiding this comment.
@MiSikora question, just to double check: I don't think there should be differences related to what we do in annotate_sentry_mapping_uuid given we're building separate app bundles anyway (that is, each app will have its own base/assets/sentry-debug-meta.properties), right?
There was a problem hiding this comment.
Good question but I don't know. I would think that no changes are required since sentry.properties do not provide any other values.
Description
Currently, our builds upload data to our main Android Sentry project. Since we decided to split Android, Automotive, and Wear apps into separate ones, this now needs to be handled in the build system.
Our Sentry configuration is stored in the
sentry.propertiesfile. However, this file lives in the root project. I wasn’t able to find anything in the Sentry documentation indicating whether it’s possible to have multiple such files one in each sub-project or to define per-project properties within the root file.To address this, I propose configuring everything through the Gradle plugin. This change will need to be accompanied by an update to the
mobile-secretsrepo to add the necessary properties. I can do it once we agree on the approach and sync the modifications in both repositories.Relates to: https://linear.app/a8c/issue/AINFRA-1610/
Testing Instructions
I'm not sure if it is possible to test it really until we create a release build. But if anyone has an idea I'd like to know.