opentelemetry-python icon indicating copy to clipboard operation
opentelemetry-python copied to clipboard

Make timeout of `Exporter.export` configurable and decide how to update Exporter interfaces

Open DylanRussell opened this issue 7 months ago • 1 comments

Is your feature request related to a problem?

Related to https://github.com/open-telemetry/opentelemetry-python/issues/4043 and https://github.com/open-telemetry/opentelemetry-python/issues/2284.

I would like to update Exporter.export to accept a timeout parameter. This way the BatchProcessor’s forceflush and shutdown can pass a timeout from the user (or a default) on to export. Right now the timeout passed to forceFlush is not used, and shutdown can hang for a few minutes if the exporters are failing/retrying.

In order to update Exporter.export I need to update the Exporter interfaces. When I looked into doing that I noticed the Exporter interfaces (trace,metrics, logs) are not in-sync with each other and not in line with the spec, it'd be good to fix that.

Describe the solution you'd like

The ideal Exporter interface to me is:

class Span/Log/MetricExporter(abc.ABC):

  @abc.abstractmethod
  def export(self, data: Metric/Span/Log, timeout_millis: Optional[int] = None, **kwargs) -> Metric/Log/TraceExportResult:


  @abc.abstractmethod
  def shutdown(self, timeout_millis: Optional[int] = 30000, **kwargs) -> None:

  @abc.abstractmethod
  def forceFlush(self, timeout_millis: Optional[int] = None, **kwargs) -> bool:

We use **kwargs to indicate that we may add additional params in the future.If **kwargs is added to the method signature and we add a param, the exporter isn’t broken.

I think we should update the existing interfaces to be as close to this as possible without breaking users:

Adding a param to an existing method is not breaking because @abc.abstractmethod does not enforce params, just that the method exists on the Class. It'd cause pylint errors, but IMO that is a good thing because it’ll notify people that they aren’t implementing the interface correctly.

Adding @abc.abstractmethod to an existing method is breaking (the SpanExporter interface doesn’t use abc.ABC at all which means there is nothing enforced on classes which implement it), adding a new method with the @abc.abstractmethod is also breaking (forceFlush method is missing from the LogExporter).

I think we should then also add v2 versions of the Span/LogExporter interface, and recommend people use those – and update any internal exporter’s use those. The MetricExporter is already almost exactly like my proposed ideal interface above..

Describe alternatives you've considered

Adding a new export_with_timeout method to the interfaces. I don't like this as much because it forces people to implement 2 methods, when 1 method with an additional param should be enough. Also MetricExporter.export already has the timeout param which makes this awkward..

Additional Context

No response

Would you like to implement a fix?

None

DylanRussell avatar May 06 '25 20:05 DylanRussell

@DylanRussell fyi https://github.com/open-telemetry/opentelemetry-python/issues/4688#issuecomment-3111858314. I think passing the timeout through would solve this, which I really want to include with Logs before making it stable.

We could hack around it by exiting after a certain number of failed attempts but it's kind of a bandaid.

aabmass avatar Jul 24 '25 03:07 aabmass