open-telemetry / opentelemetry-collector

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

[exporterhelper] Review usage of API and internalize anything not currently used #11142

Open mx-psi opened 2 months ago

mx-psi commented 2 months ago

There are parts of exporterhelper, exporterqueue that are not currently used by any component. We can make these parts internal for now to reduce the amount of API that we stabilize with exporter 1.0. One such example is exporterqueue.Config.

mx-psi commented 1 month ago

As an example here is a Github search that shows that exporterhelper.RequestErrorHandler is not used anywhere: https://github.com/search?q=language%3AGo+-is%3Afork+%2Fexporterhelper%5C.RequestErrorHandler%2F&type=code

To complete this issue we need to:

  1. Search on Github for the symbols to see if they are used or not
  2. If they are not used, open a first PR to deprecate the symbol(s)
  3. After the symbol(s) have been deprecated for one release, remove them
jade-guiton-dd commented 1 month ago

After searching for all uses of public symbols from exporterhelper and exporterqueue outside of themselves on Github (including some cases where the package was renamed on import), it seems that the unused symbols are those introduced by #8122 and marked as "experimental".

This concerns all of exporterqueue, and the following from exporterhelper:

The only experimental symbol which is used is exporterhelper.WithBatcher. Note that it takes a vararg argument of type BatcherOption, but I found no use of it.

Edit: This issue conflicts with #11143, which suggests moving these features to a new package instead of removing them. After discussing it with @dmitryax who implemented most of this, making the symbols internal is probably the easiest option. If it turns out someone uses them, then we can move them and make them public again.