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

Unified Configuration Management System #5921

Closed
anthony-murphy opened this issue Apr 23, 2021 · 6 comments · Fixed by #8497
Closed

Unified Configuration Management System #5921

anthony-murphy opened this issue Apr 23, 2021 · 6 comments · Fixed by #8497
Assignees
Labels
design-required This issue requires design thought epic An issue that is a parent to several other issues kr
Milestone

Comments

@anthony-murphy
Copy link
Contributor

Feature

Currently the framework lacks a unified configuration management system. Many of our layers take an options bag which allows some level of configuration however this system is lacking, as it often requires a code change to change configuration values. This serves us ok for things like testing but works less well for deployed applications where ideally config is easier to change than code.

Some properties that such should provide:

  • Enable onetime overrides for dev testing debugging, and protect from unintentional spread.
  • Configuration values should be immutable. once read in a session they should not change, as configs that can change increases complexity of consuming code.
  • Developers should be able to easily hook into such a system to manage deployed resources
  • Ideally configurations should continue to be strongly typed in the runtime
  • The config system itself should be low overhead in terms of performance
@anthony-murphy anthony-murphy added the design-required This issue requires design thought label Apr 23, 2021
@anthony-murphy
Copy link
Contributor Author

Some experimentation: #5691

@anthony-murphy
Copy link
Contributor Author

One option i'd considered in tying this into our logging system, so the logger also supports configuration management. The benefits is that this is already something that can be injected, and the code is already instrumented for it. However it does feel like it doesn't quite belong and is outside the abstraction the logger is meant to provide

@anthony-murphy
Copy link
Contributor Author

releate to #5783

@anthony-murphy anthony-murphy added this to the May 2021 milestone Apr 27, 2021
@anthony-murphy anthony-murphy self-assigned this May 3, 2021
@tylerbutler tylerbutler modified the milestones: May 2021, June 2021 May 24, 2021
@anthony-murphy anthony-murphy added the epic An issue that is a parent to several other issues label May 25, 2021
@anthony-murphy anthony-murphy modified the milestones: June 2021, July 2021 Jun 14, 2021
@anthony-murphy anthony-murphy removed their assignment Sep 28, 2021
@nmsimons nmsimons added the kr label Oct 20, 2021
@anthony-murphy
Copy link
Contributor Author

another set of experiments: main...anthony-murphy:config-provider

also @andre4i

@anthony-murphy
Copy link
Contributor Author

fyi @nmsimons

anthony-murphy added a commit that referenced this issue Dec 28, 2021
This change adds a simple configuration provider abstraction that can be injected via the loader. The configuration and logger are then available on an object currently called the MonitoringContext. To avoid needing to update all our api, we smuggle the MonitoringContext around via the logger. After this initial change has had time to soak we can slowly move our apis over to leveraging MonitoringContexts all rather than logger.

One are we have explicitly not tackled here is namespaces. this means we've gone with a flat structure or all config key look ups. In the future we may want to expand this introduce some kind of namespacing, but there are concerns we'll need to tackle around naming consistency. We face similar naming consistency issues with our child logger. Ideally whatever strategy we land on can help to solve the issues for both.

related to #5921
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design-required This issue requires design thought epic An issue that is a parent to several other issues kr
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants