Migration guide: How to disable Sentry when no DSN supplied

[jamescrosswell]

Problem

A couple of our customers have a setup where they enable Sentry by configuring the DSN in an environment variable. When this environment variable is not set, however, they want Sentry to be disabled.

This is tricky to do since the configuration files take precedence... then code, then environment variables. So if a disabled DSN is configured in XML/JSON/Code then it can't be conditionally overwritten by an environment variable only on those machines where customers want Sentry enabled.

However they can get Sentry to do what they want by configuring the Disabled DSN in an assembly attribute... which is the very last place that our SettingsLocator checks (even after having checked environment variables):

sentry-dotnet/src/Sentry/Internal/SettingLocator.cs

Lines 33 to 47 in 0aed969

	public string GetDsn()
	{
	// For DSN only
	// An empty string set on the option should NOT be converted to null because it is used
	// to indicate the the SDK is disabled.
	_options.Dsn ??= GetEnvironmentVariable(Constants.DsnEnvironmentVariable)
	?? AssemblyForAttributes?.GetCustomAttribute<DsnAttribute>()?.Dsn;
	if (_options.Dsn is null)
	{
	throw new ArgumentNullException("You must supply a DSN to use Sentry." +
	"To disable Sentry, pass an empty string: \"\"." +
	"See https://docs.sentry.io/platforms/dotnet/configuration/options/#dsn");
	}
	return _options.Dsn;
	}

So you can cadd a file called AssembyInfo.cs to your project with the following contents:

using Sentry;

[assembly: Dsn(SentryConstants.DisableSdkDsnValue)]

That way you end up with the same behaviour as you had in 3.x (which is that if no DSN is provided anywhere, you get a disabled DSN by default).

Originally posted by @jamescrosswell in #3134 (comment)

TODO

We should summarize the solution above and add it to the 4.0.0 migration guide.

[ericsampson]

that's kinda awkward.

What about also introducing a new setting and env var like SENTRY_ENABLED instead of messing around with fragile in-band signaling via DSN

[jamescrosswell]

What about also introducing a new setting and env var like SENTRY_ENABLED instead of messing around with fragile in-band signaling via DSN

It is already possible to enable/disable sentry by setting the SENTRY_DSN environment variable to either "" (i.e. disabled DSN) or "some_valid_dsn". For people getting started with sentry-dotnet 4.x, this is probably the right way to go.

This suggested addition to the migration guide is really for folks who want to simulate the behaviour of the 3.x SDK which was that if no DSN at all was provided (e.g. if no SENTRY_DSN environment variable was present and that was the only way the DSN was ever configured) then Sentry would be disabled. This might make it easier to migrate to v4.x of the SDK, if you're already deploying Sentry to loads of machines in a way that assumes/relies on that previous behaviour.

[ericsampson]

hmm ok, that's just different than the long-standing Unix convention of "environment variables that are unset or blank should result in the same behavior"

and so the new behavior kinda violates the "principle of least surprise" IMO

[jamescrosswell]

and so the new behavior kinda violates the "principle of least surprise" IMO

I'm not sure I follow... I confess to not having heard of the unix convention.

The original change all of this stemmed from was driven by another principle of "fail early". Generally we don't want Sentry to be the cause of any errors in in people's solutions so we try/catch and log almost all of the exceptions that might come from Sentry's code. The one exception to this is initializing Sentry itself. We don't want people to call UseSentry incorrectly and just have it inexplicably not work... So if someone calls UseSentry without a DSN, we throw. If they want to deliberately (vs accidentally) disable Sentry, they can supply a Disabled DSN (i.e. empty string).

For users who want to configure Sentry via environment variables this creates a bit of a problem... which I think stems from the fact that Sentry is using an unusual order of precedence for configuration provider sources.

Microsoft is using (from highest to lowest):

1. Environment Variables
2. User Secrets
3. appsettings-{environment}.json
4. appsettings.json

Sentry is using:

1. Code (the options callback)
2. appsettings-{environment}.json
3. appsettings.json
4. Environment variables
5. AssemblyForAttribute

So people rolling out applications instrumented with Sentry will presently have to use one set of conventions in their CI/CD for all of the MS Stuff (e.g. set default behaviour in appsettings.json but override on a per machine basis in environment variables) and a completely different set of conventions for Sentry, for the same assembly/software.

The precedence mismatch is probably the root cause of the problem but we definitely can't change something that fundamental without making a major version bump and making quite a bit of noise about it... as it would be very disruptive.

Currently then, if people want to set some default behaviour for Sentry that can be overriden with Environment variables, the only way to do it is to use an AssemblyForAttribute to configure the default behaviour... this would allow them to use similar devops conventions for Sentry and Microsoft stuff when rolling these out in their environments.

[slonopotamus]

The funny thing is that you just did a major bump and opened a can of worms by changing behavior. MS priority of things is not invented by MS, it is globally accepted order. From high to low: env, local config, global config, code defaults. This is what everyone expects. If you do anything different, it will trigger WTFs and break workflows. Things were prioritised this way for a reason.

[jamescrosswell]

The funny thing is that you just did a major bump and opened a can of worms by chsnging behavior.

Yeah it's unfortunate we didn't get any feedback about this during the beta testing and only just realized this now. I think we need to partner more closely during testing with people who are using Sentry in large real world applications.

The Sentry .NET SDK has been using a divergent order of precedence wrt configuration source providers for quite a while now (at least for all of the 3.x versions). The change we made in 4.0.0 simply made this more apparent as, previously, the consequence of this was that the SDK would be silently disabled...

We'll have to look at addressing this in v5.x now I think.

[slonopotamus]

we didn't get any feedback about this during the beta testing

This is offtopic, but I believe this is important. We use dependabot for dependency upgrades. And I believe lots of people do the same. So, until dependabot opens a PR, we do not care about your releases (sorry, we have 50 or so other dependencies, we cannot track betas of everyone). You released 4.0.0, we got a PR the next day, it ran integration tests, they failed. Maybe (maybe) you should be more active with releasing versions that are pushed to users, to get earlier feedback. But maybe I am wrong, who knows.

[bitsandfoxes]

We don't need to wait for the next major to fix what the current one broke.
We can still throw if the DSN is null (i.e. a misconfigured SDK) to catch new users and by convention allow string.Empty to be overwritten by the environment.

[jamescrosswell]

We don't need to wait for the next major to fix what the current one broke.

Check comment in the other PR.