opentelemetry-go-contrib
opentelemetry-go-contrib copied to clipboard
config: add yaml/json struct tags
This will make parsing the configuration easier
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 67.4%. Comparing base (
793b970) to head (1aac30d). Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #5433 +/- ##
=======================================
+ Coverage 66.8% 67.4% +0.6%
=======================================
Files 206 206
Lines 13211 13217 +6
=======================================
+ Hits 8833 8919 +86
+ Misses 4111 3996 -115
- Partials 267 302 +35
| Files with missing lines | Coverage Δ | |
|---|---|---|
| config/config.go | 84.3% <100.0%> (+2.0%) |
:arrow_up: |
| config/generated_config.go | 53.3% <ø> (+53.3%) |
:arrow_up: |
This needs a changelog.
I see this adds additional tags, but I'm not 100% sure how this is used. It doesn't necessarily need to be part of the PR, but could you share an example of how this was used before and how it works now?
Can you add tests that verify that the YAML/JSON files are parsed (thanks to the tags) as expected?
@MadVikingGod @pellared happy to add some functionality to test it, i didn't want to overstep on this PR https://github.com/open-telemetry/opentelemetry-go-contrib/pull/4826
@carrbs if you're ok with it I can incorporate your work here, or you can pull this change in your PR, it's all the same to me :)
@MadVikingGod @pellared happy to add some functionality to test it, i didn't want to overstep on this PR #4826
@carrbs if you're ok with it I can incorporate your work here, or you can pull this change in your PR, it's all the same to me :)
I'm good with incorporating here, thanks @codeboten!
So, what I was hoping for to understand better why this change is necessary was something like: Before:
input := map[string]interface{}
err := json.Unmarshal(rawJson, &input)
config := config{}
err = mapstructure.Decode(input, &config)
But afterwards we have
Some happy path code
While this looks to add direct ways to decode directly into our Config structures, it also means we have to test three different decoders (Mapstructure, Json, and Yaml) and find all the edge cases of each.
@MadVikingGod PTAL
Added test to validate json/yaml parsing is working as expected, also pulled in part of https://github.com/open-telemetry/opentelemetry-go-contrib/pull/4826 for adding a ParseYAML api