dotnet / aspire

An opinionated, cloud ready stack for building observable, production ready, distributed applications in .NET
https://learn.microsoft.com/dotnet/aspire
MIT License
3.38k stars 351 forks source link

gRPC client doesn't work with Service Discovery scheme resolution syntax #2896

Open DamianEdwards opened 3 months ago

DamianEdwards commented 3 months ago

The format used to support URI scheme resolution recently added to service discovery (e.g. https+http://) does not work with the gRPC client stack. The GrpcChannel class does a lot of initialization in its constructor based on the scheme of the address and special-cases https:// and http:// for numerous operations including ResolverFactory resolution and credential selection.

@JamesNK interested in your thoughts on how we might go about fixing this. I messed about a bit with a custom ResolverFactory and got further but seems we'd have to integrate more deeply to make the syntax work or update GrpcChannel to tolerate it some other way.

JamesNK commented 3 months ago

HttpClient happily sends HTTP requests to different hosts over https or http, and you can completely change a request as it goes through the handler pipeline.

GrpcChannel is different in that you specify the scheme + host, and then it only sends request to that address. Because of this, the channel has logic in it that knows whether it is https or http, and makes decisions based on that.

Maybe something could be done in Grpc.Net.Client. But GrpcChannel wasn't designed to work like this. It really wants to know what its scheme is when it's created, not when it is sending a gRPC call.

DamianEdwards commented 3 months ago

Can we integrate Service Discovery into the gRPC client in a different way, such that the address is fully resolved by the time the GrpcChannel is being created?

DamianEdwards commented 3 months ago

Ultimately the address needs to be resolved before the gRPC client is configured, as GrpcChannel performs specific logic at creation time (literally in the constructor) based on the scheme. The following "works" but has a huge issue of sync-over-async as it has to be done in an options delegate:

builder.Services.AddSingleton<BasketServiceClient>()
                .AddGrpcClient<Basket.BasketClient>((sp, o) =>
                {
                    var resolver = sp.GetRequiredService<ServiceEndPointResolver>();
                    var endpoints = resolver.GetEndPointsAsync("https+http://basketservice", default).AsTask().GetAwaiter().GetResult();
                    var address = endpoints.Where(e => e.EndPoint is UriEndPoint uriEndpoint)
                                           .Select(e => ((UriEndPoint)e.EndPoint).Uri)
                                           .First();
                    o.Address = address;
                });
DamianEdwards commented 3 months ago

Next steps are to experiment with bringing back a gRPC resolver but this time for the "https+http" scheme (or whatever combination the user wants) and see how far we get with that approach.

davidfowl commented 3 months ago

Moving to p6, unclear if it meets the bar though

joperezr commented 2 weeks ago

Is this something we want to fix 8.1 @ReubenBond?

DamianEdwards commented 2 weeks ago

No we can move it out.