#1875 Use Polly v8 syntax (#1914)

* #1844 More open customization of Polly use

* First step: implementation done, The UTs have yet to be determined

* reverted changes in PollyPoliciesDelegatingHandlerTests
created tests for  PollyResiliencePipelineDelegatingHandler
created tests for PollyQoSResiliencePipelineProvider

* renaming variables for better clarity in PollyQoSResiliencePipelineProviderTests

* The most significant changes involve updates to the `OcelotBuilderExtensions` and `PollyQoSResiliencePipelineProvider` classes, as well as modifications to the `PollyQoSTests` class.

In the `OcelotBuilderExtensions` class, the `GetDelegatingHandler` method is now used instead of `GetDelegatingHandlerV7` in the `AddPolly` methods. The `AddPolly` method without parameters has been simplified using a lambda expression. The `GetDelegatingHandler` method now returns a new instance of `PollyResiliencePipelineDelegatingHandler` instead of `PollyPoliciesDelegatingHandler`.

In the `PollyQoSResiliencePipelineProvider` class, a comment has been added suggesting the use of `ResiliencePipelineRegistry<TKey>.GetOrAddPipeline` due to its thread-safe nature. The `MinimumThroughput` property in `circuitBreakerStrategyOptions` now uses the value from `route.QosOptions.ExceptionsAllowedBeforeBreaking`.

In the `PollyQoSTests` class, the `QoSOptions` used in the test configurations have been updated. Two tests, `Should_open_circuit_breaker_then_close` and `Open_circuit_should_not_effect_different_route`, have been modified to repeat the same request as the minimum `ExceptionsAllowedBeforeBreaking` is now 2. The `GivenThereIsAPossiblyBrokenServiceRunningOn` method now delays for 2.1 seconds when the request count is 2, to ensure the circuit is open.

Changes:

1. `OcelotBuilderExtensions` class updated to use `GetDelegatingHandler` method.
2. `AddPolly` methods in `OcelotBuilderExtensions` class simplified.
3. `GetDelegatingHandler` method in `OcelotBuilderExtensions` class now returns a new instance of `PollyResiliencePipelineDelegatingHandler`.
4. Comment added in `PollyQoSResiliencePipelineProvider` class suggesting the use of `ResiliencePipelineRegistry<TKey>.GetOrAddPipeline`.
5. `MinimumThroughput` property in `circuitBreakerStrategyOptions` updated to use value from `route.QosOptions.ExceptionsAllowedBeforeBreaking`.
6. `QoSOptions` in `PollyQoSTests` class updated.
7. `Should_open_circuit_breaker_then_close` and `Open_circuit_should_not_effect_different_route` tests in `PollyQoSTests` class updated.
8. `GivenThereIsAPossiblyBrokenServiceRunningOn` method in `PollyQoSTests` class updated to delay for 2.1 seconds when request count is 2.

* end of implementation

* remove "Microsoft.VisualStudio.Azure.Containers.Tools.Targets" packages in sample

* Use `ResiliencePipelineRegistry` in `PollyQoSResiliencePipelineProvider`. This new implementation replaces the previous dictionary-based approach for managing cache for resilience pipelines.

* fix conflict

* end of merge of develop (+polly package uptdate)

* fix polly update

* #1844 More open customization of Polly use

* First step: implementation done, The UTs have yet to be determined

* reverted changes in PollyPoliciesDelegatingHandlerTests
created tests for  PollyResiliencePipelineDelegatingHandler
created tests for PollyQoSResiliencePipelineProvider

* renaming variables for better clarity in PollyQoSResiliencePipelineProviderTests

* The most significant changes involve updates to the `OcelotBuilderExtensions` and `PollyQoSResiliencePipelineProvider` classes, as well as modifications to the `PollyQoSTests` class.

In the `OcelotBuilderExtensions` class, the `GetDelegatingHandler` method is now used instead of `GetDelegatingHandlerV7` in the `AddPolly` methods. The `AddPolly` method without parameters has been simplified using a lambda expression. The `GetDelegatingHandler` method now returns a new instance of `PollyResiliencePipelineDelegatingHandler` instead of `PollyPoliciesDelegatingHandler`.

In the `PollyQoSResiliencePipelineProvider` class, a comment has been added suggesting the use of `ResiliencePipelineRegistry<TKey>.GetOrAddPipeline` due to its thread-safe nature. The `MinimumThroughput` property in `circuitBreakerStrategyOptions` now uses the value from `route.QosOptions.ExceptionsAllowedBeforeBreaking`.

In the `PollyQoSTests` class, the `QoSOptions` used in the test configurations have been updated. Two tests, `Should_open_circuit_breaker_then_close` and `Open_circuit_should_not_effect_different_route`, have been modified to repeat the same request as the minimum `ExceptionsAllowedBeforeBreaking` is now 2. The `GivenThereIsAPossiblyBrokenServiceRunningOn` method now delays for 2.1 seconds when the request count is 2, to ensure the circuit is open.

Changes:

1. `OcelotBuilderExtensions` class updated to use `GetDelegatingHandler` method.
2. `AddPolly` methods in `OcelotBuilderExtensions` class simplified.
3. `GetDelegatingHandler` method in `OcelotBuilderExtensions` class now returns a new instance of `PollyResiliencePipelineDelegatingHandler`.
4. Comment added in `PollyQoSResiliencePipelineProvider` class suggesting the use of `ResiliencePipelineRegistry<TKey>.GetOrAddPipeline`.
5. `MinimumThroughput` property in `circuitBreakerStrategyOptions` updated to use value from `route.QosOptions.ExceptionsAllowedBeforeBreaking`.
6. `QoSOptions` in `PollyQoSTests` class updated.
7. `Should_open_circuit_breaker_then_close` and `Open_circuit_should_not_effect_different_route` tests in `PollyQoSTests` class updated.
8. `GivenThereIsAPossiblyBrokenServiceRunningOn` method in `PollyQoSTests` class updated to delay for 2.1 seconds when request count is 2.

* Use `ResiliencePipelineRegistry` in `PollyQoSResiliencePipelineProvider`. This new implementation replaces the previous dictionary-based approach for managing cache for resilience pipelines.

* end of implementation

* remove "Microsoft.VisualStudio.Azure.Containers.Tools.Targets" packages in sample

* fix conflict

* end of merge of develop (+polly package uptdate)

* fix polly update

* renaming

* fix compile error

* The most significant changes involve the modification of the `errorMapping` type from `Dictionary<Type, Func<Exception, Error>>` to `IDictionary<Type, Func<Exception, Error>>` across multiple files. This change is aimed at enhancing code flexibility and testability. Additionally, the `AddPolly<T>` and `AddPollyV7<T>` methods in `OcelotBuilderExtensions.cs` have been updated to accept `IDictionary` instead of `Dictionary`.

Here are the changes in detail:

1. The type of `errorMapping` was changed from `Dictionary<Type, Func<Exception, Error>>` to `IDictionary<Type, Func<Exception, Error>>` in several places across multiple files to make the code more flexible and easier to test.
2. The `AddPolly<T>` and `AddPollyV7<T>` methods in `OcelotBuilderExtensions.cs` were updated to accept `IDictionary` instead of `Dictionary`.
3. The `_mappers` field and the corresponding assignment in the constructor in `HttpExceptionToErrorMapper.cs` were updated to use `IDictionary`.
4. The `WithExceptionsAllowedBeforeBreaking` and `WithTimeoutValue` values in `OcelotBuilderExtensionsTests.cs` and `PollyQoSResiliencePipelineProviderTests.cs` were updated.
5. The `errorMapping` variable in `HttpExceptionToErrorMapperTests.cs` was updated to use `IDictionary`.

* try to fix UTs on CircleCI

* try another settings

* one more test

* post code reveiew and develop merge

* Quick code review by @raman-m

* Re-add lost XML docs from commit 0404c24
Complete docs

* Update Polly version

* Update Polly to 8.3.1

* Code review by @raman-m

* Arrange file layout in a separate directory (V7)
Modify Obsolete attribute texts
Add note in doc

* Update qualityofservice.rst

Update docs according to the Polly v8 implementations and QoS design upgrade

* More XML docs

* Make the base class abstract

* some code cleanup

* some comments

* remove comments

* Re-use logger and fix warning

* Final review of QoS docs

---------

Co-authored-by: Ray <rmessie@traceparts.com>
Co-authored-by: Tomislav Prebeg <tprebeg@traceparts.com>
Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>
Co-authored-by: Guillaume Gnaegi <58469901+ggnaegi@users.noreply.github.com>

docs/features/qualityofservice.rst

@@ -1,55 +1,124 @@
-Quality of Service
-==================
-
-    Label: `QoS <https://github.com/ThreeMammals/Ocelot/labels/QoS>`_
-
-Ocelot supports one QoS capability at the current time. You can set on a per Route basis if you want to use a circuit breaker when making requests to a downstream service.
-This uses an awesome .NET library called `Polly <https://www.thepollyproject.org/>`_, check them out `in official repo <https://github.com/App-vNext/Polly>`_.
-
-The first thing you need to do if you want to use the :doc:`../features/administration` API is bring in the relevant NuGet `package <https://www.nuget.org/packages/Ocelot.Provider.Polly>`_:
-
-.. code-block:: powershell
-
-    Install-Package Ocelot.Provider.Polly
-
-Then in your ``ConfigureServices`` method to add Polly services we must call the ``AddPolly()`` extension of the ``OcelotBuilder`` being returned by ``AddOcelot()`` [#f1]_ like below:
-
-.. code-block:: csharp
-
-    services.AddOcelot()
-        .AddPolly();
-
-Then add the following section to a Route configuration: 
-
-.. code-block:: json
-
-  "QoSOptions": {
-    "ExceptionsAllowedBeforeBreaking": 3,
-    "DurationOfBreak": 1000,
-    "TimeoutValue": 5000
-  }
-
-- You must set a number greater than ``0`` against **ExceptionsAllowedBeforeBreaking** for this rule to be implemented. [#f2]_
-- **DurationOfBreak** means the circuit breaker will stay open for 1 second after it is tripped.
-- **TimeoutValue** means if a request takes more than 5 seconds, it will automatically be timed out. 
-
-You can set the **TimeoutValue** in isolation of the **ExceptionsAllowedBeforeBreaking** and **DurationOfBreak** options:
-
-.. code-block:: json
-
-  "QoSOptions": {
-    "TimeoutValue": 5000
-  }
-
-There is no point setting the other two in isolation as they affect each other!
-
-If you do not add a QoS section, QoS will not be used, however Ocelot will default to a **90** seconds timeout on all downstream requests.
-If someone needs this to be configurable, open an issue. [#f2]_
-
-""""
-
-.. [#f1] :ref:`di-the-addocelot-method` adds default ASP.NET services to DI container. You could call another extended :ref:`di-addocelotusingbuilder-method` while configuring services to develop your own :ref:`di-custom-builder`. See more instructions in the ":ref:`di-addocelotusingbuilder-method`" section of :doc:`../features/dependencyinjection` feature.
-.. [#f2] If something doesn't work or you get stuck, please review current `QoS issues <https://github.com/search?q=repo%3AThreeMammals%2FOcelot+QoS&type=issues>`_ filtering by |QoS_label| label.
-
-.. |QoS_label| image:: https://img.shields.io/badge/-QoS-D3ADAF.svg
-   :target: https://github.com/ThreeMammals/Ocelot/labels/QoS
+Quality of Service
+==================
+
+    Label: `QoS <https://github.com/ThreeMammals/Ocelot/labels/QoS>`_
+
+Ocelot supports one QoS capability at the current time. You can set on a per Route basis if you want to use a circuit breaker when making requests to a downstream service.
+This uses an awesome .NET library called `Polly`_, check them out `in official repository <https://github.com/App-vNext/Polly>`_.
+
+The first thing you need to do if you want to use the :doc:`../features/administration` API is bring in the relevant NuGet `package <https://www.nuget.org/packages/Ocelot.Provider.Polly>`_:
+
+.. code-block:: powershell
+
+    Install-Package Ocelot.Provider.Polly
+
+Then in your ``ConfigureServices`` method to add `Polly`_ services we must call the ``AddPolly()`` extension of the ``OcelotBuilder`` being returned by ``AddOcelot()`` [#f1]_ like below:
+
+.. code-block:: csharp
+
+    services.AddOcelot()
+        .AddPolly();
+
+Then add the following section to a Route configuration: 
+
+.. code-block:: json
+
+  "QoSOptions": {
+    "ExceptionsAllowedBeforeBreaking": 3,
+    "DurationOfBreak": 1000,
+    "TimeoutValue": 5000
+  }
+
+- You must set a number equal or greater than ``2`` against **ExceptionsAllowedBeforeBreaking** for this rule to be implemented. [#f2]_
+- **DurationOfBreak** means the circuit breaker will stay open for 1 second after it is tripped.
+- **TimeoutValue** means if a request takes more than 5 seconds, it will automatically be timed out. 
+
+You can set the **TimeoutValue** in isolation of the **ExceptionsAllowedBeforeBreaking** and **DurationOfBreak** options:
+
+.. code-block:: json
+
+  "QoSOptions": {
+    "TimeoutValue": 5000
+  }
+
+There is no point setting the other two in isolation as they affect each other!
+
+Defaults
+--------
+
+If you do not add a QoS section, QoS will not be used, however Ocelot will default to a **90** seconds timeout on all downstream requests.
+If someone needs this to be configurable, open an issue. [#f2]_
+
+.. _qos-polly-v7-vs-v8:
+
+`Polly`_ v7 vs v8
+-----------------
+
+**Important changes** of the version `23.2`_: [#f3]_
+  - with `Polly`_ version 8, ``ExceptionsAllowedBeforeBreaking`` must be equal to or greater than ``2``!
+  - the ``AddPolly`` method has been migrated from v7 policy wrappers to v8 resilience pipelines, so it has different behaviour now based on v8 pipelines! 
+
+If you don't want to change your settings, you must use `Polly`_ v7 like this:
+
+.. code-block:: csharp
+
+    services.AddOcelot()
+        .AddPollyV7();
+
+**Please note!** Support for `Polly`_ v7 will be removed in a future version, so we advise you not to use this method (tagged as ``Obsolete``) unless you really have to.
+
+.. _qos-extensibility:
+
+Extensibility [#f3]_
+--------------------
+
+If you want to use your ``ResiliencePipeline<T>`` provider, you can use the following syntax:
+
+.. code-block:: csharp
+
+    services.AddOcelot()
+        .AddPolly<MyProvider>();
+   // MyProvider should implement IPollyQoSResiliencePipelineProvider<HttpResponseMessage> 
+   // Note: you can use standard provider PollyQoSResiliencePipelineProvider
+
+If, in addition, you want to use your own ``DelegatingHandler``, you can use the following syntax:
+
+.. code-block:: csharp
+
+    services.AddOcelot()
+        .AddPolly<MyProvider>(MyQosDelegatingHandlerDelegate);
+   // MyProvider should implement IPollyQoSResiliencePipelineProvider<HttpResponseMessage> 
+   // Note: you can use standard provider PollyQoSResiliencePipelineProvider
+   // MyQosDelegatingHandlerDelegate is a delegate use to get a DelegatingHandler
+
+And finally, if you want to define your own set of exceptions to map, you can use the following syntax:
+
+.. code-block:: csharp
+
+    services.AddOcelot()
+        .AddPolly<MyProvider>(MyErrorMapping);
+    // MyProvider should implement IPollyQoSResiliencePipelineProvider<HttpResponseMessage> 
+    // Note: you can use standard provider PollyQoSResiliencePipelineProvider
+
+    // MyErrorMapping is a Dictionary<Type, Func<Exception, Error>>, eg:
+    private static readonly Dictionary<Type, Func<Exception, Error>> MyErrorMapping = new()
+    {
+        {typeof(TaskCanceledException), CreateError},
+        {typeof(TimeoutRejectedException), CreateError},
+        {typeof(BrokenCircuitException), CreateError},
+        {typeof(BrokenCircuitException<HttpResponseMessage>), CreateError},
+    };
+    private static Error CreateError(Exception e) => new RequestTimedOutError(e);
+
+""""
+
+.. [#f1] :ref:`di-the-addocelot-method` adds default ASP.NET services to DI container. You could call another extended :ref:`di-addocelotusingbuilder-method` while configuring services to develop your own :ref:`di-custom-builder`. See more instructions in the ":ref:`di-addocelotusingbuilder-method`" section of :doc:`../features/dependencyinjection` feature.
+.. [#f2] If something doesn't work or you get stuck, please review current `QoS issues <https://github.com/search?q=repo%3AThreeMammals%2FOcelot+QoS&type=issues>`_ filtering by |QoS_label| label.
+.. [#f3] We upgraded `Polly`_ version from v7.x to v8.x! The :ref:`qos-extensibility` feature was requested in issue `1875`_ and delivered by PR `1914`_ as a part of the version `23.2`_.
+
+.. _Polly: https://www.thepollyproject.org
+.. _1875: https://github.com/ThreeMammals/Ocelot/issues/1875
+.. _1914: https://github.com/ThreeMammals/Ocelot/pull/1914
+.. _23.2: https://github.com/ThreeMammals/Ocelot/releases/tag/23.2.0
+.. |QoS_label| image:: https://img.shields.io/badge/-QoS-D3ADAF.svg
+   :target: https://github.com/ThreeMammals/Ocelot/labels/QoS

samples/OcelotKube/ApiGateway/Ocelot.Samples.OcelotKube.ApiGateway.csproj

@@ -6,9 +6,6 @@
     <AspNetCoreHostingModel>InProcess</AspNetCoreHostingModel>
     <DockerDefaultTargetOS>Linux</DockerDefaultTargetOS>
   </PropertyGroup>
-  <ItemGroup>
-    <PackageReference Include="Microsoft.VisualStudio.Azure.Containers.Tools.Targets" Version="1.19.5" />
-  </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\..\..\src\Ocelot.Provider.Kubernetes\Ocelot.Provider.Kubernetes.csproj" />
     <ProjectReference Include="..\..\..\src\Ocelot\Ocelot.csproj" />

samples/OcelotServiceDiscovery/DownstreamService/Ocelot.Samples.ServiceDiscovery.DownstreamService.csproj

@@ -11,7 +11,6 @@
 
   <ItemGroup>
     <PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="7.0.7" />
-    <PackageReference Include="Microsoft.VisualStudio.Azure.Containers.Tools.Targets" Version="1.18.1" />
     <PackageReference Include="Swashbuckle.AspNetCore" Version="6.5.0" />
   </ItemGroup>
 

src/Ocelot.Provider.Polly/Interfaces/IPollyQoSResiliencePipelineProvider.cs

@@ -0,0 +1,16 @@
+using Ocelot.Configuration;
+
+namespace Ocelot.Provider.Polly.Interfaces;
+
+/// <summary>Defines provider for Polly V8 pipelines.</summary>
+/// <typeparam name="TResult">An HTTP result type, usually it is <see cref="HttpResponseMessage"/> type.</typeparam>
+public interface IPollyQoSResiliencePipelineProvider<TResult>
+    where TResult : IDisposable
+{
+    /// <summary>
+    /// Gets Polly v8 pipeline.
+    /// </summary>
+    /// <param name="route">The route to apply a pipeline for.</param>
+    /// <returns>A <see cref="ResiliencePipeline{T}"/> object where T is <typeparamref name="TResult"/>.</returns>
+    ResiliencePipeline<TResult> GetResiliencePipeline(DownstreamRoute route);
+}

src/Ocelot.Provider.Polly/Ocelot.Provider.Polly.csproj

@@ -34,7 +34,7 @@
     <PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.507">
       <PrivateAssets>all</PrivateAssets>
     </PackageReference>
-    <PackageReference Include="Polly" Version="8.2.0" />
+    <PackageReference Include="Polly" Version="8.3.1" />
   </ItemGroup>
   <ItemGroup>
     <PackageReference Update="Microsoft.SourceLink.GitHub" Version="1.0.0" />