Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Non-blocking batch span processor? #4264

Open
trask opened this issue Mar 14, 2022 · 4 comments · May be fixed by #4280
Open

Non-blocking batch span processor? #4264

trask opened this issue Mar 14, 2022 · 4 comments · May be fixed by #4280
Labels
Feature Request Suggest an idea for this project

Comments

@trask
Copy link
Member

trask commented Mar 14, 2022

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 trask added the Feature Request Suggest an idea for this project label Mar 14, 2022
@trask trask changed the title How to implement concurrent exporter? How to implement non-blocking exporter? Mar 14, 2022
@anuraaga
Copy link
Contributor

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.

@trask
Copy link
Member Author

trask commented Mar 14, 2022

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

💯

will update title of this issue

@trask trask changed the title How to implement non-blocking exporter? Non-blocking batch span processor? Mar 14, 2022
@trask
Copy link
Member Author

trask commented Mar 14, 2022

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
Copy link
Member Author

trask commented Mar 14, 2022

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 trask linked a pull request Mar 20, 2022 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature Request Suggest an idea for this project
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add option in BatchSpanProcessor to support multiple pending exports trask/opentelemetry-java
2 participants
@anuraaga @trask