config: Parse model from JSON

[carrbs]

resolves #4412

Implements parsing of a JSON configuration file into an OpenTelemetryConfiguration, and tests it.

[pellared]

This would return the same result

Suggested change

	input: `{"file_format": "json", "disabled": false}`,
	input: `{"file_format": "json", "disabled": false}THIS IS INTENTIONALLY MALFORMED`,

Reference: https://dev.to/taqkarim/you-might-not-be-using-json-decoder-correctly-in-golang-12mb

Maybe it would be safer to change the API to accept []byte instead of io.Reader and use json.Unmarshal? Especially that the performance difference is not big. Reference: https://dev.to/jpoly1219/to-unmarshal-or-to-decode-json-processing-in-go-explained-5870

Or maybe we should even not add such function, but simply add an an example to docs instead?

E.g.

func ExampleParseJSON() {
	input := `{"file_format": "0.1", "disabled": true}`

	var cfg config.OpenTelemetryConfiguration
	if err := json.Unmarshal(([]byte)(input), &cfg); err != nil {
		panic(err)
	}

	fmt.Printf("Disabled: %v", cfg.Disabled)

	// Output: Disabled: true
}

I am leaning towards adding a runnable example in docs instead of adding a function.

[carrbs]

A runnable example in the docs sounds just fine to me. I can also rewrite this to accept the []byte string so we can use the more dependable json.Unmarshal method in the parser if we decide to take this route.

[pellared]

I am leaning towards a runnable a example to reduce the API surface.

When I created #4412 I did NOT want OpenTelemetryConfiguration to have mapstructure tags, but after some time and disucssion agreed that it is acceptable. I will update the issue.

CC @codeboten

[pellared]

Can you make some more advanced test? I am not sure if it works "out of the box". We may be missing JSON tags. See:

opentelemetry-go-contrib/Makefile

Line 322 in ba19074

--tags mapstructure \

@codeboten Am I correct?

[carrbs]

@pellared / @codeboten, I wasn't super clear on what the expected input would be, so perhaps I've overlooked something. I'd thought the OpenTelemetryConfiguration was specifically designed to build the parameters as mapstructures so the user could choose to define their config using either yaml or json. I (possibly naively) was building this to accept any valid json that had the required property from the configuration input used in the Makefile:

opentelemetry-go-contrib/Makefile

Line 324 in ba19074

${OPENTELEMETRY_CONFIGURATION_JSONSCHEMA_SRC_DIR}/schema/opentelemetry_configuration.json

Perhaps we can clarify what are the expected valid inputs for this (and #4373), and what we'd expect as output on valid/invalid cases? I feel this would help with implementation and writing more comprehensive tests. lmkwyt!

[pellared]

I was thinking of removing this line:

opentelemetry-go-contrib/Makefile

Line 322 in ba19074

--tags mapstructure \

As then the OpenTelemetryConfiguration type will have mapstructure, json and yaml tags. See:

https://github.com/omissis/go-jsonschema/blob/6fcc83b8eebe874eeebc20474775280877cc62fe/main.go#L167-L168

Then we could simply provide examples how one can parse input to OpenTelemetryConfiguration using different libraries.

The parsing method do NOT need to do any validation e.g. check for required fields. The validation has to be done by NewSDK because the user could even create OpenTelemetryConfiguration "by hand" without parsing any JSON or YAML.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3cecdcf) 81.1% compared to head (d6381c7) 81.2%.
Report is 20 commits behind head on main.

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #4806   +/-   ##
=====================================
  Coverage   81.1%   81.2%           
=====================================
  Files        150     150           
  Lines      10753   10759    +6     
=====================================
+ Hits        8725    8740   +15     
+ Misses      1872    1861   -11     
- Partials     156     158    +2     

Files	Coverage Δ
config/config.go	100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

[pellared]

Now, I see that the specification currently only requires implementing parsing of YAML: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/file-configuration.md#parse

@codeboten should we change to draft (or even close) this PR? WDYT?

[codeboten]

@pellared it might make sense to only implement YAML and leave JSON until there's a need for it. @carrbs do you have a specific need for JSON parsing to be implemented?

[carrbs]

@carrbs do you have a specific need for JSON parsing to be implemented?

@codeboten, not at all, I was just looking for low hanging fruit to work on. Closing this sounds good to me!