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]: Enumerate policies in ResiliencePipelineRegistry #1828

Closed
IGx89 opened this issue Nov 28, 2023 · 2 comments
Closed

[Feature request]: Enumerate policies in ResiliencePipelineRegistry #1828

IGx89 opened this issue Nov 28, 2023 · 2 comments

Comments

@IGx89
Copy link

IGx89 commented Nov 28, 2023

Is your feature request related to a specific problem? Or an existing feature?

Hello! In Polly v7 we have health checks in our APIs that enumerate over policies in IReadOnlyPolicyRegistry, find the circuit breaker policies, and alert if any are not open. I'm working on porting that code to Polly.Core v8 and have run into a roadblock: ResiliencePipelineRegistry doesn't implement IEnumerable nor provides any way to get a list of the pipelines. Was that removed deliberately? If not, could that functionality be added back?

If my use case above makes no sense anymore in v8, please let me know too! The migration docs were silent on this topic.

Describe the solution you'd like

ResiliencePipelineRegistry implements IEnumerable or at least adds a GetPipelines() method or similar.

Additional context

No response

@martintmk
Copy link
Contributor

martintmk commented Nov 29, 2023

Hey @IGx89

In Polly V8 there is no way to retrieve the circuit breaker state from an instance of ResiliencePipeline. Even if enumerating pipeline from registry was available it wouldn't help.

What you want to do is to keep registry of CircuitBreakerStateProvider as you are creating the pipelines. See this test that demonstrates your use-case:

// Arrange
var states = new ConcurrentBag<CircuitBreakerStateProvider>();
using var registry = new ResiliencePipelineRegistry<string>();

// Act
_ = registry.GetOrAddPipeline("A", builder =>
{
    var stateProvider = new CircuitBreakerStateProvider();
    builder.AddCircuitBreaker(new() { StateProvider = stateProvider });
    states.Add(stateProvider);
});

_ = registry.GetOrAddPipeline("B", builder =>
{
    var stateProvider = new CircuitBreakerStateProvider();
    builder.AddCircuitBreaker(new() { StateProvider = stateProvider });
    states.Add(stateProvider);
});

_ = registry.TryAddBuilder("C", (builder, _) =>
{
    var stateProvider = new CircuitBreakerStateProvider();
    builder.AddCircuitBreaker(new() { StateProvider = stateProvider });
    states.Add(stateProvider);
});

// Assert
states.Should().HaveCount(2);
registry.GetPipeline("C");
states.Should().HaveCount(3);

foreach (var state in states)
{
    state.CircuitState.Should().Be(CircuitState.Closed);
}

doesn't implement IEnumerable nor provides any way to get a list of the pipelines. Was that removed deliberately? If not, could that functionality be added back?

This was deliberate choice due to dynamic nature of registry. The pipelines are not explicitly added, instead these are created on demand when requested. Additionally, with the minimal nature of ResiliencePipeline there is now way to retrieve any information about strategies from it.

Also created a PR that adds this test: #1829

@IGx89
Copy link
Author

IGx89 commented Nov 29, 2023

Thanks a lot for the detailed explanation and code example! That's very helpful. I definitely have a lot more learning to do it looks :)

@IGx89 IGx89 closed this as completed Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants