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

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

Open
cbertolasio opened this issue Jan 19, 2024 · 1 comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-resilience

Comments

@cbertolasio
Copy link

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 cbertolasio added api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged labels Jan 19, 2024
@martintmk
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-resilience
Projects
None yet
Development

No branches or pull requests

3 participants