Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Proposal: Microsoft.Extensions.ServiceDiscovery #53715

Closed
ReubenBond opened this issue Jan 30, 2024 · 8 comments · Fixed by dotnet/aspire#3413
Closed

API Proposal: Microsoft.Extensions.ServiceDiscovery #53715

ReubenBond opened this issue Jan 30, 2024 · 8 comments · Fixed by dotnet/aspire#3413
Labels
api-approved API was approved in API review, it can be implemented area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions

Comments

@ReubenBond
Copy link
Member

ReubenBond commented Jan 30, 2024

Background and Motivation

We added Service Discovery APIs for Aspire to improve the dev inner loop experience for multi-service apps. The core idea behind these APIs is to give developers a way to identify services using logical names (eg, "https://catalog") in both development and production environments, with the Service Discovery library performing the work of translating those logical names into something which a network connection can be established with. The core duties of the Service Discovery APIs are:

  • Name Resolution: Resolve service names to collections of endpoints for those services. React to changes in the set of resolved endpoints by notifying callers.
  • Client-side load balancing: Resolve a service name to a single endpoint, balancing load locally across the set of known endpoints.

The API should be extensible enough to support the most common service discovery mechanisms:

  • Configuration for environments where service endpoints are defined in configuration. This is used in the dev inner-loop scenario for Aspire.
  • DNS A/AAAA records for environments where addresses are exposed via DNS queries and ports are the default for whatever protocol is being resolved (eg, 80 for HTTP, 433 for HTTPS)
  • DNS SRV records for Kubernetes, Consul, and other environments which expose service discovery via the common SRV record format. In Kubernetes, headless Service resources can be used to expose the addresses and ports of services via DNS SRV records. This allows for multiple, named ports on service. For example, this allows a developer can query for "the dashboard port of the catalog service".
  • Pass-through for environments which offer their own service discovery mechanism, such as Azure Container Apps, which exposes endpoints through a service mesh running next to every container. This relies on ACA app names to line up with the logical name specified by the developer (eg, if the logical name of a service is catalog, the corresponding ACA app named catalog will be resolved).

The API also aims to support:

  • Resolution through HttpClient instances which have been configured with Service Discovery.
  • Integration with YARP via YARP's IDestinationResolver API.
  • Integration with other libraries both as consumers/watchers of service resolution and providers of service endpoints.

The primary touch points for developers are:

  • Configuring and enabling Service Discovery during application startup
  • Transparent service resolution through HttpClient.

High level design

The library is designed around a provider model. Integration with HttpClient is implemented using a middleware, ResolvingHttpClientMiddleware. Here's the approximate flow for resolving a, reading depth-first from top to bottom, then left to right.

flowchart TD
   H[HttpClient]
   RHCH[ResolvingHttpClientHandler]
   HSEPR[HttpServiceEndPointResolver]
   SEPW[ServiceEndPointWatcher]
   ISEPS[IServiceEndPointSelector impl]
   ISEPP[Multiple IServiceEndPointProvider impls]
   HCH[HttpClientHandler]
   H--SendAsync-->RHCH
   RHCH--GetEndPointAsync-->HSEPR
   HSEPR--endpoint-->RHCH
   HSEPR--GetEndPointsAsync-->SEPW
   SEPW--endpoints-->HSEPR
   HSEPR--SetEndpointCollection-->ISEPS
   HSEPR--GetEndPoint-->ISEPS
   ISEPS--endpoint-->HSEPR
   SEPW--ResolveAsync-->ISEPP
   RHCH--base.SendAsync-->HCH
Loading

Key design notes

  • The underlying endpoint is described using an System.Net.EndPoint. It is presented to consumers via a new type, ServiceEndPoint which contains two properties: the EndPoint and an IFeatureCollection property which can be used to attach metadata and functionality to exposed endpoints.
    • The intention is to support things such as:
      • Attaching AuthN/AuthZ information to endpoints
      • Attaching the original hostname via IHostNameFeature to the endpoint.
        • This host name is propagated to the HttpClient via the HttpRequestMessage.Headers.Host property.
      • Attaching the IServiceEndPointProvider which resolved the endpoint to the endpoint for diagnostics
      • Per-endpoint health reporting (exceptions & latency) via IEndPointHealthFeature - not implemented.
      • Querying per-endpoint load querying via IEndPointLoadFeature (eg, for Power-of-two-choices load balancing by IServiceEndPointSelector) - not implemented, no resolvers currently expose load metrics.
  • Endpoints are returned in a collection of type ServiceEndPointCollection which implements IReadOnlyList<ServiceEndPoint> and adds:
    • An IChangeToken property, which is used to signal when a collection should be refreshed. Providers invalidate the change token in several ways:
      • DNS SRV uses the minimum of the DNS TTL and the configured refresh time.
      • DNS A/AAAA uses configured refresh time (no TTL available from System.Net.Dns).
      • Configuration invalidates the token when the relevant IConfigurationSection's IChangeToken is invalidated
      • Pass-thru never invalidates the token.
    • An IFeatureCollection property to add per-collection features.
    • The service name (a string)
  • Resolution is naturally asynchronous, since these service discovery mechanisms largely involve I/O to some remote service. Therefore, there are no synchronous APIs for resolving a service.

Proposed API

The API consists of 4 libraries, structured as follows:

graph BT;
    Abstractions["Microsoft.Extensions.ServiceDiscovery.Abstractions"]
    Core["Microsoft.Extensions.ServiceDiscovery"]
    Dns["Microsoft.Extensions.ServiceDiscovery.Dns"]
    Yarp["Microsoft.Extensions.ServiceDiscovery.Yarp"]
    Core --> Abstractions
    Dns --> Abstractions
    Yarp --> Abstractions
    Dns -- AddServiceDiscoveryCore --> Core
    Yarp -- AddServiceDiscoveryCore\nServiceEndPointResolver --> Core 
Loading

The DNS & YARP libraries reference the core Service Discovery library for access to a hosting configuration method, AddServiceDiscoveryCore(), and in the case of YARP, for ServiceEndPointResolver, which is the central point for resolving endpoints from libraries.

APIs in Microsoft.Extensions.ServiceDiscovery.Abstractions.dll

Microsoft.Extensions.ServiceDiscovery.Abstractions

 namespace Microsoft.Extensions.ServiceDiscovery.Abstractions {
     public interface IEndPointHealthFeature {
         void ReportHealth(TimeSpan responseTime, Exception exception);
     }
     public interface IEndPointLoadFeature {
         double CurrentLoad { get; }
     }
     public interface IHostNameFeature {
         string HostName { get; }
     }
     public interface IServiceEndPointProvider : IAsyncDisposable {
         ValueTask<ResolutionStatus> ResolveAsync(ServiceEndPointCollectionSource endPoints, CancellationToken cancellationToken);
     }
     public interface IServiceEndPointResolverProvider {
         bool TryCreateResolver(string serviceName, out IServiceEndPointProvider? resolver);
     }
     public interface IServiceEndPointSelector {
         ServiceEndPoint GetEndPoint(object? context);
         void SetEndPoints(ServiceEndPointCollection endPoints);
     }
     public interface IServiceEndPointSelectorProvider {
         IServiceEndPointSelector CreateSelector();
     }
     public readonly struct ResolutionStatus : IEquatable<ResolutionStatus> {
         public static readonly ResolutionStatus Cancelled;
         public static readonly ResolutionStatus None;
         public static readonly ResolutionStatus Pending;
         public static readonly ResolutionStatus Success;
         public ResolutionStatus(ResolutionStatusCode statusCode, Exception? exception, string message);
         public Exception? Exception { get; }
         public string Message { get; }
         public ResolutionStatusCode StatusCode { get; }
         public static ResolutionStatus CreateNotFound(string message);
         public bool Equals(ResolutionStatus other);
         public override bool Equals(object? obj);
         public static ResolutionStatus FromException(Exception exception);
         public static ResolutionStatus FromPending(Exception? exception = null);
         public override int GetHashCode();
         public static bool operator ==(ResolutionStatus left, ResolutionStatus right);
         public static bool operator !=(ResolutionStatus left, ResolutionStatus right);
         public override string ToString();
     }
     public enum ResolutionStatusCode {
         Cancelled = 4,
         Error = 5,
         None = 0,
         NotFound = 2,
         Pending = 1,
         Success = 3,
     }
     public abstract class ServiceEndPoint {
         protected ServiceEndPoint();
         public abstract EndPoint EndPoint { get; }
         public abstract IFeatureCollection Features { get; }
         public static ServiceEndPoint Create(EndPoint endPoint, IFeatureCollection? features = null);
         public virtual string GetEndPointString();
     }
     public class ServiceEndPointCollection : IEnumerable, IEnumerable<ServiceEndPoint>, IReadOnlyCollection<ServiceEndPoint>, IReadOnlyList<ServiceEndPoint> {
         public ServiceEndPointCollection(string serviceName, List<ServiceEndPoint>? endpoints, IChangeToken changeToken, IFeatureCollection features);
         public IChangeToken ChangeToken { get; }
         public int Count { get; }
         public IFeatureCollection Features { get; }
         public string ServiceName { get; }
         public ServiceEndPoint this[int index] { get; }
         public IEnumerator<ServiceEndPoint> GetEnumerator();
         IEnumerator IEnumerable.GetEnumerator();
         public override string ToString();
     }
     public class ServiceEndPointCollectionSource {
         public ServiceEndPointCollectionSource(string serviceName, IFeatureCollection features);
         public IList<ServiceEndPoint> EndPoints { get; }
         public IFeatureCollection Features { get; }
         public string ServiceName { get; }
         public void AddChangeToken(IChangeToken changeToken);
         public static ServiceEndPointCollection CreateServiceEndPointCollection(ServiceEndPointCollectionSource source);
         public IChangeToken GetChangeToken();
     }
     public sealed class ServiceEndPointResolverResult {
         public ServiceEndPointResolverResult(ServiceEndPointCollection endPoints, ResolutionStatus status);
         public ServiceEndPointCollection EndPoints { get; }
         public bool ResolvedSuccessfully { get; }
         public ResolutionStatus Status { get; }
     }
 }

APIs in Microsoft.Extensions.ServiceDiscovery.dll

Microsoft.Extensions.DependencyInjection

 namespace Microsoft.Extensions.DependencyInjection {
     public static class HttpClientBuilderExtensions {
         public static IHttpClientBuilder UseServiceDiscovery(this IHttpClientBuilder httpClientBuilder);
         public static IHttpClientBuilder UseServiceDiscovery(this IHttpClientBuilder httpClientBuilder, IServiceEndPointSelectorProvider selectorProvider);
     }
 }

The rationale for putting these extension methods in this namespace is that this is where the IHttpClientBuilder.AddStandardResilienceHandler() extension lives. In Aspire apps, at least, these two often appear together in the ServiceDefaults project. Example:

builder.Services.ConfigureHttpClientDefaults(http =>
{
    // Turn on resilience by default
    http.AddStandardResilienceHandler();

    // Turn on service discovery by default
    http.UseServiceDiscovery();
});

Microsoft.Extensions.Hosting

 namespace Microsoft.Extensions.Hosting {
     public static class HostingExtensions {
         public static IServiceCollection AddConfigurationServiceEndPointResolver(this IServiceCollection services);
         public static IServiceCollection AddConfigurationServiceEndPointResolver(this IServiceCollection services, Action<ConfigurationServiceEndPointResolverOptions>? configureOptions = null);
         public static IServiceCollection AddPassThroughServiceEndPointResolver(this IServiceCollection services);
         public static IServiceCollection AddServiceDiscovery(this IServiceCollection services);
         public static IServiceCollection AddServiceDiscoveryCore(this IServiceCollection services);
     }
 }

Microsoft.Extensions.ServiceDiscovery

 namespace Microsoft.Extensions.ServiceDiscovery {
     public sealed class ServiceEndPointResolver : IAsyncDisposable {
         public ValueTask DisposeAsync();
         public ValueTask<ServiceEndPointCollection> GetEndPointsAsync(string serviceName, CancellationToken cancellationToken);
     }
     public class ServiceEndPointResolverFactory {
         public ServiceEndPointResolverFactory(IEnumerable<IServiceEndPointResolverProvider> resolvers, ILogger<ServiceEndPointWatcher> resolverLogger, IOptions<ServiceEndPointResolverOptions> options, TimeProvider timeProvider);
         public ServiceEndPointWatcher CreateResolver(string serviceName);
     }
     public sealed class ServiceEndPointWatcher : IAsyncDisposable {
         public ServiceEndPointWatcher(IServiceEndPointProvider[] resolvers, ILogger logger, string serviceName, TimeProvider timeProvider, IOptions<ServiceEndPointResolverOptions> options);
         public Action<ServiceEndPointResolverResult>? OnEndPointsUpdated { get; set; }
         public string ServiceName { get; }
         public ValueTask DisposeAsync();
         public ValueTask<ServiceEndPointCollection> GetEndPointsAsync(CancellationToken cancellationToken = default(CancellationToken));
         public void Start();
     }
 }

Microsoft.Extensions.ServiceDiscovery.Abstractions

 namespace Microsoft.Extensions.ServiceDiscovery.Abstractions {
     public class ConfigurationServiceEndPointResolverOptions {
         public ConfigurationServiceEndPointResolverOptions();
         public Func<ServiceEndPoint, bool> ApplyHostNameMetadata { get; set; }
         public string? SectionName { get; set; }
     }
     public class ConfigurationServiceEndPointResolverProvider : IServiceEndPointResolverProvider {
         public ConfigurationServiceEndPointResolverProvider(IConfiguration configuration, IOptions<ConfigurationServiceEndPointResolverOptions> options, ILoggerFactory loggerFactory);
         public bool TryCreateResolver(string serviceName, out IServiceEndPointProvider? resolver);
     }
     public class PickFirstServiceEndPointSelector : IServiceEndPointSelector {
         public PickFirstServiceEndPointSelector();
         public ServiceEndPoint GetEndPoint(object? context);
         public void SetEndPoints(ServiceEndPointCollection endPoints);
     }
     public class PickFirstServiceEndPointSelectorProvider : IServiceEndPointSelectorProvider {
         public PickFirstServiceEndPointSelectorProvider();
         public static PickFirstServiceEndPointSelectorProvider Instance { get; }
         public IServiceEndPointSelector CreateSelector();
     }
     public class PowerOfTwoChoicesServiceEndPointSelector : IServiceEndPointSelector {
         public PowerOfTwoChoicesServiceEndPointSelector();
         public ServiceEndPoint GetEndPoint(object? context);
         public void SetEndPoints(ServiceEndPointCollection endPoints);
     }
     public class PowerOfTwoChoicesServiceEndPointSelectorProvider : IServiceEndPointSelectorProvider {
         public PowerOfTwoChoicesServiceEndPointSelectorProvider();
         public static PowerOfTwoChoicesServiceEndPointSelectorProvider Instance { get; }
         public IServiceEndPointSelector CreateSelector();
     }
     public class RandomServiceEndPointSelector : IServiceEndPointSelector {
         public RandomServiceEndPointSelector();
         public ServiceEndPoint GetEndPoint(object? context);
         public void SetEndPoints(ServiceEndPointCollection endPoints);
     }
     public class RandomServiceEndPointSelectorProvider : IServiceEndPointSelectorProvider {
         public RandomServiceEndPointSelectorProvider();
         public static RandomServiceEndPointSelectorProvider Instance { get; }
         public IServiceEndPointSelector CreateSelector();
     }
     public class RoundRobinServiceEndPointSelector : IServiceEndPointSelector {
         public RoundRobinServiceEndPointSelector();
         public ServiceEndPoint GetEndPoint(object? context);
         public void SetEndPoints(ServiceEndPointCollection endPoints);
     }
     public class RoundRobinServiceEndPointSelectorProvider : IServiceEndPointSelectorProvider {
         public RoundRobinServiceEndPointSelectorProvider();
         public static RoundRobinServiceEndPointSelectorProvider Instance { get; }
         public IServiceEndPointSelector CreateSelector();
     }
     public sealed class ServiceEndPointResolverOptions {
         public ServiceEndPointResolverOptions();
         public TimeSpan PendingStatusRefreshPeriod { get; set; }
         public TimeSpan RefreshPeriod { get; set; }
     }
 }

Microsoft.Extensions.ServiceDiscovery.Http

 namespace Microsoft.Extensions.ServiceDiscovery.Http {
     public class HttpServiceEndPointResolver : IAsyncDisposable {
         public HttpServiceEndPointResolver(ServiceEndPointResolverFactory resolverFactory, IServiceEndPointSelectorProvider selectorProvider, TimeProvider timeProvider);
         public ValueTask DisposeAsync();
         public ValueTask<ServiceEndPoint> GetEndpointAsync(HttpRequestMessage request, CancellationToken cancellationToken);
     }
     public class ResolvingHttpClientHandler : HttpClientHandler {
         public ResolvingHttpClientHandler(HttpServiceEndPointResolver resolver);
         protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken);
     }
     public class ResolvingHttpDelegatingHandler : DelegatingHandler {
         public ResolvingHttpDelegatingHandler(HttpServiceEndPointResolver resolver);
         public ResolvingHttpDelegatingHandler(HttpServiceEndPointResolver resolver, HttpMessageHandler innerHandler);
         protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken);
     }
 }

Usage Examples

Using Service Discovery with HttpClient

The primary scenario for service discovery is for HttpClient, where
A common approach is to add Service Discovery to the default HttpClient factory as follows. This example comes from the Aspire ServiceDefaults project which we generate on behalf of users:

builder.Services.AddServiceDiscovery();

builder.Services.ConfigureHttpClientDefaults(http =>
{
    // Turn on resilience by default
    http.AddStandardResilienceHandler();

    // Turn on service discovery by default
    http.UseServiceDiscovery();
});

Once service discovery is configured on the application host, individual typed HttpClients are configured by setting the BaseAddress to include the logical service name. In the following example, there are two logical service names:

builder.Services.AddHttpClient<CatalogServiceClient>(c => c.BaseAddress = new("http://catalogservice"));

builder.Services.AddSingleton<BasketServiceClient>()
                .AddGrpcClient<Basket.BasketClient>(o => o.Address = new("http://basketservice"));

The corresponding configuration sections in JSON might like this:

{
  "services":
  {
    "basketservice":
    {
      "http": "127.0.0.1:8080", // A single IP endpoint
    },
    "catalogservice":
    { // An IP and a DNS endpoint
      "http": [ "localhost:8888", "10.0.0.1:5032" ]
    }
  }
}

Named endpoints

Similarly, named endpoints can be used to specify a particular endpoint exposed by service, for example:

  • The "https" endpoint on catalog service (versus the "http" endpoint, for example)
  • The "dashboard" endpoint on the basket service

In configuration, each named endpoint has its own section and the configuration path takes the format: $"services:{serviceName}:{endpointName}".

To specify which endpoint should be resolved during consumption, the endpoint name is encoded into the input string, following the format: $"_{endpointName}.{serviceName}".

Scheme selection

During development time, it is currently much easier for developers to use plain-text HTTP. In production, however, this is not acceptable, and HTTPS is used instead. Therefore, we ideally want a way to allow developers to switch between HTTP at development time and HTTPS in production. The mechanism chosen for this is to allow specifying both schemes in the input string passed to service discovery, separated by a + symbol. For example, "https+http://catalogapi" specifies that either HTTPS or HTTP can be used, and it is up to the Service Discovery system to select an appropriate scheme. Service Discovery evaluates these schemes in order and selects the first scheme which has corresponding configuration. For pass-through and DNS providers, the first scheme is always used.

To limit the schemes permissible at runtime, an option is provided:

public sealed class ServiceDiscoveryOptions
{
    /// <summary>
    /// The value for <see cref="AllowedSchemes"/> which indicates that all schemes are allowed.
    /// </summary>
    public static readonly string[] AllSchemes = new string[0];

    /// <summary>
    /// Gets or sets the collection of allowed URI schemes for URIs resolved by the service discovery system when multiple schemes are specified, for example "https+http://_endpoint.service".
    /// </summary>
    /// <remarks>
    /// When set to <see cref="AllSchemes"/>, all schemes are allowed.
    /// Schemes are not case-sensitive.
    /// </remarks>
    public string[] AllowedSchemes { get; set; } = AllSchemes;
}

By default, all schemes are allowed, so the schemes specified in the input to service discovery will be used. These can be limited by specifying a set of allowed schemes, eg ["https"] to allow only HTTPS.

Ad-hoc service resolution

Service endpoints can also be resolved outside of HttpClient's integration. The most straight-forward approach to this is to use ServiceEndPointResolver.GetEndPointsAsync(string serviceName, CancellationToken cancellationToken) like so:

var app = builder.Build();
await app.StartAsync();
ServiceEndPointResolver resolver = app.Services.GetRequiredService<ServiceEndPointResolver>();

// Resolve the service and enumerate the endpoints
ServiceEndPointCollection endpoints
  = await resolver.GetEndPointsAsync("catalogservice", CancellationToken.None);
foreach (ServiceEndPoint endpoint in endpoints)
{
    Console.WriteLine(endpoint);
}

Alternative Designs

Config-only: Scrap most of the library and only support host name replacement via IConfiguration. One benefit to that approach is that we could adopt a simpler, synchronous API. For the two main cases which we target with Aspire, that would be sufficient: we already use IConfiguration in the inner-loop. When deployed to Azure Container Apps, we use the pass-thru provider since ACA has its own service discovery via a service mesh.
Downsides to this approach include that it is not flexible or extensible outside of the IConfiguration extensibility model. Eg, it could not be extended to support purpose-built service discovery mechanisms like Consul and DNS or anything which requires I/O. It also would not support client-side load balancing.

Risks & deficiencies

  • The current usage of the API does not offer zero-downtime upgrades without relying on retries in Kubernetes or other environments where DNS is used because pods can be taken down without clients first removing them from the rotation. Systems such as Meta's Shard Manager integrate with the cluster manager so that clients can gracefully drain load before the target service is terminated. There are some mitigating factors to this risk:
    • The IFeatureCollection allows for extensibility if we ever wanted to support this (and had a means to).
    • Defunct endpoints could be removed from the rotation after exceptions are observed by implementing IEndPointHealthFeature and integrating it with IServiceEndPointSelector.
    • Services can always crash, that is not new here: resilient apps must deal with crashes by using retries, etc, already.
  • The API currently provides no clear way to change the scheme of an HttpClient request. Eg, we cannot currently change the scheme from "http://" to "https://" using service discovery. We could offer this functionality using a feature on the returned ServiceEndPoint or by returning a UriEndPoint, which we do not currently have.
  • Due to the asynchronous nature of the API, there are many places where it does not integrate cleanly. For example, during application startup, while configuring IOptions<T> or other DI services, in constructors, etc.

Designer notes:

  • We should change IEndPointLoadFeature.CurrentLoad to a float (from double), since it has plenty of precision for the purposes of load balancing.
@ReubenBond ReubenBond added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 30, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Jan 30, 2024
@gfoidl gfoidl added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Jan 31, 2024
@ReubenBond ReubenBond added the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label Feb 14, 2024
@ReubenBond
Copy link
Member Author

cc @halter73

@halter73 halter73 removed the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 29, 2024
@halter73
Copy link
Member

halter73 commented Mar 8, 2024

API Review Notes:

  • Let's move types from "Abstractions" namespaces like Microsoft.Extensions.ServiceDiscovery.Abstractions to one without like Microsoft.Extensions.ServiceDiscovery even if they're in an Abstractions assembly.
  • Let's remove IEquatable from ResolutionStatus.
  • Do we need a UriEndPoint? Can we use the one in Microsoft.AspNetCore.Connections.Abstractions? Define a new one?
    • Ideally it would go in System.Net.
  • What's the difference between ResolutionStatusCode.None vs ResolutionStatusCode.NotFound?
    • Can we combine None and NotFound? And make Pending the default? TBD
  • When is ResolutionStatusCode.Canceled returned? If you cancel ResolveAsync with the cancellation token, don't you just get a canceled task with no statuses? TBD
  • Why is ServiceEndPoint abstract with a factory rather than a ctor?
    • So we can more easily optimize the ServiceEndPoint implementation by doing things like making it implement IFeatureCollection without an additional allocation.
  • IServiceEndPointResolverProvider with TryCreateResolver which creates a IServiceEndPointProvider is weird naming.
    • Let's go with IServiceEndPointProviderFactory with TryCreateProvider.
    • Let's rename IServiceEndPointSelectorProvider to IServiceEndPointSelectorFactory for consistency if it continues to exist.
  • Better yet would be to get rid of IServiceEndPointSelectorProvider entirely and register the IServiceEndPointSelector directly DI as a transient service.
  • Instead of using a mutable ServiceEndPointCollectionSource, could we add an Order or Priority to the ServiceEndPoint itself similar to endpoint routing with its EndpointDataSource? TBD
  • Let's rename UseServiceDiscovery to AddServiceDiscovery.
  • Move the IServiceCollection extension methods out of Microsoft.Extensions.Hosting.HostingExtensions.
    • Move into Microsoft.Extensions.DependencyInjection with a more unique class name.
  • Why make the ServiceEndPointResolverFactory and ServiceEndPointWatcher ctors public? We might have to add more ctors in the future. Can we make the implementations internal and use interfaces instead. TBD
    • If we keep the constructors, why is there none for ServiceEndPointResolver?

@ReubenBond
Copy link
Member Author

ReubenBond commented Mar 22, 2024

Here is an updated proposal with a reduced surface area, and reflecting the discussion from the preview meeting. The only thing which is not here is

Microsoft.Extensions.ServiceDiscovery.Abstractions.dll

 namespace Microsoft.Extensions.ServiceDiscovery {
     public interface IHostNameFeature {
         string HostName { get; }
     }
     public interface IServiceEndPointBuilder {
         IList<ServiceEndPoint> EndPoints { get; }
         IFeatureCollection Features { get; }
         void AddChangeToken(IChangeToken changeToken);
     }
     public interface IServiceEndPointProvider : IAsyncDisposable {
         ValueTask PopulateAsync(IServiceEndPointBuilder endPoints, CancellationToken cancellationToken);
     }
     public interface IServiceEndPointProviderFactory {
         bool TryCreateProvider(ServiceEndPointQuery query, out IServiceEndPointProvider? resolver);
     }
     public abstract class ServiceEndPoint {
         protected ServiceEndPoint();
         public abstract EndPoint EndPoint { get; }
         public abstract IFeatureCollection Features { get; }
         public static ServiceEndPoint Create(EndPoint endPoint, IFeatureCollection? features = null);
         public virtual string GetEndPointString();
     }
     public sealed class ServiceEndPointQuery {
         public string? EndPointName { get; }
         public IReadOnlyList<string> IncludeSchemes { get; }
         public string OriginalString { get; }
         public string ServiceName { get; }
         public override string? ToString();
         public static bool TryParse(string input, out ServiceEndPointQuery? query);
     }
     public sealed class ServiceEndPointSource {
         public ServiceEndPointSource(List<ServiceEndPoint>? endpoints, IChangeToken changeToken, IFeatureCollection features);
         public IChangeToken ChangeToken { get; }
         public IReadOnlyList<ServiceEndPoint> EndPoints { get; }
         public IFeatureCollection Features { get; }
         public override string ToString();
     }
 }

Microsoft.Extensions.ServiceDiscovery.dll

 namespace Microsoft.Extensions.DependencyInjection {
     public static class ServiceDiscoveryHttpClientBuilderExtensions {
         public static IHttpClientBuilder AddServiceDiscovery(this IHttpClientBuilder httpClientBuilder);
     }
     public static class ServiceDiscoveryServiceCollectionExtensions {
         public static IServiceCollection AddConfigurationServiceEndPointResolver(this IServiceCollection services);
         public static IServiceCollection AddConfigurationServiceEndPointResolver(this IServiceCollection services, Action<ConfigurationServiceEndPointResolverOptions>? configureOptions);
         public static IServiceCollection AddPassThroughServiceEndPointResolver(this IServiceCollection services);
         public static IServiceCollection AddServiceDiscovery(this IServiceCollection services);
         public static IServiceCollection AddServiceDiscovery(this IServiceCollection services, Action<ServiceDiscoveryOptions>? configureOptions);
         public static IServiceCollection AddServiceDiscoveryCore(this IServiceCollection services);
         public static IServiceCollection AddServiceDiscoveryCore(this IServiceCollection services, Action<ServiceDiscoveryOptions>? configureOptions);
     }
 }

 namespace Microsoft.Extensions.ServiceDiscovery {
     public sealed class ConfigurationServiceEndPointResolverOptions {
         public ConfigurationServiceEndPointResolverOptions();
         public Func<ServiceEndPoint, bool> ApplyHostNameMetadata { get; set; }
         public string SectionName { get; set; }
     }
     public sealed class ServiceDiscoveryOptions {
         public static readonly string[] AllowAllSchemes;
         public ServiceDiscoveryOptions();
         public string[] AllowedSchemes { get; set; }
         public TimeSpan RefreshPeriod { get; set; }
     }
     public sealed class ServiceEndPointResolver : IAsyncDisposable {
         public ValueTask DisposeAsync();
         public ValueTask<ServiceEndPointSource> GetEndPointsAsync(string serviceName, CancellationToken cancellationToken);
     }
 }

 namespace Microsoft.Extensions.ServiceDiscovery.Http {
     public interface IServiceDiscoveryHttpMessageHandlerFactory {
         HttpMessageHandler CreateHandler(HttpMessageHandler handler);
     }
 }

@halter73
Copy link
Member

API Review Notes:

  • The new API proposal is far smaller.
  • New ServiceEndPointQuery type was introduced.
    • IncludeSchemes should be renamed to the past tense IncludedSchemes.
    • Why not make OriginalString the ToString() implementation? This copies Uri.
      • Let's start with just ToString and delete OriginalString.
  • All "EndPoint"s should become "Endpoint" or "endpoint" except for the ServiceEndpoint.EndPoint property which is a System.Net.EndPoint
  • Are we okay with ServiceDiscoveryOptions.AllowAllSchemes rely on reference equality?
    • This is similar to the AnyKey for keyed DI. And a little similar to Timeout.Infinite in that it's a sentinel value.
    • An AllowAllSchemes bool could be bound from configuration.
      • If AllowAllSchemes is true and the AllowedSchemes array is non-empty, should it throw? No. It should allow all schemes.
  • Should configureOptions be non-nullable? AddOpenApi, AddMvc, AddSignalR don't allow it to be nullable.
    • Yes. It might be annoying sometimes, but rarely, and it's more consistent with other APIs.
  • Let's rename ApplyHostNameMetadata to ShouldApplyHostNameMetadata.
  • Can we remove IServiceDiscoveryHttpMessageHandlerFactory and rely just on IHttpMessageHandlerFactory? If so, let's remove it since it's always easier to add rather than remove APIs later.
  • Does ServiceEndPointSource need a public constructor?
    • It's not necessary, but it's very straightforward, so why not?

API Approved!

 namespace Microsoft.Extensions.ServiceDiscovery {
     public interface IHostNameFeature {
         string HostName { get; }
     }
     public interface IServiceEndpointBuilder {
         IList<ServiceEndpoint> Endpoints { get; }
         IFeatureCollection Features { get; }
         void AddChangeToken(IChangeToken changeToken);
     }
     public interface IServiceEndpointProvider : IAsyncDisposable {
         ValueTask PopulateAsync(IServiceEndpointBuilder endpoints, CancellationToken cancellationToken);
     }
     public interface IServiceEndpointProviderFactory {
         bool TryCreateProvider(ServiceEndpointQuery query, out IServiceEndpointProvider? resolver);
     }
     public abstract class ServiceEndpoint {
         protected ServiceEndpoint();
         public abstract EndPoint EndPoint { get; }
         public abstract IFeatureCollection Features { get; }
         public static ServiceEndpoint Create(Endpoint endpoint, IFeatureCollection? features = null);
         public virtual string GetEndpointString();
     }
     public sealed class ServiceEndpointQuery {
         public string? EndpointName { get; }
         public IReadOnlyList<string> IncludedSchemes { get; }
         public string ServiceName { get; }
         public override string? ToString();
         public static bool TryParse(string input, out ServiceEndpointQuery? query);
     }
     public sealed class ServiceEndpointSource {
         public ServiceEndpointSource(List<ServiceEndpoint>? endpoints, IChangeToken changeToken, IFeatureCollection features);
         public IChangeToken ChangeToken { get; }
         public IReadOnlyList<ServiceEndpoint> Endpoints { get; }
         public IFeatureCollection Features { get; }
         public override string ToString();
     }
 }

  namespace Microsoft.Extensions.DependencyInjection {
     public static class ServiceDiscoveryHttpClientBuilderExtensions {
         public static IHttpClientBuilder AddServiceDiscovery(this IHttpClientBuilder httpClientBuilder);
     }
     public static class ServiceDiscoveryServiceCollectionExtensions {
         public static IServiceCollection AddConfigurationServiceEndpointResolver(this IServiceCollection services);
         public static IServiceCollection AddConfigurationServiceEndpointResolver(this IServiceCollection services, Action<ConfigurationServiceEndpointResolverOptions> configureOptions);
         public static IServiceCollection AddPassThroughServiceEndpointResolver(this IServiceCollection services);
         public static IServiceCollection AddServiceDiscovery(this IServiceCollection services);
         public static IServiceCollection AddServiceDiscovery(this IServiceCollection services, Action<ServiceDiscoveryOptions> configureOptions);
         public static IServiceCollection AddServiceDiscoveryCore(this IServiceCollection services);
         public static IServiceCollection AddServiceDiscoveryCore(this IServiceCollection services, Action<ServiceDiscoveryOptions> configureOptions);
     }
 }

 namespace Microsoft.Extensions.ServiceDiscovery {
     public sealed class ConfigurationServiceEndpointResolverOptions {
         public ConfigurationServiceEndpointResolverOptions();
         public Func<ServiceEndpoint, bool> ShouldApplyHostNameMetadata { get; set; }
         public string SectionName { get; set; }
     }
     public sealed class ServiceDiscoveryOptions {
         public ServiceDiscoveryOptions();
         public bool AllowAllSchemes { get; set; }
         public IList<string> AllowedSchemes { get; set; }
         public TimeSpan RefreshPeriod { get; set; }
     }
     public sealed class ServiceEndpointResolver : IAsyncDisposable {
         public ValueTask DisposeAsync();
         public ValueTask<ServiceEndpointSource> GetEndpointsAsync(string serviceName, CancellationToken cancellationToken);
     }
 }

 namespace Microsoft.Extensions.ServiceDiscovery.Http {
     public interface IServiceDiscoveryHttpMessageHandlerFactory {
         HttpMessageHandler CreateHandler(HttpMessageHandler handler);
     }
 }

@halter73 halter73 added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Mar 29, 2024
@ReubenBond
Copy link
Member Author

Addendum, we removed ServiceEndpoint.GetEndpointString() in favor of using ServiceEndpoint.ToString().

@gdantuono
Copy link

Hi @ReubenBond
With the latest release I got a regression on the ability to use Ad-hoc service resolution.
The ServiceEndPointResolver class is no longer public.
The ServiceEndpointWatcherFactory class is no longer public.
How can I resolve this?
This is my code:

var endPointResolverFactory = serviceProvider.GetRequiredService<ServiceEndPointResolverFactory>();
var endpoints = await endPointResolverFactory
               .CreateResolver(serviceName)
               .GetEndPointsAsync();

return endpoints.Select(endpoint => endpoint.GetEndPointString()).FirstOrDefault();

@ReubenBond
Copy link
Member Author

ReubenBond commented Apr 12, 2024

@gdantuono the equivalent now is this:

var resolver = services.GetRequiredService<ServiceEndpointResolver>();
var endpoints = await resolver.GetEndpointsAsync(serviceName, cancellationToken).ConfigureAwait(false);
return endpoints.Endpoints?[0].ToString();

@gdantuono
Copy link

@ReubenBond thanks you, very much.

This code also handles no results:

var source = await endPointResolver.GetEndPointsAsync(serviceName, cancellationToken).ConfigureAwait(false);
return (source.EndPoints?.Count > 0) ? source.EndPoints[0].ToString() : null;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@halter73 @ReubenBond @gfoidl @gdantuono and others