Allow changing the severity of resilience events

[martintmk]

Pull Request

The issue or feature being addressed

#1859

Details on the issue fix or feature implementation

This can be used to customize the severity of resilience events as:

services.AddResiliencePipeline("my-strategy", (builder, context) =>
{
    // Create a new instance of telemetry options by using copy-constructor of the global ones.
    // This ensures that common configuration is preserved.
    var telemetryOptions = new TelemetryOptions(context.GetOptions<TelemetryOptions>());

    telemetryOptions.SeverityProvider = ev =>
    {
        if (ev.EventName == "OnRetry")
        {
            // Decrease the severity of particular event.
            return ResilienceEventSeverity.Debug;
        }

        return ev.Severity;
    };

    builder.AddTimeout(TimeSpan.FromSeconds(1));

    // Override the telemetry configuration for this pipeline.
    builder.ConfigureTelemetry(telemetryOptions);
});

Confirm the following

• I started this PR by branching from the head of the default branch
• I have targeted the PR to merge into the default branch
• I have included unit tests for the issue/feature
• I have successfully run a local build

[martincostello]

Seems fine to me - just needs docs and tests!

[peter-csala]

I think if we would expose all the telemetry event name constants via a simple static class then the branching logic would safer. It would also help users of the API to use switch expression.

[martintmk]

It would require to expose a lot of public members for a limited audience. Let's wait and see. My vote would be to not expose anything unless absolutely necessary. Consumers of this API can just hardcode the strings, these won't change anyway as they are considered public contract as part of telemetry.

[martincostello]

In this specific case you could also do nameof(RetryStrategyOptions<>.OnRetry) (I think).

[peter-csala]

The nameof won't work for all cases. For example

• in case of rate limiter OnRateLimiterRejected event name vs OnRejected options property
• in case of circuit breaker OnCircuitHalfOpened event name vs OnHalfOpened options property
• in case of latency Chaos.OnLatency event name vs OnLatencyInjected options property
• etc.

[peter-csala]

It would require to expose a lot of public members for a limited audience. Let's wait and see. My vote would be to not expose anything unless absolutely necessary. Consumers of this API can just hardcode the strings, these won't change anyway as they are considered public contract as part of telemetry.

If you are saying that it is already implicitly part of the public contract then I don't see why it is a problem to make it explicit?

[martintmk]

Just more public area to cover :)

[martincostello]

We can consider adding it at a later date if this functionality becomes popular and there's user demand for it.

[wjrogers]

This would meet my needs. Thanks for the heads up!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.77%. Comparing base (b89ce23) to head (8178123).
Report is 19 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2072      +/-   ##
==========================================
+ Coverage   83.69%   83.77%   +0.07%     
==========================================
  Files         312      313       +1     
  Lines        7114     7148      +34     
  Branches     1054     1055       +1     
==========================================
+ Hits         5954     5988      +34     
  Misses        789      789              
  Partials      371      371              

Flag	Coverage Δ
linux	83.77% <100.00%> (+0.07%)	⬆️
macos	?
windows	83.77% <100.00%> (+0.07%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[peter-csala]

This example seems a bit pointless because

• you have a single Timeout strategy
• and you override the OnRetry telemetry event's severity

Can you either override the OnTimeout or add a Retry strategy to the pipeline?

[martintmk]

Done.

[martincostello]

@martintmk Are you planning on any other PRs anytime soon? If not, once this is merged we can do an 8.4.0 to release this and the other recent changes.

[martintmk]

@martintmk Are you planning on any other PRs anytime soon? If not, once this is merged we can do an 8.4.0 to release this and the other recent changes.

I am thinking about finalizing the nullability changes during the weekend and that one chaos outcome strategy bug where exceptions are ignored.

[pisethdanh]

Hi @martintmk,

Thank you for supporting this feature!

We currently configure Polly using the AddStandardResilienceHandler from the Microsoft.Extensions.Http.Resilience package:
e.g.

public static IHttpStandardResiliencePipelineBuilder AddResilienceStrategies(this IHttpClientBuilder builder) => builder
        .AddStandardResilienceHandler()
        .Configure((options, serviceProvider) =>
        {
            // configure ...
        });

We add the standard resilience strategies to each typed http client.

Is it possible to set the event severity with the above approach?

[martintmk]

You can configure global TelemetryOptions to configure the severity for all resilience pipelines.

var serviceCollection = new ServiceCollection()
    .Configure<TelemetryOptions>(options =>
    {
        // Configure options here
    });