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

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

Open
karoldeland opened this issue Mar 15, 2021 · 6 comments
Open
Assignees
Labels
documentation Improvements or additions to documentation help wanted Extra attention is needed
Milestone

Comments

@karoldeland
Copy link

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.

@rynowak
Copy link
Contributor

rynowak commented Mar 15, 2021

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:

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"));
}

@karoldeland
Copy link
Author

karoldeland commented Mar 15, 2021

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.

@rynowak
Copy link
Contributor

rynowak commented Mar 19, 2021

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 rynowak added the documentation Improvements or additions to documentation label Mar 19, 2021
@rynowak rynowak added this to the v1.1 milestone Mar 19, 2021
@rynowak rynowak self-assigned this Mar 19, 2021
@rynowak rynowak removed this from the v1.1 milestone Mar 23, 2021
@rynowak rynowak added this to the Future milestone Apr 16, 2021
@rynowak rynowak added the help wanted Extra attention is needed label Apr 16, 2021
@sameer106
Copy link

@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.

@jkears
Copy link

jkears commented May 18, 2022

@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
Copy link

jkears commented May 19, 2022

@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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants