Support Mixer HTTP filter to report sent.bytes and received.bytes attributes (#1372)

JimmyCYJ · istio-merge-robot · commit d20cd6d89dfa · 2018-04-16T10:29:44.000-07:00
Automatic merge from submit-queue. Support Mixer HTTP filter to report sent.bytes and received.bytes attributes **What this PR does / why we need it**:Support Mixer HTTP filter to send attributes "sent.bytes" and "received.bytes" in Report() calls. "sent.bytes" records total response sent in bytes, including response headers, body, and trailers. "received.bytes" records total request received in bytes, including request headers, body, and trailers. **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #1370 **Special notes for your reviewer**: **Release note**: ```release-note NONE ```
diff --git a/include/istio/control/http/report_data.h b/include/istio/control/http/report_data.h
@@ -34,8 +34,10 @@ class ReportData {
 
   // Get additional report info.
   struct ReportInfo {
-    uint64_t send_bytes;
-    uint64_t received_bytes;
+    uint64_t response_totle_size;
+    uint64_t request_totle_size;
+    uint64_t request_body_size;
+    uint64_t response_body_size;
     std::chrono::nanoseconds duration;
     int response_code;
   };
diff --git a/src/envoy/http/mixer/filter.cc b/src/envoy/http/mixer/filter.cc
@@ -107,6 +107,7 @@ void Filter::ReadPerRouteConfig(
 
 FilterHeadersStatus Filter::decodeHeaders(HeaderMap& headers, bool) {
   ENVOY_LOG(debug, "Called Mixer::Filter : {}", __func__);
+  received_bytes_ += headers.byteSize();
 
   ::istio::control::http::Controller::PerRouteConfig config;
   auto route = decoder_callbacks_->route();
@@ -135,14 +136,16 @@ FilterHeadersStatus Filter::decodeHeaders(HeaderMap& headers, bool) {
 FilterDataStatus Filter::decodeData(Buffer::Instance& data, bool end_stream) {
   ENVOY_LOG(debug, "Called Mixer::Filter : {} ({}, {})", __func__,
             data.length(), end_stream);
+  received_bytes_ += data.length();
   if (state_ == Calling) {
     return FilterDataStatus::StopIterationAndWatermark;
   }
   return FilterDataStatus::Continue;
 }
 
-FilterTrailersStatus Filter::decodeTrailers(HeaderMap&) {
+FilterTrailersStatus Filter::decodeTrailers(HeaderMap& trailers) {
   ENVOY_LOG(debug, "Called Mixer::Filter : {}", __func__);
+  received_bytes_ += trailers.byteSize();
   if (state_ == Calling) {
     return FilterTrailersStatus::StopIteration;
   }
@@ -182,6 +185,35 @@ void Filter::completeCheck(const Status& status) {
   }
 }
 
+// Http::StreamEncoderFilter
+FilterHeadersStatus Filter::encode100ContinueHeaders(Http::HeaderMap& headers) {
+  ENVOY_LOG(debug, "Called Mixer::Filter : {}", __func__);
+  sent_bytes_ += headers.byteSize();
+  return FilterHeadersStatus::Continue;
+}
+
+FilterHeadersStatus Filter::encodeHeaders(Http::HeaderMap& headers, bool) {
+  ENVOY_LOG(debug, "Called Mixer::Filter : {}", __func__);
+  sent_bytes_ += headers.byteSize();
+  return Http::FilterHeadersStatus::Continue;
+}
+
+FilterDataStatus Filter::encodeData(Buffer::Instance& data, bool) {
+  ENVOY_LOG(debug, "Called Mixer::Filter : {}", __func__);
+  sent_bytes_ += data.length();
+  return Http::FilterDataStatus::Continue;
+}
+
+FilterTrailersStatus Filter::encodeTrailers(Http::HeaderMap& trailers) {
+  sent_bytes_ += trailers.byteSize();
+  return Http::FilterTrailersStatus::Continue;
+}
+
+void Filter::setEncoderFilterCallbacks(
+    StreamEncoderFilterCallbacks& callbacks) {
+  encoder_callbacks_ = &callbacks;
+}
+
 void Filter::onDestroy() {
   ENVOY_LOG(debug, "Called Mixer::Filter : {} state: {}", __func__, state_);
   if (state_ != Calling) {
@@ -212,7 +244,8 @@ void Filter::log(const HeaderMap* request_headers,
     CheckData check_data(*request_headers, nullptr);
     handler_->ExtractRequestAttributes(&check_data);
   }
-  ReportData report_data(response_headers, request_info);
+  ReportData report_data(response_headers, request_info, sent_bytes_,
+                         received_bytes_);
   handler_->Report(&report_data);
 }
 
diff --git a/src/envoy/http/mixer/filter.h b/src/envoy/http/mixer/filter.h
@@ -24,7 +24,7 @@ namespace Envoy {
 namespace Http {
 namespace Mixer {
 
-class Filter : public Http::StreamDecoderFilter,
+class Filter : public Http::StreamFilter,
                public AccessLog::Instance,
                public Logger::Loggable<Logger::Id::filter> {
  public:
@@ -33,9 +33,20 @@ class Filter : public Http::StreamDecoderFilter,
   // Implementing virtual functions for StreamDecoderFilter
   FilterHeadersStatus decodeHeaders(HeaderMap& headers, bool) override;
   FilterDataStatus decodeData(Buffer::Instance& data, bool end_stream) override;
-  FilterTrailersStatus decodeTrailers(HeaderMap&) override;
+  FilterTrailersStatus decodeTrailers(HeaderMap& trailers) override;
   void setDecoderFilterCallbacks(
       StreamDecoderFilterCallbacks& callbacks) override;
+
+  // Http::StreamEncoderFilter
+  FilterHeadersStatus encode100ContinueHeaders(
+      Http::HeaderMap& headers) override;
+  FilterHeadersStatus encodeHeaders(HeaderMap& headers, bool) override;
+  FilterDataStatus encodeData(Buffer::Instance& data, bool) override;
+  FilterTrailersStatus encodeTrailers(HeaderMap& trailers) override;
+  void setEncoderFilterCallbacks(
+      StreamEncoderFilterCallbacks& callbacks) override;
+
+  // Http::StreamFilterBase
   void onDestroy() override;
 
   // This is the callback function when Check is done.
@@ -67,8 +78,16 @@ class Filter : public Http::StreamDecoderFilter,
   // Point to the request HTTP headers
   HeaderMap* headers_;
 
-  // The filter callback.
-  StreamDecoderFilterCallbacks* decoder_callbacks_;
+  // Total number of bytes received, including request headers, body, and
+  // trailers.
+  uint64_t received_bytes_{0};
+  // Total number of bytes sent, including response headers, body, and trailers.
+  uint64_t sent_bytes_{0};
+
+  // The stream decoder filter callback.
+  StreamDecoderFilterCallbacks* decoder_callbacks_{nullptr};
+  // The stream encoder filter callback.
+  StreamEncoderFilterCallbacks* encoder_callbacks_{nullptr};
 };
 
 }  // namespace Mixer
diff --git a/src/envoy/http/mixer/report_data.h b/src/envoy/http/mixer/report_data.h
@@ -32,10 +32,16 @@ const std::set<std::string> ResponseHeaderExclusives = {};
 class ReportData : public ::istio::control::http::ReportData {
   const HeaderMap *headers_;
   const RequestInfo::RequestInfo &info_;
+  uint64_t sent_bytes_;
+  uint64_t received_bytes_;
 
  public:
-  ReportData(const HeaderMap *headers, const RequestInfo::RequestInfo &info)
-      : headers_(headers), info_(info) {}
+  ReportData(const HeaderMap *headers, const RequestInfo::RequestInfo &info,
+             uint64_t sent_bytes, uint64_t received_bytes)
+      : headers_(headers),
+        info_(info),
+        sent_bytes_(sent_bytes),
+        received_bytes_(received_bytes) {}
 
   std::map<std::string, std::string> GetResponseHeaders() const override {
     if (headers_) {
@@ -46,8 +52,10 @@ class ReportData : public ::istio::control::http::ReportData {
 
   void GetReportInfo(
       ::istio::control::http::ReportData::ReportInfo *data) const override {
-    data->received_bytes = info_.bytesReceived();
-    data->send_bytes = info_.bytesSent();
+    data->request_body_size = info_.bytesReceived();
+    data->response_body_size = info_.bytesSent();
+    data->response_totle_size = sent_bytes_;
+    data->request_totle_size = received_bytes_;
     data->duration =
         info_.requestComplete().value_or(std::chrono::nanoseconds{0});
     // responseCode is for the backend response. If it is not valid, the request
diff --git a/src/istio/control/attribute_names.cc b/src/istio/control/attribute_names.cc
@@ -28,15 +28,17 @@ const char AttributeName::kRequestMethod[] = "request.method";
 const char AttributeName::kRequestPath[] = "request.path";
 const char AttributeName::kRequestReferer[] = "request.referer";
 const char AttributeName::kRequestScheme[] = "request.scheme";
-const char AttributeName::kRequestSize[] = "request.size";
+const char AttributeName::kRequestBodySize[] = "request.size";
+const char AttributeName::kRequestTotleSize[] = "request.totle_size";
 const char AttributeName::kRequestTime[] = "request.time";
 const char AttributeName::kRequestUserAgent[] = "request.useragent";
 const char AttributeName::kRequestApiKey[] = "request.api_key";
 
 const char AttributeName::kResponseCode[] = "response.code";
 const char AttributeName::kResponseDuration[] = "response.duration";
 const char AttributeName::kResponseHeaders[] = "response.headers";
-const char AttributeName::kResponseSize[] = "response.size";
+const char AttributeName::kResponseBodySize[] = "response.size";
+const char AttributeName::kResponseTotleSize[] = "response.totle_size";
 const char AttributeName::kResponseTime[] = "response.time";
 
 // TCP attributes
diff --git a/src/istio/control/attribute_names.h b/src/istio/control/attribute_names.h
@@ -34,15 +34,21 @@ struct AttributeName {
   static const char kRequestPath[];
   static const char kRequestReferer[];
   static const char kRequestScheme[];
-  static const char kRequestSize[];
+  static const char kRequestBodySize[];
+  // Total size of request received, including request headers, body, and
+  // trailers.
+  static const char kRequestTotleSize[];
   static const char kRequestTime[];
   static const char kRequestUserAgent[];
   static const char kRequestApiKey[];
 
   static const char kResponseCode[];
   static const char kResponseDuration[];
   static const char kResponseHeaders[];
-  static const char kResponseSize[];
+  static const char kResponseBodySize[];
+  // Total size of response sent, including response headers, body, and
+  // trailers.
+  static const char kResponseTotleSize[];
   static const char kResponseTime[];
 
   // TCP attributes
diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc
@@ -180,8 +180,10 @@ void AttributesBuilder::ExtractReportAttributes(ReportData *report_data) {
 
   ReportData::ReportInfo info;
   report_data->GetReportInfo(&info);
-  builder.AddInt64(AttributeName::kRequestSize, info.received_bytes);
-  builder.AddInt64(AttributeName::kResponseSize, info.send_bytes);
+  builder.AddInt64(AttributeName::kRequestBodySize, info.request_body_size);
+  builder.AddInt64(AttributeName::kResponseBodySize, info.response_body_size);
+  builder.AddInt64(AttributeName::kRequestTotleSize, info.request_totle_size);
+  builder.AddInt64(AttributeName::kResponseTotleSize, info.response_totle_size);
   builder.AddDuration(AttributeName::kResponseDuration, info.duration);
   if (!request_->check_status.ok()) {
     builder.AddInt64(
diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc
@@ -222,6 +222,18 @@ attributes {
     int64_value: 200
   }
 }
+attributes {
+  key: "response.totle_size"
+  value {
+    int64_value: 120
+  }
+}
+attributes {
+  key: "request.totle_size"
+  value {
+    int64_value: 240
+  }
+}
 attributes {
   key: "response.time"
   value {
@@ -423,8 +435,10 @@ TEST(AttributesBuilderTest, TestReportAttributes) {
       }));
   EXPECT_CALL(mock_data, GetReportInfo(_))
       .WillOnce(Invoke([](ReportData::ReportInfo *info) {
-        info->received_bytes = 100;
-        info->send_bytes = 200;
+        info->request_body_size = 100;
+        info->response_body_size = 200;
+        info->response_totle_size = 120;
+        info->request_totle_size = 240;
         info->duration = std::chrono::nanoseconds(1);
         info->response_code = 404;
       }));
@@ -463,8 +477,10 @@ TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) {
       }));
   EXPECT_CALL(mock_data, GetReportInfo(_))
       .WillOnce(Invoke([](ReportData::ReportInfo *info) {
-        info->received_bytes = 100;
-        info->send_bytes = 200;
+        info->request_body_size = 100;
+        info->response_body_size = 200;
+        info->response_totle_size = 120;
+        info->request_totle_size = 240;
         info->duration = std::chrono::nanoseconds(1);
         info->response_code = 404;
       }));
