Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Proposal: size of http response headers #31003

Closed
zivkan opened this issue Sep 27, 2019 · 7 comments
Closed

API Proposal: size of http response headers #31003

zivkan opened this issue Sep 27, 2019 · 7 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Http
Milestone

Comments

@zivkan
Copy link
Member

zivkan commented Sep 27, 2019

Rationale and Usage

Download managers, or other applications that wish to closely monitor network performance, when using HttpClient may be interested in the number of bytes a response header was, in order to calculate a bytes/second metric from that HTTP server. Right now the best we can do count the bytes of the response's content stream, but responses with no body are treated as zero bytes transferred, despite the fact that it did indeed transfer bytes over the network.

Here's a simplistic example of what I'd like to be able to do:

var requestTime = Stopwatch.StartNew();
var headerTime = Stopwatch.StartNew();
var response = client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead);
headerTime.Stop();

var responseData = await response.Content.ReadAsByteArrayAsync();
requestTime.Stop();

networkTelemetry.OnComplete(request.RequestUri, headerTime.Elapsed, response.HeaderBytesTransferred, responseTime.Elapsed, responseData.Length);

I work on NuGet and due to the design on how it works, we expect a large number of our requests to return 404's. Therefore not being able to take into account header size impacts our ability to better understand our own network usage, or the network connectivity of the customer's machine to the host it's trying to download from.

Proposed API

I'd like the input of experts in the area, as I'm not sure where this would go, assuming the concept is agreeable.

One possibility is to add a Size property to HttpResponseHeaders, however, this makes it sound like there was a header returned with the name "Size". So it doesn't feel right to me.

Another possibility is to add a HeaderSize property to HttpResponseMessage, but it also feels a bit out of place.

A third option is along the lines of #31000, but rather than having position set to zero at the start of the body, the position is zero before headers are read and the position when HttpClient.SendAsync returns determines the size of the headers. The position once the full body has been read is therefore the body size plus the header size. For example:

var response = client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead);
var stream = await response.Content.ReadAsStreamAsync();
var headerSize = stream.Position;
await ProcessStreamToEnd(stream);
var bodySize = stream.Position - headerSize;

Open Questions

Unfortunately I'm not an expert in the HTTP protocol. My understanding is that with HTTP 1.x, response headers are not compressed, but with HTTP 2.0 they will be. Therefore there is a question if the header size should be the number of bytes before of after decompression. Since I am interested in measuring the network connectivity to the HTTP host, I would prefer the compressed size, which would give a more consistent result, regardless of whether the header was highly or poorly compressible for a particular response.

I assume that TLS streams are the same size encrypted and decrypted and therefore they hopefully won't complicate the issue further.

@scalablecory
Copy link
Contributor

I think that for your purpose, the proposal in #30995 would be more flexible by letting you inject your own speed measuring Stream implementation. What do you think?

@zivkan
Copy link
Member Author

zivkan commented Sep 27, 2019

Thanks for such a quick comment.

From my understanding, it would be a trade-off. Indeed, if I create a TCP stream and do HTTP over it, I can count bytes and check the position before and after each HTTP request, at least for HTTP 1.x. It wouldn't work for HTTP 2.0 if concurrent streams are used (if I want to measure per-request size). Additionally, it sounds like I'd need to implement my own DNS lookups, connection pooling, retries/reconnects and possibly other things that HttpClient currently does for us.

It sounds interesting, and losing per-request granularity might be acceptable, but taking on responsibility for DNS, connection pooling and reconnections sounds like I'd lose more than I gain.

@scalablecory
Copy link
Contributor

scalablecory commented Sep 27, 2019

Yea, the other proposal wouldn't work for per-request data. What's the benefit you expect there versus per-host data?

Not really worried about all the other things -- we'd almost definitely provide a default implementation so you could do something like:

Stream s = OpenDefault(hostname, ...);
return MyMonitoringStream(s);

There are other things to think about like compression, encryption, redirects, proxies, and other general overhead of the protocol that is not accounted for in Header Size + Content Size. So getting just the size of the headers would only give you a rough idea of bandwidth used and be far more inaccurate in some cases than in others. I don't know if it's worth implementing something so unpredictably useful.

@zivkan
Copy link
Member Author

zivkan commented Sep 27, 2019

In that case, it should be acceptable.

As previously mentioned, I work on NuGet, and I'm trying to measure the impact of source performance on overall restore performance. Some hosts, like myget, Azure Artifacts and .NET Core's feeds on blob storage, host multiple feeds on a single hostname. So, to monitor network performance per source, I'd need to create a new HttpClient per package source to avoid reusing the same socket, and when the stream factory is called, know which source it's associated with. But I expect that will be possible.

It would be easier if the response somehow had the header bytes information, but I expect this would benefit so few applications that it may not be worth adding support for.

As long as I have some way of associating an object instance with my custom monitoring socket, despite the fact that multiple sockets will be to the same host name, then I'm more than happy to wait for it and re-evaluate once it's available. By then the NuGet team should know more about what we want to monitor and whether header size is still something that would be beneficial to measure.

@karelz
Copy link
Member

karelz commented Jan 16, 2020

Triage: It seems that the value looks questionable. Bandwidth monitoring sounds like a good idea, but it may be via diagnostics EventSource or something else. If there are API requests, we would like to see full end-to-end scenario and its usage described to make sure we design APIs that make sense and are truly valuable for customers.

@karelz karelz closed this as completed Jan 16, 2020
@karelz
Copy link
Member

karelz commented Jan 16, 2020

@zivkan let me know if you disagree or if we missed anything.

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@scalablecory
Copy link
Contributor

This has been resolved via the API added here in .NET 5: #41949

@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Http
Projects
None yet
Development

No branches or pull requests

4 participants