Refactor config sources into provider wrapper and support all service providers #2893
Conversation
rmfitzpatrick
force-pushed
the
configproviderreduction
branch
5 times, most recently
from
463d044
to
91fef18
Compare
| @@ -10,21 +10,21 @@ resource_metrics: | |||
| state: used | |||
| single-dollar: an-envvar-value-suffix | |||
| single-dollar-no-curly-braces: prefix-an-envvar-value | |||
| double-dollar: ${env:AN_ENVVAR}-suffix | |||
There was a problem hiding this comment.
Because of open-telemetry/opentelemetry-collector#7055 I think this is now guaranteed to be expanded by the upstream provider. Existing compatibility persists in the non-${env:..} case.
| OnShutdown() | ||
| } | ||
|
|
||
| type Provider interface { |
There was a problem hiding this comment.
Can we make this easier to understand for reviewers and maintainers? Looks like this interface is called Provider, its method is called Provider, and the method accepts a Provider and returns a Provider :). Do we need the interface -- it seems like the only implementer is ProviderWrapper (which is exported). And can we rename the Provider function to WrapProvider (or something) if that's what it does?
There was a problem hiding this comment.
It's a configsource.Provider that takes a confmap.Provider and returns a confmap.Provider. I initially named the method Wrap but thought there were too many "wrap" prefixes so relied on the package name to help differentiate. The interface is helpful imo to help keep the surface area minimal while allowing changes to the implementation. Went ahead and unexported the implementation since the confmap.Provider is what's important. Also updated to use the clearer method name and added some doc comments.
rmfitzpatrick
force-pushed
the
configproviderreduction
branch
from
7fa9906
to
fea3d88
Compare
These changes get us closer to reconciling the original config sources and manager with the core resolver and providers that were based on them. In the process they remove unused/unnecessary fields and method requirements for the config source interfaces along with some general renaming to help with accuracy/maintainability. They also add support for existing service confmap.Provider's that would otherwise require settings declarations in our config without upstream equivalents, resolving #2536.