extensions icon indicating copy to clipboard operation
extensions copied to clipboard

[API Proposal]: add AddResiliencePipelineHandlerFromRegistry and AddResiliencePipelineHandler to Microsoft.Extensions.Http.Resilience

Open cbertolasio opened this issue 1 year ago • 1 comments

Background and motivation

In the aspnetcore extensions repo, in the Microsoft.Extensions.Http.Polly project there are extension methods for HttpClientBuilder so that you can use polly v7 constructs to append policies to an http client. The same methods have not been ported to use the polly v8 constructs in the Microsoft.Extensions.Http.Resilience package.

Are their any plans to port the following: public static IHttpClientBuilder AddPolicyHandler( this IHttpClientBuilder builder, Func<HttpRequestMessage, IAsyncPolicy<HttpResponseMessage>> policySelector)

and

public static IHttpClientBuilder AddPolicyHandlerFromRegistry( this IHttpClientBuilder builder, Func<IReadOnlyPolicyRegistry<string>, HttpRequestMessage, IAsyncPolicy<HttpResponseMessage>> policySelector)

along with the HttpRequestMessageExtensions so that we can use native polly v8 contructs from the Microsoft.Extensions.Http.Resilience package? At this point I have to use the sundry.extensions.http.polly library to bridge this gap.

Maybe there is a way to do what I need with existing libaries and I just cannot see how to do it with polly v8 constructs.

My main motivation for being able to do this is b/c I like to define a TransientHttpFaultPipeline, a StandardResiliencePipeline, and a NoOpPipeline and add them to a ResiliencePipelineRegistry and then lookup which pipeline to use based on the Http Verb being sent into the PolicySelector / PipelineSelector. I still need to be able to do noop on http post.

API Proposal

public static IHttpClientBuilder AddResiliencePipelineHandler(this IHttpClientBuilder builder, Func<IServiceProvider, HttpRequestMessage, ResiliencePipeline<HttpResponseMessage>> policySelector) {
// add implementation details in a polly v8 centric way
}

public static IHttpClientBuilder AddResiliencePipelineHandlerFromRegistry(this IHttpClientBuilder builder, Func<ResiliencePipelineRegistry<string>, HttpRequestMessage, ResiliencePipeline<HttpResponseMessage>> policySelector) {
// add implementation details in a polly v8 centric way
}

//TODO: port HttpRequestMessageExtensions so that we can get & set ResilienceContext on HttpRequestMessages if that is still a relevant construct for v8

API Usage

define pipeline names...

    public const string NoOpPipelineName = "NoOpPipeline";
    public const string StandardResiliencePipelineName = "StandardResiliencePipeline";

setup the registry, pipelines, & decorate the http clients...

 // add the resilience pipelines - somewhere in startup or some method that has access to IServiceCollection... :
 // Create (and register with DI) a resilience pipeline registry containing some strategies we want to use.
 // standard resilience pipeline
 services.AddResiliencePipeline<string, HttpResponseMessage>(StandardResiliencePipelineName, builder =>
  {
      var standardResilienceOptions = new HttpStandardResilienceOptions();

      builder.AddRateLimiter(standardResilienceOptions.RateLimiter)
          .AddTimeout(standardResilienceOptions.TotalRequestTimeout)  // 30s
          .AddRetry(standardResilienceOptions.Retry) // max 3
          .AddCircuitBreaker(standardResilienceOptions.CircuitBreaker)
          .AddTimeout(standardResilienceOptions.AttemptTimeout); // 10s
  });

// noop pipeline
  services.AddResiliencePipeline<string, HttpResponseMessage>(NoOpPipelineName, builder =>
  {
      builder.AddPipeline(ResiliencePipeline.Empty);
  });

// add resilience pipeline handlers to all your http clients
services.AddHttpClient<ResilientHttpClient>().AddResiliencePipelineHandler(PipelineSelector);
or alternately...
services.AddHttpClient<ResilientHttpClient>().AddResiliencePipelineHandlerFromRegistry(PipelineSelector);

create a PipelineSelector method that can be used to dynamically select the Pipeline based on the HttpVerb

  public static ResiliencePipeline<HttpResponseMessage> PipelineSelector(IServiceProvider serviceProvider, HttpRequestMessage httpRequestMessage)
  {
      var pipelineRegistry = serviceProvider.GetRequiredService<ResiliencePipelineRegistry<string>>();
      if (httpRequestMessage.Method == HttpMethod.Post)
      {
          return pipelineRegistry.GetPipeline<HttpResponseMessage>(NoOpPipelineName);
      }
      else
      {
          return pipelineRegistry.GetPipeline<HttpResponseMessage>(StandardResiliencePipelineName);
      }
  }

Alternative Designs

cannot think of any

Risks

this might not be relevant for v8? but it seems like it is?

there might already be a way to do this and I just cannot find it, but ive spent a solid 5 business days on it digging thru polly v8, simmy, micrososft.extensions.http.polly, and micrososft.extensions.http.resilience.

cbertolasio avatar Jan 19 '24 16:01 cbertolasio

This is duplicate of #4666, but I vote to keep this one as it has more details :)

Anyway, in #4858 we exposed ResilienceHandler so it's now relatively easy to support your scenarios:

services
    .AddHttpClient("my-client")
    .AddHttpMessageHandler(servicerProvider =>
    {
        return new ResilienceHandler(request => PipelineSelector(servicerProvider, request));
    });

Give how simple it is, I am thinking whether it makes sense to add these APIs?

FYI, the ResilienceHandler will be available in version 8.2.0 of Microsoft.Extensions.Http.Resilience package.

martintmk avatar Jan 20 '24 19:01 martintmk

Hi @geeknoid @martintmk ,

For #4858 , do we have plan to remove the Experimental attribute?

  • As I really love the direction and want/need to introduce it into our services for replacing some customized resilience handlers and standardize and modernize the code.
  • The case in our code is that there is one existing handlers list for creating http client. In the handler list, there is some existing customized retry handler. we want to replace it with Polly resilience.
  • Similar case for fault injection.

As I am not sure if the Experimental API could be introduced into production code. // Yes, I am working for/within Microsoft. This is not sensitive info I believe.

caigen avatar Nov 18 '24 06:11 caigen

/cc: @joperezr @dotnet/dotnet-extensions-resilience

RussKie avatar Nov 18 '24 07:11 RussKie

By removing the Experimental attribute we'll make the following public APIs stable:

// The class name will become "stable".
public class ResilienceHandler
{
    // The following two ctors will become "stable" too.
    public ResilienceHandler(Func<HttpRequestMessage, ResiliencePipeline<HttpResponseMessage>> pipelineProvider);
    public ResilienceHandler(ResiliencePipeline<HttpResponseMessage> pipeline);
}

These APIs look solid, and I think we can make them stable.

This issue is not directly related to making ResilienceHanlder stable. We'd better create a separate issue for that.

iliar-turdushev avatar Nov 18 '24 12:11 iliar-turdushev

Let's get these in, these are useful additions.

geeknoid avatar Nov 18 '24 16:11 geeknoid