Skip to content

Commit 35dbc9d

Browse files
raman-mRaynaldM
andauthored
#585 #2338 Quality of Service global configuration (#2339)
* Delete QoSOptionsBuilder * FileRouteBase abstract class * Rename ExceptionsAllowedBeforeBreaking to MinimumThroughput * Rename DurationOfBreak to BreakDuration * Rename TimeoutValue to Timeout so the QoSOptions model aligns with the Polly schema * Deprecate props of FileQoSOptions model * Fix unstable unit test in PollKubeTests * Organize QualityOfService feature into its own folder * Implement global configuration feat * Improve code coverage * Add history of QoS feat evolving in acceptance tests * Remove the BDDfy wrapping from async operations to measure timings more accurately and achieve more stable behavior * Review current QoS Polly acceptance tests * Let JSON deserializer handle creating the options * Acceptance tests for static routes * Code review by @RaynaldM, issue 1 * Code review by @RaynaldM, issue 2 * Fix tests after code review by @RaynaldM * Add QualityOfService folder * Refactor acceptance tests to prepare them for reuse * Bump Ocelot.Testing pack to ver 24.0.3-beta.4 * Acceptance tests for dynamic routes * Update docs --------- Co-authored-by: Raynald Messié <[email protected]>
1 parent 4730613 commit 35dbc9d

File tree

72 files changed

+2019
-1197
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

72 files changed

+2019
-1197
lines changed

docs/features/configuration.rst

Lines changed: 67 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -825,81 +825,100 @@ For comprehensive documentation, please refer to the :doc:`../features/metadata`
825825
``Timeout``
826826
-----------
827827

828-
[#f5]_ This feature is designed as part of the ``MessageInvokerPool``, which contains cached ``HttpMessageInvoker`` objects per route.
828+
This feature [#f5]_ is designed as part of the ``MessageInvokerPool``, which contains cached ``HttpMessageInvoker`` objects per route.
829829
Each created ``HttpMessageInvoker`` encapsulates an ``HttpMessageHandler``, specifically a ``SocketsHttpHandler`` instance, which serves as the base handler for the request pipeline.
830830
This pipeline also includes all user-defined :doc:`../features/delegatinghandlers`.
831831
Finally, both the :doc:`../features/delegatinghandlers` and the base ``SocketsHttpHandler`` are wrapped by Ocelot's custom ``TimeoutDelegatingHandler``, which provides the internal timeout functionality.
832832

833833
**Note**: This design is subject to future review because ``TimeoutDelegatingHandler`` overrides/mimics the default timeout properties of ``SocketsHttpHandler``, as well as the behavior of ``HttpMessageInvoker`` as a controller for ``HttpMessageHandler`` objects.
834834

835-
To configure timeouts (in seconds) at different levels, choose the appropriate level and provide the corresponding JSON configuration:
835+
To configure timeouts (in seconds) at different levels, choose the appropriate level and provide the corresponding JSON configuration.
836836

837-
- **A route timeout** can be easily defined using the following JSON, according to the :ref:`config-route-schema`:
837+
Route timeout
838+
^^^^^^^^^^^^^
838839

839-
.. code-block:: json
840+
A *route timeout* (also known as Requester middleware timeout based on ``TimeoutDelegatingHandler``) can be easily defined using the following JSON, according to the :ref:`config-route-schema`:
840841

841-
{
842-
// upstream props
843-
// downstream props
844-
"Timeout": 3 // seconds
845-
}
842+
.. code-block:: json
843+
844+
{
845+
// upstream props
846+
// downstream props
847+
"Timeout": 3 // seconds
848+
}
846849
847-
Please note that the route-level timeout takes precedence over the global timeout.
848-
The same configuration applies to *dynamic routes*, according to the :ref:`config-dynamic-route-schema`.
850+
Please note that the route-level timeout takes precedence over the global timeout.
851+
The same configuration applies to *dynamic routes*, according to the :ref:`config-dynamic-route-schema`.
849852

850-
- **A global configuration timeout** can be defined using the following JSON, according to the :ref:`config-global-configuration-schema`:
853+
Global timeout
854+
^^^^^^^^^^^^^^
851855

852-
.. code-block:: json
856+
A *global configuration timeout* can be defined using the following JSON, according to the :ref:`config-global-configuration-schema`:
853857

854-
{
855-
// routes...
856-
"GlobalConfiguration": {
857-
// other props
858-
"Timeout": 60 // seconds, 1 minute
859-
}
858+
.. code-block:: json
859+
860+
{
861+
// routes...
862+
"GlobalConfiguration": {
863+
// other props
864+
"Timeout": 60 // seconds, 1 minute
860865
}
866+
}
861867
862-
Please note that the global timeout is substituted into a route if the route-level timeout is not defined, and it takes precedence over the absolute :ref:`config-default-timeout`.
863-
Additionally, the global timeout may be omitted in the JSON configuration in favor of the absolute :ref:`config-default-timeout`, which is also configurable via a property of the C# static class.
868+
Please note that the global timeout is substituted into a route if the route-level timeout is not defined, and it takes precedence over the absolute :ref:`config-default-timeout`.
869+
Additionally, the global timeout may be omitted in the JSON configuration in favor of the absolute :ref:`config-default-timeout`, which is also configurable via a property of the C# static class.
864870

865-
- **A** :doc:`../features/qualityofservice` **timeout** can be defined according to the QoS :ref:`qos-configuration-schema` and the QoS :ref:`qos-timeout-strategy`:
871+
QoS timeout
872+
^^^^^^^^^^^
866873

867-
.. code-block:: json
874+
A :doc:`../features/qualityofservice` (QoS) *timeout* can be defined using the :ref:`qos-schema` and the QoS :ref:`qos-timeout-strategy`:
868875

869-
"QoSOptions": {
870-
"TimeoutValue": 5000 // milliseconds
871-
}
876+
.. code-block:: json
872877
873-
Please note, the *Quality of Service* timeout takes precedence over both route-level and global timeouts, which are ignored when QoS is enabled.
874-
Additionally, avoid defining both timeouts in the same route, as the QoS timeout (``TimeoutValue``) has higher priority than the route-level timeout.
875-
Therefore, the following route configuration is not recommended:
878+
"QoSOptions": {
879+
"Timeout": 5000 // milliseconds
880+
}
876881
877-
.. code-block:: json
882+
Please note, the *Quality of Service* timeout takes precedence over both route-level and global timeouts, which are ignored when QoS is enabled.
883+
Additionally, avoid defining both *timeouts* in the same route, as the QoS timeout has higher priority than the route-level timeout.
884+
Therefore, the following route configuration **is not** recommended:
878885

879-
{
880-
// route props...
881-
"Timeout": 3, // seconds
882-
"QoSOptions": {
883-
"TimeoutValue": 5000 // milliseconds
884-
}
886+
.. code-block:: json
887+
888+
{
889+
// route props...
890+
"Timeout": 3, // seconds
891+
"QoSOptions": {
892+
"Timeout": 5000 // milliseconds
885893
}
894+
}
886895
887-
So, ``Timeout`` will be ignored in favor of ``TimeoutValue``.
888-
Moreover, because the 3-second duration is shorter than 5000 milliseconds, you may observe warning messages in the logs that begin with the following sentence:
896+
So, route ``Timeout`` will be ignored in favor of QoS ``Timeout``.
897+
Moreover, because the 3-second duration is shorter than 5000 milliseconds, you may observe warning messages in the logs that begin with the following sentence:
889898

890-
.. code-block:: text
899+
.. code-block:: text
891900
892-
Route '/xxx' has Quality of Service settings (QoSOptions) enabled, but either the route Timeout or the QoS TimeoutValue is misconfigured: ...
901+
Route '/xxx' has Quality of Service settings (QoSOptions) enabled, but either the route Timeout or the QoS Timeout is misconfigured: ...
893902
894-
For more details about this warning, refer to the :ref:`qos-notes-qos-and-route-global-timeouts` note in the :doc:`../features/qualityofservice` chapter.
895-
Your next recommended action is to completely remove the ``Timeout`` property.
903+
For more details about this warning, refer to the :ref:`qos-notes-qos-and-route-global-timeouts` note in the :doc:`../features/qualityofservice` chapter.
904+
Your next recommended action is to completely remove the 3-second ``Timeout`` property or comment it out:
896905

897-
.. _break4: http://break.do
906+
.. code-block:: json
907+
908+
{
909+
// "Timeout": 3, // seconds
910+
"QoSOptions": {
911+
"Timeout": 5000 // milliseconds
912+
}
913+
}
914+
915+
.. note::
898916

899-
**Note 1**: Both ``Timeout`` and ``TimeoutValue`` are nullable positive integers, with a minimum valid value of ``1``.
917+
1. Both route ``Timeout`` and QoS ``Timeout`` are nullable positive integers, with a minimum valid value of ``1``.
900918
Values in the range ``(−∞, 0]`` are treated as "no value" and will be automatically converted to the absolute :ref:`config-default-timeout`, effectively ignoring the property.
901919

902-
**Note 2**: The unit of measurement for ``Timeout`` is seconds, whereas ``TimeoutValue`` (used in QoS) is measured in milliseconds.
920+
2. The unit of measurement for route ``Timeout`` is seconds,
921+
whereas QoS ``Timeout`` is measured in milliseconds.
903922

904923
.. _config-default-timeout:
905924

@@ -929,9 +948,9 @@ However, keep in mind that the absolute timeout has the lowest priority—theref
929948

930949
""""
931950

932-
.. [#f1] The ":ref:`config-merging-files`" feature was requested in issue `296`_, since then we extended it in issue `1216`_ (PR `1227`_) as ":ref:`config-merging-tomemory`" subfeature which was released as a part of version `23.2`_.
933-
.. [#f2] The ":ref:`config-merging-tomemory`" feature is based on the `MergeOcelotJson <https://github.com/ThreeMammals/Ocelot/blob/main/src/Ocelot/DependencyInjection/MergeOcelotJson.cs>`_ enumeration type with values: ``ToFile`` and ``ToMemory``. The 1st one is implicit by default, and the second one is exactly what you need when merging to memory. See more details on implementations in the `ConfigurationBuilderExtensions`_ class.
934-
.. [#f3] The ":ref:`config-version-policy`" feature was requested in issue `1672`_ as a part of version `23.3`_.
951+
.. [#f1] The ":ref:`Merging Files <config-merging-files>`" feature was requested in issue `296`_, since then we extended it in issue `1216`_ (PR `1227`_) as ":ref:`Merging files to memory <config-merging-tomemory>`" subfeature which was released as a part of version `23.2`_.
952+
.. [#f2] The ":ref:`Merging files to memory <config-merging-tomemory>`" feature is based on the `MergeOcelotJson <https://github.com/ThreeMammals/Ocelot/blob/main/src/Ocelot/DependencyInjection/MergeOcelotJson.cs>`_ enumeration type with values: ``ToFile`` and ``ToMemory``. The 1st one is implicit by default, and the second one is exactly what you need when merging to memory. See more details on implementations in the `ConfigurationBuilderExtensions`_ class.
953+
.. [#f3] The ":ref:`DownstreamHttpVersionPolicy <config-version-policy>`" feature was requested in issue `1672`_ as a part of version `23.3`_.
935954
.. [#f4] The ":ref:`config-route-metadata`" feature was requested in issues `738`_ and `1990`_, and it was released as part of version `23.3`_.
936955
.. [#f5] The initial draft design of the :ref:`config-timeout` feature was implemented in pull request `1824`_ as ``TimeoutDelegatingHandler`` (released in version `23.0`_), but this version supported only the built-in `default timeout of 90 seconds`_.
937956
The full :ref:`config-timeout` feature was requested in issue `1314`_, implemented in pull request `2073`_, and officially released as part of version `24.1`_.

0 commit comments

Comments
 (0)