open-telemetry / open-telemetry/opentelemetry-python

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

Open
#4,568 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

feature-request
Dominant language
Python
Stars
2.6k
Forks
1k
Avg merge
4d 15h
Merged PRs (30d)
19

Description

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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by comparing the trace, metrics, and logs Exporter interfaces in the linked SDK files, then read related issues 4043 and 2284 and the referenced specification section. Determine a compatible timeout and interface-versioning approach, including effects on BatchProcessor forceflush and shutdown; the issue is complete when the interface changes and affected internal exporters are defined and covered by tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
observability-sre
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.