Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align configuration binding precedence with Microsoft priorities #3143

Closed
jamescrosswell opened this issue Feb 12, 2024 · 3 comments
Closed

Comments

@jamescrosswell
Copy link
Collaborator

jamescrosswell commented Feb 12, 2024

Problem Summary

For users who want to configure Sentry via environment variables we have a problem, which stems from a discrepency between the precedence of configuration providers used by Microsoft and Sentry.

In ASP.NET Core Microsoft is using (from highest to lowest):

  1. Command line arguments
  2. Environment Variables
  3. User Secrets
  4. appsettings-{environment}.json
  5. 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.

Suggested change

We should change the priority for configuration provider sources when configuring Sentry to align with the Microsoft way.

This would be a very disruptive change so we'd need to do it in a major version bump and communicate it very carefully.

Workaround

Currently, if people want to set some default behaviour for Sentry that can be overriden with Environment variables, they need to use this workaround:

@bitsandfoxes
Copy link
Contributor

As far as I can tell the SDK tries to locate DSN, Environment, and Release on the environment via the SettingLocator.
All three are now using the same mechanism of if(!string.NullOrWhiteSpace()) as check whether to take the variable from the environment.

@jamescrosswell
Copy link
Collaborator Author

As far as I can tell the SDK tries to locate DSN, Environment, and Release on the environment via the SettingLocator.

I wonder if we need to be doing anything special for these in the SettingLocator at all... This could just be handled by the Environment variable configuration provider right? That would allow you to override any of the Bindable options via environment variables.

The only thing that might need to be wired up manually in the SettingLocator is setting the DSN via an assemby attribute.

Would need to play around with this a bit.

@bitsandfoxes
Copy link
Contributor

We just went back and forth break-fixing the overwriting rules. The DSN, environment, and release now use the same mechanism.
I don't see us changing that again any time soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

2 participants