Skip to content

Commit

Permalink
Revert "Adjust the DNS lookup duration metric (#93254)"
Browse files Browse the repository at this point in the history 
This reverts commit 946c524.
  • Loading branch information
@lewing
lewing committed Oct 12, 2023
1 parent 946c524 commit eb17eb1
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 58 deletions.
34 changes: 15 additions & 19 deletions src/libraries/System.Net.NameResolution/src/System/Net/Dns.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ public static string GetHostName()
{
name = NameResolutionPal.GetHostName();
}
catch (Exception ex) when (LogFailure(string.Empty, startingTimestamp, ex))
catch when (LogFailure(string.Empty, startingTimestamp))
{
Debug.Fail("LogFailure should return false");
throw;
}

NameResolutionTelemetry.Log.AfterResolution(string.Empty, startingTimestamp);
NameResolutionTelemetry.Log.AfterResolution(string.Empty, startingTimestamp, successful: true);

if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(null, name);
return name;
Expand Down Expand Up @@ -394,13 +394,13 @@ private static object GetHostEntryOrAddressesCore(string hostName, bool justAddr
Aliases = aliases
};
}
catch (Exception ex) when (LogFailure(hostName, startingTimestamp, ex))
catch when (LogFailure(hostName, startingTimestamp))
{
Debug.Fail("LogFailure should return false");
throw;
}

NameResolutionTelemetry.Log.AfterResolution(hostName, startingTimestamp);
NameResolutionTelemetry.Log.AfterResolution(hostName, startingTimestamp, successful: true);

return result;
}
Expand Down Expand Up @@ -434,13 +434,13 @@ private static object GetHostEntryOrAddressesCore(IPAddress address, bool justAd
}
Debug.Assert(name != null);
}
catch (Exception ex) when (LogFailure(address, startingTimestamp, ex))
catch when (LogFailure(address, startingTimestamp))
{
Debug.Fail("LogFailure should return false");
throw;
}

NameResolutionTelemetry.Log.AfterResolution(address, startingTimestamp);
NameResolutionTelemetry.Log.AfterResolution(address, startingTimestamp, successful: true);

// Do the forward lookup to get the IPs for that host name
startingTimestamp = NameResolutionTelemetry.Log.BeforeResolution(name);
Expand All @@ -464,13 +464,13 @@ private static object GetHostEntryOrAddressesCore(IPAddress address, bool justAd
AddressList = addresses
};
}
catch (Exception ex) when (LogFailure(name, startingTimestamp, ex))
catch when (LogFailure(name, startingTimestamp))
{
Debug.Fail("LogFailure should return false");
throw;
}

NameResolutionTelemetry.Log.AfterResolution(name, startingTimestamp);
NameResolutionTelemetry.Log.AfterResolution(name, startingTimestamp, successful: true);

// One of three things happened:
// 1. Success.
Expand Down Expand Up @@ -577,7 +577,7 @@ private static Task GetHostEntryOrAddressesCoreAsync(string hostName, bool justR
}

private static Task<T>? GetAddrInfoWithTelemetryAsync<T>(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);
Expand All @@ -594,19 +594,15 @@ private static Task GetHostEntryOrAddressesCoreAsync(string hostName, bool justR
static async Task<T> CompleteAsync(Task task, string hostName, long startingTimestamp)
{
_ = NameResolutionTelemetry.Log.BeforeResolution(hostName);
Exception? exception = null;
T? result = null;
try
{
return await ((Task<T>)task).ConfigureAwait(false);
}
catch (Exception ex)
{
exception = ex;
throw;
result = await ((Task<T>)task).ConfigureAwait(false);
return result;
}
finally
{
NameResolutionTelemetry.Log.AfterResolution(hostName, startingTimestamp, exception);
NameResolutionTelemetry.Log.AfterResolution(hostName, startingTimestamp, successful: result is not null);
}
}
}
Expand All @@ -631,9 +627,9 @@ private static void ValidateHostName(string hostName)
}
}

private static bool LogFailure(object hostNameOrAddress, long? startingTimestamp, Exception exception)
private static bool LogFailure(object hostNameOrAddress, long? startingTimestamp)
{
NameResolutionTelemetry.Log.AfterResolution(hostNameOrAddress, startingTimestamp, exception);
NameResolutionTelemetry.Log.AfterResolution(hostNameOrAddress, startingTimestamp, successful: false);
return false;
}

Expand Down
26 changes: 3 additions & 23 deletions src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionMetrics.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,35 +13,15 @@ internal static class NameResolutionMetrics
private static readonly Meter s_meter = new("System.Net.NameResolution");

private static readonly Histogram<double> s_lookupDuration = s_meter.CreateHistogram<double>(
name: "dns.lookup.duration",
name: "dns.lookups.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, Exception? exception)
public static void AfterResolution(TimeSpan duration, string 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);
}
s_lookupDuration.Record(duration.TotalSeconds, KeyValuePair.Create("dns.question.name", (object?)hostName));
}

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!
};
}
}
6 changes: 3 additions & 3 deletions src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionTelemetry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public long BeforeResolution(object hostNameOrAddress)
}

[NonEvent]
public void AfterResolution(object hostNameOrAddress, long? startingTimestamp, Exception? exception = null)
public void AfterResolution(object hostNameOrAddress, long? startingTimestamp, bool successful)
{
Debug.Assert(startingTimestamp.HasValue);
if (startingTimestamp == 0)
Expand All @@ -99,7 +99,7 @@ public void AfterResolution(object hostNameOrAddress, long? startingTimestamp, E

if (IsEnabled(EventLevel.Informational, EventKeywords.None))
{
if (exception is not null)
if (!successful)
{
ResolutionFailed();
}
Expand All @@ -110,7 +110,7 @@ public void AfterResolution(object hostNameOrAddress, long? startingTimestamp, E

if (NameResolutionMetrics.IsEnabled())
{
NameResolutionMetrics.AfterResolution(duration, GetHostnameFromStateObject(hostNameOrAddress), exception);
NameResolutionMetrics.AfterResolution(duration, GetHostnameFromStateObject(hostNameOrAddress));
}
}

Expand Down
17 changes: 4 additions & 13 deletions src/libraries/System.Net.NameResolution/tests/FunctionalTests/MetricsTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace System.Net.NameResolution.Tests
{
public class MetricsTest
{
private const string DnsLookupDuration = "dns.lookup.duration";
private const string DnsLookupDuration = "dns.lookups.duration";

[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public static void ResolveValidHostName_MetricsRecorded()
Expand Down Expand Up @@ -57,26 +57,17 @@ public static async Task ResolveInvalidHostName_MetricsRecorded()
Assert.ThrowsAny<SocketException>(() => Dns.EndGetHostEntry(Dns.BeginGetHostEntry(InvalidHostName, null, null)));
Assert.ThrowsAny<SocketException>(() => Dns.EndGetHostAddresses(Dns.BeginGetHostAddresses(InvalidHostName, null, null)));

double[] measurements = GetMeasurementsForHostname(recorder, InvalidHostName, "host_not_found");
double[] measurements = GetMeasurementsForHostname(recorder, InvalidHostName);

Assert.Equal(6, measurements.Length);
Assert.All(measurements, m => Assert.True(m > double.Epsilon));
}

private static double[] GetMeasurementsForHostname(InstrumentRecorder<double> recorder, string hostname, string? expectedErrorType = null)
private static double[] GetMeasurementsForHostname(InstrumentRecorder<double> recorder, string hostname)
{
return recorder
.GetMeasurements()
.Where(m =>
{
KeyValuePair<string, object?>[] 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;
})
.Where(m => m.Tags.ToArray().Any(t => t.Key == "dns.question.name" && t.Value is string hostnameTag && hostnameTag == hostname))
.Select(m => m.Value)
.ToArray();
}
Expand Down

0 comments on commit eb17eb1

Please sign in to comment.