Define file configuration file format and env var substitution #3744
Conversation
|
cc @open-telemetry/configuration-maintainers |
|
The common expectation among developers is that env vars will automatically overwrite config parameters. If we do it the other way, I am concerned that until the end of time we will have a steady stream of users lodging issues about this and then becoming very frustrated when we explain that they need to modify the config file to use an env var. The reason that these users will be upset is that they are in a situation where having to define the env vars in the config file is a non-starter. Their use case is that for some reason they either can't get at or are not allowed to modify the config file template, but they really need to override a parameter. For reference, this is an issue that has come up on many OSS projects I have been involved with where it is common to have both operators and application developers wanting to configure the same thing. Often it's some kind of emergency situation where the person with the rights to change the config file is unavailable. I should note that of course the opposite situation could be true, where for some reason you want to disable an env var but don't have access to it. But it's probably easier to give users the ability to disable an env var via the config file than it is to give them the ability to disable a config parameter via an env var. |
To be fair, from experience, you're going to get people complaining either way. If you choose environment variable priority over a configuration users will complain that their deployment was altered and failed when an environment variable was set that took precedence. However, if you make environment variables take precedence you will also need to make some pretty sever and subjective choices on how they are mapped to a config. Do the BSP environment variables apply to all batch span processors or just one? Is the sampler environment variable use for all tracer providers in the config, even ones that specify alternate samplers? Should propagators be merged or overridden? If an exporter is defined by environment, does that stop the console logging exporter used for debugging as well as all the other exporters defined in configuration? Ultimately, I think the current changes are going to be the most appropriate. They allow users to make their own choice in precedence without making subjective choices for them on how to map things. If a user want environment variables to take precedence, all they need to do is use the OTel environment keys in the related parts of their configuration. In doing so they will answer each of the above questions their own way. |
This is my feeling as well, especially for things like:
I totally understand the nightmare that is merging configuration from multiple sources though. I wonder if we would have created many of the other env vars (e.g. |
|
I want to clarify that while this conversation about the precedence of environment variables and the configuration file is important, there is nothing in this PR related to that. This conversation popped up as a bit of a tangent in the 10/30/23 maintainers meeting, but I wasn't referencing language in this PR - I was referencing language from the OTEP:
This language was originally included in #3437 but appears to be have been lost in the shuffle of reviews / changes. I propose we open a new issue to discuss. While its tempting to think that this PR should resolve the issue because its related to environment variables, I think environment variable substitution syntax is independent of any sort of merging / priority. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Few comments:
Any chance the env subst is postponed until some progress is made on representing telemetry settings in K8S ? |
|
I support removing the |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
@open-telemetry/configuration-maintainers please review |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
👍 |
|
Will merge tomorrow if there are no additional comments. |
Part of incorporating [OTEP #225](open-telemetry/oteps#225) into the specification. Followup to #3744. This defines how file configuration works with custom SDK extension components (Samplers, Exporters, etc). It defines the concept of a Component Provider: - Component providers are registered with the type of extension component they provide and a name. Component providers are registered automatically or manually based on what is idiomatic in the language. - Component providers have a Create Plugin method, which passes configuration properties as a parameter and returns the configured component - When Create is called to interpret a file configuration model, and it comes across a reference to a extension component which is not built-in, it invokes Create Plugin on the corresponding component provider. If no corresponding component provider exists, or if Create Plugin returns an Error, Create returns an error. Prototype implementation in java here: open-telemetry/opentelemetry-java-examples#227 cc @open-telemetry/configuration-maintainers
Part of incorporating OTEP #225 into the specification.
Followup to #3437.
The adds the YAML file format, and leaves an open TODO for the JSON format discussed in the original OTEP. It also define environment variable substitution. IMO its necessary to define environment variable substitution at the same time as file format because they interact in ways that aren't immediately obvious and need to be prototyped.
The opentelemetry-java implementation of file configuration already accepts YAML encoding and performs environment variable substitution as described in this PR. It does not support JSON, and I don't think we should unless there is good reason. I omitted the JSON file format because I don't know of any prototypes that permit it. We should add JSON once a language implements it and explores how environment variable substitution works with JSON.