Use plain text values for Sentry + env-injected token - #1850
Conversation
mokagio
added
the
tooling
There was a problem hiding this comment.
Pull request overview
This PR updates Simplenote Android’s Sentry configuration to use a checked-in, plain-text DSN in BuildConfig and to source the Sentry Gradle plugin auth token from an environment variable, while removing the previously encrypted sentry.properties flow from .configure.
Changes:
- Hardcode
BuildConfig.SENTRY_DSNinSimplenote/build.gradleand stop injectingSENTRY_DSNfromgradle.properties. - Configure the Sentry Gradle plugin with org/project and
SENTRY_AUTH_TOKENfrom the environment. - Remove the encrypted
sentry.propertiesartifact and its.configurecopy/encrypt step.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
Simplenote/gradle.properties-example |
Removes the sentryDsn example entry since DSN is no longer provided via properties. |
Simplenote/build.gradle |
Adds a hardcoded SENTRY_DSN BuildConfig field and configures Sentry Gradle plugin auth via env var. |
.configure-files/sentry.properties.enc |
Removes the encrypted Sentry properties file from the repo. |
.configure |
Removes the step that provisioned sentry.properties via .configure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| minSdkVersion 23 | ||
| targetSdkVersion 35 | ||
|
|
||
| buildConfigField "String", "SENTRY_DSN", "\"https://89e443ca3fd84e588d1809cfbfcc40f4@sentry.io/1455558\"" | ||
| } |
There was a problem hiding this comment.
@ParaskP7 @wzieba what do you think of this?
This is the first app, as far as I can tell, that makes the DSN public, so I think it'd be good to think it through well so we can adopt the same approach to the other apps.
On the one hand, the crashLoggingEnabled() implementation is such that we can afford to have a single, static DSN because only the release builds will effectively use it. On the other, that assumption can change at any time so I don't like relying on it.
There was a problem hiding this comment.
I think it doesn't matter how we provide the DSN: as long as we have autoInstallation disabled (in projects that use newer versyion of Sentry Gradle Plugin), the way we provice DSN shouldn't matter.
It feels slightly odd to share the DSN in plaintext, though, but it's maybe the fact that I'm so used to keeping this in secrets (for side readers: the DSN is not technically a secret, because anyone can extract it from the app bundle from Google Play Store anyway)
There was a problem hiding this comment.
as long as we have autoInstallation disabled (in projects that use newer versyion of Sentry Gradle Plugin), the way we provice DSN shouldn't matter.
Noted. Implemented in a816342
for side readers: the DSN is not technically a secret, because anyone can extract it from the app bundle from Google Play Store anyway
Moreover, the DSN is designed by Sentry to be public in the first place. See https://docs.sentry.io/concepts/key-terms/dsn-explainer/#dsn-utilization
|
📲 You can test the changes from this Pull Request in Simplenote Android by scanning the QR code below to install the corresponding build.
|
fc140e0 to
0c2badc
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Now that the Sentry DSN is committed rather than decrypted, every build carries the production DSN — including local, contributor, and screenshot builds, which previously fell back to the dummy DSN in `gradle.properties-example`. `analyticsIsEnabled()` defaults to `true` when the preferences bucket is absent, so nothing else stopped a fresh contributor build from reporting into the production project. This matches `WPCrashLoggingDataProvider` and WooCommerce, which both keep debug builds out of Sentry. Prototype builds are `Debug`, so they stop reporting too — consistent with wpandroid, whose prototypes are already silenced the same way. --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Every build type now carries the production DSN as a compile-time constant. Auto-installation would have the SDK initialize itself from that constant, bypassing `SimplenoteCrashLoggingDataProvider` — the one place that decides whether crash logging is on, and the reason debug builds stay out of Sentry. This is also the condition wzieba's review answer rests on: how the DSN is provided does not matter as long as auto-installation is off. `wpandroid` and `wcandroid` both disable it. --- Generated with the help of Claude Code, https://claude.ai/code Co-Authored-By: Claude Code Opus 4.8 <noreply@anthropic.com>
0c2badc to
a816342
Compare
https://linear.app/a8c/issue/AINFRA-2714