opentelemetry-collector
opentelemetry-collector copied to clipboard
[confmap] Unify behavior of `${env:ENV}` and `${ENV}`
As decided on #9854, we want to unify the behavior of ${env:ENV} and ${ENV}. Currently, each has a different behavior: the ${ENV} syntax does not do any YAML parsing, while the ${env:ENV} syntax does parse the contents as YAML. We want to unify both and make them have the same behavior, described in RFC #9854.
One approach for this is to have some 'default' confmap provider used by the ${..} syntax, and control what provider is used (a expandconverter provider or the env provider) through a feature gate.
cc @TylerHelmuth
@mx-psi I see a couple approaches.
- Allow
envproviderto expand${env:}and${}. This is conceptually simple, but I think breaks some logic inresolver.expandValuethat requires providers to have an schema so it can identify which provider to use. - Allow
resolver.expandValueto expand${}as an env var when it doesn't contain a uri (basically do env var expansion here). This is doable, but would mean duplicating the yaml parsing logic and logging logic (which means adding a Logger to the ResolverSettings) from theenvprovide. - Leave expandconverter in place, but restrict it to only allowing
${}and add yaml parsing. - Create the concept of a "default" provider that is used by the Resolver when no schema is set. This would be a new field on the ResolverSettings. For our collector instances we'd set the default to be the envprovider. The envprovider would need to be updated to not error when no schema is provided.
I've experimented with 2 and think it would be workable. It would take care of this issue and https://github.com/open-telemetry/opentelemetry-collector/issues/8215 at once. It did feel a little messy but there are probably ways to share code between the 2 places.
I would also be ok with option 4, as it provides a lot of flexibility.
Option 3 is pretty simple as well.
Option 4 feels the cleanest to me. I think we should do it without the awareness of any of the Providers by prepending the default schema to the URI when the URI is given to the Provider, like the Resolver currently does with files passed to config arguments. If we provide this option, we may also want to consider whether to make it configurable for the command line flags
I think we should do it without the awareness of any of the Providers by prepending the default schema to the URI when the URI is given to the Provider
Ya that would work. I don't love the idea of messing with the incoming values, but if we already have precedence I can be ok with it.
If we provide this option, we may also want to consider whether to make it configurable for the command line flags
I think this is an option, but not required for 1.0
Agreed that option 4 feels like the cleanest. If we go with (4), for now I don't think we need to make this configurable anywhere other than the Go API, but we can open the door to make this configurable in the future by end-users
If we go with (4), for now I don't think we need to make this configurable anywhere other than the Go API, but we can open the door to make this configurable in the future by end-users
Agreed. I'll start work on option 4. I try to do this and https://github.com/open-telemetry/opentelemetry-collector/issues/7111 in one.