Add more args to RequestListener

[trask]

@HaloFour in slack asked about passing the request to RequestListener.start() in order to amend the context with info from the request, which seems to be a common use case, e.g. HttpServerTracer.customizeContext(Context, REQUEST) -> Context.

And if we're going to pass request to start(), for consistency it seems to make sense to pass request, response, error to end().

Though I'm not sure adding those to end() has a real use case.

An alternative to this PR is a more targeted approach which adds a new interface that just supports the "customize context" use case, something like just a single method Context start(Context, REQUEST).

I'm kind of torn between the two options. I do prefer the more targeted solution, but the kitchen sink option (this PR) may be more future proof.

cc: @anuraaga @mateuszrzeszutek

[anuraaga]

Hmm - I don't think such a mechanism is needed when writing instrumentation, because semantic conventions don't exist (unlike something like span name) I can't think of any functionality or convenience benefit of implementing a listener vs just calling the code directly after creating the context.

Users may want to do it though, even a propagator doesn't seem to work to populate something from the request body into baggage for example. Something like a context customizer would be a more targeted so somewhat easier to implement knob for this specific use case.

There is something to say about our request listeners being the way libraries expose interception to the world, in which case we do need the kitchen sink API. But there seems to be little interest in the OTel community in this direction so if focusing on an API for instrumentation only I guess we don't need it (unless I missed something in my first paragraph of this comment).

[mateuszrzeszutek]

Hmm - I don't think such a mechanism is needed when writing instrumentation, because semantic conventions don't exist (unlike something like span name) I can't think of any functionality or convenience benefit of implementing a listener vs just calling the code directly after creating the context.

💯
Setting a checkpoint in Reactor (which was the use case described in the linked slack thread) could easily be done after start() call - it's not related to telemetry/semantic conventions at all, it doesn't need to be implemented in a RequestListener.

[mateuszrzeszutek]

Oh, there was one issue about adding exception to RequestListener#end() for metrics: #3730
But we came to a conclusion that whatever was needed for metrics should be extracted to Attributes

[HaloFour]

Thanks for opening the PR. The comment was mostly about exploring options and stylistic preferences, especially as in this case the instrumentation is split between two parts. I do have a local branch where I've refactored the REQUEST instances to implement a common interface which allows the Reactor async strategy to extract this information from the Context. I've cleaned it up and created the following draft PR:

#4114

[trask]

closing this, discussed with @anuraaga and @mateuszrzeszutek, I will send a PR to implement "context customizer" knob that has single method start(Context, REQUEST, Attributes) -> Context