Clarify spans export concurrency

[pellared]

Same as #4173 for trace SDK.

Fixes #4134

[tsloughter]

Necessary changes but had some questions.

[carlosalberto]

Ping @tsloughter

[tsloughter]

Before my PR #2452 I was originally pushing for a change to allow Export() to be called concurrently. I wish I could remember who it was that most convinced me so I could ping them. I believe it was the simplicity that sold me

If we make a change that Export can be called concurrently or make the current BSP legacy or add a new ConcurrentBSP it means exporters written with this guarantee in mind (example: writing to a raw file) will now fail in confusing ways until they are updated to support concurrent calls. I don't think keeping the current processor and having another that calls concurrently is great either since now the user has to ensure they use the right one.

Implementations exist that support concurrency from the BatchProcessor based on the changes we did before:

• Rust: Support concurrent exports opentelemetry-rust#781
• C++: Async/callback based interface between Span/Log Processor and Span/Log Exporter opentelemetry-cpp#1239
• I think Java

I know this PR was only about limiting it to the builtin processors and not changing the builtin processors but I think important to keep this standard for all processors, even if it can't be guaranteed by the API design by everyone to not add this confusion to the user. Originally I thought it was just clarifying it for all processors but had misread :).

[pellared]

@tsloughter, I think that this PR still keeps the statements that discourages from having concurrent-safe exporters. It mostly changes "will" into "should" as e.g. not all languages are able to enforce such constraint. It adds normative requirements to built-in processors as this is a place where all languages have control. Having more normative statements in places where they can be added helps in reviewing/auditing the SDK implementations. As you can see, I do not want to make further changes in this PR which may be controversial. I think the proposed changes are mostly editorial+clarification.

[tsloughter]

@pellared right, that is how I initially read it but I think the should instead of must for replacing will changes the meaning too much. A should on this basically makes it a must support concurrency since you don't actually know. Even if today you know all processors in your language won't call it concurrently that is no guarantee that an upgrade won't change that in a third party processor or a new processor added to the spec that the user could enable instead of Simple or Batch.

Having it be a must you can at least point to the spec if it breaks and say, 'this processor isn't spec compliant', when an exporter doesn't function properly :).

I mainly want to make sure my concerns are clear but not actually hold this up if it has enough support. I think it prevents merging if there are unresolved conversations? If no one shares my concerns, or thinks things need to be done in a new PR after this one is merged, I can resolve them to not block on just my concern.

[pellared]

@carlosalberto, I think we have a consensus and this PR can be merged. More refining PRs are welcome but let's avoid scope creep in this PR.

[carlosalberto]

Will merge by EOD today as, yes, we simply clarify the existing behavior of simple/batch processors.

Btw @jmacd @bogdandrutu as you are interested in concurrent capabilities, maybe we can think of expanding this for future processors. Maybe create an issue to track that? Otherwise, wait for it to happen, etc.

[cijothomas]

Will merge by EOD today as, yes, we simply clarify the existing behavior of simple/batch processors.

Btw @jmacd @bogdandrutu as you are interested in concurrent capabilities, maybe we can think of expanding this for future processors. Maybe create an issue to track that? Otherwise, wait for it to happen, etc.

Opened this for tracking #4231