opentelemetry-collector icon indicating copy to clipboard operation
opentelemetry-collector copied to clipboard

Add config marshaler.

Open jefchien opened this issue 2 years ago • 7 comments

Description: Add Marshal function to confmap. Update types implementing encoding.TextUnmarshaler to implement encoding.TextMarshaler as well. Add a mapstructure encoder that encodes the config provided using the existing mapstructure tags.

Link to tracking Issue: https://github.com/open-telemetry/opentelemetry-collector/issues/5418

Testing: Added unit test to validate the unmarshal/marshal cycle.

jefchien avatar Jun 21 '22 15:06 jefchien

Codecov Report

Base: 91.77% // Head: 91.87% // Increases project coverage by +0.10% :tada:

Coverage data is based on head (deeacb6) compared to base (8060037). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5566      +/-   ##
==========================================
+ Coverage   91.77%   91.87%   +0.10%     
==========================================
  Files         218      219       +1     
  Lines       13364    13533     +169     
==========================================
+ Hits        12265    12434     +169     
  Misses        870      870              
  Partials      229      229              
Impacted Files Coverage Δ
config/configtelemetry/configtelemetry.go 100.00% <100.00%> (ø)
config/identifiable.go 100.00% <100.00%> (ø)
confmap/confmap.go 93.19% <100.00%> (+2.20%) :arrow_up:
confmap/internal/mapstructure/encoder.go 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Jun 22 '22 18:06 codecov[bot]

@bogdandrutu PTAL when you get the chance. Refactored the marshaler to be more similar to the unmarshaler.

jefchien avatar Jun 28 '22 21:06 jefchien

This PR was marked stale due to lack of activity. It will be closed in 14 days.

github-actions[bot] avatar Jul 29 '22 03:07 github-actions[bot]

Changed the Marshallable interface to take in a confmap.Conf. Added a function on the confmap.Conf that uses the encoder and uses koanf.Merge to "marshal" it into the confmap.Conf.

jefchien avatar Aug 03 '22 20:08 jefchien

The Encoder handles all of the reflection needed to respect the mapstructure tags and is used by confmap to marshal. Without it, the marshal/unmarshal cycle would break and the configs being generated would be invalid.

jefchien avatar Sep 08 '22 14:09 jefchien

The Encoder handles all of the reflection needed to respect the mapstructure tags and is used by confmap to marshal. Without it, the marshal/unmarshal cycle would break and the configs being generated would be invalid.

Then the dependency graph is totally unexpected. Move that to confmap/internal/mapstructure if needed.

bogdandrutu avatar Sep 08 '22 15:09 bogdandrutu

The Encoder handles all of the reflection needed to respect the mapstructure tags and is used by confmap to marshal. Without it, the marshal/unmarshal cycle would break and the configs being generated would be invalid.

Then the dependency graph is totally unexpected. Move that to confmap/internal/mapstructure if needed.

Moved under confmap/internal/mapstructure. The old path was a holdover from the initial implementation where the confmap didn't handle the marshalling.

jefchien avatar Sep 08 '22 15:09 jefchien

This PR was marked stale due to lack of activity. It will be closed in 14 days.

github-actions[bot] avatar Sep 24 '22 04:09 github-actions[bot]