Skip to content

Add IHttpSysRequestTimingFeature to support accessing http.sys timing info #48218

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

Merged
merged 9 commits into from
May 22, 2023

Conversation

NGloreous
Copy link
Contributor

@NGloreous NGloreous commented May 12, 2023

Adding IHttpSysRequestTimingFeature to support a well-defined type for accessing http.sys request timing info

Addresses a portion of #35012 by exposing a well-defined API to access http.sys timing info.

Description

Currently http.sys request info is exposed as a ROM via IHttpSysRequestInfoFeature in #14119. Now that the API has stabilized, we want to add well-defined APIs to expose this information. This PR adds APIs to access all timings, individual timings by type, and to get elapsed time between two timings.

The request timing information from http.sys provides high resolution timestamps, obtained from calling QueryPerformanceCounter, of the various steps of request processing. Exposing this information to the consuming service can be helpful in understanding the total time spent to process requests, attribute that time to the various stages of request processing, and diagnose issues when they arise.

Contributes to #35012

@ghost ghost added area-runtime community-contribution Indicates that the PR has been added by a community member labels May 12, 2023
@ghost
Copy link

ghost commented May 12, 2023

Thanks for your PR, @NGloreous. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@amcasey
Copy link
Member

amcasey commented May 16, 2023

Looks like

src/Servers/HttpSys/src/RequestProcessing/Request.cs(4,1): error IDE0005: Using directive is unnecessary.

@Tratcher Tratcher self-assigned this May 17, 2023
Co-authored-by: Chris Ross <Tratcher@Outlook.com>
@Tratcher Tratcher enabled auto-merge (squash) May 17, 2023 19:41
@Tratcher Tratcher added this to the 8.0-preview5 milestone May 17, 2023
@Tratcher Tratcher disabled auto-merge May 17, 2023 19:41
Co-authored-by: Stephen Halter <halter73@gmail.com>
@Tratcher Tratcher enabled auto-merge (squash) May 22, 2023 18:55
@Tratcher Tratcher merged commit e6ca829 into dotnet:main May 22, 2023
@adityamandaleeka adityamandaleeka added the blog-candidate Consider mentioning this in the release blog post label Jun 5, 2023
@ghost
Copy link

ghost commented Jun 5, 2023

@NGloreous, this change will be considered for inclusion in the blog post for the release it'll ship in. Nice work!

Please ensure that the original comment in this thread contains a clear explanation of what the change does, why it's important (what problem does it solve?), and, if relevant, include things like code samples and/or performance numbers.

This content may not be exactly what goes into the blog post, but it will help the team putting together the announcement.

Thanks!

@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions blog-candidate Consider mentioning this in the release blog post community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants