opentelemetry-collector
opentelemetry-collector copied to clipboard
Allow builder to accept multiple config providers
This is the builder part of #4567.
Question to @pavankrish123, @pmm-sumo, and @bogdandrutu: how would the main.go look like to support this? Once I know that, we can find a way to support it in the builder.
Without looking into many details about the config provider feature, I think it could work in a similar way as the components: it could accept a list of factories which would be loaded in order. The builder could then have a new config option, "configproviders", accepting go modules for those providers like we do for the other components.
I think for this to work we'd need either a conventional way to instantiate a provider given just a package name, or we'd need to include an option to specify a function to invoke to instantiate the provider.
We should probably do the same for map converters, as well.
or we'd need to include an option to specify a function to invoke to instantiate the provider.
We assume "NewFactory" for the components:
https://github.com/open-telemetry/opentelemetry-collector/blob/32301d929b413918efeefc1d9a1165b03ac71988/cmd/builder/internal/builder/templates/components.go.tmpl#L27
I like the idea with "configproviders" (or "configmapproviders" actually?).
As @Aneurysm9 noted, doing it at the level of configmapproviders also brings a question how to handle map converters and perhaps unmarshaller (separate set of config entries I presume?). I think refactoring it to make it consistent with other components ("NewFactory()") would be the way to go
One limitation of the conventional approach is that it can only support one (provider|converter|unmarshaller) per package. The collector currently has several of each in common packages and those would need to be restructured.
This seems like something we should address in a short order. Any ideas?
I believe that my prior concern about conventional initialization regarding multiple providers living in the same package has been addressed and now all provider implementations in this and the contrib repos are in independent packages and have a New() initializer. I think we should have a clear path to including config provider implementations via ocb if we're comfortable with New() as a conventional initializer. I might be slightly more comfortable with NewProvider(), but I think it generally best for these implementations to remain in standalone packages so there should be little risk of conflict in the initializer name.
I think the providers are in a good place for this to move forward. Per the discussion in https://github.com/open-telemetry/opentelemetry-collector/issues/6956, we want to configure providers using URI fragments, which are passed when they resolve a config URI, so the New() function in each provider package should be sufficient to instantiate the provider.
In order to allow the builder to specify confmap providers (and converters), we will need to make some modifications to the otelcol package to make this easier. Right now this isn't really feasible since NewCommand takes otelCol.CollectorSettings which only accepts an instantiated otelcol.ConfigProvider. This creates a bit of a circular dependency since the otelcol Command is what reads in config URIs from the command line arguments, but the ConfigProvider must be created prior to the arguments being read. I can think of a few ways to approach this:
- Create
otelcol.CommandSettingsthat translates between the Command andotelcol.CollectorSettings. This would allow exposing options to pass in a slice of providers. We could also potentially hide the option to pass in a custom ConfigProvider in the command to simplify the API. A third option could be to allow passing providers in through an optional argument if we deem that would result in the cleanest API. - Have
otelcol.Collectorcreate its own ConfigProvider. This would remove the ability to provide a custom ConfigProvider, but I think the existing one should be sufficient for nearly all users. - Allow updating the set of URIs used in a ConfigProvider. So the builder could create a ConfigProvider which the Command then updates to include the URIs passed in through the command line arguments. We would need to determine how to handle an empty set of URIs or an appropriate default value.