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

Feature Request: Customize Polly policies #264

Closed
mjrousos opened this issue Mar 7, 2018 · 9 comments
Closed

Feature Request: Customize Polly policies #264

mjrousos opened this issue Mar 7, 2018 · 9 comments
Labels
feature A new feature good first issue Should be pretty easy to do help wanted Not actively being worked on. If you plan to contribute, please drop a note. medium effort Likely a few days of development effort

Comments

@mjrousos
Copy link

mjrousos commented Mar 7, 2018

Feature Request

This isn't urgent, but it would be great if we could somehow customize Polly policies used for some reroutes. As examples, I'd like to be able to add a retry policy or change the callback used when the circuit breaker opens.

It would be easy to add my own policies in a DelgateHandler but then they would apply to all reroutes (which is undesirable since one bad API would open the circuit breaker for all).

Perhaps there's a way we could inject Policy[] via DI to override the defaults?

@TomPallister
Copy link
Member

@mjrousos Yeh this is a useful feature. I will have a look ASAP.

@TomPallister TomPallister added feature A new feature help wanted Not actively being worked on. If you plan to contribute, please drop a note. good first issue Should be pretty easy to do medium effort Likely a few days of development effort labels Mar 7, 2018
@TomPallister
Copy link
Member

TomPallister commented Mar 9, 2018

Maybe we want ReRoute specific delegating handlers, do you agree? I had this functionality requested before.

So we need some way of mapping ReRoutes to handlers, the easiest way to do this would be to have an array of delegating handlers on the ReRoute in configuration.json.

Maybe on the ReRoute we have

"Handlers": [
    "Tracer",
    "PollyOne"
]

then in Ocelot

services.AddOcelot()
    .AddDelegatingHandler<PollyOne>()
    .AddDelegatingHandler<Tracer>()
    .AddDelegatingHandler<Foo>();

When the http client is built for the ReRoute we pick out the delegating handlers that matched the string in json.

Seems simple enough!

TomPallister pushed a commit that referenced this issue Mar 9, 2018
@mjrousos
Copy link
Author

mjrousos commented Mar 9, 2018

Yea, that would do the trick. In this model, I could put whatever custom Polly policies I want in a delegating handler and then use it with particular reroutes. A couple concerns, though -

  • Some delegating handlers would still be useful to apply globally, so we may need a way to specify in AddDelegatingHandler if the handler should always apply or if it should only reply when requested.
  • In the Polly case that I'm thinking of specifically, I may want to create one delegating handler with a circuit breaker, for example, and request it on multiple reroutes via "Handlers": [] but I won't want that circuit breaker opening for one of the reroutes to affect the other. So, it will be good if each reroute that uses a delegating handler could get its own instance (so that they can have separate instances of policies or other local state).

@TomPallister
Copy link
Member

Cool sounds like a plan...I’ll make sure each reroute gets its own!

TomPallister pushed a commit that referenced this issue Mar 9, 2018
TomPallister added a commit that referenced this issue Mar 10, 2018
TomPallister added a commit that referenced this issue Mar 10, 2018
* #264 added handlers to config

* #264 added global handlers object and defaut param for method, not sure this is correct api for users yet

* #264 Can now add all sorts of delegating handlers in all sorts of ways

* +semver: breaking #264
@TomPallister
Copy link
Member

@mjrousos OK ive just pushed version 5.0.0 that has changes that should let you do all of the things we talked about (hopefully). Let me know if this isn't the case.

Updated docs here hopefully they make sense.

TomPallister added a commit that referenced this issue Mar 10, 2018
@mjrousos
Copy link
Author

mjrousos commented Mar 10, 2018

Awesome; thanks! I'll check it out and let you know. Thanks for the quick turn-around!

@Cristiano7Zhang
Copy link

Is it compatible with core 2.x?

@david-sackstein
Copy link

Is it possible to get the context of the request in the delegating handler?
I need this in order to enrich the request from information that I extract from the HttpContext.User

@raman-m
Copy link
Member

raman-m commented Jul 7, 2023

@david-sackstein commented on Nov 22, 2021

Is not it too late? 😃
Use this coding recipe to have HttpContext in your custom delegating handler:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature good first issue Should be pretty easy to do help wanted Not actively being worked on. If you plan to contribute, please drop a note. medium effort Likely a few days of development effort
Projects
None yet
Development

No branches or pull requests

5 participants