Add System.Net.NameResolution metrics #88773
Conversation
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsContributes to #48885 The implementation piggybacks off of the existing I added a
The existing (do tell me if I'm committing any metering crimes - I haven't dealt with the new APIs yet) P.S. What's the story with
|
There was a problem hiding this comment.
Looks good, but we need to agree on the naming.
P.S. What's the story with
InstrumentRecorder? It's a bit sad to see every change duplicate it in tests.
It almost made its' way to the BCL but was removed in #87873. We shall create a copy of MetricCollector or a simplified variant in Common/tests as we keep adding more metrics I guess.
| private static readonly Meter s_meter = new("System.Net.NameResolution"); | ||
|
|
||
| private static readonly Counter<long> s_lookupsRequestedCounter = s_meter.CreateCounter<long>( | ||
| name: "dns-lookups-requested", |
There was a problem hiding this comment.
/cc @noahfalk to figure out the naming, since this is establishing a precedent. Should it be dns.lookups.requested?
(The instruments in my pr are to be renamed.)
src/libraries/System.Net.NameResolution/tests/FunctionalTests/MetricsTest.cs
Outdated
Show resolved
Hide resolved
ghost
locked as resolved and limited conversation to collaborators
Contributes to #48885
The implementation piggybacks off of the existing
EventSourcelogic to minimize the maintainability burden.I added a
System.Net.NameResolutionmeter with:Counter<long> "dns-lookups-requested"that just does+1,tags: hostname=foo.The existing
EventCountersalso have adns-lookups-durationcounter -- do we want aMeterequivalent?(do tell me if I'm committing any metering crimes - I haven't dealt with the new APIs yet)
P.S. What's the story with
InstrumentRecorder? It's a bit sad to see every change duplicate it in tests.cc: @davidfowl