Opentelemetry-java: Allow SpanProcessor#onStart to modify received Span

[mateuszrzeszutek]

Is your feature request related to a problem? Please describe.
There's an inconsistency with the spec: it clearly states that SpanProcessor#OnStart should receive a read/write instance of Span. Current Java SDK implementation receives only a ReadableSpan which makes implementing any Span enrichment/modification in SpanProcessor#onStart() impossible.

Describe the solution you'd like
A proposed solution is to add a second Span parameter to the onStart function - it will provide the "write" capability. I believe that adding a second parameter that represents a write-only span is in accordance with the spec ("...the Span could be one of the parameters passed to such a function, or a getter could be provided...").

Describe alternatives you've considered
Main motivation for this change is https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/942 . Instead of adding thread info using a SpanProcessor (as was suggested here) I can always just add those attributes directly in the Java agent.

Additional context
Related issues/PRs/comments:

• https://github.com/open-telemetry/opentelemetry-java/pull/1554
• https://github.com/open-telemetry/opentelemetry-specification/pull/669#discussion_r458145581

[jkwatson]

This is relatively recent spec change, and yes, we need to get the implementation updated! Thanks for pointing it out!

[jkwatson]

I think we might want another interface for this which joins the readability of the ReadableSpan and the write-capability of the Span itself.

[carlosalberto]

(Or pass both)

[jkwatson]

I think passing both might be extra confusing for users, especially since we'll be passing the same implementation in for both parameters. :)

[carlosalberto]

Fair enough. Let's keep the idea of passing both as plan B ;)

[jkwatson]

I think it's as easy as this:

public interface ReadWriteSpan extends Span, ReadableSpan {
}

Then just have RecordEventsReadableSpan implement that, and change the onStart take the new interface.

I'll PR it. :)

[jkwatson]

it's that easy except for the Disruptor async exporter which needs a new event type.

[mateuszrzeszutek]

I'll PR it. :)

Great, thanks!
Do you have an estimate when you'll finish it?

[jkwatson]

I'll PR it. :)

Great, thanks!
Do you have an estimate when you'll finish it?

I'm working on it this afternoon. 🤞 that I get the disruptor sorted out by then.