-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Enable confmap expand feature #6276
Conversation
bc363a8
to
c89630d
Compare
Codecov ReportBase: 92.30% // Head: 92.31% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #6276 +/- ##
==========================================
+ Coverage 92.30% 92.31% +0.01%
==========================================
Files 219 219
Lines 13466 13473 +7
==========================================
+ Hits 12430 12438 +8
+ Misses 806 805 -1
Partials 230 230
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we enable it through a feature gate at the beginning? I believe it might introduce some unexpected outcomes for distros that do it using another approach...
No, because that has a different syntax which does not match this one, so the mechanism will not trigger. The reason a featureflag is not required is because is a new syntax, and if users are using this new syntax is clear that they want this feature, otherwise this feature is not triggered. |
051cb2a
to
47dfcbc
Compare
Signed-off-by: Bogdan <bogdandrutu@gmail.com>
@dmitryax convinced me that this may overlap with splunk's distros feature, so protect this by a feature gate for the moment. |
47dfcbc
to
ace188f
Compare
This is a significant feature of the configuration system used by many users in many ways. We need to be very careful about this transition and ensure that it is seamless as possible. I don't think we can simply say it will go away in a couple of releases. |
@Aneurysm9 when that moment comes, happy to hear opinions, for the moment let's aim for that (be positive). |
All I'm saying is that if we want to make this change the time to plan for transition is now, not when we're ready to disable functionality that users have relied on up to this point. |
Examples of usages
Retrieving token from zookeeper (scheme not yet defined)
Retrieving an entire exporter config from a file
Expanding env vars
The old way of expanding env vars (without a scheme) is still supported (for at least few versions) but discouraged at this point and should be replaced by the new usage pattern.