search

open-telemetry / opentelemetry-collector

OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
4.34k stars 1.44k forks source link

Remove one of the nopexporter.NewFactory or exportertest.NewNopFactory #11369

Open bogdandrutu opened 3 days ago

bogdandrutu commented 3 days ago

These 2 are exactly the same. We should keep only one.

cc @open-telemetry/collector-approvers

dmitryax commented 3 days ago

Sounds good to me. We can remove exportertest.NewNopFactory

narcis96 commented 2 days ago

@bogdandrutu @dmitryax I have created https://github.com/open-telemetry/opentelemetry-collector/pull/11381

mx-psi commented 1 day ago

Should we also create nopprocessor instead of https://pkg.go.dev/go.opentelemetry.io/collector/processor@v0.111.0/processortest#NewNopFactory?

bogdandrutu commented 1 day ago

Can I ask a different question. Is the nop exporter/processor really useful as a standalone component? Can I get some arguments why we need that?

mx-psi commented 1 day ago

I believe @evan-bradley worked on this primarily for OpAMP, the processor would not be useful for that use case but consistency is a relevant issue

bogdandrutu commented 1 day ago

Why is the exporter than. I 100% agree with you for consistency, but still not seeing usefulness for exporter either.

evan-bradley commented 1 day ago

The exporter has two uses that I'm aware of:

  1. It allows us to start the Collector only running extensions. This is useful for the bootstrapping process done by the OpAMP Supervisor.
  2. It can be used for load testing a live Collector if the focus is a receiver/processor, since the exporter will not have any overhead.

The original issue may also provide some context: https://github.com/open-telemetry/opentelemetry-collector/issues/7316.

bogdandrutu commented 10 hours ago

It allows us to start the Collector only running extensions. This is useful for the bootstrapping process done by the OpAMP Supervisor.

For this an alternative is very simple to allow no pipelines, but write an warning.

It can be used for load testing a live Collector if the focus is a receiver/processor, since the exporter will not have any overhead.

Interesting.

The original issue may also provide some context: https://github.com/open-telemetry/opentelemetry-collector/issues/7316.

The reason for having nop receiver is wrong, because we made the processor a connector for spanmetricsprocessor

Overall very few reasons in my opinion, but decision was made and we definitely want to continue with this.

Different question: Should be nop in the distributions we produce?