Skip to content

Commit

Permalink
docs: clarity around http filter ordering (#11057)
Browse files Browse the repository at this point in the history
I still had an arcane mental model (pre #5955) where both decoder and encoder filters where invoked in the same order as the configuration order. My mind was so used to this I even failed to notice the code that prepends encoder filters into the encoder_filters_ list in the conn manager. These documentation additions are trying to make the behavior as explicit as possible so others are not confused.

Risk Level: low - doc updates, no behavior change.
Docs Changes: updated inline comments in the code I failed to notice, and in the project docs.

Signed-off-by: Jose Nino <jnino@lyft.com>
  • Loading branch information
junr03 authored May 5, 2020
1 parent 893ed58 commit 4abe685
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,8 @@ message HttpConnectionManager {
}

// A list of individual HTTP filters that make up the filter chain for
// requests made to the connection manager. Order matters as the filters are
// processed sequentially as request events happen.
// requests made to the connection manager. :ref:`Order matters <arch_overview_http_filters_ordering>`
// as the filters are processed sequentially as request events happen.
repeated HttpFilter http_filters = 5;

// Whether the connection manager manipulates the :ref:`config_http_conn_man_headers_user-agent`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,8 @@ message HttpConnectionManager {
}

// A list of individual HTTP filters that make up the filter chain for
// requests made to the connection manager. Order matters as the filters are
// processed sequentially as request events happen.
// requests made to the connection manager. :ref:`Order matters <arch_overview_http_filters_ordering>`
// as the filters are processed sequentially as request events happen.
repeated HttpFilter http_filters = 5;

// Whether the connection manager manipulates the :ref:`config_http_conn_man_headers_user-agent`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,8 @@ message HttpConnectionManager {
}

// A list of individual HTTP filters that make up the filter chain for
// requests made to the connection manager. Order matters as the filters are
// processed sequentially as request events happen.
// requests made to the connection manager. :ref:`Order matters <arch_overview_http_filters_ordering>`
// as the filters are processed sequentially as request events happen.
repeated HttpFilter http_filters = 5;

// Whether the connection manager manipulates the :ref:`config_http_conn_man_headers_user-agent`
Expand Down
23 changes: 23 additions & 0 deletions docs/root/intro/arch_overview/http/http_filters.rst
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,26 @@ themselves within the context of a single request stream. Refer to :ref:`data sh
between filters <arch_overview_data_sharing_between_filters>` for more details. Envoy already
includes several HTTP level filters that are documented in this architecture overview as well as
the :ref:`configuration reference <config_http_filters>`.

.. _arch_overview_http_filters_ordering:

Filter ordering
---------------

Filter ordering in the :ref:`http_filters field <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.http_filters>`
matters. If filters are configured in the following order (and assuming all three filters are
decoder/encoder filters):

.. code-block:: yaml
http_filters:
- A
- B
# The last configured filter has to be a terminal filter, as determined by the
# NamedHttpFilterConfigFactory::isTerminalFilter() function. This is most likely the router
# filter.
- C
The connection manager will invoke decoder filters in the order: ``A``, ``B``, ``C``.
On the other hand, the connection manager will invoke encoder filters in the **reverse**
order: ``C``, ``B``, ``A``.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -695,13 +695,29 @@ void ConnectionManagerImpl::ActiveStream::addStreamDecoderFilterWorker(
StreamDecoderFilterSharedPtr filter, bool dual_filter) {
ActiveStreamDecoderFilterPtr wrapper(new ActiveStreamDecoderFilter(*this, filter, dual_filter));
filter->setDecoderFilterCallbacks(*wrapper);
// Note: configured decoder filters are appended to decoder_filters_.
// This means that if filters are configured in the following order (assume all three filters are
// both decoder/encoder filters):
// http_filters:
// - A
// - B
// - C
// The decoder filter chain will iterate through filters A, B, C.
wrapper->moveIntoListBack(std::move(wrapper), decoder_filters_);
}

void ConnectionManagerImpl::ActiveStream::addStreamEncoderFilterWorker(
StreamEncoderFilterSharedPtr filter, bool dual_filter) {
ActiveStreamEncoderFilterPtr wrapper(new ActiveStreamEncoderFilter(*this, filter, dual_filter));
filter->setEncoderFilterCallbacks(*wrapper);
// Note: configured encoder filters are prepended to encoder_filters_.
// This means that if filters are configured in the following order (assume all three filters are
// both decoder/encoder filters):
// http_filters:
// - A
// - B
// - C
// The encoder filter chain will iterate through filters C, B, A.
wrapper->moveIntoList(std::move(wrapper), encoder_filters_);
}

Expand Down

0 comments on commit 4abe685

Please sign in to comment.