Add System.Net.NameResolution metrics #68
Conversation
|
Hey, thanks for the submission! I'm on holiday from now until the end of May but I'll try and take a look at this over the next couple of weeks. |
| while (!_collector.EventListeners.All(x => x.StartedReceivingEvents)) | ||
|
|
||
| Console.Write("Waiting for event listeners to be active.. "); | ||
| if (!SpinWait.SpinUntil(() => |
There was a problem hiding this comment.
Is there a benefit in switching from a blocking wait to a spin wait?
| lastLookups = e.Mean; | ||
| }; | ||
|
|
||
| DnsLookupDuration = metrics.CreateHistogram("dotnet_dns_lookup_duration_avg_seconds", |
There was a problem hiding this comment.
I have some issues with taking the average and representing it as a histogram:
- The single sample produced from the event parser every
nseconds will see invalid values set for the_countand_sumvalues. - The name includes
_avg_which is confusing for a histogram
I would instead look at creating a gauge to capture the average lookup duration. A histogram would only be viable if we were listening to the individual DNS lookup events and we could track the individual times of each resolution.
| .Where(s => allMetrics.SourceToMetrics[s].Count > 0) | ||
| .ToList(); | ||
|
|
||
| if (nonEmptySources.Count == 0) |
There was a problem hiding this comment.
It was my original intention to not render anything for runtime versions where no metrics could be produced. Do you find this alternative easier to understand?
Adds new metrics on .NET 5+:
dotnet_dns_lookups_total: The number of DNS lookups requested since the process starteddotnet_dns_lookup_duration_avg_seconds: The average time taken for a DNS lookup