Skip to content
Merged
7 changes: 5 additions & 2 deletions api/envoy/api/v2/route/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,12 @@ message VirtualHost {

// Decides whether the :ref:`x-envoy-attempt-count
// <config_http_filters_router_x-envoy-attempt-count>` header should be included
// in the upstream request. Setting this option will cause it to override any existing header
// in the upstream request and the downstream response.
// Setting this option will cause it to override any existing header
// value, so in the case of two Envoys on the request path with this option enabled, the upstream
// will see the attempt count as perceived by the second Envoy. Defaults to false.
// will see the attempt count as perceived by the second Envoy. Similarly, if there are multiple Envoys
// on the response path with this option enabled, the downstream will see the attempt count
// as perceived by the first Envoy in the path. This field Defaults to false.
Comment thread
junr03 marked this conversation as resolved.
Outdated
Comment thread
junr03 marked this conversation as resolved.
Outdated
// This header is unaffected by the
// :ref:`suppress_envoy_headers
// <envoy_api_field_config.filter.http.router.v2.Router.suppress_envoy_headers>` flag.
Expand Down
7 changes: 5 additions & 2 deletions api/envoy/config/route/v3/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,12 @@ message VirtualHost {

// Decides whether the :ref:`x-envoy-attempt-count
// <config_http_filters_router_x-envoy-attempt-count>` header should be included
// in the upstream request. Setting this option will cause it to override any existing header
// in the upstream request and the downstream response.
// Setting this option will cause it to override any existing header
// value, so in the case of two Envoys on the request path with this option enabled, the upstream
// will see the attempt count as perceived by the second Envoy. Defaults to false.
// will see the attempt count as perceived by the second Envoy. Similarly, if there are multiple Envoys
// on the response path with this option enabled, the downstream will see the attempt count
// as perceived by the first Envoy in the path. This field Defaults to false.
// This header is unaffected by the
// :ref:`suppress_envoy_headers
// <envoy_api_field_extensions.filters.http.router.v3.Router.suppress_envoy_headers>` flag.
Expand Down
11 changes: 6 additions & 5 deletions docs/root/configuration/http/http_filters/router_filter.rst
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ or :ref:`config_http_filters_router_x-envoy-retry-grpc-on` headers are not speci
A few notes on how Envoy does retries:

* The route timeout (set via :ref:`config_http_filters_router_x-envoy-upstream-rq-timeout-ms` or the
:ref:`timeout <envoy_api_field_route.RouteAction.timeout>` in route configuration or set via
`grpc-timeout header <https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md>`_ by specifying
:ref:`timeout <envoy_api_field_route.RouteAction.timeout>` in route configuration or set via
`grpc-timeout header <https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md>`_ by specifying
:ref:`max_grpc_timeout <envoy_api_field_route.RouteAction.timeout>` in route configuration) **includes** all
retries. Thus if the request timeout is set to 3s, and the first request attempt takes 2.7s, the
retry (including back-off) has .3s to complete. This is by design to avoid an exponential
Expand Down Expand Up @@ -232,8 +232,8 @@ x-envoy-upstream-rq-timeout-ms
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Setting this header on egress requests will cause Envoy to override the :ref:`route configuration timeout
<envoy_api_field_route.RouteAction.timeout>` or gRPC client timeout set via `grpc-timeout header
<https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md>`_ by specifying :ref:`max_grpc_timeout
<envoy_api_field_route.RouteAction.timeout>` or gRPC client timeout set via `grpc-timeout header
<https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md>`_ by specifying :ref:`max_grpc_timeout
<envoy_api_field_route.RouteAction.timeout>`. The timeout must be specified in millisecond
units. See also :ref:`config_http_filters_router_x-envoy-upstream-rq-per-try-timeout-ms`.

Expand Down Expand Up @@ -318,7 +318,8 @@ x-envoy-attempt-count
^^^^^^^^^^^^^^^^^^^^^

Sent to the upstream to indicate which attempt the current request is in a series of retries. The value
will be "1" on the initial request, incrementing by one for each retry. Only set if the
will be "1" on the initial request, incrementing by one for each retry.
Also sent on downstream response headers if a response is received from upstream. Only set if the
:ref:`include_attempt_count_header <envoy_api_field_route.VirtualHost.include_request_attempt_count>`
flag is set to true.

Expand Down

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.

2 changes: 1 addition & 1 deletion include/envoy/http/header_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,6 @@ class HeaderEntry {
HEADER_FUNC(AccessControlRequestMethod) \
HEADER_FUNC(Authorization) \
HEADER_FUNC(ClientTraceId) \
HEADER_FUNC(EnvoyAttemptCount) \
HEADER_FUNC(EnvoyDownstreamServiceCluster) \
HEADER_FUNC(EnvoyDownstreamServiceNode) \
HEADER_FUNC(EnvoyExpectedRequestTimeoutMs) \
Expand Down Expand Up @@ -344,6 +343,7 @@ class HeaderEntry {
HEADER_FUNC(Connection) \
HEADER_FUNC(ContentLength) \
HEADER_FUNC(ContentType) \
HEADER_FUNC(EnvoyAttemptCount) \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes me happy to see this working properly. :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is awesome! Hard/tedious work is now paying off :) thanks for doing this. I took the liberty to narrow some more types in this PR to ensure static checking, really rad.

HEADER_FUNC(EnvoyDecoratorOperation) \
HEADER_FUNC(KeepAlive) \
HEADER_FUNC(NoChunks) \
Expand Down
10 changes: 10 additions & 0 deletions source/common/router/router.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1229,6 +1229,16 @@ void Filter::onUpstreamHeaders(uint64_t response_code, Http::ResponseHeaderMapPt
headers->addReferenceKey(Http::Headers::get().SetCookie, header_value);
}

// This header is added without checking for config_.suppress_envoy_headers_ to mirror what is
// done for upstream requests. Note: this work is done here instead of in
// route_entry_->finalizeResponse headers because the attempt_count_ data pertains to the router.
// This could be moved, if desired, by charging callbacks_->streamInfo with the attempt count.
// Adding the information to the stream info would also be necessary if we wanted this header to
// be added on sendLocalReply.
if (include_attempt_count_) {
Comment thread
junr03 marked this conversation as resolved.
Outdated
headers->setEnvoyAttemptCount(attempt_count_);
}

// TODO(zuercher): If access to response_headers_to_add (at any level) is ever needed outside
// Router::Filter we'll need to find a better location for this work. One possibility is to
// provide finalizeResponseHeaders functions on the Router::Config and VirtualHost interfaces.
Expand Down
3 changes: 3 additions & 0 deletions test/integration/http_integration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,9 @@ void HttpIntegrationTest::testRetryAttemptCountHeader() {
EXPECT_TRUE(response->complete());
EXPECT_EQ("200", response->headers().Status()->value().getStringView());
EXPECT_EQ(512U, response->body().size());
EXPECT_EQ(
2,
atoi(std::string(response->headers().EnvoyAttemptCount()->value().getStringView()).c_str()));
}

void HttpIntegrationTest::testGrpcRetry() {
Expand Down
3 changes: 3 additions & 0 deletions test/integration/protocol_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,9 @@ TEST_P(DownstreamProtocolIntegrationTest, RetryAttemptCountHeader) {
EXPECT_TRUE(response->complete());
EXPECT_EQ("200", response->headers().Status()->value().getStringView());
EXPECT_EQ(512U, response->body().size());
EXPECT_EQ(
2,
atoi(std::string(response->headers().EnvoyAttemptCount()->value().getStringView()).c_str()));
}

// Verifies that a retry priority can be configured and affect the host selected during retries.
Expand Down