dotnet-sdk icon indicating copy to clipboard operation
dotnet-sdk copied to clipboard

Add an extension method to add Dapr HTTP Client to service collection

Open karoldeland opened this issue 3 years ago • 6 comments

Describe the proposal

Currently, we can call the CreateInvokeHttpClient from DaprClient to create a preconfigured HttpClient. It would be great to leverage the IHttpClientFactory of ASP.NET Core with the same level of integration as the CreateInvokeHttpClient.

In short, there's how it could be used :

services.AddDaprHttpClient<CartClient, CartClient>("appId");

I've created a PR to see how it could be implemented.

karoldeland avatar Mar 15 '21 01:03 karoldeland

Thanks @karoldeland - this makes what you're proposing more clear.

Can you answer one question for me? What features of the client factory do you need for your scenario?

  • Typed clients (CartClient)
  • Configuration of other handlers (retries)
  • Built-in logging
  • Handler rotation (not likely)

I'm yet convinced that this is the right thing to do on a few axes:

  • It's easy to configure this today but it's not in docs (use AddMessageHandler and set the BaseUri) - this is the first time this feedback has come up so I'm hesitant to pursue expensive solutions
  • This would best be served as a separate package (we won't add a dependency to Dapr.Client but Dapr.AspNetCore is probably not the right place).
  • There are many overloads of AddHttpClient<T> if we add one of these we kinda need to have them all or issues/requests will trickle in over time: https://docs.microsoft.com/en-us/dotnet/api/microsoft.extensions.dependencyinjection.httpclientfactoryservicecollectionextensions.addhttpclient?view=dotnet-plat-ext-5.0

Personally, my approach for these things is avoid over-indexing on whether something is one line of code or multiple.

Providing a convenience API for helps when you want users to find a feature without documentation, or when the task is so shocking common that it's going to help 95% of people for 95% of their use cases.

I think what would change my mind would be:

  • Us starting with the sample of doing this "by hand" and working backwards to a good solution
  • Thinking about other options that don't try to pack 3-4 features in single method call

To continue this discussion, here's an example of what this might look like today if I put it in the docs without adding any new features.

public void ConfigureServices(IServiceCollection services)
{
    services.AddHttpClient<CartClient>(c =>
    {
        c.BaseAddress = new Uri("http://cart");
    })
    .AddHttpMessageHandler(() => new InvocationHandler());
}

Depending on your answers to the questions I asked, here's an even simpler example without the factory:

public void ConfigureServices(IServiceCollection services)
{
    services.AddSingleton<CartClient>(new CartClient(DaprClient.CreateInvokeHttpClient("cart"));
}

rynowak avatar Mar 15 '21 19:03 rynowak

Hi @rynowak, I have to agree with you. I realized yesterday while implementing the basic method that it will imply to have a lot of code to support all overloads.

For me, the need was mainly a concern of adopting the best-practices for ASP.NET about the IHttpClientFactory. It will be useful to have the possibility to add global policies like retries, but it seems that Dapr will solve this early soon. So I think it's not a good idea to add code in the SDK that have high chance to be useless in the short term.

IMHO, the second sample you provide is nice. I personaly don't like to have http:// strings in the code.

For sure, adding this to documentation would be a great addition.

karoldeland avatar Mar 15 '21 22:03 karoldeland

Ok, thanks @karoldeland - with that resolved I'm going to close your PR and treat this as a docs issue. Feel free to discuss more if you have other ideas.

rynowak avatar Mar 19 '21 03:03 rynowak

@rynowak Are you looking for any help? I would love to start my open source journey with this as am really interested in Dapr project.

sameer106 avatar Nov 16 '21 10:11 sameer106

@rynowak I hope all is well.

I realize this thread is over a year old but I have a similar need..

I am trying to upgrade an existing GraphQL framework to run on Dapr.

Pre-Dapr Existing

In the Pre-Dapr configuration there is a GraphQL Gateway service, which uses Named Http Clients, via a HttpClientFactory to each HttpClient per each downstream service as follows...

Add Http Endpoints ...


 services.AddHttpClient(WellKnownSchemaNames.InventoryServices, c => c.BaseAddress = new Uri(configuration.GetConnectionString("InventoryServices"))).AddHeaderPropagation();
 services.AddHttpClient(WellKnownSchemaNames.OrderServices, c => c.BaseAddress = new Uri(configuration.GetConnectionString("OrderServices"))).AddHeaderPropagation();

Note : the AddHeaderPropagation() at the end of each HttpClient add, more on that in a second..

When a GraphQL query is issued to the Gateway it will craft up a query plan and invoke the necessary query(s) to each down-stream GraphQL service. It will execute each query through the specific, previously configured HttpClient as follows;

using HttpClient httpClient = _clientFactory.CreateClient(targetSchema);

Where the targetSchema in this example is either "OrderServices" or "InventoryServices".

image

Using-Dapr

When converting to Dapr, I will be running the Gateway and all downstream services via Dapr Side Cars and as such do not need the HttpClientFactory, as I will create the HttpClient in the following way, in which the targetSchema is the Dapr AppId....

using HttpClient httpClient = Dapr.Client.DaprClient.CreateInvokeHttpClient(targetSchema);

The solution would then look like this..

image

The above works great without applied security, however I am further utilizing Azure B2C to secure both the clients as well as the all services (including the Gateway).

As such, I would like to propagate the "authorization" header (JWT) that is received at the Gateway down to each downstream service.

Pre-Dapr I was using Microsoft.AspNetCore.HeaderPropagation and this works perfectly to propagate the "authorization" (JWT) from the client->gateway->service1/2/etc..

That said, you'll note that the way I was able to make that work was adding the .AddHeaderPropagation() when adding the client to the gateway.

Given that Darp Client is creating the HttpClient, I was expecting it would also propagate the Headers I configured to propagate but sadly no.

Is there a way to add the Dapr produced Http Client into the HttpClientFactory?

In that way I may be able to take advantage of Header propagation.

If not that way, can you please suggest another way?

Many thanks in advance, John

jkears avatar May 18 '22 21:05 jkears

@rynowak Actually I got it to work based upon your example....

namespace NextWare.Domain.Gateway.DependencyInjection
{
    public static class EndPointConfiguration
    {
        public static IServiceCollection AddEndPoints(this IServiceCollection services, ConfigurationManager configuration)
        {

            // Non-Dapr
            //services.AddHttpClient(WellKnownSchemaNames.OrderServices, c => c.BaseAddress = new Uri(configuration.GetConnectionString("OrderServices"))).AddHeaderPropagation();
            //services.AddHttpClient(WellKnownSchemaNames.InventoryServices, c => c.BaseAddress = new Uri(configuration.GetConnectionString("InventoryServices"))).AddHeaderPropagation();

            // With-Dapr
            services.AddHttpClient(WellKnownSchemaNames.OrderServices, c =>
            {
                c.BaseAddress = new Uri($"http://{WellKnownSchemaNames.OrderServices}");
            }).AddHttpMessageHandler(() => new InvocationHandler()).AddHeaderPropagation();

            services.AddHttpClient(WellKnownSchemaNames.InventoryServices, c =>
            {
                c.BaseAddress = new Uri($"http://{WellKnownSchemaNames.InventoryServices}");
            }).AddHttpMessageHandler(() => new InvocationHandler()).AddHeaderPropagation();
 
            return services;
        }
    }
}

Very simple so thank you!

jkears avatar May 19 '22 03:05 jkears