HttpClient and Activity lifetime

[JamesNK]

It appears that HttpClient's HTTP request activity ends when HTTP headers are received.

Discussion about it in OpenTelemetry - open-telemetry/opentelemetry-dotnet#687 (comment)

I don't know whether this is a bug or intended behavior. Wouldn't it be more accurate for the activity to end with the response finishing?

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

[davidsh]

I don't know whether this is a bug or intended behavior. Wouldn't it be more accurate for the activity to end with the response finishing?

In general, it is by design. Many of the HttpClient APIs will return immediately after reading just the headers, i.e. HttpCompletionOption.ResponseHeadersRead.

Also, the design of the HttpClient and the handler pipeline based on HttpMessageHandler is that the pipeline works by getting an HttpRequestMessage and returning an HttpResponseMessage. And handlers returns an HttpResponseMessage without reading the response body.

[JamesNK]

Ok. It seems like the results it produces would be inconsistent with how requests in other network graphs are displayed (e.g. network graph in Chrome), but I don't have a requirement to change it. The gRPC client has its own activity that does run until the response is complete.

[davidsh]

Re-activating as per #37619 (comment).

This issue here is primarily about how DiagnosticHandler works (which is based on DiagnosticSource and not EventSource).

[karelz]

@davidsh are you working on this as part of YARP Telemetry?

[davidsh]

@davidsh are you working on this as part of YARP Telemetry?

I am not currently.

DiagnosticHandler is separate from YARP Telemetry. YARP Telemetry is about adding new EventSource and EventCounters for telemetry. This issue is about fixing (changing the behavior) of the existing DiagnosticSource events so that RequestStop is issued when the request body is fully processed. That will be non-trivial for DiagnosticHandler because as a handler for HttpClient, it only knows about response headers. To fix this issue, DiagnosticHandler will need to wrap HttpResponseMessage.Content objects in some way to get access to the response body and trigger an event when the response body is fully processed. However, an alternate solution is possible... it could piggyback on my work in PR #37619 by adding an internal EventSource listener to the RequestStop event that will be done for telemetry.

[scalablecory]

@JamesNK how critical is this for you? .NET 5? or can we push it?

[JamesNK]

It's not important. Push away.

[JamesNK]

This is what HttpClient telemetry looks like today when captured (highlighted in red):

HttpClient telemetry stops as soon as the headers are received. Meanwhile the server call (bottom record) continues until it finishes the response.