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

Additional Metrics for System.Net.Http #88384

Open
antonfirsov opened this issue Jul 4, 2023 · 4 comments
Open

Additional Metrics for System.Net.Http #88384

antonfirsov opened this issue Jul 4, 2023 · 4 comments
Labels
area-System.Net.Http enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@antonfirsov
Copy link
Member

antonfirsov commented Jul 4, 2023

Opening a separate issue for the HTTP part of #79371, also including requests from #79459. We should eventually implement the Metrics counterpart of all System.Net.Http EventCounters.

Implemented as part of #84978

  • requests-failed, current-requests
  • http(11|20|30)-connections-current-total

Note that Metrics variants of requests-failed, current-requests will be semantically different from EventCounters since we need to comply with the OTel specification which wants us to record the counters until "headers receieved" and exclude response buffering in case HttpCompletionOption.ResponseContentRead is passed. See the discussion in open-telemetry/opentelemetry-specification#3520.

Event counters not discussed in #84978

  • requests-started
  • requests-started-rate
  • requests-failed-rate
  • http(11|20|30)-requests-queue-duration

Additional requests from #79459; we don't have existing counters for these

  • number of idle connections
  • average idle connection time
  • number of connections evicted from pool due to idle timeout
  • number of connection timeouts
  • average time to connect
  • queue length
  • number of http 2 keep alive pings which didn't receive a response
  • average response time of http 2 keep alive pings

Regarding tags, there is a wish in #79459:

Ideally these metrics would be available per FQDN:Port combination, broken down by type (i.e. HTTP1.1/HTTP2) and there would be some way to track which http client each pool belongs to.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 4, 2023
@ghost
Copy link

ghost commented Jul 4, 2023

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Opening a separate issue for the HTTP part of #79371, also including requests from #79459. We should eventually implement the Metrics counterpart of all System.Net.Http EventCounters.

The following counters already have their Metrics counterparts specified and tracked by #84978

  • requests-failed, current-requests
  • http11-connections-current-total, http20-connections-current-total, http30-connections-current-total
  • http11-requests-queue-duration, http20-requests-queue-duration, http30-requests-queue-duration

Event counters not discussed in #84978

  • requests-started
  • requests-started-rate
  • requests-failed-rate

Additional requests from #79459, not listed above

  • number of idle connections
  • average idle connection time
  • number of connections evicted from pool due to idle timeout
  • number of connection timeouts
  • average time to connect
  • queue length
  • number of http 2 keep alive pings which didn't receive a response
  • average response time of http 2 keep alive pings

Regarding tags, there is a wish in #79459:

Ideally these metrics would be available per FQDN:Port combination, broken down by type (i.e. HTTP1.1/HTTP2) and there would be some way to track which http client each pool belongs to.

Author: antonfirsov
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@karelz karelz added enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner labels Jul 4, 2023
@karelz karelz added this to the 8.0.0 milestone Jul 4, 2023
@antonfirsov antonfirsov modified the milestones: 8.0.0, Future Jul 11, 2023
@antonfirsov antonfirsov removed their assignment Jul 12, 2023
@antonfirsov
Copy link
Member Author

@davidfowl all connection instruments from #84978 have been added now. I mistakenly though that http(11|20|30)-requests-queue-duration is also tracked there which is not the case, and it fits the definition of the connection pooling data that exists inside of SocketHttpHandler's guts from #48885 (comment). Do you find this critical for 8.0?

@davidfowl
Copy link
Member

davidfowl commented Jul 16, 2023

Yes, that's very important to add. One of the most important counters when looking at slowness of requests.

@MihaZupan
Copy link
Member

requests-queue-duration has been added in #88981

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net.Http enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

4 participants