arcus-azure / arcus.eventgrid

Azure Event Grid development in a breeze
https://eventgrid.arcus-azure.net/
MIT License
17 stars 6 forks source link

Inject service registered `IHttpClient` into EventGrid publisher instead of using our own #225

Closed stijnmoreels closed 2 years ago

stijnmoreels commented 2 years ago

Is your feature related to a problem? Please describe. The EventGridPublisher uses its own HttpClient when sending out HTTP POST requests that places events on the subscription.

What solution do you propose? In the context of service-to-service correlation, this could maybe be improved. With built-in HTTP message handlers, one could have automatic dependency ID addition to the HTTP headers on each of these send-out HTTP POST requests to Azure EventGrid.

Could be, yeah, but we didn't do that yet. 😄 Maybe that would make it better as we introduce HTTP message handlers that append the dependency ID automatically. 🤔

But yeah, we could open an issue for that to inject the IHttpClient, but this is not always used in a HTTP context, so not sure if that service dependency is always available, then.

_Originally posted by @stijnmoreels in https://github.com/arcus-azure/arcus.eventgrid/pull/224#discussion_r840520961_

stijnmoreels commented 2 years ago

As a way to simplify this, we could consider adding extensions so people are not obligated to add the .AddHttpClient to the dependency system. It would also simplify at adding our EventGrid publisher to the application. By the look of it, the publisher is mostly added to the dependency system anyway to inject it in the necessary parts of the application. Just look at the places where we used it in Arcus, both messaging and background jobs. It's all injected.

So, currently, when using the publishers, one has to do this:

services.AddSingleton(serviceProvider =>
{
    ILogger logger = serviceProvider = serviceProvider.GetRequiredService<ILogger<EventGridPublisher>>();
    return EventGridPublisherBuilder.WithTopic(..., logger)
                                    .UsingAuthenticationKey(...)
                                    .WithCircuitBreaker(...) // <- optional, like other stuff after auth key.
                                    .Build();
});

We can improve this like more complex systems in the ASP.NET Core and our messaging handlers are configured. Something like this:

services.AddEventGridPublishing(topicUrl, secretName)
        .WithCircuitBroker(...);

Internally, we can make use of the Arcus secret store and the IHttpClient.

We will need to use the Microsoft.Extensions.Http package for this. We could consider moving to the Microsoft.Extensions.Http.Polly package if we want to use those built-in stuff. But that's taking things to far, maybe, for now, as we should probably focus on .NET 6 support for now.

stijnmoreels commented 2 years ago

Maybe we can move this entirely to v3.3 so we don't block the release with a new feature.