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

Shared circuit breaker when registering a resilience handler with ConfigureHttpClientDefaults #5021

Open
sveinungf opened this issue Mar 12, 2024 · 5 comments
Assignees
Labels
area-resilience bug This issue describes a behavior which is not expected - a bug. untriaged work in progress 🚧

Comments

@sveinungf
Copy link

Description

When registering the standard resilience handler using builder.Services.ConfigureHttpClientDefaults(x => x.AddStandardResilienceHandler), all HttpClients seem to be using a shared circuit breaker. When one client is causing the circuit to open, the circuit opens for all clients.

Reproduction Steps

Here is my project file:

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <Nullable>enable</Nullable>
    <ImplicitUsings>enable</ImplicitUsings>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Http.Resilience" Version="8.2.0" />
  </ItemGroup>

</Project>

Here is my appsettings.json (modified to be able to more easily reproduce the issue):

{
  "RetryOptions": {
    "Retry": {
      "Delay": "00:00:00",
      "MaxRetryAttempts": 1
    }
  }
}

And here is the Program.cs:

var builder = WebApplication.CreateBuilder(args);

var section = builder.Configuration.GetSection("RetryOptions");

var succeedingClientBuilder = builder.Services.AddHttpClient<SucceedingClient>(x =>
    x.BaseAddress = new Uri("https://ipv4.icanhazip.com/"));

var failingClientBuilder = builder.Services.AddHttpClient<FailingClient>(x =>
    x.BaseAddress = new Uri("http://the-internet.herokuapp.com/status_codes/500"));

var useHttpClientDefaults = true;
if (useHttpClientDefaults)
{
    builder.Services.ConfigureHttpClientDefaults(x => x.AddStandardResilienceHandler(section));
}
else
{
    succeedingClientBuilder.AddStandardResilienceHandler(section);
    failingClientBuilder.AddStandardResilienceHandler(section);
}

var app = builder.Build();

app.MapGet("/test", async (SucceedingClient succeedingClient, FailingClient failingClient) =>
{
    var succeedingClientResult = await succeedingClient.Get();
    var failingClientResult = await failingClient.Get();

    return $"""
        SucceedingClient result: {succeedingClientResult}
        FailingClient result: {failingClientResult}
        """;
});

app.Run();

public class SucceedingClient(HttpClient client)
{
    public async Task<string> Get()
    {
        try
        {
            using var response = await client.GetAsync("");
            return $"Status code {response.StatusCode}";
        }
        catch (Exception e)
        {
            return $"Exception {e.Message}";
        }
    }
}

public class FailingClient(HttpClient client)
{
    public async Task<string> Get()
    {
        try
        {
            using var response = await client.GetAsync("");
            return $"Status code {response.StatusCode}";
        }
        catch (Exception e)
        {
            return $"Exception {e.Message}";
        }
    }
}

Run this sample code and continuously hit the /test endpoint until the circuit breaker kicks in.

Expected behavior

I would expect the circuit would only open for the client that is facing issues. From the reproduction steps, the circuit should not open for SucceedingClient when it opens for FailingClient.

Actual behavior

The circuit breaker opens for both the SucceedingClient and the FailingClient, even though only FailingClient is receiving status code 500.

Regression?

No response

Known Workarounds

Use AddStandardResilienceHandler on each HttpClient instead of using ConfigureHttpClientDefaults.

Configuration

dotnet --info .NET SDK: Version: 8.0.200 Commit: 438cab6a9d Workload version: 8.0.200-manifests.e575128c

Runtime Environment:
OS Name: Windows
OS Version: 10.0.22631
OS Platform: Windows
RID: win-x64
Base Path: C:\Program Files\dotnet\sdk\8.0.200\

.NET workloads installed:
There are no installed workloads to display.

Host:
Version: 8.0.2
Architecture: x64
Commit: 1381d5ebd2

.NET SDKs installed:
8.0.200 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
Microsoft.AspNetCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.27 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.16 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 8.0.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.27 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.16 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 8.0.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.27 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 7.0.16 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 8.0.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
x86 [C:\Program Files (x86)\dotnet]
registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
Not set

global.json file:
Not found

Other information

No response

@sveinungf sveinungf added bug This issue describes a behavior which is not expected - a bug. untriaged labels Mar 12, 2024
@geeknoid
Copy link
Member

@martintmk Do you have time to take a look please?

@iliar-turdushev
Copy link
Contributor

The issue occurs because ConfigureHttpClientDefaults(x => x.AddStandardResilienceHandler()) registers a single resilience pipeline with a name standard that is then shared by both SucceedingClient and FailingClient. As a result, they both share the circuit breaker of the standard pipeline.

When we use two separate statements to register the resilience pipeline

succeedingClientBuilder.AddStandardResilienceHandler(section)
failingClientBuilder.AddStandardResilienceHandler(section)

we create two distinct resilience pipelines named SucceedingClient-standard and FailingClient-standard, and each of them has its own circuit breaker. Each of these pipelines is then used by the respective HttpClients.

The reason for that is how AddStandardResilienceHandler() and ConfigureHttpClientDefaults() methods work. AddStandardResilienceHandler() relies on IHttpClientBuilder.Name when registering and resolving the corresponding resilience pipeline. Whenever you call AddStandardResilienceHandler() you register a resilience pipeline for the given IHttpClientBuilder.Name, and when you create an HttpClient the corresponding resilience pipeline is retrieved by the name or type of the HttpClient, i.e. by the given IHttpClientBuilder.Name. ConfigureHttpClientDefaults() executes its lambda x => x.AddStandardResilienceHandler() only once, and the x argument is an IHttpClienBuilder with the Name=null, i.e. it doesn't have information about the actual name of HttpClient. It doesn't execute the lambda for each registered HttpClient. As a result, we have a single resilience pipeline (and hence a single shared circuit breaker) which is applied to all HttpClients regardless of their name or type.

@JamesNK Can ConfigureHttpClientDefaults(Action<IHttpClientBuilder> configure) be updated to provide the actual name/type of the HttpClient being resolved? As I understand, currently its configure action is executed immediately when ConfigureHttpClientDefaults() is invoked, therefore it doesn't have the actual Name of HttpClient. And to be able to provide the Name it should be updated to execute the configure action when the actual HttpClient is being resolved/created, i.e. when there is information about its type/name. Can we update ConfigureHttpClientDefaults() in a such way?

Workaround

@sveinungf As a workaround, you can configure the standard resilience pipeline to be applied per HTTP request authority:

builder.Services.ConfigureHttpClientDefaults(x => x
    .AddStandardResilienceHandler(section)
    .SelectPipelineByAuthority());

If you do so, you'll have distinct resilience pipelines per each HTTP request authority. It'll solve the shared circuit breaker issue. You can even specify a custom criteria how to select resilience pipelines using the method SelectPipelineBy.

@JamesNK
Copy link
Member

JamesNK commented Apr 30, 2024

@JamesNK Can ConfigureHttpClientDefaults(Action configure) be updated to provide the actual name/type of the HttpClient being resolved?

IHttpClientBuilder has a name property on it. Have you tried using that value?

This feature lives in dotnet/runtime - https://github.com/dotnet/runtime/blob/55c904024601c133f8ad081bc704c3c1fc5c7c9b/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientFactoryServiceCollectionExtensions.cs#L80. I don't work on the HTTP client team and I don't have time to investigate more. I suggest creating an issue there to talk about it with the HTTP client folks.

@iliar-turdushev
Copy link
Contributor

IHttpClientBuilder has a name property on it. Have you tried using that value?

Yes, I have. IHttpClientBuilder.Name is always null.

I don't work on the HTTP client team and I don't have time to investigate more. I suggest creating an issue there to talk about it with the HTTP client folks.

Got it. I noticed that it was you who proposed the API #87914 and then implemented it #87953, therefore I mentioned you in the discussion and asked for help. Sure, I'll create an issue and start discussion with HTTP client folks. Thank you.

@iliar-turdushev
Copy link
Contributor

Here is an issue in dotnet/runtime to discuss ConfigureHttpClientDefaults.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-resilience bug This issue describes a behavior which is not expected - a bug. untriaged work in progress 🚧
Projects
None yet
Development

No branches or pull requests

4 participants