router: allow propagating attempt count in header (#4536)

Signed-off-by: Snow Pettersen <snowp@squareup.com>

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 <config_http_filters>`
   // for if and how it is utilized.
   map<string, google.protobuf.Struct> per_filter_config = 12;
+
+  // 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
+  // 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
+  // <envoy_api_field_config.filter.http.router.v2.Router.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

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 <envoy_api_field_route.VirtualHost.include_request_attempt_count>`
+flag is set to true.
+
 .. _config_http_filters_router_x-envoy-expected-rq-timeout-ms:
 
 x-envoy-expected-rq-timeout-ms

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 <envoy_api_field_route.VirtualHost.include_request_attempt_count>`.
 * stats: added :ref:`stats_matcher <envoy_api_field_config.metrics.v2.StatsConfig.stats_matcher>` to the bootstrap config for granular control of stat instantiation.
 * fault: removed integer percentage support.
 * router: when :ref:`max_grpc_timeout <envoy_api_field_route.RouteAction.max_grpc_timeout>`

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)                                                          \

include/envoy/router/router.h

@@ -345,6 +345,11 @@ class VirtualHost {
   template <class Derived> const Derived* perFilterConfigTyped(const std::string& name) const {
     return dynamic_cast<const Derived*>(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 <class Derived> const Derived* perFilterConfigTyped(const std::string& name) const {
     return dynamic_cast<const Derived*>(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;
 };
 
 /**

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_;

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"};

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;

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<VirtualHostImpl> 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; }

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,13 +767,18 @@ bool Filter::setupRetry(bool end_stream) {
 
 void Filter::doRetry() {
   is_retry_ = true;
+  attempt_count_++;
   Http::ConnectionPool::Instance* conn_pool = getConnPool();
   if (!conn_pool) {
     sendNoHealthyUpstreamResponse();
     cleanup();
     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));

source/common/router/router.h

@@ -408,6 +408,8 @@ class Filter : Logger::Loggable<Logger::Id::router>,
   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 {

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<Server::Configuration::MockFactoryContext> 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:

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);

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);

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) {

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");

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();

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(); }

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<std::string, std::string> opaque_config_;

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