Address review feedback

Author: bart-vmware

src/Libraries/Microsoft.Extensions.ServiceDiscovery.Dns/DnsSrvServiceEndpointProviderFactory.cs

@@ -32,20 +32,26 @@ public bool TryCreateProvider(ServiceEndpointQuery query, [NotNullWhen(true)] ou
         // Otherwise, the namespace can be read from /var/run/secrets/kubernetes.io/serviceaccount/namespace and combined with an assumed suffix of "svc.cluster.local".
         // The protocol is assumed to be "tcp".
         // The portName is the name of the port in the service definition. If the serviceName parses as a URI, we use the scheme as the port name, otherwise "default".
-        if (optionsValue.GetQueryText == null && string.IsNullOrWhiteSpace(_querySuffix))
+        if (optionsValue.ServiceDomainNameCallback == null && string.IsNullOrWhiteSpace(_querySuffix))
         {
             DnsServiceEndpointProviderBase.Log.NoDnsSuffixFound(logger, query.ToString()!);
             provider = default;
             return false;
         }
 
-        var srvQuery = optionsValue.GetQueryText != null
-            ? optionsValue.GetQueryText(query)
-            : optionsValue.GetDefaultQueryText(query);
+        var srvQuery = optionsValue.ServiceDomainNameCallback != null
+            ? optionsValue.ServiceDomainNameCallback(query)
+            : DefaultServiceDomainNameCallback(query, optionsValue);
         provider = new DnsSrvServiceEndpointProvider(query, srvQuery, hostName: query.ServiceName, options, logger, resolver, timeProvider);
         return true;
     }
 
+    private static string DefaultServiceDomainNameCallback(ServiceEndpointQuery query, DnsSrvServiceEndpointProviderOptions options)
+    {
+        var portName = query.EndpointName ?? "default";
+        return $"_{portName}._tcp.{query.ServiceName}.{options.QuerySuffix}";
+    }
+
     private static string? GetKubernetesHostDomain()
     {
         // Check that we are running in Kubernetes first.

src/Libraries/Microsoft.Extensions.ServiceDiscovery.Dns/DnsSrvServiceEndpointProviderOptions.cs

@@ -31,9 +31,9 @@ public class DnsSrvServiceEndpointProviderOptions
     public double RetryBackOffFactor { get; set; } = 2;
 
     /// <summary>
-    /// Gets or sets the options used to configure the resolver's behavior.
+    /// Gets or sets the options used to configure the underlying DNS resolution behavior.
     /// </summary>
-    public ResolverOptions ResolverOptions { get; set; } = new();
+    public DnsResolverOptions DnsResolverOptions { get; set; } = new();
 
     /// <summary>
     /// Gets or sets the default DNS query suffix for services resolved via this provider.
@@ -46,16 +46,10 @@ public class DnsSrvServiceEndpointProviderOptions
     /// <summary>
     /// Gets or sets a delegate that generates a DNS SRV query from a specified <see cref="ServiceEndpointQuery"/> instance.
     /// </summary>
-    public Func<ServiceEndpointQuery, string>? GetQueryText { get; set; }
+    public Func<ServiceEndpointQuery, string>? ServiceDomainNameCallback { get; set; }
 
     /// <summary>
     /// Gets or sets a delegate used to determine whether to apply host name metadata to each resolved endpoint. Defaults to <c>false</c>.
     /// </summary>
     public Func<ServiceEndpoint, bool> ShouldApplyHostNameMetadata { get; set; } = _ => false;
-
-    internal string GetDefaultQueryText(ServiceEndpointQuery query)
-    {
-        var portName = query.EndpointName ?? "default";
-        return $"_{portName}._tcp.{query.ServiceName}.{QuerySuffix}";
-    }
 }

src/Libraries/Microsoft.Extensions.ServiceDiscovery.Dns/Resolver/DnsResolver.cs

@@ -25,12 +25,12 @@ internal sealed partial class DnsResolver : IDnsResolver, IDisposable
     private static readonly TimeSpan s_maxTimeout = TimeSpan.FromMilliseconds(int.MaxValue);
 
     private bool _disposed;
-    private readonly ResolverOptions _options;
+    private readonly DnsResolverOptions _options;
     private readonly CancellationTokenSource _pendingRequestsCts = new();
     private readonly TimeProvider _timeProvider;
     private readonly ILogger<DnsResolver> _logger;
 
-    public DnsResolver(TimeProvider timeProvider, ILogger<DnsResolver> logger, ResolverOptions options)
+    public DnsResolver(TimeProvider timeProvider, ILogger<DnsResolver> logger, DnsResolverOptions options)
     {
         _timeProvider = timeProvider;
         _logger = logger;
@@ -58,23 +58,23 @@ public DnsResolver(TimeProvider timeProvider, ILogger<DnsResolver> logger, Resol
         }
     }
 
-    internal DnsResolver(ResolverOptions options) : this(TimeProvider.System, NullLogger<DnsResolver>.Instance, options)
+    internal DnsResolver(DnsResolverOptions options) : this(TimeProvider.System, NullLogger<DnsResolver>.Instance, options)
     {
     }
 
-    internal DnsResolver(IEnumerable<IPEndPoint> servers) : this(new ResolverOptions { Servers = servers.ToArray() })
+    internal DnsResolver(IEnumerable<IPEndPoint> servers) : this(new DnsResolverOptions { Servers = servers.ToArray() })
     {
     }
 
-    internal DnsResolver(IPEndPoint server) : this(new ResolverOptions { Servers = [server] })
+    internal DnsResolver(IPEndPoint server) : this(new DnsResolverOptions { Servers = [server] })
     {
     }
 
     internal static DnsResolver Create(IServiceProvider serviceProvider)
     {
         ArgumentNullException.ThrowIfNull(serviceProvider);
 
-        var resolverOptions = serviceProvider.GetRequiredService<IOptions<DnsSrvServiceEndpointProviderOptions>>().Value.ResolverOptions;
+        var resolverOptions = serviceProvider.GetRequiredService<IOptions<DnsSrvServiceEndpointProviderOptions>>().Value.DnsResolverOptions;
         var timeProvider = serviceProvider.GetRequiredService<TimeProvider>();
         var logger = serviceProvider.GetRequiredService<ILoggerFactory>().CreateLogger<DnsResolver>();
         return new DnsResolver(timeProvider, logger, resolverOptions);
@@ -387,7 +387,7 @@ internal struct SendQueryResult
         {
             IPEndPoint serverEndPoint = _options.Servers[index];
 
-            for (int attempt = 1; attempt <= _options.Attempts; attempt++)
+            for (int attempt = 1; attempt <= _options.MaxAttempts; attempt++)
             {
                 DnsResponse response = default;
                 try

src/Libraries/Microsoft.Extensions.ServiceDiscovery.Dns/Resolver/ResolverOptions.cs renamed to src/Libraries/Microsoft.Extensions.ServiceDiscovery.Dns/Resolver/DnsResolverOptions.cs

@@ -8,21 +8,24 @@ namespace Microsoft.Extensions.ServiceDiscovery.Dns.Resolver;
 /// <summary>
 /// Provides configuration options for DNS resolution, including server endpoints, retry attempts, and timeout settings.
 /// </summary>
-public class ResolverOptions
+public class DnsResolverOptions
 {
     /// <summary>
     /// Gets or sets the collection of server endpoints used for network connections.
     /// </summary>
     public IList<IPEndPoint> Servers { get; set; } = new List<IPEndPoint>();
 
     /// <summary>
-    /// Gets or sets the number of allowed attempts for the operation.
+    /// Gets or sets the maximum number of attempts per server.
     /// </summary>
-    public int Attempts { get; set; } = 2;
+    public int MaxAttempts { get; set; } = 2;
 
     /// <summary>
-    /// Gets or sets the maximum duration to wait for an operation to complete before timing out.
+    /// Gets or sets the maximum duration per attempt to wait before timing out.
     /// </summary>
+    /// <remarks>
+    /// The maximum time for resolving a query is <see cref="MaxAttempts"/> * <see cref="Servers"/> count * <see cref="Timeout"/>.
+    /// </remarks>
     public TimeSpan Timeout { get; set; } = TimeSpan.FromSeconds(3);
 
     // override for testing purposes

test/Libraries/Microsoft.Extensions.ServiceDiscovery.Dns.Tests.Fuzzing/Fuzzers/DnsResponseFuzzer.cs

@@ -19,11 +19,11 @@ public void FuzzTarget(ReadOnlySpan<byte> data)
         if (_resolver == null)
         {
             _buffer = new byte[4096];
-            _resolver = new DnsResolver(new ResolverOptions
+            _resolver = new DnsResolver(new DnsResolverOptions
             {
                 Servers = [new IPEndPoint(IPAddress.Loopback, 53)],
                 Timeout = TimeSpan.FromSeconds(5),
-                Attempts = 1,
+                MaxAttempts = 1,
                 _transportOverride = (buffer, length) =>
                 {
                     // the first two bytes are the random transaction ID, so we keep that

test/Libraries/Microsoft.Extensions.ServiceDiscovery.Dns.Tests/Resolver/LoopbackDnsTestBase.cs

@@ -18,7 +18,7 @@ public abstract class LoopbackDnsTestBase : IDisposable
     internal LoopbackDnsServer DnsServer { get; }
     private readonly Lazy<DnsResolver> _resolverLazy;
     internal DnsResolver Resolver => _resolverLazy.Value;
-    internal ResolverOptions Options { get; }
+    internal DnsResolverOptions Options { get; }
     protected readonly FakeTimeProvider TimeProvider;
 
     public LoopbackDnsTestBase(ITestOutputHelper output)
@@ -30,7 +30,7 @@ public LoopbackDnsTestBase(ITestOutputHelper output)
         {
             Servers = [DnsServer.DnsEndPoint],
             Timeout = TimeSpan.FromSeconds(5),
-            Attempts = 1,
+            MaxAttempts = 1,
         };
         _resolverLazy = new(InitializeResolver);
     }
@@ -40,7 +40,7 @@ DnsResolver InitializeResolver()
         ServiceCollection services = new();
         services.AddXunitLogging(Output);
 
-        // construct DnsResolver manually via internal constructor which accepts ResolverOptions
+        // construct DnsResolver manually via internal constructor which accepts DnsResolverOptions
         var resolver = new DnsResolver(TimeProvider, services.BuildServiceProvider().GetRequiredService<ILogger<DnsResolver>>(), Options);
         return resolver;
     }

test/Libraries/Microsoft.Extensions.ServiceDiscovery.Dns.Tests/Resolver/RetryTests.cs

@@ -11,7 +11,7 @@ public class RetryTests : LoopbackDnsTestBase
 {
     public RetryTests(ITestOutputHelper output) : base(output)
     {
-        Options.Attempts = 3;
+        Options.MaxAttempts = 3;
     }
 
     private Task SetupUdpProcessFunction(LoopbackDnsServer server, Func<LoopbackDnsResponseBuilder, Task> func)
@@ -49,7 +49,7 @@ public async Task Retry_Simple_Success()
         Task t = SetupUdpProcessFunction(builder =>
         {
             attempt++;
-            if (attempt == Options.Attempts)
+            if (attempt == Options.MaxAttempts)
             {
                 builder.Answers.AddAddress(hostName, 3600, address);
             }
@@ -214,7 +214,7 @@ public async Task ExhaustedRetries_FailoverToNextServer()
                 return Task.CompletedTask;
             });
 
-        Assert.Equal(Options.Attempts, primaryAttempt);
+        Assert.Equal(Options.MaxAttempts, primaryAttempt);
         Assert.Equal(1, secondaryAttempt);
 
         AddressResult res = Assert.Single(results);

test/Libraries/Microsoft.Extensions.ServiceDiscovery.Dns.Tests/Resolver/TcpFailoverTests.cs

@@ -42,7 +42,7 @@ public async Task TcpFailover_Simple_Success()
     public async Task TcpFailover_ServerClosesWithoutData_EmptyResult()
     {
         string hostName = "tcp-server-closes.test";
-        Options.Attempts = 1;
+        Options.MaxAttempts = 1;
         Options.Timeout = TimeSpan.FromSeconds(60);
 
         _ = DnsServer.ProcessUdpRequest(builder =>
@@ -66,7 +66,7 @@ public async Task TcpFailover_ServerClosesWithoutData_EmptyResult()
     public async Task TcpFailover_TcpNotAvailable_EmptyResult()
     {
         string hostName = "tcp-not-available.test";
-        Options.Attempts = 1;
+        Options.MaxAttempts = 1;
         Options.Timeout = TimeSpan.FromMilliseconds(100000);
 
         _ = DnsServer.ProcessUdpRequest(builder =>

test/Libraries/Microsoft.Extensions.ServiceDiscovery.Yarp.Tests/YarpServiceDiscoveryTests.cs

@@ -232,7 +232,7 @@ public async Task ServiceDiscoveryDestinationResolverTests_Configuration_Disallo
     [Fact]
     public async Task ServiceDiscoveryDestinationResolverTests_Dns()
     {
-        DnsResolver resolver = new DnsResolver(TimeProvider.System, NullLogger<DnsResolver>.Instance, new ResolverOptions());
+        DnsResolver resolver = new DnsResolver(TimeProvider.System, NullLogger<DnsResolver>.Instance, new DnsResolverOptions());
 
         await using var services = new ServiceCollection()
             .AddSingleton<IDnsResolver>(resolver)