opentelemetry-collector
opentelemetry-collector copied to clipboard
Create tool or subcommand that allows migrating configuration
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:
- Allow adding support for components providing custom migration rules (similar to what we do for unmarshaling and validation). We should take this into account in the design phase but leave implementation explicitly out of the first version of this subcommand.
-
Work at the confmap level, at least for the
${..}
to${env:...}
migration. It probably is useful also for components to work at theconfmap
level. -
Provide output that is as close as possible to the way the user provided the configuration. With the current implementation it's hard to map back a given configuration to the individual files/environment variables/other sources where it comes from, so doing this 'the right way' means adding some sort of metadata to
confmap.Conf
noting the source of each configuration section/scalar value.
Open questions
- How should we provide the output? If configuration comes from multiple sources, should we map back to these sources? How do we do this given merging happens very early? We can't modify user input in some cases (protected files, env variables, URLs), so should we just print things out to stdout?
- Should components be able to recommend configuration changes outside of their configuration section? This could be useful for situations like #4646, where we want to move configuration from one component to another.
- For some configuration changes, we may not be able to fully automate the process no matter how clever we are (e.g. if we drop some functionality because it is moved to the SDKs). What should we do in these cases? Should we have support for warnings/lints? This functionality would also be useful for #5615.
Some follow up tasks once we have this
- [ ] Add this to our CI to run on examples for dogfooding
- [ ] Add warnings in logs to nudge users to migrate to the new configuration (e.g. by doing a dry-run and checking if there are changes reported)
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 https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/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?
file:path/to/file/one
---
# Contents of file path/to/file/one go here
---
env:SOME_ENV
---
# Contents of environment variable SOME_ENV go here
---
...
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 map[string]any
. The output file is a Go text/template
that expresses the new configuration, and contains references to existing config values (included in the template as .Data
) where appropriate.
An example from the template showing a renamed config item is:
Logging:
## HoneycombAPI is the URL for the upstream Honeycomb API for logs; this
## is the destination to which refinery sends its own logs. Only used if
## the logging type is "honeycomb".
##
## Not eligible for live reload.
{{ nonDefaultOnly .Data "HoneycombAPI" "HoneycombLogger.LoggerHoneycombAPI" "https://api.honeycomb.io" }}
nonDefaultOnly
is defined as:
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:
- The config file itself should be versioned going forward so that future converters know what they're trying to load and can stamp what they're writing.
- Any components that need a converter should also be required to include a version within that component's config in all future configurations for the same reason.
- It wouldn't hurt for the config to include a "minimum supported version" of the collector to which it applies. This would allow a collector to avoid trying to load a config it won't understand.
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: https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/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 ./otelcolcore --auto-migrate --dry-run
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 $ENV
to ${ENV}
. I guess whether we need this for 1.0 depends on what amount of people we expect would have to do the migration. We don't need to build this tool specifically for it, but having some sort of helper would be very helpful in the transition to 1.0 IMO