From 946c5245aea149d9ece53fd0debc18328c29083b Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 12 Oct 2023 21:34:26 +0200 Subject: [PATCH] Adjust the DNS lookup duration metric (#93254) --- .../src/System/Net/Dns.cs | 34 +++++++++++-------- .../src/System/Net/NameResolutionMetrics.cs | 26 ++++++++++++-- .../src/System/Net/NameResolutionTelemetry.cs | 6 ++-- .../tests/FunctionalTests/MetricsTest.cs | 17 +++++++--- 4 files changed, 58 insertions(+), 25 deletions(-) diff --git a/src/libraries/System.Net.NameResolution/src/System/Net/Dns.cs b/src/libraries/System.Net.NameResolution/src/System/Net/Dns.cs index f4b09f346b3d9..8227d58ebb61b 100644 --- a/src/libraries/System.Net.NameResolution/src/System/Net/Dns.cs +++ b/src/libraries/System.Net.NameResolution/src/System/Net/Dns.cs @@ -23,13 +23,13 @@ public static string GetHostName() { name = NameResolutionPal.GetHostName(); } - catch when (LogFailure(string.Empty, startingTimestamp)) + catch (Exception ex) when (LogFailure(string.Empty, startingTimestamp, ex)) { Debug.Fail("LogFailure should return false"); throw; } - NameResolutionTelemetry.Log.AfterResolution(string.Empty, startingTimestamp, successful: true); + NameResolutionTelemetry.Log.AfterResolution(string.Empty, startingTimestamp); if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(null, name); return name; @@ -394,13 +394,13 @@ private static object GetHostEntryOrAddressesCore(string hostName, bool justAddr Aliases = aliases }; } - catch when (LogFailure(hostName, startingTimestamp)) + catch (Exception ex) when (LogFailure(hostName, startingTimestamp, ex)) { Debug.Fail("LogFailure should return false"); throw; } - NameResolutionTelemetry.Log.AfterResolution(hostName, startingTimestamp, successful: true); + NameResolutionTelemetry.Log.AfterResolution(hostName, startingTimestamp); return result; } @@ -434,13 +434,13 @@ private static object GetHostEntryOrAddressesCore(IPAddress address, bool justAd } Debug.Assert(name != null); } - catch when (LogFailure(address, startingTimestamp)) + catch (Exception ex) when (LogFailure(address, startingTimestamp, ex)) { Debug.Fail("LogFailure should return false"); throw; } - NameResolutionTelemetry.Log.AfterResolution(address, startingTimestamp, successful: true); + NameResolutionTelemetry.Log.AfterResolution(address, startingTimestamp); // Do the forward lookup to get the IPs for that host name startingTimestamp = NameResolutionTelemetry.Log.BeforeResolution(name); @@ -464,13 +464,13 @@ private static object GetHostEntryOrAddressesCore(IPAddress address, bool justAd AddressList = addresses }; } - catch when (LogFailure(name, startingTimestamp)) + catch (Exception ex) when (LogFailure(name, startingTimestamp, ex)) { Debug.Fail("LogFailure should return false"); throw; } - NameResolutionTelemetry.Log.AfterResolution(name, startingTimestamp, successful: true); + NameResolutionTelemetry.Log.AfterResolution(name, startingTimestamp); // One of three things happened: // 1. Success. @@ -577,7 +577,7 @@ private static Task GetHostEntryOrAddressesCoreAsync(string hostName, bool justR } private static Task? GetAddrInfoWithTelemetryAsync(string hostName, bool justAddresses, AddressFamily addressFamily, CancellationToken cancellationToken) - where T : class + where T : class { long startingTimestamp = Stopwatch.GetTimestamp(); Task? task = NameResolutionPal.GetAddrInfoAsync(hostName, justAddresses, addressFamily, cancellationToken); @@ -594,15 +594,19 @@ private static Task GetHostEntryOrAddressesCoreAsync(string hostName, bool justR static async Task CompleteAsync(Task task, string hostName, long startingTimestamp) { _ = NameResolutionTelemetry.Log.BeforeResolution(hostName); - T? result = null; + Exception? exception = null; try { - result = await ((Task)task).ConfigureAwait(false); - return result; + return await ((Task)task).ConfigureAwait(false); + } + catch (Exception ex) + { + exception = ex; + throw; } finally { - NameResolutionTelemetry.Log.AfterResolution(hostName, startingTimestamp, successful: result is not null); + NameResolutionTelemetry.Log.AfterResolution(hostName, startingTimestamp, exception); } } } @@ -627,9 +631,9 @@ private static void ValidateHostName(string hostName) } } - private static bool LogFailure(object hostNameOrAddress, long? startingTimestamp) + private static bool LogFailure(object hostNameOrAddress, long? startingTimestamp, Exception exception) { - NameResolutionTelemetry.Log.AfterResolution(hostNameOrAddress, startingTimestamp, successful: false); + NameResolutionTelemetry.Log.AfterResolution(hostNameOrAddress, startingTimestamp, exception); return false; } diff --git a/src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionMetrics.cs b/src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionMetrics.cs index 180f492b3408e..fe1048e90b22d 100644 --- a/src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionMetrics.cs +++ b/src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionMetrics.cs @@ -13,15 +13,35 @@ internal static class NameResolutionMetrics private static readonly Meter s_meter = new("System.Net.NameResolution"); private static readonly Histogram s_lookupDuration = s_meter.CreateHistogram( - name: "dns.lookups.duration", + name: "dns.lookup.duration", unit: "s", description: "Measures the time taken to perform a DNS lookup."); public static bool IsEnabled() => s_lookupDuration.Enabled; - public static void AfterResolution(TimeSpan duration, string hostName) + public static void AfterResolution(TimeSpan duration, string hostName, Exception? exception) { - s_lookupDuration.Record(duration.TotalSeconds, KeyValuePair.Create("dns.question.name", (object?)hostName)); + var hostNameTag = KeyValuePair.Create("dns.question.name", (object?)hostName); + + if (exception is null) + { + s_lookupDuration.Record(duration.TotalSeconds, hostNameTag); + } + else + { + var errorTypeTag = KeyValuePair.Create("error.type", (object?)GetErrorType(exception)); + s_lookupDuration.Record(duration.TotalSeconds, hostNameTag, errorTypeTag); + } } + + private static string GetErrorType(Exception exception) => (exception as SocketException)?.SocketErrorCode switch + { + SocketError.HostNotFound => "host_not_found", + SocketError.TryAgain => "try_again", + SocketError.AddressFamilyNotSupported => "address_family_not_supported", + SocketError.NoRecovery => "no_recovery", + + _ => exception.GetType().FullName! + }; } } diff --git a/src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionTelemetry.cs b/src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionTelemetry.cs index ef43b59d15a13..73ed325712ac5 100644 --- a/src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionTelemetry.cs +++ b/src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionTelemetry.cs @@ -81,7 +81,7 @@ public long BeforeResolution(object hostNameOrAddress) } [NonEvent] - public void AfterResolution(object hostNameOrAddress, long? startingTimestamp, bool successful) + public void AfterResolution(object hostNameOrAddress, long? startingTimestamp, Exception? exception = null) { Debug.Assert(startingTimestamp.HasValue); if (startingTimestamp == 0) @@ -99,7 +99,7 @@ public void AfterResolution(object hostNameOrAddress, long? startingTimestamp, b if (IsEnabled(EventLevel.Informational, EventKeywords.None)) { - if (!successful) + if (exception is not null) { ResolutionFailed(); } @@ -110,7 +110,7 @@ public void AfterResolution(object hostNameOrAddress, long? startingTimestamp, b if (NameResolutionMetrics.IsEnabled()) { - NameResolutionMetrics.AfterResolution(duration, GetHostnameFromStateObject(hostNameOrAddress)); + NameResolutionMetrics.AfterResolution(duration, GetHostnameFromStateObject(hostNameOrAddress), exception); } } diff --git a/src/libraries/System.Net.NameResolution/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.NameResolution/tests/FunctionalTests/MetricsTest.cs index d3c0990dbf9c4..a19d9edc476ab 100644 --- a/src/libraries/System.Net.NameResolution/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.NameResolution/tests/FunctionalTests/MetricsTest.cs @@ -14,7 +14,7 @@ namespace System.Net.NameResolution.Tests { public class MetricsTest { - private const string DnsLookupDuration = "dns.lookups.duration"; + private const string DnsLookupDuration = "dns.lookup.duration"; [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public static void ResolveValidHostName_MetricsRecorded() @@ -57,17 +57,26 @@ public static async Task ResolveInvalidHostName_MetricsRecorded() Assert.ThrowsAny(() => Dns.EndGetHostEntry(Dns.BeginGetHostEntry(InvalidHostName, null, null))); Assert.ThrowsAny(() => Dns.EndGetHostAddresses(Dns.BeginGetHostAddresses(InvalidHostName, null, null))); - double[] measurements = GetMeasurementsForHostname(recorder, InvalidHostName); + double[] measurements = GetMeasurementsForHostname(recorder, InvalidHostName, "host_not_found"); Assert.Equal(6, measurements.Length); Assert.All(measurements, m => Assert.True(m > double.Epsilon)); } - private static double[] GetMeasurementsForHostname(InstrumentRecorder recorder, string hostname) + private static double[] GetMeasurementsForHostname(InstrumentRecorder recorder, string hostname, string? expectedErrorType = null) { return recorder .GetMeasurements() - .Where(m => m.Tags.ToArray().Any(t => t.Key == "dns.question.name" && t.Value is string hostnameTag && hostnameTag == hostname)) + .Where(m => + { + KeyValuePair[] tags = m.Tags.ToArray(); + if (!tags.Any(t => t.Key == "dns.question.name" && t.Value is string hostnameTag && hostnameTag == hostname)) + { + return false; + } + string? actualErrorType = tags.FirstOrDefault(t => t.Key == "error.type").Value as string; + return expectedErrorType == actualErrorType; + }) .Select(m => m.Value) .ToArray(); }