You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Technically, we could do steps 1-3 again for only these components but it looks like a substantial overhaul to separate this functionality from the unmarshaler.
That was more or less the approach I had in mind. I agree it would probably require a substantial overhaul, but I thought about it some more after reading your summary and I still think it is possibly worth the effort. I've attempted to draft a solution that would solve all our issues with minimal downsides, but let me know if there's some fatal flaw that I've missed.
I think the problems with implementing templating within our current unmarshaling facilities are that they expect a one-to-one mapping between component configs and factories, and that all component unmarshaling is done within a section of the config instead of in the scope of the whole config.
We would probably still want to take advantage of configunmarshaler.Configs to unmarshal individual components, but would want to process the whole config after that to do template expansion. I would see a "config processor" being something similar to a converter that implements the following interface. A list of these would be registered with otelcol.ConfigProvider and would be used during config unmarshaling.
typeConfigProcessorinterface {
// Prefix for configs like "template/"Prefix() string// Config type for component configs that need// processing like `template/my_receiver`ComponentConfig() any// Key and type for the processor's config (e.g. "templates" and `templatesConfig`)ProcessorConfig() (string, any)
// Handles configs starting with the processor's prefix// given the available factories and its config.// All instances of the ComponentConfig type should be// replaced by component.Config when this function returns.Process(pc*PartialConfig, factoriesFactories, configany) error
}
We could then split config unmarshaling into stages:
First, unmarshal the config as much as possible, considering during unmarshaling that there may be some keys that need further processing. So we could unmarshal components, but any config keys prepended with template/ would need to be handled by a processor. I think this first step would produce a struct like:
typePartialConfigstruct {
Receivers: map[string]any// Factory or types returned by ConfigProcessor.ComponentConfig()
[...]
Service: PartialService// Some type that enables accessing/creating pipelines for the case of templating
}
We would then run the Process method of each ConfigProcessor on the PartialConfig, with the expectation that after all processors have run, we're left with map[string]component.Config for each component class. I think the template converter would run in a similar fashion to how it runs now with this: it would remove the component template from the config, then add all components and add them to the pipelines.
I didn't specify a mechanism here, but at least for templates we would also want to provide a common function to take a confmap.Conf and return a component.Config. Component config unmarshaling would still happen at two different steps, but would at least be done consistently.
We currently only have two use-cases in mind, which I'm not sure is enough to say that this design will serve all our needs going forward or will be used enough to justify the additional complexity.
Overall I think this will be cleaner and for templating will enable things like checking whether the forward connector is available. However, at least for our current use-cases, this doesn't enable anything that we can't already do with a Converter.
We would need to meddle with service.Config (or produce it from another struct), for which the whole point is to be directly unmarshaled from a confmap.Conf. We'd likely have to refactor something here.
"Expand" the top-level component graph by substituting the subcomponents in place of the templated component. (Again, see Template provider #8372)
The first approach has a number of problems, described in detail here.
The second approach is currently possible, but the implementation (see #8507) is somewhat messy because we are forced to reason about the structure of otelcol.Config _but must actually perform all work on a raw confmap.Conf.
This issue represents a potential path forward based on the goal of obtaining a proper otelcol.Configbefore attempting to substitute subcomponents into the component graph.
I have a working prototype for this, but there are still some rough edges. I hope to have a PR that we can use as a basis for further discussion sometime soon.
That was more or less the approach I had in mind. I agree it would probably require a substantial overhaul, but I thought about it some more after reading your summary and I still think it is possibly worth the effort. I've attempted to draft a solution that would solve all our issues with minimal downsides, but let me know if there's some fatal flaw that I've missed.
I think the problems with implementing templating within our current unmarshaling facilities are that they expect a one-to-one mapping between component configs and factories, and that all component unmarshaling is done within a section of the config instead of in the scope of the whole config.
We would probably still want to take advantage of
configunmarshaler.Configs
to unmarshal individual components, but would want to process the whole config after that to do template expansion. I would see a "config processor" being something similar to a converter that implements the following interface. A list of these would be registered withotelcol.ConfigProvider
and would be used during config unmarshaling.We could then split config unmarshaling into stages:
First, unmarshal the config as much as possible, considering during unmarshaling that there may be some keys that need further processing. So we could unmarshal components, but any config keys prepended with
template/
would need to be handled by a processor. I think this first step would produce a struct like:We would then run the
Process
method of eachConfigProcessor
on thePartialConfig
, with the expectation that after all processors have run, we're left withmap[string]component.Config
for each component class. I think the template converter would run in a similar fashion to how it runs now with this: it would remove the component template from the config, then add all components and add them to the pipelines.I didn't specify a mechanism here, but at least for templates we would also want to provide a common function to take a
confmap.Conf
and return acomponent.Config
. Component config unmarshaling would still happen at two different steps, but would at least be done consistently.I think this design would also provide a straightforward path for open-telemetry/opentelemetry-collector-contrib#18509, which would probably work similar to templates but be slightly simpler.
I see a few downsides to this route:
service.Config
(or produce it from another struct), for which the whole point is to be directly unmarshaled from aconfmap.Conf
. We'd likely have to refactor something here.Originally posted by @evan-bradley in #8507 (comment)
The text was updated successfully, but these errors were encountered: