fix: Allow a DSN string.Empty to be overwritten by the environment

[bitsandfoxes]

TLDR;

This PR aims to let users disable the SDK with options.Dsn = ""; and by convention, let this be overwritten by an environment variable. This "" behavior is 10 years old and set in all SDks. #2494 (comment)

Context

We've introduced some friction by letting the SDK throw if there was no DSN provided during initialization. The idea was to make the onboarding process more explicit and let new users know immediately that there was something wrong with their current configuration. #2494

The change made it very awkward to control the SDK's behavior through the environment since now not setting the DSN via code implicitly gets treated as a configuration error.

The current order of priority:

• Code
• Binding through config files
• Environment
• AssemblyAttribute

2. and 3. are now following the convention of letting the environment overwrite config files but that's OK for now. We can track and document this.

Fixes #3136,

[bruno-garcia]

with these tests passing I believe we have a good logic to deal with this
other than nitpicking on the code itself this LGTM

[jamescrosswell]

Broadly I'm in favour of this change, as it brings us a little closer to aligning with the Microsoft priorities but just be aware that this is going to be a change in the behaviour of the SDK.

Prevsiously, when using version 3.x or version 4.x of the SDK, if you configured a disabled DSN in appsettings.json, this would not be overwritten by any environment variable configuration (nulls would, but disabled wouldn't - that's the whole point of this change).

After this change, a disabled DSN configuration via appsettings.json will be overwritten by environment variable configuration.

I have no idea how many customers (if any) were relying on the assumption that what they configured in appsettings.json would not be overwritten by environment variables. Just a risk to be aware of.

[slonopotamus]

You're already different from 3.x. This PR makes code different in another direction that is supposed to fix currently broken usecases. And yes, it will involve adjustments on user side (we need to pass empty string to UseSentry).

[jamescrosswell]

You're already different from 3.x. This PR makes code different in another direction that is supposed to fix currently broken usecases. And yes, it will involve adjustments on user side (we need to pass empty string to UseSentry).

That's true. The other workaround I'd proposed (relying on assembly attributes) would not be breaking things for customers who were relying on them being able to configure a valid or disabled DSN in appsettings that would not be overwritten by env variables though. This PR would break things for those customers.

As I say, I'm not sure how many (if any) customers have their CI/CD setup that way, but it's a risk to be aware of. We could be inadvertedly breaking things for a bunch of customers here.

[slonopotamus]

We could be inadvertedly breaking things for a bunch of customers here.

Hah. I have a bias here, because this PR will fix my workflow :D But I believe that cases "we have SENTRY_DSN defined in env, but expect it to be ignored" are nonexistent.

[bruno-garcia]

Hah. I have a bias here, because this PR will fix my workflow :D But I believe that cases "we have SENTRY_DSN defined in env, but expect it to be ignored" are nonexistent.

Yeah env var should trump all other values and it's how a lot of software works so I feel OK about this change

[jamescrosswell]

Yeah env var should trump all other values and it's how a lot of software works so I feel OK about this change

Do we just want to do it for the DSN property, or should we make this change instead (it would render this PR irrelevant)?

• Align configuration binding precedence with Microsoft priorities #3143

[bruno-garcia]

Yeah env var should trump all other values and it's how a lot of software works so I feel OK about this change

Do we just want to do it for the DSN property, or should we make this change instead (it would render this PR irrelevant)?

* [Align configuration binding precedence with Microsoft priorities #3143](https://github.com/getsentry/sentry-dotnet/issues/3143)

makes sense to change the behavior across all env vars

[ericsampson]

thanks for reconsidering the original direction all, it's an appreciated sign of customer focus <3

Maybe you can partner with some poweruser companies/people who would be a "beta test" partner group? I'd be happy to, just LMK. If you gave me a heads-up when there are design questions or a beta version to check out, then I'd be happy to give it a whirl and provide feedback. As long as it was at like 3 or 4 weeks before the planned SDK GA, to give time to schedule the change/deploy n our side.

Cheers