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

router: add x-envoy-attempt-count on downstream responses #10325

Merged
merged 20 commits into from
Mar 16, 2020
14 changes: 13 additions & 1 deletion api/envoy/api/v2/route/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ option (udpa.annotations.file_migrate).move_to_package = "envoy.config.route.v3"
// host header. This allows a single listener to service multiple top level domain path trees. Once
// a virtual host is selected based on the domain, the routes are processed in order to see which
// upstream cluster to route to or whether to perform a redirect.
// [#next-free-field: 19]
// [#next-free-field: 20]
message VirtualHost {
enum TlsRequirementType {
// No TLS requirement for the virtual host.
Expand Down Expand Up @@ -143,8 +143,20 @@ message VirtualHost {
// This header is unaffected by the
// :ref:`suppress_envoy_headers
// <envoy_api_field_config.filter.http.router.v2.Router.suppress_envoy_headers>` flag.
//
// [#next-major-version: rename to include_attempt_count_in_request.]
Copy link
Member

Choose a reason for hiding this comment

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

You can do this now with [(udpa.annotations.field_migrate).rename right? Or is the issue that this will right now change it in v3 and we have to circle back and do it after we stop v2 updates?

Copy link
Member Author

Choose a reason for hiding this comment

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

there's a wrinkle with the annotation right now #10325 (comment). I am keeping a note so that when this is fix we can change to use the annotation.

bool include_request_attempt_count = 14;

// Decides whether the :ref:`x-envoy-attempt-count
// <config_http_filters_router_x-envoy-attempt-count>` header should be included
// in the downstream response. Setting this option will cause the router to override any existing header
// value, so in the case of two Envoys on the request path with this option enabled, the downstream
// will see the attempt count as perceived by the Envoy closest upstream from itself. Defaults to false.
// This header is unaffected by the
// :ref:`suppress_envoy_headers
// <envoy_api_field_config.filter.http.router.v2.Router.suppress_envoy_headers>` flag.
bool include_attempt_count_in_response = 19;

// Indicates the retry policy for all routes in this virtual host. Note that setting a
// route level entry will take precedence over this config and it'll be treated
// independently (e.g.: values are not inherited).
Expand Down
14 changes: 13 additions & 1 deletion api/envoy/config/route/v3/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ option java_multiple_files = true;
// host header. This allows a single listener to service multiple top level domain path trees. Once
// a virtual host is selected based on the domain, the routes are processed in order to see which
// upstream cluster to route to or whether to perform a redirect.
// [#next-free-field: 19]
// [#next-free-field: 20]
message VirtualHost {
option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.VirtualHost";

Expand Down Expand Up @@ -140,8 +140,20 @@ message VirtualHost {
// This header is unaffected by the
// :ref:`suppress_envoy_headers
// <envoy_api_field_extensions.filters.http.router.v3.Router.suppress_envoy_headers>` flag.
//
// [#next-major-version: rename to include_attempt_count_in_request.]
bool include_request_attempt_count = 14;

// Decides whether the :ref:`x-envoy-attempt-count
// <config_http_filters_router_x-envoy-attempt-count>` header should be included
// in the downstream response. Setting this option will cause the router to override any existing header
// value, so in the case of two Envoys on the request path with this option enabled, the downstream
// will see the attempt count as perceived by the Envoy closest upstream from itself. 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.
bool include_attempt_count_in_response = 19;

// Indicates the retry policy for all routes in this virtual host. Note that setting a
// route level entry will take precedence over this config and it'll be treated
// independently (e.g.: values are not inherited).
Expand Down
16 changes: 11 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 @@ -319,7 +319,13 @@ 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
:ref:`include_attempt_count_header <envoy_api_field_route.VirtualHost.include_request_attempt_count>`
:ref:`include_request_attempt_count <envoy_api_field_route.VirtualHost.include_request_attempt_count>`
flag is set to true.

Sent to the downstream to indicate how many upstream requests took place. The header will be absent if
the router did not send any upstream requests. The value will be "1" if only the original upstream
request was sent, incrementing by one for each retry. Only set if the
:ref:`include_attempt_count_in_response <envoy_api_field_route.VirtualHost.include_attempt_count_in_response>`
flag is set to true.

.. _config_http_filters_router_x-envoy-expected-rq-timeout-ms:
Expand Down
2 changes: 2 additions & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ Version history
* router: added support for :ref:`regex_rewrite
<envoy_api_field_route.RouteAction.regex_rewrite>` for path rewriting using regular expressions and capture groups.
* router: don't ignore :ref:`per_try_timeout <envoy_api_field_route.RetryPolicy.per_try_timeout>` when the :ref:`global route timeout <envoy_api_field_route.RouteAction.timeout>` is disabled.
* router: added ability to set attempt count in downstream response, see :ref:`virtual host's include response
attempt count config <envoy_api_field_route.VirtualHost.include_attempt_count_in_response>`.
* router: strip whitespace for :ref:`retry_on <envoy_api_field_route.RetryPolicy.retry_on>`, :ref:`grpc-retry-on header <config_http_filters_router_x-envoy-retry-grpc-on>` and :ref:`retry-on header <config_http_filters_router_x-envoy-retry-on>`.
* runtime: enabling the runtime feature "envoy.deprecated_features.allow_deprecated_extension_names"
disables the use of deprecated extension names.
Expand Down
14 changes: 13 additions & 1 deletion generated_api_shadow/envoy/api/v2/route/route_components.proto

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
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
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
16 changes: 14 additions & 2 deletions include/envoy/router/router.h
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,12 @@ class VirtualHost {
/**
* @return bool whether to include the request count header in upstream requests.
*/
virtual bool includeAttemptCount() const PURE;
virtual bool includeAttemptCountInRequest() const PURE;

/**
* @return bool whether to include the request count header in the downstream response.
*/
virtual bool includeAttemptCountInResponse() const PURE;

/**
* @return uint32_t any route cap on bytes which should be buffered for shadowing or retries.
Expand Down Expand Up @@ -778,7 +783,14 @@ class RouteEntry : public ResponseEntry {
* count header.
* @return bool whether x-envoy-attempt-count should be included on the upstream request.
*/
virtual bool includeAttemptCount() const PURE;
virtual bool includeAttemptCountInRequest() const PURE;

/**
* True if the virtual host this RouteEntry belongs to is configured to include the attempt
* count header.
* @return bool whether x-envoy-attempt-count should be included on the downstream response.
*/
virtual bool includeAttemptCountInResponse() const PURE;

using UpgradeMap = std::map<std::string, bool>;
/**
Expand Down
6 changes: 4 additions & 2 deletions source/common/http/async_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ class AsyncStreamImpl : public AsyncClient::Stream,
const Router::RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override {
return nullptr;
}
bool includeAttemptCount() const override { return false; }
bool includeAttemptCountInRequest() const override { return false; }
bool includeAttemptCountInResponse() const override { return false; }
uint32_t retryShadowBufferLimit() const override {
return std::numeric_limits<uint32_t>::max();
}
Expand Down Expand Up @@ -268,7 +269,8 @@ class AsyncStreamImpl : public AsyncClient::Stream,
return nullptr;
}

bool includeAttemptCount() const override { return false; }
bool includeAttemptCountInRequest() const override { return false; }
bool includeAttemptCountInResponse() const override { return false; }
const Router::RouteEntry::UpgradeMap& upgradeMap() const override { return upgrade_map_; }
Router::InternalRedirectAction internalRedirectAction() const override {
return Router::InternalRedirectAction::PassThrough;
Expand Down
3 changes: 2 additions & 1 deletion source/common/router/config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,8 @@ VirtualHostImpl::VirtualHostImpl(const envoy::config::route::v3::VirtualHost& vi
validator),
retry_shadow_buffer_limit_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(
virtual_host, per_request_buffer_limit_bytes, std::numeric_limits<uint32_t>::max())),
include_attempt_count_(virtual_host.include_request_attempt_count()),
include_attempt_count_in_request_(virtual_host.include_request_attempt_count()),
include_attempt_count_in_response_(virtual_host.include_attempt_count_in_response()),
virtual_cluster_catch_all_(stat_name_pool_) {

switch (virtual_host.require_tls()) {
Expand Down
20 changes: 16 additions & 4 deletions source/common/router/config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ class VirtualHostImpl : public VirtualHost {
const RateLimitPolicy& rateLimitPolicy() const override { return rate_limit_policy_; }
const Config& routeConfig() const override;
const RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override;
bool includeAttemptCount() const override { return include_attempt_count_; }
bool includeAttemptCountInRequest() const override { return include_attempt_count_in_request_; }
bool includeAttemptCountInResponse() const override { return include_attempt_count_in_response_; }
const absl::optional<envoy::config::route::v3::RetryPolicy>& retryPolicy() const {
return retry_policy_;
}
Expand Down Expand Up @@ -223,7 +224,8 @@ class VirtualHostImpl : public VirtualHost {
HeaderParserPtr response_headers_parser_;
PerFilterConfigs per_filter_configs_;
uint32_t retry_shadow_buffer_limit_{std::numeric_limits<uint32_t>::max()};
const bool include_attempt_count_;
const bool include_attempt_count_in_request_;
const bool include_attempt_count_in_response_;
absl::optional<envoy::config::route::v3::RetryPolicy> retry_policy_;
absl::optional<envoy::config::route::v3::HedgePolicy> hedge_policy_;
const CatchAllVirtualCluster virtual_cluster_catch_all_;
Expand Down Expand Up @@ -451,7 +453,12 @@ class RouteEntryImplBase : public RouteEntry,
const envoy::config::core::v3::Metadata& metadata() const override { return metadata_; }
const Envoy::Config::TypedMetadata& typedMetadata() const override { return typed_metadata_; }
const PathMatchCriterion& pathMatchCriterion() const override { return *this; }
bool includeAttemptCount() const override { return vhost_.includeAttemptCount(); }
bool includeAttemptCountInRequest() const override {
return vhost_.includeAttemptCountInRequest();
}
bool includeAttemptCountInResponse() const override {
return vhost_.includeAttemptCountInResponse();
}
const UpgradeMap& upgradeMap() const override { return upgrade_map_; }
InternalRedirectAction internalRedirectAction() const override {
return internal_redirect_action_;
Expand Down Expand Up @@ -572,7 +579,12 @@ class RouteEntryImplBase : public RouteEntry,
return parent_->pathMatchCriterion();
}

bool includeAttemptCount() const override { return parent_->includeAttemptCount(); }
bool includeAttemptCountInRequest() const override {
return parent_->includeAttemptCountInRequest();
}
bool includeAttemptCountInResponse() const override {
return parent_->includeAttemptCountInResponse();
}
const UpgradeMap& upgradeMap() const override { return parent_->upgradeMap(); }
InternalRedirectAction internalRedirectAction() const override {
return parent_->internalRedirectAction();
Expand Down
Loading