Breaking Change: Change confmap.Provider to return pointer to Retrieved.

[bogdandrutu]

This change makes implementations cleaner, since they can return nil, err in case of an error instead of a zero initialized Retrieved.

This is a breaking change, but there are very very few implementation of the Provider, so it should be safe to just break it.

Signed-off-by: Bogdan [email protected]

[bogdandrutu]

I need to get @open-telemetry/collector-approvers consensus, since this is a breaking change without deprecation.

[codecov]

Codecov Report

Merging #5839 (3669bcc) into main (b9fa036) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #5839   +/-   ##
=======================================
  Coverage   91.82%   91.82%           
=======================================
  Files         195      195           
  Lines       11920    11920           
=======================================
  Hits        10945    10945           
  Misses        768      768           
  Partials      207      207           

Impacted Files	Coverage Δ
confmap/provider.go	100.00% <100.00%> (ø)
confmap/provider/envprovider/provider.go	100.00% <100.00%> (ø)
confmap/provider/fileprovider/provider.go	100.00% <100.00%> (ø)
confmap/provider/internal/provider.go	100.00% <100.00%> (ø)
confmap/provider/yamlprovider/provider.go	100.00% <100.00%> (ø)
confmap/resolver.go	97.22% <100.00%> (ø)

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

[mx-psi]

LGTM, I am fine with breaking this in one go

[codeboten]

This is a breaking change, but there are very very few implementation of the Provider, so it should be safe to just break it.

Is there a way to know this outside of the repository? I agree with this change, however i would prefer we follow the same path we always do for breaking changes.

[bogdandrutu]

Is there a way to know this outside of the repository?

• Contrib passes (so that is a good sign).
• Based on my knowledge only AWS may have an implementation of this for http/s3. If that is the case, the change is very small for them.

however i would prefer we follow the same path we always do for breaking changes.

• Because Provider is an interface it is extremely hard base on what I know to make this fully backwards compatible. We can define a new interface and provide some "Converters" then we need to change lots of other public APIs wherever we expect this Interface to expect the other one, and deprecate there, etc. Because the good names are taken we have to do everything in 3 steps. I think this is definitely overkill based on the very limited usage and the amount of changes needed.

[codeboten]

Right, since any changes to the interface would cause any implementor to need to update anyways. I'm ok going forward with this.

[mx-psi]

S3 provider is here, I think: open-telemetry/opentelemetry-collector-contrib/pull/13113

[bogdandrutu]

Looks like we have 3 approvers, and first provider coming to contrib soon. I will merge this and update contrib so we can have that one implementing the new interface.

[junhaoyu-aws]

That's a great work, I will fix my s3provider PR according to this.