Define file configuration parse and create operations #3437
Conversation
arminru
added
area:configuration
…y-specification into parse-and-configure
…entelemetry-specification into parse-and-configure
|
@MrAlias, @pellared, @tsloughter PTAL. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
@MrAlias @codeboten @tsloughter @carlosalberto @yurishkuro @aabmass please take another look when you can! |
There was a problem hiding this comment.
Thanks for the reminder @jack-berg. This looks good and matches what we've discussed in the working group previously 👍🏻
…y-specification into parse-and-configure
|
FYI, I'd like to merge this on 10/16 if there are no further comments. @carlosalberto I requested an additional review from you since the content changed since your original would be good if you could take another look and comment or approve. |
|
Overall LGTM, although I'm curious about whether the config model returned by |
I think long term the model should be a struct / interface that the user can read from and update (or if not update provide their own implementation of the interface with updated values). This would allow users to define their own higher owner functions for merging multiple config sources. I do think allowing users to define their own merge logic is important because its unlikely we'd be able to come up with merge behavior that satisfies everyone. But yeah, I think we can expand on the specifics of that in followup PRs. |
Part of incorporating [OTEP #225](open-telemetry/oteps#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.
…#3437) Part of incorporating [OTEP open-telemetry#225](open-telemetry/oteps#225) into the specification. Followup to open-telemetry#3360.
…telemetry#3744) Part of incorporating [OTEP open-telemetry#225](open-telemetry/oteps#225) into the specification. Followup to open-telemetry#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.
Part of incorporating OTEP #225 into the specification.
Followup to #3360.