Add System.Net.NameResolution metrics (#88773)
authorMiha Zupan <mihazupan.zupan1@gmail.com>
Sat, 15 Jul 2023 18:37:53 +0000 (20:37 +0200)
committerGitHub <noreply@github.com>
Sat, 15 Jul 2023 18:37:53 +0000 (11:37 -0700)
* Add System.Net.NameResolution metrics

* Avoid potential race condition in test

* Fix metrics counter firing twice when called from RunAsync without telemetry

* Add an extra assert

* More RemoteExecutor

src/libraries/System.Net.NameResolution/src/System.Net.NameResolution.csproj
src/libraries/System.Net.NameResolution/src/System/Net/Dns.cs
src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionMetrics.cs [new file with mode: 0644]
src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionTelemetry.cs
src/libraries/System.Net.NameResolution/tests/FunctionalTests/MetricsTest.cs [new file with mode: 0644]
src/libraries/System.Net.NameResolution/tests/FunctionalTests/System.Net.NameResolution.Functional.Tests.csproj
src/libraries/System.Net.NameResolution/tests/FunctionalTests/TelemetryTest.cs

index 254be8e..e064468 100644 (file)
@@ -13,6 +13,7 @@
     <Compile Include="System\Net\Dns.cs" />
     <Compile Include="System\Net\IPHostEntry.cs" />
     <Compile Include="System\Net\NetEventSource.NameResolution.cs" />
+    <Compile Include="System\Net\NameResolutionMetrics.cs" />
     <Compile Include="System\Net\NameResolutionTelemetry.cs" />
     <!-- Logging -->
     <Compile Include="$(CommonPath)System\Net\Logging\NetEventSource.Common.cs"
   <ItemGroup>
     <Reference Include="Microsoft.Win32.Primitives" />
     <Reference Include="System.Collections" />
+    <Reference Include="System.Diagnostics.DiagnosticSource" />
     <Reference Include="System.Diagnostics.Tracing" />
     <Reference Include="System.Memory" />
     <Reference Include="System.Net.Primitives" />
index 0ee39a3..46c1eff 100644 (file)
@@ -156,8 +156,8 @@ namespace System.Net
                 throw new ArgumentException(SR.net_invalid_ip_addr, nameof(address));
             }
 
-            return RunAsync(static (s, stopwatch) => {
-                IPHostEntry ipHostEntry = GetHostEntryCore((IPAddress)s, AddressFamily.Unspecified, stopwatch);
+            return RunAsync(static (s, startingTimestamp) => {
+                IPHostEntry ipHostEntry = GetHostEntryCore((IPAddress)s, AddressFamily.Unspecified, startingTimestamp);
                 if (NetEventSource.Log.IsEnabled()) NetEventSource.Info((IPAddress)s, $"{ipHostEntry} with {ipHostEntry.AddressList.Length} entries");
                 return ipHostEntry;
             }, address, CancellationToken.None);
@@ -362,20 +362,18 @@ namespace System.Net
             return ipHostEntry;
         }
 
-        private static IPHostEntry GetHostEntryCore(string hostName, AddressFamily addressFamily, long startingTimestamp = 0) =>
+        private static IPHostEntry GetHostEntryCore(string hostName, AddressFamily addressFamily, long? startingTimestamp = null) =>
             (IPHostEntry)GetHostEntryOrAddressesCore(hostName, justAddresses: false, addressFamily, startingTimestamp);
 
-        private static IPAddress[] GetHostAddressesCore(string hostName, AddressFamily addressFamily, long startingTimestamp = 0) =>
+        private static IPAddress[] GetHostAddressesCore(string hostName, AddressFamily addressFamily, long? startingTimestamp = null) =>
             (IPAddress[])GetHostEntryOrAddressesCore(hostName, justAddresses: true, addressFamily, startingTimestamp);
 
-        private static object GetHostEntryOrAddressesCore(string hostName, bool justAddresses, AddressFamily addressFamily, long startingTimestamp = 0)
+        private static object GetHostEntryOrAddressesCore(string hostName, bool justAddresses, AddressFamily addressFamily, long? startingTimestamp = null)
         {
             ValidateHostName(hostName);
 
-            if (startingTimestamp == 0)
-            {
-                startingTimestamp = NameResolutionTelemetry.Log.BeforeResolution(hostName);
-            }
+            // startingTimestamp may have already been set if we're being called from RunAsync.
+            startingTimestamp ??= NameResolutionTelemetry.Log.BeforeResolution(hostName);
 
             object result;
             try
@@ -408,24 +406,22 @@ namespace System.Net
             return result;
         }
 
-        private static IPHostEntry GetHostEntryCore(IPAddress address, AddressFamily addressFamily, long startingTimestamp = 0) =>
+        private static IPHostEntry GetHostEntryCore(IPAddress address, AddressFamily addressFamily, long? startingTimestamp = null) =>
             (IPHostEntry)GetHostEntryOrAddressesCore(address, justAddresses: false, addressFamily, startingTimestamp);
 
-        private static IPAddress[] GetHostAddressesCore(IPAddress address, AddressFamily addressFamily, long startingTimestamp) =>
+        private static IPAddress[] GetHostAddressesCore(IPAddress address, AddressFamily addressFamily, long? startingTimestamp = null) =>
             (IPAddress[])GetHostEntryOrAddressesCore(address, justAddresses: true, addressFamily, startingTimestamp);
 
         // Does internal IPAddress reverse and then forward lookups (for Legacy and current public methods).
-        private static object GetHostEntryOrAddressesCore(IPAddress address, bool justAddresses, AddressFamily addressFamily, long startingTimestamp)
+        private static object GetHostEntryOrAddressesCore(IPAddress address, bool justAddresses, AddressFamily addressFamily, long? startingTimestamp = null)
         {
             // Try to get the data for the host from its address.
             // We need to call getnameinfo first, because getaddrinfo w/ the ipaddress string
             // will only return that address and not the full list.
 
             // Do a reverse lookup to get the host name.
-            if (startingTimestamp == 0)
-            {
-                startingTimestamp = NameResolutionTelemetry.Log.BeforeResolution(address);
-            }
+            // startingTimestamp may have already been set if we're being called from RunAsync.
+            startingTimestamp ??= NameResolutionTelemetry.Log.BeforeResolution(address);
 
             SocketError errorCode;
             string? name;
@@ -535,12 +531,11 @@ namespace System.Net
                     ValidateHostName(hostName);
 
                     Task? t;
-                    if (NameResolutionTelemetry.Log.IsEnabled())
+                    if (NameResolutionTelemetry.Log.IsEnabled() || NameResolutionMetrics.IsEnabled())
                     {
                         t = justAddresses
                             ? GetAddrInfoWithTelemetryAsync<IPAddress[]>(hostName, justAddresses, family, cancellationToken)
                             : GetAddrInfoWithTelemetryAsync<IPHostEntry>(hostName, justAddresses, family, cancellationToken);
-
                     }
                     else
                     {
@@ -599,7 +594,7 @@ namespace System.Net
 
             static async Task<T> CompleteAsync(Task task, string hostName, long startingTimestamp)
             {
-                _  = NameResolutionTelemetry.Log.BeforeResolution(hostName);
+                _ = NameResolutionTelemetry.Log.BeforeResolution(hostName);
                 T? result = null;
                 try
                 {
@@ -633,7 +628,7 @@ namespace System.Net
             }
         }
 
-        private static bool LogFailure(long startingTimestamp)
+        private static bool LogFailure(long? startingTimestamp)
         {
             NameResolutionTelemetry.Log.AfterResolution(startingTimestamp, successful: false);
             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
new file mode 100644 (file)
index 0000000..df1f5a2
--- /dev/null
@@ -0,0 +1,53 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Diagnostics.Metrics;
+using System.Net.Sockets;
+
+namespace System.Net
+{
+    internal static class NameResolutionMetrics
+    {
+        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",
+            description: "Number of DNS lookups requested.");
+
+        public static bool IsEnabled() => s_lookupsRequestedCounter.Enabled;
+
+        public static void BeforeResolution(object hostNameOrAddress, out string? host)
+        {
+            if (s_lookupsRequestedCounter.Enabled)
+            {
+                host = GetHostnameFromStateObject(hostNameOrAddress);
+
+                s_lookupsRequestedCounter.Add(1, KeyValuePair.Create("hostname", (object?)host));
+            }
+            else
+            {
+                host = null;
+            }
+        }
+
+        public static string GetHostnameFromStateObject(object hostNameOrAddress)
+        {
+            Debug.Assert(hostNameOrAddress is not null);
+
+            string host = hostNameOrAddress switch
+            {
+                string h => h,
+                KeyValuePair<string, AddressFamily> t => t.Key,
+                IPAddress a => a.ToString(),
+                KeyValuePair<IPAddress, AddressFamily> t => t.Key.ToString(),
+                _ => null!
+            };
+
+            Debug.Assert(host is not null, $"Unknown hostNameOrAddress type: {hostNameOrAddress.GetType().Name}");
+
+            return host;
+        }
+    }
+}
index eef4b0a..893659a 100644 (file)
@@ -1,10 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Collections.Generic;
 using System.Diagnostics;
 using System.Diagnostics.Tracing;
-using System.Net.Sockets;
 using System.Threading;
 
 namespace System.Net
@@ -62,14 +60,10 @@ namespace System.Net
         [NonEvent]
         public long BeforeResolution(object hostNameOrAddress)
         {
-            Debug.Assert(hostNameOrAddress != null);
-            Debug.Assert(
-                hostNameOrAddress is string ||
-                hostNameOrAddress is IPAddress ||
-                hostNameOrAddress is KeyValuePair<string, AddressFamily> ||
-                hostNameOrAddress is KeyValuePair<IPAddress, AddressFamily>,
-                $"Unknown hostNameOrAddress type: {hostNameOrAddress.GetType().Name}");
+            // System.Diagnostics.Metrics part
+            NameResolutionMetrics.BeforeResolution(hostNameOrAddress, out string? host);
 
+            // System.Diagnostics.Tracing part
             if (IsEnabled())
             {
                 Interlocked.Increment(ref _lookupsRequested);
@@ -77,14 +71,8 @@ namespace System.Net
 
                 if (IsEnabled(EventLevel.Informational, EventKeywords.None))
                 {
-                    string host = hostNameOrAddress switch
-                    {
-                        string h => h,
-                        KeyValuePair<string, AddressFamily> t => t.Key,
-                        IPAddress a => a.ToString(),
-                        KeyValuePair<IPAddress, AddressFamily> t => t.Key.ToString(),
-                        _ => null!
-                    };
+                    host ??= NameResolutionMetrics.GetHostnameFromStateObject(hostNameOrAddress);
+
                     ResolutionStart(host);
                 }
 
@@ -95,13 +83,15 @@ namespace System.Net
         }
 
         [NonEvent]
-        public void AfterResolution(long startingTimestamp, bool successful)
+        public void AfterResolution(long? startingTimestamp, bool successful)
         {
+            Debug.Assert(startingTimestamp.HasValue);
+
             if (startingTimestamp != 0)
             {
                 Interlocked.Decrement(ref _currentLookups);
 
-                _lookupsDuration?.WriteMetric(Stopwatch.GetElapsedTime(startingTimestamp).TotalMilliseconds);
+                _lookupsDuration?.WriteMetric(Stopwatch.GetElapsedTime(startingTimestamp.Value).TotalMilliseconds);
 
                 if (IsEnabled(EventLevel.Informational, EventKeywords.None))
                 {
diff --git a/src/libraries/System.Net.NameResolution/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.NameResolution/tests/FunctionalTests/MetricsTest.cs
new file mode 100644 (file)
index 0000000..262626c
--- /dev/null
@@ -0,0 +1,98 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Concurrent;
+using System.Collections.Generic;
+using System.Diagnostics.Metrics;
+using System.Linq;
+using System.Net.Sockets;
+using System.Threading.Tasks;
+using Microsoft.DotNet.RemoteExecutor;
+using Xunit;
+
+namespace System.Net.NameResolution.Tests
+{
+    public class MetricsTest
+    {
+        private const string DnsLookupsRequested = "dns-lookups-requested";
+
+        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        public static void ResolveValidHostName_MetricsRecorded()
+        {
+            RemoteExecutor.Invoke(async () =>
+            {
+                const string ValidHostName = "localhost";
+
+                using var recorder = new InstrumentRecorder<long>(DnsLookupsRequested);
+
+                await Dns.GetHostEntryAsync(ValidHostName);
+                await Dns.GetHostAddressesAsync(ValidHostName);
+
+                Dns.GetHostEntry(ValidHostName);
+                Dns.GetHostAddresses(ValidHostName);
+
+                Dns.EndGetHostEntry(Dns.BeginGetHostEntry(ValidHostName, null, null));
+                Dns.EndGetHostAddresses(Dns.BeginGetHostAddresses(ValidHostName, null, null));
+
+                long[] measurements = GetMeasurementsForHostname(recorder, ValidHostName);
+
+                Assert.Equal(6, measurements.Length);
+                Assert.All(measurements, m => Assert.Equal(1, m));
+            }).Dispose();
+        }
+
+        [Fact]
+        public static async Task ResolveInvalidHostName_MetricsRecorded()
+        {
+            const string InvalidHostName = $"invalid...example.com...{nameof(ResolveInvalidHostName_MetricsRecorded)}";
+
+            using var recorder = new InstrumentRecorder<long>(DnsLookupsRequested);
+
+            await Assert.ThrowsAnyAsync<SocketException>(async () => await Dns.GetHostEntryAsync(InvalidHostName));
+            await Assert.ThrowsAnyAsync<SocketException>(async () => await Dns.GetHostAddressesAsync(InvalidHostName));
+
+            Assert.ThrowsAny<SocketException>(() => Dns.GetHostEntry(InvalidHostName));
+            Assert.ThrowsAny<SocketException>(() => Dns.GetHostAddresses(InvalidHostName));
+
+            Assert.ThrowsAny<SocketException>(() => Dns.EndGetHostEntry(Dns.BeginGetHostEntry(InvalidHostName, null, null)));
+            Assert.ThrowsAny<SocketException>(() => Dns.EndGetHostAddresses(Dns.BeginGetHostAddresses(InvalidHostName, null, null)));
+
+            long[] measurements = GetMeasurementsForHostname(recorder, InvalidHostName);
+
+            Assert.Equal(6, measurements.Length);
+            Assert.All(measurements, m => Assert.Equal(1, m));
+        }
+
+        private static long[] GetMeasurementsForHostname(InstrumentRecorder<long> recorder, string hostname)
+        {
+            return recorder
+                .GetMeasurements()
+                .Where(m => m.Tags.ToArray().Any(t => t.Key == "hostname" && t.Value is string hostnameTag && hostnameTag == hostname))
+                .Select(m => m.Value)
+                .ToArray();
+        }
+
+        private sealed class InstrumentRecorder<T> : IDisposable where T : struct
+        {
+            private readonly MeterListener _meterListener = new();
+            private readonly ConcurrentQueue<Measurement<T>> _values = new();
+
+            public InstrumentRecorder(string instrumentName)
+            {
+                _meterListener.InstrumentPublished = (instrument, listener) =>
+                {
+                    if (instrument.Meter.Name == "System.Net.NameResolution" && instrument.Name == instrumentName)
+                    {
+                        listener.EnableMeasurementEvents(instrument);
+                    }
+                };
+                _meterListener.SetMeasurementEventCallback<T>(OnMeasurementRecorded);
+                _meterListener.Start();
+            }
+
+            private void OnMeasurementRecorded(Instrument instrument, T measurement, ReadOnlySpan<KeyValuePair<string, object?>> tags, object? state) => _values.Enqueue(new Measurement<T>(measurement, tags));
+            public IReadOnlyList<Measurement<T>> GetMeasurements() => _values.ToArray();
+            public void Dispose() => _meterListener.Dispose();
+        }
+    }
+}
index b70d263..4028681 100644 (file)
@@ -10,6 +10,7 @@
     <Compile Include="GetHostByAddressTest.cs" />
     <Compile Include="GetHostByNameTest.cs" />
     <Compile Include="ResolveTest.cs" />
+    <Compile Include="MetricsTest.cs" />
     <Compile Include="GetHostNameTest.cs" />
     <Compile Include="GetHostEntryTest.cs" />
     <Compile Include="GetHostAddressesTest.cs" />
index e42b3d9..c113591 100644 (file)
@@ -8,9 +8,7 @@ using System.Linq;
 using System.Net.Sockets;
 using System.Threading.Tasks;
 using Microsoft.DotNet.RemoteExecutor;
-using Microsoft.DotNet.XUnitExtensions;
 using Xunit;
-using Xunit.Sdk;
 
 namespace System.Net.NameResolution.Tests
 {