diff --git a/api/envoy/api/v2/route/route.proto b/api/envoy/api/v2/route/route.proto index 3d95d009ff22..0763ce617fab 100644 --- a/api/envoy/api/v2/route/route.proto +++ b/api/envoy/api/v2/route/route.proto @@ -25,7 +25,7 @@ option (gogoproto.equal_all) = 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. -// [#comment:next free field: 14] +// [#comment:next free field: 15] message VirtualHost { // The logical name of the virtual host. This is used when emitting certain // statistics but is not relevant for routing. @@ -109,6 +109,16 @@ message VirtualHost { // specific; see the :ref:`HTTP filter documentation ` // for if and how it is utilized. map per_filter_config = 12; + + // Decides whether the :ref:`x-envoy-attempt-count + // ` header should be included + // in the upstream request. 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. + // This header is unaffected by the + // :ref:`suppress_envoy_headers + // ` flag. + bool include_request_attempt_count = 14; } // A route is both a specification of how to match a request as well as an indication of what to do diff --git a/docs/root/configuration/http_filters/router_filter.rst b/docs/root/configuration/http_filters/router_filter.rst index 2ae24ac7e73b..902ab9954ad3 100644 --- a/docs/root/configuration/http_filters/router_filter.rst +++ b/docs/root/configuration/http_filters/router_filter.rst @@ -226,6 +226,16 @@ ingress/response path. They are documented in this section. .. contents:: :local: +.. _config_http_filters_router_x-envoy-attempt-count: + +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, incremeting by one for each retry. Only set if the +:ref:`include_attempt_count_header ` +flag is set to true. + .. _config_http_filters_router_x-envoy-expected-rq-timeout-ms: x-envoy-expected-rq-timeout-ms diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index eef5fd701936..bce670a2dd79 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -3,6 +3,8 @@ Version history 1.9.0 (pending) =============== +* router: added ability to set attempt count in upstream requests, see :ref:`virtual host's include request + attempt count flag `. * stats: added :ref:`stats_matcher ` to the bootstrap config for granular control of stat instantiation. * fault: removed integer percentage support. * router: when :ref:`max_grpc_timeout ` diff --git a/include/envoy/http/header_map.h b/include/envoy/http/header_map.h index d7af629b84c6..ff9cc2830e4e 100644 --- a/include/envoy/http/header_map.h +++ b/include/envoy/http/header_map.h @@ -243,6 +243,7 @@ class HeaderEntry { HEADER_FUNC(ContentLength) \ HEADER_FUNC(ContentType) \ HEADER_FUNC(Date) \ + HEADER_FUNC(EnvoyAttemptCount) \ HEADER_FUNC(EnvoyDecoratorOperation) \ HEADER_FUNC(EnvoyDownstreamServiceCluster) \ HEADER_FUNC(EnvoyDownstreamServiceNode) \ diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index 5129ba9feef3..6983cecad96e 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -345,6 +345,11 @@ class VirtualHost { template const Derived* perFilterConfigTyped(const std::string& name) const { return dynamic_cast(perFilterConfig(name)); } + + /** + * @return bool whether to include the request count header in upstream requests. + */ + virtual bool includeAttemptCount() const PURE; }; /** @@ -612,7 +617,14 @@ class RouteEntry : public ResponseEntry { */ template const Derived* perFilterConfigTyped(const std::string& name) const { return dynamic_cast(perFilterConfig(name)); - } + }; + + /** + * 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 upstream request. + */ + virtual bool includeAttemptCount() const PURE; }; /** diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index be2ca36f5634..d4f34fe8e0be 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -162,6 +162,7 @@ class AsyncStreamImpl : public AsyncClient::Stream, const Router::RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override { return nullptr; } + bool includeAttemptCount() const override { return false; } static const NullRateLimitPolicy rate_limit_policy_; static const NullConfig route_configuration_; @@ -230,6 +231,8 @@ class AsyncStreamImpl : public AsyncClient::Stream, return nullptr; } + bool includeAttemptCount() const override { return false; } + static const NullRateLimitPolicy rate_limit_policy_; static const NullRetryPolicy retry_policy_; static const NullShadowPolicy shadow_policy_; diff --git a/source/common/http/headers.h b/source/common/http/headers.h index abe23615dfa2..18fd25aa9055 100644 --- a/source/common/http/headers.h +++ b/source/common/http/headers.h @@ -33,6 +33,7 @@ class HeaderValues { const LowerCaseString ContentType{"content-type"}; const LowerCaseString Cookie{"cookie"}; const LowerCaseString Date{"date"}; + const LowerCaseString EnvoyAttemptCount{"x-envoy-attempt-count"}; const LowerCaseString EnvoyDownstreamServiceCluster{"x-envoy-downstream-service-cluster"}; const LowerCaseString EnvoyDownstreamServiceNode{"x-envoy-downstream-service-node"}; const LowerCaseString EnvoyExternalAddress{"x-envoy-external-address"}; diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 50e434031299..be7d7c520ce3 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -737,7 +737,8 @@ VirtualHostImpl::VirtualHostImpl(const envoy::api::v2::route::VirtualHost& virtu virtual_host.request_headers_to_remove())), response_headers_parser_(HeaderParser::configure(virtual_host.response_headers_to_add(), virtual_host.response_headers_to_remove())), - per_filter_configs_(virtual_host.per_filter_config(), factory_context) { + per_filter_configs_(virtual_host.per_filter_config(), factory_context), + include_attempt_count_(virtual_host.include_request_attempt_count()) { switch (virtual_host.require_tls()) { case envoy::api::v2::route::VirtualHost::NONE: ssl_requirements_ = SslRequirements::NONE; diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index f8e2322e8c77..f6026b513446 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -140,6 +140,7 @@ 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_; } private: enum class SslRequirements { NONE, EXTERNAL_ONLY, ALL }; @@ -176,6 +177,7 @@ class VirtualHostImpl : public VirtualHost { HeaderParserPtr request_headers_parser_; HeaderParserPtr response_headers_parser_; PerFilterConfigs per_filter_configs_; + const bool include_attempt_count_; }; typedef std::shared_ptr VirtualHostSharedPtr; @@ -353,6 +355,7 @@ class RouteEntryImplBase : public RouteEntry, bool includeVirtualHostRateLimits() const override { return include_vh_rate_limits_; } const envoy::api::v2::core::Metadata& metadata() const override { return metadata_; } const PathMatchCriterion& pathMatchCriterion() const override { return *this; } + bool includeAttemptCount() const override { return vhost_.includeAttemptCount(); } // Router::DirectResponseEntry std::string newPath(const Http::HeaderMap& headers) const override; @@ -454,6 +457,8 @@ class RouteEntryImplBase : public RouteEntry, return parent_->pathMatchCriterion(); } + bool includeAttemptCount() const override { return parent_->includeAttemptCount(); } + // Router::Route const DirectResponseEntry* directResponseEntry() const override { return nullptr; } const RouteEntry* routeEntry() const override { return this; } diff --git a/source/common/router/router.cc b/source/common/router/router.cc index f0cfc0d93019..9d966e10a67f 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -296,6 +296,11 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e headers.removeEnvoyUpstreamRequestTimeoutAltResponse(); } + include_attempt_count_ = route_entry_->includeAttemptCount(); + if (include_attempt_count_) { + headers.insertEnvoyAttemptCount().value(attempt_count_); + } + route_entry_->finalizeRequestHeaders(headers, callbacks_->streamInfo(), !config_.suppress_envoy_headers_); FilterUtility::setUpstreamScheme(headers, *cluster_); @@ -762,6 +767,7 @@ bool Filter::setupRetry(bool end_stream) { void Filter::doRetry() { is_retry_ = true; + attempt_count_++; Http::ConnectionPool::Instance* conn_pool = getConnPool(); if (!conn_pool) { sendNoHealthyUpstreamResponse(); @@ -769,6 +775,10 @@ void Filter::doRetry() { return; } + if (include_attempt_count_) { + downstream_headers_->insertEnvoyAttemptCount().value(attempt_count_); + } + ASSERT(response_timeout_ || timeout_.global_timeout_.count() == 0); ASSERT(!upstream_request_); upstream_request_.reset(new UpstreamRequest(*this, *conn_pool)); diff --git a/source/common/router/router.h b/source/common/router/router.h index 57501b990082..61c8b95656a2 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -408,6 +408,8 @@ class Filter : Logger::Loggable, bool downstream_end_stream_ : 1; bool do_shadowing_ : 1; bool is_retry_ : 1; + bool include_attempt_count_ : 1; + uint32_t attempt_count_{1}; }; class ProdFilter : public Filter { diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 1ca9aa6bd3db..fc77bbffb00d 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -2199,6 +2199,26 @@ TEST(RouteMatcherTest, ClusterNotFoundNotCheckingViaConfig) { TestConfigImpl(parseRouteConfigurationFromJson(json), factory_context, true); } +TEST(RouteMatcherTest, AttemptCountHeader) { + std::string yaml = R"EOF( +virtual_hosts: + - name: "www2" + domains: ["www.lyft.com"] + include_request_attempt_count: true + routes: + - match: { prefix: "/"} + route: + cluster: "whatever" + )EOF"; + + NiceMock factory_context; + TestConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), factory_context, true); + + EXPECT_TRUE(config.route(genHeaders("www.lyft.com", "/foo", "GET"), 0) + ->routeEntry() + ->includeAttemptCount()); +} + TEST(RouteMatchTest, ClusterNotFoundResponseCode) { std::string yaml = R"EOF( virtual_hosts: diff --git a/test/config/utility.cc b/test/config/utility.cc index 5ecae1359e92..5501b61189f7 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -328,7 +328,8 @@ void ConfigHelper::addRoute(const std::string& domains, const std::string& prefi const std::string& cluster, bool validate_clusters, envoy::api::v2::route::RouteAction::ClusterNotFoundResponseCode code, envoy::api::v2::route::VirtualHost::TlsRequirementType type, - envoy::api::v2::route::RouteAction::RetryPolicy retry_policy) { + envoy::api::v2::route::RouteAction::RetryPolicy retry_policy, + bool include_attempt_count_header) { RELEASE_ASSERT(!finalized_, ""); envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager hcm_config; loadHttpConnectionManager(hcm_config); @@ -337,6 +338,7 @@ void ConfigHelper::addRoute(const std::string& domains, const std::string& prefi route_config->mutable_validate_clusters()->set_value(validate_clusters); auto* virtual_host = route_config->add_virtual_hosts(); virtual_host->set_name(domains); + virtual_host->set_include_request_attempt_count(include_attempt_count_header); virtual_host->add_domains(domains); virtual_host->add_routes()->mutable_match()->set_prefix(prefix); virtual_host->mutable_routes(0)->mutable_route()->set_cluster(cluster); diff --git a/test/config/utility.h b/test/config/utility.h index 68aee64c332b..f640b9bdc3ab 100644 --- a/test/config/utility.h +++ b/test/config/utility.h @@ -79,7 +79,8 @@ class ConfigHelper { envoy::api::v2::route::RouteAction::ClusterNotFoundResponseCode code, envoy::api::v2::route::VirtualHost::TlsRequirementType type = envoy::api::v2::route::VirtualHost::NONE, - envoy::api::v2::route::RouteAction::RetryPolicy retry_policy = {}); + envoy::api::v2::route::RouteAction::RetryPolicy retry_policy = {}, + bool include_attempt_count_header = false); // Add an HTTP filter prior to existing filters. void addFilter(const std::string& filter_yaml); diff --git a/test/integration/http2_integration_test.cc b/test/integration/http2_integration_test.cc index d869a36a7c66..3c4dd561b407 100644 --- a/test/integration/http2_integration_test.cc +++ b/test/integration/http2_integration_test.cc @@ -91,6 +91,8 @@ TEST_P(Http2IntegrationTest, TwoRequestsWithForcedBackup) { testTwoRequests(true TEST_P(Http2IntegrationTest, Retry) { testRetry(); } +TEST_P(Http2IntegrationTest, RetryAttemptCount) { testRetryAttemptCountHeader(); } + TEST_P(Http2IntegrationTest, EnvoyHandling100Continue) { testEnvoyHandling100Continue(); } TEST_P(Http2IntegrationTest, EnvoyHandlingDuplicate100Continue) { diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index c037c8139d79..77e868068f92 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -735,6 +735,47 @@ void HttpIntegrationTest::testRetry() { EXPECT_EQ(512U, response->body().size()); } +// Tests that the x-envoy-attempt-count header is properly set on the upstream request +// and updated after the request is retried. +void HttpIntegrationTest::testRetryAttemptCountHeader() { + config_helper_.addRoute("host", "/test_retry", "cluster_0", false, + envoy::api::v2::route::RouteAction::NOT_FOUND, + envoy::api::v2::route::VirtualHost::NONE, {}, true); + initialize(); + codec_client_ = makeHttpConnection(lookupPort("http")); + auto response = + codec_client_->makeRequestWithBody(Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/test_retry"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, + {"x-envoy-retry-on", "5xx"}}, + 1024); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "503"}}, false); + + EXPECT_EQ(atoi(upstream_request_->headers().EnvoyAttemptCount()->value().c_str()), 1); + + if (fake_upstreams_[0]->httpType() == FakeHttpConnection::Type::HTTP1) { + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + } else { + ASSERT_TRUE(upstream_request_->waitForReset()); + } + waitForNextUpstreamRequest(); + EXPECT_EQ(atoi(upstream_request_->headers().EnvoyAttemptCount()->value().c_str()), 2); + upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false); + upstream_request_->encodeData(512, true); + + response->waitForEndStream(); + EXPECT_TRUE(upstream_request_->complete()); + EXPECT_EQ(1024U, upstream_request_->bodyLength()); + + EXPECT_TRUE(response->complete()); + EXPECT_STREQ("200", response->headers().Status()->value().c_str()); + EXPECT_EQ(512U, response->body().size()); +} + // Change the default route to be restrictive, and send a request to an alternate route. void HttpIntegrationTest::testGrpcRouterNotFound() { config_helper_.setDefaultHostAndRoute("foo.com", "/found"); diff --git a/test/integration/http_integration.h b/test/integration/http_integration.h index 0559715cc784..10308c2c6de2 100644 --- a/test/integration/http_integration.h +++ b/test/integration/http_integration.h @@ -170,6 +170,7 @@ class HttpIntegrationTest : public BaseIntegrationTest { void testDrainClose(); void testRetry(); void testRetryHittingBufferLimit(); + void testRetryAttemptCountHeader(); void testGrpcRouterNotFound(); void testGrpcRetry(); void testRetryPriority(); diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index fd1050cce4ea..fe0103724c72 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -109,6 +109,8 @@ TEST_P(IntegrationTest, RouterUpstreamResponseBeforeRequestComplete) { TEST_P(IntegrationTest, Retry) { testRetry(); } +TEST_P(IntegrationTest, RetryAttemptCount) { testRetryAttemptCountHeader(); } + TEST_P(IntegrationTest, RetryHostPredicateFilter) { testRetryHostPredicateFilter(); } TEST_P(IntegrationTest, RetryPriority) { testRetryPriority(); } diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 1ce5bb83f1f3..d146dd0132bc 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -175,6 +175,7 @@ class MockVirtualHost : public VirtualHost { MOCK_CONST_METHOD0(corsPolicy, const CorsPolicy*()); MOCK_CONST_METHOD0(routeConfig, const Config&()); MOCK_CONST_METHOD1(perFilterConfig, const RouteSpecificFilterConfig*(const std::string&)); + MOCK_CONST_METHOD0(includeAttemptCount, bool()); MOCK_METHOD0(retryPriority, Upstream::RetryPrioritySharedPtr()); MOCK_METHOD0(retryHostPredicate, Upstream::RetryHostPredicateSharedPtr()); @@ -258,6 +259,7 @@ class MockRouteEntry : public RouteEntry { MOCK_CONST_METHOD0(metadata, const envoy::api::v2::core::Metadata&()); MOCK_CONST_METHOD0(pathMatchCriterion, const PathMatchCriterion&()); MOCK_CONST_METHOD1(perFilterConfig, const RouteSpecificFilterConfig*(const std::string&)); + MOCK_CONST_METHOD0(includeAttemptCount, bool()); std::string cluster_name_{"fake_cluster"}; std::multimap opaque_config_; diff --git a/tools/spelling_whitelist_words.txt b/tools/spelling_whitelist_words.txt index 7594b1c61fe4..c2924df99f67 100644 --- a/tools/spelling_whitelist_words.txt +++ b/tools/spelling_whitelist_words.txt @@ -1,4 +1,4 @@ -# One word per line, these words are not spell checked. +# One word per line, these words are not spell checked. # you can add a comment to each word to explain why you don't need to do a spell check. # bazel keywords in bazel/cc_configure.bzl