-
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
Create tool or subcommand that allows migrating configuration #7631
Comments
cc @Aneurysm9 @bogdandrutu because of #7111, also cc @TylerHelmuth to review requirements/needs for the open-telemetry/opentelemetry-collector-contrib/issues/18642 example (this is probably also useful for the long term transform processor work). To be clear one more time, I want to get all requirements/desiderata for use cases related to this first so we consider them during the design phase, even if the subcommand does not do everything we would want it to at first. |
This sounds great and will be a big deal for open-telemetry/opentelemetry-collector-contrib#18643 as a whole. I have started work on a bridge between the old configuration and OTTL and it would be great to be able to convert user's configs for them instead of only printing out the equivalent statement and hoping they update. |
Thinking about this a bit more, the biggest open question is what the output should be. Should we have something like this?
How would that work if and when we add support for custom migration rules from components? |
It so happens that I'm working on a tool right now for my product that does configuration conversion in a way that maintains existing choices but allows reconfiguration. The strategy I'm using is to load the existing config into a generic An example from the template showing a renamed config item is:
func nonDefaultOnly(data map[string]any, key, oldkey string, def any) string {
...
} Its behavior is that the item is commented out if the original value was either missing or equivalent to the default; otherwise it's included with the original value of this field (but now it has a new name). This function is an example of one of several special-purpose functions for our particular conversion needs. The value is that the conversion code is quite generic -- most of the work is in the template and the helper functions the template is given. The disadvantage is that it's very "meta" -- coding in templates requires thinking about the template logic at a different level than the template support code. It can get confusing to write, nevermind read. However, because templates are so expressive, this model could allow components to include templates for converting component configurations to a new format without having to consider the larger file within which they're contained, and the tool could know to look for those in a well-defined place. Independent of the stuff above, I do strongly recommend that:
|
To follow up from my comment above, we deployed this last summer and have had very good results. I'd be happy to talk about whether there's a way to extract it for collector use. |
I wasn't aware this issue existed, linking a proposal to donate our configconverter module: open-telemetry/opentelemetry-collector-contrib#30654 |
The converter implementation is part of the core API already https://github.com/open-telemetry/opentelemetry-collector/blob/main/confmap/converter.go. Do we really need a separate subcommand or we can jus utilize it instead? |
One thing that is unclear to me with the converter approach is if we are encouraging people to change their actual configuration files. At least before 1.0, if we are to remove old functionality users need to actually change their configuration files. The way to push for that IMO is to either offer the option to modify the files for them (if they are using files and these are writable) or, if that's not possible, offer users something that they can copy-paste |
Applying conversions to migrate configuration to the new version should probably be opt-in; maybe we can use another flag for that. We can also have a flag to show the final configuration without starting the collector. Applying those two options would provide pretty much the same behavior as described in the issue |
So it sounds like we still want the migration functionality, but not with a subcommand, rather 2 flags to ./otelcolcore. Something like Is this a fair summary? @dmitryax |
@mx-psi is this feature required for 1.0 or could it be added after? |
@TylerHelmuth The intent of having this on 1.0 was to help people migrate from |
@mx-psi with the new error logs for |
@TylerHelmuth Oki, I am fine with that. I think this continue to be a high priority issue, so I am going to leave |
Is your feature request related to a problem? Please describe.
Changes in recommended configuration happen frequently in the Collector, including both Collector-wide changes (e.g. see #7111 which is the main motivation for this) and in individual components (e.g. most recently see open-telemetry/opentelemetry-collector-contrib/issues/18642, see also #6334 for a change that probably affected many configurations). They are a normal part of component stabilization and will continue happening across the Collector.
Currently, we rely on warnings, updated examples and documentation to reflect our recommendation and expect users to check these and then upgrade their configuration manually. Our current approach probably means that only a small number of users migrate to the new configuration, and this makes it hard for us to drop old syntax/configuration flags without hurting many users. It also makes it hard to come to a consensus in issues like #5686.
Describe the solution you'd like
Add a subcommand that automates as much as possible of any given configuration migration. Ideally, the user passes the configuration in the same way they would do it for a normal Collector run and the Collector then prints out the updated configuration as well as a list of the changes that were made and their rationale.
Describe alternatives you've considered
An alternative is to provide a separate tool for this. This would work for the
${...} -> ${env:..}
migration, but would make it hard for components to introduce custom migration rules.Additional context
Note: This section lists some requirements of a fully working solution. While we should consider all of these when designing a first solution, we don't have to support all of them, and an MVP should leave things like custom component migration out of scope for now.
Some requirements
The solution should:
${..}
to${env:...}
migration. It probably is useful also for components to work at theconfmap
level.confmap.Conf
noting the source of each configuration section/scalar value.Open questions
exporterhelper
#4646, where we want to move configuration from one component to another.Some follow up tasks once we have this
The text was updated successfully, but these errors were encountered: