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

Non-blocking batch span processor?

Open trask opened this issue 2 years ago • 4 comments

Is your feature request related to a problem? Please describe.

I'd like to increase export throughput, and the most obvious way (at least in my case with a remote ingestion service) is to not block waiting for a response from the ingestion service.

However the spec explicitly disallows calling the exporter concurrently:

Export() will never be called concurrently for the same exporter instance. Export() can be called again only after the current call returns.

So exporters need to implement the non-blocking behavior themselves.

But the only way to tell the batch span processor not to block, is to return an immediately completed CompletableResultCode, instead of a real CompletableResultCode.

Which sort of defeats the purpose of the async-friendly CompletableResultCode return type from export.

In any case though, if this is a supported pattern (returning an immediately completed CompletableResultCode from the exporter), it would be nice for the batch span processor to flush the span exporter after if flushes itself, or to document on the batch span processor's flush() method that you may still need to flush the underlying span exporter after calling that method.

trask avatar Mar 14 '22 22:03 trask

Hi @trask - I would say exporters are already non-blocking since they make the export request but don't block waiting for it to complete, there would only be blocking if the caller decides to call join. This is the standard non-blocking pattern in Java.

The question is more about how to make a non-blocking batch span processor I think. FWIW, the spec wording seems to be a bug

Export() will never be called concurrently for the same exporter instance. Export() can be called again only after the current call returns.

This seems to be too strict and I would be surprised if any exporter in the languages actually relies on this because OTLP conversion is known to be stateless. Maybe it could be removed from the spec safely.

Also just for reference, a very early version of the BatchSpanProcessor never blocked but we switched back to the blocking pattern to simplify the code.

anuraaga avatar Mar 14 '22 23:03 anuraaga

The question is more about how to make a non-blocking batch span processor I think

💯

will update title of this issue

trask avatar Mar 14 '22 23:03 trask

FWIW, the spec wording seems to be a bug

Export() will never be called concurrently for the same exporter instance. Export() can be called again only after the current call returns.

This seems to be too strict and I would be surprised if any exporter in the languages actually relies on this because OTLP conversion is known to be stateless. Maybe it could be removed from the spec safely.

ya, I will open a spec issue 👍

trask avatar Mar 14 '22 23:03 trask

re-reading the spec now:

Export() will never be called concurrently for the same exporter instance. Export() can be called again only after the current call returns.

I'm reading it completely different now, that this is just "concurrent" from a thread perspective, and still allows what we want, which is to call Export() again from the same thread after the previous call returns.

trask avatar Mar 14 '22 23:03 trask