-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Privately expose MsQuic performance counters #98422
Conversation
Tagging subscribers to this area: @dotnet/ncl Issue DetailsContributes to #55979. The main problem is that we can only get the MsQuic perf counters in bulk, so any observable/polling metrics/coutners API would require fetching entire array (32 items) for each separate counter and there is no reasonable way to extend the existing API without needing public API changes. I think using the new Metrics API is the best middle-ground solution. We can use one ObservableGauge and one ObservableCounter and report multiple counters by tagging each measurement with perf counter name. While we can't specify individual descriptions and units for each individual counter, I think this is acceptable for internal diagnostics. example when monitoring functional tests run:
|
@@ -929,6 +929,7 @@ internal enum QUIC_PERFORMANCE_COUNTERS | |||
PATH_FAILURE, | |||
SEND_STATELESS_RESET, | |||
SEND_STATELESS_RETRY, | |||
CONN_LOAD_REJECT, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is related to microsoft/msquic#4120, we should wait until that one merges.
We don't need to wait with MsQuic update before merging, the counter will be 0 until a version which supports it is loaded.
src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.NativeMethods.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicApi.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/src/System/Net/Quic/NetEventSource.Quic.Counters.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/tests/FunctionalTests/QuicCountersFixture.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/tests/FunctionalTests/QuicCountersFixture.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/src/System/Net/Quic/NetEventSource.Quic.Counters.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/src/System/Net/Quic/NetEventSource.Quic.Counters.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general LGTM, thanks.
src/libraries/System.Net.Quic/src/System/Net/Quic/NetEventSource.Quic.Counters.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/src/System/Net/Quic/NetEventSource.Quic.Counters.cs
Show resolved
Hide resolved
src/libraries/System.Net.Quic/tests/FunctionalTests/QuicCountersFixture.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/tests/FunctionalTests/QuicCountersFixture.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Quic/tests/FunctionalTests/QuicCountersFixture.cs
Outdated
Show resolved
Hide resolved
Cc @JamesNK in case relevant |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
src/libraries/System.Net.Quic/tests/FunctionalTests/QuicCountersFixture.cs
Outdated
Show resolved
Hide resolved
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
CI failures are unrelated. |
Contributes to #55979.
The main problem is that we can only get the MsQuic perf counters in bulk, so any observable/polling metrics/coutners API would require fetching entire array (32 items) for each separate counter and there is no reasonable way to extend the existing API without needing public API changes.
Since this is going to be internal use only, I went with capping the refresh rate of backgorund counters at once every 50ms, current monitoring tools usually query every X seconds so this should be low enough not to confuse any monitoring tool and high enough to finish querying all counters before the next refresh.
example when monitoring functional tests run:
This PR also adds dump of selected metrics after running QUIC functional tests.