Log rate limited requests with new response flag. (envoyproxy#346)

kyessenov · Jan 11, 2017 · 453b511 · 453b511
1 parent c0deb5b
commit 453b511
Show file tree

Hide file tree

Showing 7 changed files with 20 additions and 2 deletions.
diff --git a/docs/configuration/http_conn_man/access_log.rst b/docs/configuration/http_conn_man/access_log.rst
@@ -85,6 +85,7 @@ The following command operators are supported:
   * **NR**: No :ref:`route configured <arch_overview_http_routing>` for a given request in addition to 404 response code.
   * **DI**: The request processing was delayed for a period specified via :ref:`fault injection <config_http_filters_fault_injection>`.
   * **FI**: The request was aborted with a response code specified via :ref:`fault injection <config_http_filters_fault_injection>`.
+  * **RL**: The request was ratelimited locally by the :ref:`HTTP rate limit filter <config_http_filters_rate_limit>` in addition to 429 response code.
 
 %UPSTREAM_HOST%
   Upstream host URL (e.g., tcp://ip:port for TCP connections).

diff --git a/include/envoy/http/access_log.h b/include/envoy/http/access_log.h
@@ -32,7 +32,9 @@ enum ResponseFlag {
   // Request was delayed before proxying.
   DelayInjected = 0x200,
   // Abort with error code was injected.
-  FaultInjected = 0x400
+  FaultInjected = 0x400,
+  // Request was ratelimited locally by rate limit filter.
+  RateLimited = 0x800
 };
 
 /**

diff --git a/source/common/http/access_log/access_log_formatter.cc b/source/common/http/access_log/access_log_formatter.cc
@@ -18,6 +18,7 @@ const std::string ResponseFlagUtils::UPSTREAM_OVERFLOW = "UO";
 const std::string ResponseFlagUtils::NO_ROUTE_FOUND = "NR";
 const std::string ResponseFlagUtils::DELAY_INJECTED = "DI";
 const std::string ResponseFlagUtils::FAULT_INJECTED = "FI";
+const std::string ResponseFlagUtils::RATE_LIMITED = "RL";
 
 void ResponseFlagUtils::appendString(std::string& result, const std::string& append) {
   if (result.empty()) {
@@ -74,6 +75,10 @@ const std::string ResponseFlagUtils::toShortString(const RequestInfo& request_in
     appendString(result, FAULT_INJECTED);
   }
 
+  if (request_info.getResponseFlag(ResponseFlag::RateLimited)) {
+    appendString(result, RATE_LIMITED);
+  }
+
   return result.empty() ? NONE : result;
 }
 

diff --git a/source/common/http/access_log/access_log_formatter.h b/source/common/http/access_log/access_log_formatter.h
@@ -29,6 +29,7 @@ class ResponseFlagUtils {
   const static std::string NO_ROUTE_FOUND;
   const static std::string DELAY_INJECTED;
   const static std::string FAULT_INJECTED;
+  const static std::string RATE_LIMITED;
 };
 
 /**

diff --git a/source/common/http/filter/ratelimit.cc b/source/common/http/filter/ratelimit.cc
@@ -94,6 +94,7 @@ void Filter::complete(::RateLimit::LimitStatus status) {
     state_ = State::Responded;
     Http::HeaderMapPtr response_headers{new HeaderMapImpl(*TOO_MANY_REQUESTS_HEADER)};
     callbacks_->encodeHeaders(std::move(response_headers), true);
+    callbacks_->requestInfo().setResponseFlag(Http::AccessLog::ResponseFlag::RateLimited);
   } else if (!initiating_call_) {
     callbacks_->continueDecoding();
   }

diff --git a/test/common/http/access_log/access_log_formatter_test.cc b/test/common/http/access_log/access_log_formatter_test.cc
@@ -25,7 +25,8 @@ TEST(ResponseFlagUtilsTest, toShortStringConversion) {
       std::make_pair(ResponseFlag::UpstreamOverflow, "UO"),
       std::make_pair(ResponseFlag::NoRouteFound, "NR"),
       std::make_pair(ResponseFlag::DelayInjected, "DI"),
-      std::make_pair(ResponseFlag::FaultInjected, "FI")};
+      std::make_pair(ResponseFlag::FaultInjected, "FI"),
+      std::make_pair(ResponseFlag::RateLimited, "RL")};
 
   for (const auto& testCase : expected) {
     NiceMock<MockRequestInfo> request_info;

diff --git a/test/common/http/filter/ratelimit_test.cc b/test/common/http/filter/ratelimit_test.cc
@@ -137,6 +137,8 @@ TEST_F(HttpRateLimitFilterTest, OkResponse) {
   EXPECT_EQ(FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_));
 
   EXPECT_CALL(filter_callbacks_, continueDecoding());
+  EXPECT_CALL(filter_callbacks_.request_info_,
+              setResponseFlag(Http::AccessLog::ResponseFlag::RateLimited)).Times(0);
   request_callbacks_->complete(::RateLimit::LimitStatus::OK);
 
   EXPECT_EQ(1U, cm_.cluster_.info_->stats_store_.counter("ratelimit.ok").value());
@@ -180,6 +182,8 @@ TEST_F(HttpRateLimitFilterTest, ErrorResponse) {
 
   EXPECT_EQ(FilterDataStatus::Continue, filter_->decodeData(data_, false));
   EXPECT_EQ(FilterTrailersStatus::Continue, filter_->decodeTrailers(request_headers_));
+  EXPECT_CALL(filter_callbacks_.request_info_,
+              setResponseFlag(Http::AccessLog::ResponseFlag::RateLimited)).Times(0);
 
   EXPECT_EQ(1U, cm_.cluster_.info_->stats_store_.counter("ratelimit.error").value());
 }
@@ -199,6 +203,9 @@ TEST_F(HttpRateLimitFilterTest, LimitResponse) {
   Http::TestHeaderMapImpl response_headers{{":status", "429"}};
   EXPECT_CALL(filter_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true));
   EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0);
+  EXPECT_CALL(filter_callbacks_.request_info_,
+              setResponseFlag(Http::AccessLog::ResponseFlag::RateLimited));
+
   request_callbacks_->complete(::RateLimit::LimitStatus::OverLimit);
 
   EXPECT_EQ(1U, cm_.cluster_.info_->stats_store_.counter("ratelimit.over_limit").value());