getsentry / sentry-android-gradle-plugin

Gradle plugin for Sentry Android. Upload proguard, debug files, and more.
https://docs.sentry.io/platforms/android/gradle/
MIT License
145 stars 33 forks source link

SENTRY_DSN is not picked up #782

Closed WebTiger89 closed 3 weeks ago

WebTiger89 commented 1 month ago

Gradle Version

8.10.2

AGP Version

8,7,0

Code Minifier/Optimizer

Proguard

Version

4.12.0

Sentry SDK Version

8.0.0-beta.1

Steps to Reproduce

-

Expected Result

The documentation states this variable is piucked up automatically from environment variables if it is not set.

Actual Result

The environment variable _SENTRYDSN is not picked up in my android/kotlin application. This is confirmed by an exception on app start:

DSN is required. Use empty string or set enabled to false in SentryOptions to disable SDK.

I'm working on Windows and running env in terminal confirms the presence of the variable. In my AndroidManifest.xml file I have set auto-init false and initialization is done in a Application class:

class MyApplication : Application() {
    override fun onCreate() {
        super.onCreate()
        SentryAndroid.init(this) { options ->
            options.environment = "development"
            options.isEnableUserInteractionTracing = true
            options.isEnableUserInteractionBreadcrumbs = true
        }
    }
}

The sentry android gradle plugin is used.

Did I miss something?

romtsn commented 1 month ago

Hi, we don't read env variables in the sdk, you have to explicitly pass it either through the manifest, or in the init configuration block. Where did you get that the SENTRY_DSN env variable can be used with the android sdk?

WebTiger89 commented 1 month ago

See https://docs.sentry.io/platforms/android/configuration/options/

The list of common options across SDKs. These work more or less the same in all SDKs, but some subtle differences will exist to better support the platform. Options that can be read from an environment variable (SENTRY_DSN, SENTRY_ENVIRONMENT, SENTRY_RELEASE) are read automatically.

romtsn commented 1 month ago

@WebTiger89 ok, this is wrong, thanks for letting us know. We'll fix the docs!

WebTiger89 commented 1 month ago

There is more that needs to be fixed, see https://github.com/getsentry/sentry-android-gradle-plugin/issues/779

WebTiger89 commented 1 month ago

There is a lot of confusing stuff since configuration can be done by gradle (sentry block in gradle.kts file), manifest, init function and combinations of them, e.g. init function + gradle

romtsn commented 1 month ago

779 is actually a feature request, so it's not about incorrect docs really. We'll look into it a bit later.

There is a lot of confusing stuff since configuration can be done by gradle (sentry block in gradle.kts file), manifest, init function and combinations of them, e.g. init function + gradle

I agree it can be a bit confusing, but not sure how the gradle configuration clashes with the init block? I don't think there's a setting in the gradle config that has an alternative in the init block and vice versa, or am I missing something?

WebTiger89 commented 1 month ago

779 is also my ticket. I have declared it as feature request but actually it is wrong behavior if documentation is correct about that, see https://docs.sentry.io/platforms/android/configuration/gradle/#auto-installation

The documentation states:

The plugin offers the automated installation feature of the Sentry Android SDK and the Fragment, Timber, OkHttp, and Jetpack Compose integrations. Starting with version 3.1.0, the feature is enabled by default, so you don't need to add any dependencies — you just use the Sentry Gradle plugin.

so you don't need to add any dependencies — you just use the Sentry Gradle plugin.

But that is wrong. I have to add sentry compose library explicitly otherwise I get build errors, see ticket and the linked ticket to the doc repository within.

WebTiger89 commented 1 month ago

I agree it can be a bit confusing, but not sure how the gradle configuration clashes with the init block? I don't think there's a setting in the gradle config that has an alternative in the init block and vice versa, or am I missing something?

I have to dig deeper to make more concrete examples, but what I mainly mean is that you can confiure a little bit in gradle, a little bit in manifest and a little bit in code.

romtsn commented 1 month ago

I agree it can be a bit confusing, but not sure how the gradle configuration clashes with the init block? I don't think there's a setting in the gradle config that has an alternative in the init block and vice versa, or am I missing something?

I have to dig deeper to make more concrete examples, but what I mainly mean is that you can confiure a little bit in gradle, a little bit in manifest and a little bit in code.

Would you like the manifest support to be dropped completely? We're discussing this internally, but our stats show that there's quite a large user-base that use manifest for configuring the sdk, so we're hesitant.

Not sure we could drop the gradle config though, as it's really just a build-time configuration as opposed to run-time config of the init block.

WebTiger89 commented 1 month ago

I'm not sure if I'm in a position to make a recommendation on such a relatively important decision, but these are my thoughts about that:

Configuration in AndroidManifest.xml remain readable in the APK and isn't obfuscated by R8/ProGuard, so it may make sense to move away from manifest-based configuration for sensitive settings like DSNs. However, since it’s convenient for many users, a phased approach might work best.

One option could be to deprecate manifest configuration over a few releases, encouraging users to migrate to safer alternatives, like environment variables or code-based configuration. A well-documented guide on securely setting up DSNs and other configurations in code or via Gradle could support this transition.

This would improve security for the broader Sentry user base while providing ample time for current manifest users to adjust.

WebTiger89 commented 1 month ago

A good starting point would be to provide DSN via gradle like it is done with authToken:

authToken.set(System.getenv("SENTRY_AUTH_TOKEN"))

Since DSN is considered sensitive this approach is a good alternative.

romtsn commented 3 weeks ago

Since DSN is considered sensitive this approach is a good alternative.

No the DSN should not be considered sensitive, see here: https://docs.sentry.io/concepts/key-terms/dsn-explainer/#dsn-utilization

But I can see the value in exposing the runtime sentry options via the gradle plugin configuration to keep everything in the same place. The challenge would be to keep them in sync though. Thanks for you opinions!

romtsn commented 3 weeks ago

@WebTiger89 we discussed this internally and concluded that we're not going to expose runtime options through the gradle plugin configuration for the time-being, as it would mean we'd have to maintain 3 different ways of setting the options: Manifest, SentryOptions and the Gradle plugin

If you want to wire your env variables with the sentry config, you could make use of manifestPlaceholders, e.g. for DSN it'd look something like this:

// in your build.gradle.kts
    buildTypes {
        getByName("debug") {
            addManifestPlaceholders(
                mapOf(
                    "sentryDsn" to System.getenv("SENTRY_DSN"),
                    "sentryEnvironment" to "debug"
                )
            )
        }
    }

// and then in your manifest
        <meta-data android:name="io.sentry.dsn" android:value="${sentryDsn}" />
        <meta-data android:name="io.sentry.environment" android:value="${sentryEnvironment}" />