diagnostic channel events on body consume / cancel

[artur-ma]

Discussed in https://github.com/nodejs/undici/discussions/1170

^{Originally posted by artur-ma January 19, 2022} Following this https://github.com/DataDog/dd-trace-js/pull/1642#issuecomment-1009298056 Looks like to have full visibility and have good APM instrumentation, the APM agent should know when the request actually finished, including reading the body from the socket.

Do you have any thoughts or ideas on what is the right way to implement it?

AFAIK the body object always be consumed or cancelled according to the docs, so the right direction to look on is the Readable object

Specifically these parts:

https://github.com/nodejs/undici/blob/main/lib/api/readable.js#L41-L56 - Destroy https://github.com/nodejs/undici/blob/main/lib/api/readable.js#L266-L283 - ConsumeFinish

IMO just sending an event from "ConsumeFinish" is not enough since it will need some reference on the request itself to be able to finish the right span which currently is missing on Readable

[mcollina]

@artur-ma I think the best path forward for this is for you to open a PR that publishes those events exactly where you need them. It would be hard for us to guess exactly what you need.

[artur-ma]

@mcollina This is what I planned to do, but there are questions, as I mentioned in the discussion

1. What do you think is the right way to pass the request context to Readable object.
2. Is the Readable object is the only point that should send the event, or there are any other places (for example what do you think should be the solution for streams)

It would be hard for us to guess exactly what you need

I personally do not need this specific development, I just saw a discussion on DataDog Agent repo (we are not using dataDog) so I though helping undici to get better instrumentation on data dog side.

[mcollina]

What do you think is the right way to pass the request context to Readable object.

I would pass it through the constructor https://github.com/nodejs/undici/blob/main/lib/api/readable.js#L20

Is the Readable object is the only point that should send the event, or there are any other places (for example what do you think should be the solution for streams)

I do not have a good answer for that. I think the responsibility for undici finishes once the body is consumed.