OTLP exporter should support concurrent sending

[codeboten]

As per spec: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/protocol/otlp.md#concurrent-requests

[dmarar]

@codeboten , pls assign this to me.

[codeboten]

@dmarar done, thanks!

[dmarar]

Hello,
Before I implement this, thought of clarifying some points from spec
Following from the spec:
https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/protocol/otlp.md#concurrent-requests

The maximum achievable throughput is max_concurrent_requests * max_request_size / (network_latency +
server_response_time). For example if the request can contain at most 100 spans, network roundtrip latency is 200ms and server response time is 300 ms, then the maximum achievable throughput with one concurrent request is 100 spans / (200ms+300ms) or 200 spans per second. It is easy to see that in high latency networks or when the server response time is high to achieve good throughput the requests need to be very big or a lot concurrent requests must be done.

I had some questions regarding the spec before i start the implementation:

• number of spans in a single request? Do we have any number that we are targeting?
• Say for eg out of 5 concurrent request, one did not get acknowledgment, the export function should "FAILURE" ?
• if client configured to "NOT WAIT FOR ACK" during shutdown export function should return "SUCCESS" ?

Please let me know if I need to put this question in channel for the broader audience?

[aabmass]

Related: open-telemetry/opentelemetry-dotnet#1777 (thanks Leighton)

number of spans in a single request? Do we have any number that we are targeting?

That's a good question, and we should probably discuss with a wider audience. My thoughts: the BatchSpanExporter's default maxExportBatchSize is 512 (here) which is what most people will probably use. For all the defaults to work together nicely, max_concurrent_requests * num_spans_per_request = 512. So maybe default to max_concurrent_requests = 8 and num_spans_per_request = 64 or similar.

Say for eg out of 5 concurrent request, one did not get acknowledgment, the export function should "FAILURE" ?

Going off the spec for export(), I believe so: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#exportbatch

if client configured to "NOT WAIT FOR ACK" during shutdown export function should return "SUCCESS" ?

That makes sense to me

[lzchen]

@dmarar
Hey are you planning on taking this on? This is a blocker for our rc1 release.

[lzchen]

@aabmass
Aaron to open up a spec issue about clarifying behavior. Perhaps we don't need this by rc1 due to it being an additive change.

[aabmass]

The spec issue open-telemetry/opentelemetry-specification#1405 was tagged as after-ga

[dmarar]

@lzchen , My apologies, had a personal emergency and had taken a break for the last one month and hadnt checked my mails as well.
I can continue to work on it unless the team wants me to take something else.
Thanks!

[lzchen]

@dmarar
No worries. I believe you can continue to work on this unless @aabmass has made some progress?

[dmarar]

@lzchen , since we have a spec issue for this, i was thinking whether I should wait a till we get a clarity on the spec issue. Once we have enough info, i will revisit this issue. Till that time I could pick up any other issue. Any thoughts?

[lzchen]

@dmarar
I agree. We can wait until there's more clarity in the specs until implementing this. For now, feel free to pick up some other issues :)

[github-actions]

This issue was marked stale due to lack of activity. It will be closed in 30 days.