Add span processing function from application code

[laurit]

Some users wish to add extra attributes or update the name of http client spans. This can be troublesome because there isn't an obvious place where you could stick Span.current() and add the attributes. This pr introduces an api for adding a span processing function to context and a span processor that applies that function when the span is created.

[anuraaga]

It would be good to keep new public API here in a separate incubator artifact if needing to merge this soon. As there isn't any instrumentation-specific issue here, at a glance it seems that the SDK is a better place for this, though as usual I generally wouldn't stop active avoidance of the spec process if that's the main motivation :)

[trask]

Some users wish to add extra attributes or update the name of http client spans. This can be troublesome because there isn't an obvious place where you could stick Span.current() and add the attributes.

If not using the Javaagent, would registering a SpanProcessor solve this use case?

And if so, what do you think about providing a javaagent-only hook to register/bridge SpanProcessor, instead of introducing new concept?

[anuraaga]

Realized also that attributesextractor also should be able to hook this and maybe is simpler than span processor.

[laurit]

If not using the Javaagent, would registering a SpanProcessor solve this use case?

I believe so. It should be possible to implement a SpanProcessor that reads the actual processing function from context like in this pr, or use some other way to communicate what needs to be done to the SpanProcessor. It should be something that users can implement on their own if they need this.
With java agent doing the same is a lot harder because of the need to bridge.

And if so, what do you think about providing a javaagent-only hook to register/bridge SpanProcessor, instead of introducing new concept?

You mean using SpanProcessor instead of BiConsumer? I did consider it and chose to go with BiConsumer because SpanProcessor.onEnd doesn't have access to Context so I couldn't look up the actual function to call. Also SpanProcessor.onEnd shouldn't change the span anyway.

[mateuszrzeszutek]

Can you move this class to instrumentation-api-semconv? (I guess it's probably the best place for incubating code?) As we want to release stable instrumentation-api soon, we shouldn't add new experimental utils there.

Also, there are other angles that we might want to consider in the future:

• making it possible to load SPI implementations directly from the application code; you could add a SpanProcessor without an extension this way;
• or going through spec with this -- there's an OTEP about Context-scoped attributes (Propose Context-scoped attributes. oteps#207) that looks really similar to this.
  If any of these get implemented at some point we'll want to deprecate and remove the ContextSpanProcessor.

[trask]

oh interesting I hadn't made the connection with the context-scoped attributes OTEP 👍

[trask]

are we comfortable dropping backwards compatibility for this in future Javaagent versions (e.g. if open-telemetry/oteps#207 gets implemented)?

I don't mean without a deprecation cycle, but rather as opposed to ServerSpan which we continue to preserve backwards compatibility for in the Javaagent to avoid breaking users of older instrumentation-api.

[laurit]

@trask I added a note to the javadoc that marks this as experimental. We'll sooner or later wish to introduce experimental APIs, we might as well start with this one and see how it plays out. By clearly stating that this API can be removed we should absolve ourselves of the consequences of removing it. Not that something particularly bad would happen, the way it is built when agent drops support for it there will just be a loss of functionality not exceptions flying around.

[trask]

👍

might be nice to get rename suggested into same release to avoid breaking/dealing with deprecation right away #6310 (comment)

[trask]

let's hold this until after 1.16.0, so we can sort out #6310 first