Allow loading incomplete configuration

[pavolloffay]

Allow loading incomplete configuration with missing root level nodes like exporters, receivers.

Jaeger OTEL collector uses hardcoded default configuration. OTEL configuration file is also supported and it is merged with the default configuration. The issue is that users have to specify all parts of OTEL config even when they want to override a single property or just add one additional processor.

This PR adds config "merge" capability to Jaeger jaegertracing/jaeger#2211.

However config.Load() fails if receivers/exporters/service/pipeline is missing e.g.

opentelemetry-collector/config/config.go

Line 357 in 9d16f59

msg: "no receivers specified in config",

.

[pavolloffay]

Also note that there is a separate public API for config validation - config.ValidateConfig

opentelemetry-collector/config/config.go

Line 597 in 9d16f59

func ValidateConfig(cfg *configmodels.Config, logger *zap.Logger) error {

[tigrannajaryan]

So, just to clarify. You suggest to move the check for presence of top-level "receivers" and "exporters" from config.Load to config.ValidateConfig.

What would be the next step? To have a default hard-coded config with which the user defined config is merged or there is a different goal?

[pavolloffay]

So, just to clarify. You suggest to move the check for presence of top-level "receivers" and "exporters" from config.Load to config.ValidateConfig.

Correct, if needed we can split it into two functions.

What would be the next step? To have a default hard-coded config with which the user defined config is merged or there is a different goal?

In Jaeger we want to merge the configs. Here is a blog post that explains configuration https://medium.com/jaegertracing/jaeger-embraces-opentelemetry-collector-90a545cbc24.

Whether a similar approach would be implemented here it's up to the decision - I am not proposing it.

[tigrannajaryan]

Makes sense to me. There are several benefits: 1) it fits the Load/Validate model nicely, 2) it improves the customizability of the Collector, 3) potentially allows us to implement a similar "default config" capability in the core in the future (which you refer to in #886).

Do you want to do a PR that implements your proposal?

[bogdandrutu]

I will do this, because I got also confused about test coverage dropped in #909 and is because currently I cannot test verify config unless I am writing my own Load :) - first world problems I know :))

[bogdandrutu]

@pavolloffay I am worried that in #909 I removed some functionality you rely on. Can you tell me if you can still achieve what you want without that?

[pavolloffay]

Correct the #909 removed functionality Jaeger OTEL collector relied on. We wanted to use the disabled option to disable components without overriding the default/hardcoded service.pipeline.

Let me illustrate it: Jaeger by default uses batch processors in default/hardcoded configuration. If a user wants to disable it (with #909 merged) he has to override the service.pipeline.traces.processor array:

service:
  pipelines:
    traces:
      processors: []

We wanted to also support adding components (e.g. processors) to the default/hardcoded pipeline - a user would just specify the processor in the service.pipelines.traces.processors array without repeating default/hardcoded processors (defautl/hardcoed and user config array would be merged/unified). This however brings problems, there is no way to override the order and with the removed disabled flag in #909 there is no way to disable a default/hardcoded components. So instead we decided that any arrays would be fully overridden by user-provided config.

There is one problem though, for instance Jaeger receiver implements multiple receivers (grpc, thrift, HTTP) and they cannot be disabled individually as a result of #909. So if one wants to disabled grpc endpoint that is by default enabled he will have to specify new jaeger receiver e.g. jaeger/override and add it to service.pipelines.traces.receiver.

[bogdandrutu]

I will make sure for multi-protocol receivers to keep the disable at the protocol level and not the component level. Does that solve all your issues? Or you need back the entire functionality? I would also recommend to not do too much magic when it comes to add/remove components to the pipeline because that can cause unexpected behaviors.

[pavolloffay]

I will make sure for multi-protocol receivers to keep the disable at the protocol level and not the component level. Does that solve all your issues?

This is enough thanks. I can submit a PR to bring it back there.