feat: Add support to Polly Resilience Pipeline#3677
Conversation
# Conflicts: # src/Paramore.Brighter.Extensions.DependencyInjection/BrighterOptions.cs # src/Paramore.Brighter/CommandProcessorBuilder.cs
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
Gates Failed
(1 file with Large Method)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| ControlBusReceiverBuilder.cs | 1 advisory rule | 8.72 → 8.72 | Suppress |
Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
There was a problem hiding this comment.
Gates Failed
(1 file with Large Method)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| ControlBusReceiverBuilder.cs | 1 advisory rule | 8.72 → 8.72 | Suppress |
Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
lillo42
requested review from
DevJonny,
holytshirt,
iancooper and
preardon
as code owners
lillo42
added
3 - Done
Breaking Change
v10
.NET
iancooper
left a comment
iancooper
left a comment
There was a problem hiding this comment.
Thanks for this, it will be great to fix this in V10.
- I wonder about having both DefaultPolicy and DefaultResiliencePolicy on the HostBuilder extensions, maybe we just need the new one, and call the old one inside>
- I think we should replace our internal uses of Polly with the Resilience Builder. I think that is mainly in OutboxProducerMediator, though it may have crept into some other places. Our internal code should move on, and no one externally depends on that.
- We may want to mark UsePolicy as obsolete in the next version; it would be good to clean up and drop it by then.
There was a problem hiding this comment.
(1 files improve in Code Health)
Gates Failed
(1 hotspot with Large Method)
(1 file with Large Method)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| ControlBusReceiverBuilder.cs | 1 rule in this hotspot | 8.72 → 8.72 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| ControlBusReceiverBuilder.cs | 1 advisory rule | 8.72 → 8.72 | Suppress |
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| OutboxProducerMediator.cs | 6.95 → 7.16 | Deep, Nested Complexity |
Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
slang25
left a comment
slang25
left a comment
There was a problem hiding this comment.
By complete coincidence, I made a start on a similar change this morning, and then found this PR 😅
I would expect that this change also removes the "old" non-core approach, so the end result would be only a reference to Polly.Core, it would be more of an upgrade burden for folks, but I think it keeps things more simple.
As we use an attribute to indicate what Middleware you want, it's easier for us to declare that Obsolete and indicate that we will drop in V11, and then add an attribute for the new Middleware that adopts upgraded Polly policies. That gives folks time to be aware change is coming, and then shift. I agree that internally to Brighter, though, so not in a middleware pipeline but where the framework uses it for resilience (such as producers etc.) we should move to the new approach. We should only keep the old approach for end users |
There was a problem hiding this comment.
(1 files improve in Code Health)
Gates Failed
(2 hotspots with Complex Method, Large Method)
(3 files with Complex Method, Large Method, Constructor Over-Injection)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| ServiceCollectionExtensions.cs | 1 rule in this hotspot | 9.13 → 9.10 | Suppress |
| ControlBusReceiverBuilder.cs | 1 rule in this hotspot | 8.72 → 8.70 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| ServiceCollectionBrighterBuilder.cs | 1 advisory rule | 9.39 → 9.10 | Suppress |
| ServiceCollectionExtensions.cs | 1 advisory rule | 9.13 → 9.10 | Suppress |
| ControlBusReceiverBuilder.cs | 1 advisory rule | 8.72 → 8.70 | Suppress |
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| OutboxProducerMediator.cs | 6.95 → 7.16 | Deep, Nested Complexity |
Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
| brighterBuilder.Services.TryAddSingleton<IUseRpc>(new UseRpc(busConfiguration.UseRpc, busConfiguration.ReplyQueueSubscriptions!)); | ||
|
|
||
| brighterBuilder.Services.TryAddSingleton<IAmProducersConfiguration>(busConfiguration); | ||
| brighterBuilder.ResiliencePolicyRegistry ??= new ResiliencePipelineRegistry<string>().AddBrighterDefault(); |
There was a problem hiding this comment.
❌ Getting worse: Complex Method
AddProducers increases in cyclomatic complexity from 21 to 22, threshold = 9
| IPolicyRegistry<string>? policyRegistry = null, | ||
| ResiliencePipelineRegistry<string>? resiliencePipelineRegistry = null) |
There was a problem hiding this comment.
❌ New issue: Constructor Over-Injection
ServiceCollectionBrighterBuilder has 6 arguments, threshold = 5
There was a problem hiding this comment.
(1 files improve in Code Health)
Gates Failed
(2 hotspots with Complex Method, Large Method)
(3 files with Complex Method, Large Method, Constructor Over-Injection)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| ServiceCollectionExtensions.cs | 1 rule in this hotspot | 9.13 → 9.10 | Suppress |
| ControlBusReceiverBuilder.cs | 1 rule in this hotspot | 8.72 → 8.70 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| ServiceCollectionBrighterBuilder.cs | 1 advisory rule | 9.39 → 9.10 | Suppress |
| ServiceCollectionExtensions.cs | 1 advisory rule | 9.13 → 9.10 | Suppress |
| ControlBusReceiverBuilder.cs | 1 advisory rule | 8.72 → 8.70 | Suppress |
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| OutboxProducerMediator.cs | 6.95 → 7.16 | Deep, Nested Complexity |
Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
# Conflicts: # src/Paramore.Brighter/OutboxProducerMediator.cs
There was a problem hiding this comment.
Gates Failed
(2 hotspots with Complex Method, Large Method)
(3 files with Complex Method, Large Method, Constructor Over-Injection)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| ServiceCollectionExtensions.cs | 1 rule in this hotspot | 9.13 → 9.10 | Suppress |
| ControlBusReceiverBuilder.cs | 1 rule in this hotspot | 8.72 → 8.70 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| ServiceCollectionBrighterBuilder.cs | 1 advisory rule | 9.39 → 9.10 | Suppress |
| ServiceCollectionExtensions.cs | 1 advisory rule | 9.13 → 9.10 | Suppress |
| ControlBusReceiverBuilder.cs | 1 advisory rule | 8.72 → 8.70 | Suppress |
Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
|
|
||
| var policyRegistry = new PolicyRegistry | ||
| { | ||
| #pragma warning disable CS0618 // Type or member is obsolete |
There was a problem hiding this comment.
❌ Getting worse: Large Method
Build increases from 76 to 80 lines of code, threshold = 70
There was a problem hiding this comment.
Gates Failed
(2 hotspots with Complex Method, Large Method)
(3 files with Complex Method, Large Method, Constructor Over-Injection)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| ServiceCollectionExtensions.cs | 1 rule in this hotspot | 9.13 → 9.10 | Suppress |
| ControlBusReceiverBuilder.cs | 1 rule in this hotspot | 8.72 → 8.70 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| ServiceCollectionBrighterBuilder.cs | 1 advisory rule | 9.39 → 9.10 | Suppress |
| ServiceCollectionExtensions.cs | 1 advisory rule | 9.13 → 9.10 | Suppress |
| ControlBusReceiverBuilder.cs | 1 advisory rule | 8.72 → 8.70 | Suppress |
Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
# Conflicts: # tests/Paramore.Brighter.Core.Tests/CommandProcessors/Post/When_Posting_A_Message_And_There_Is_No_Message_Transformer.cs # tests/Paramore.Brighter.Core.Tests/CommandProcessors/Scheduler/When_Scheduling_A_Message_To_The_Command_Processor_Async.cs # tests/Paramore.Brighter.Core.Tests/Observability/CommandProcessor/Clear/When_Clearing_A_Message_A_Span_Is_Exported.cs
There was a problem hiding this comment.
Gates Failed
(2 hotspots with Complex Method, Large Method)
(3 files with Complex Method, Large Method, Constructor Over-Injection)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| ServiceCollectionExtensions.cs | 1 rule in this hotspot | 9.13 → 9.10 | Suppress |
| ControlBusReceiverBuilder.cs | 1 rule in this hotspot | 8.72 → 8.70 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| ServiceCollectionBrighterBuilder.cs | 1 advisory rule | 9.39 → 9.10 | Suppress |
| ServiceCollectionExtensions.cs | 1 advisory rule | 9.13 → 9.10 | Suppress |
| ControlBusReceiverBuilder.cs | 1 advisory rule | 8.72 → 8.70 | Suppress |
Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
| /// Register that policy with your <see cref="IPolicyRegistry{TKey}"/> such as <see cref="PolicyRegistry"/> | ||
| /// You can use this an identifier for you own policies, if your generic policy is the same as your Work Queue policy. | ||
| /// </summary> | ||
| [Obsolete("Migrate to Resilience Pipeline with OutboxProducer", error: false)] |
There was a problem hiding this comment.
Not a problem but I expect we tell users to migrate to the resilience pipeline, and keep our strings for internal usage
# Conflicts: # src/Paramore.Brighter/OutboxProducerMediator.cs
There was a problem hiding this comment.
Gates Failed
(2 hotspots with Complex Method, Large Method)
(3 files with Complex Method, Large Method, Constructor Over-Injection)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| ServiceCollectionExtensions.cs | 1 rule in this hotspot | 9.13 → 9.10 | Suppress |
| ControlBusReceiverBuilder.cs | 1 rule in this hotspot | 8.72 → 8.70 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| ServiceCollectionBrighterBuilder.cs | 1 advisory rule | 9.39 → 9.10 | Suppress |
| ServiceCollectionExtensions.cs | 1 advisory rule | 9.13 → 9.10 | Suppress |
| ControlBusReceiverBuilder.cs | 1 advisory rule | 8.72 → 8.70 | Suppress |
Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
|
|
||
| else | ||
| { | ||
| await resiliencePipeline.ExecuteAsync(async _ => await send(cancellationToken), cancellationToken) |
There was a problem hiding this comment.
You should pass through the cancellation token from the delegate arguments, rather than re-use cancellationToken in the closure, as the token might be different (e.g. combined with a token internal to Polly for handling timeouts).
For example:
await resiliencePipeline.ExecuteAsync(async token => await send(token), cancellationToken)
* Add Resilience Pipeline * fix unit tests * add unit test * fix build * Add more unit tests * fix build * feat: Use Resilience Pipeline internally * fix: unit tests * feat: update comment * feat: Rename register name * rename method * fix pipeline and add obsolete attribute * feat: mark more property as Obsolete and fix unit tests * fix: build * Improve outbox producer * fix: build * fix: OutboxProducerMediator.cs build
4 participants
Add support to Polly Resilience Pipeline via a new attribute
UseResiliencePipelineAttributeandUseResiliencePipelineAsyncAttribute)IRequestContextto have support toResiliencePipelineandResilienceContextTimeoutPolicyAttributeas deprecatedThis PR should cover this GH Issues:
#2880
#1197
#1065