-
Notifications
You must be signed in to change notification settings - Fork 755
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
Drop FailureResultContext #4600
Conversation
...Libraries/Microsoft.Extensions.Http.Resilience/Resilience/Internal/HttpResilienceTagNames.cs
Outdated
Show resolved
Hide resolved
cc @joperezr |
…ilureresultcontext
@@ -20,7 +19,7 @@ | |||
|
|||
<PropertyGroup> | |||
<Stage>normal</Stage> | |||
<MinCodeCoverage>98</MinCodeCoverage> | |||
<MinCodeCoverage>97</MinCodeCoverage> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be at 100, but code coverage is reporting lower number :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When going over #4482 I realized that
FailureResultContext
is something that is not required with Polly v8.The idea behind
FailureResultContext
was to allow to customize the metering dimensions based on the result type. However, Polly v8 supports telemetry enrichment natively and this API feels unnecessary now.Instead of using
FailureResultContext
, the consumer of Polly can just register their ownMeteringEnricher
and use it to extract the result and add whatever dimensions they want.Microsoft Reviewers: Open in CodeFlow