From eb5b380f2c5a4ae5bac6889926427d69e0aa4792 Mon Sep 17 00:00:00 2001
From: Shriram Rajagopalan <shriramr@vmware.com>
Date: Thu, 1 Nov 2018 15:31:35 +0000
Subject: [PATCH 1/4] use string map instead of opaque struct for dynamic
 metadata

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
---
 include/istio/control/http/report_data.h      |  4 +-
 include/istio/control/tcp/report_data.h       |  4 +-
 include/istio/utils/attribute_names.h         |  6 --
 include/istio/utils/attributes_builder.h      | 15 ++++
 src/envoy/http/mixer/report_data.h            | 10 +--
 src/envoy/tcp/mixer/filter.cc                 | 17 +---
 src/envoy/tcp/mixer/filter.h                  |  5 +-
 src/istio/control/http/attributes_builder.cc  |  5 +-
 .../control/http/attributes_builder_test.cc   | 60 +++++++++++---
 src/istio/control/http/mock_report_data.h     |  2 +-
 .../control/http/request_handler_impl_test.cc |  5 ++
 src/istio/control/tcp/attributes_builder.cc   |  6 +-
 .../control/tcp/attributes_builder_test.cc    | 79 +++++++++++++++----
 src/istio/control/tcp/mock_report_data.h      |  2 +-
 .../control/tcp/request_handler_impl_test.cc  |  3 +
 src/istio/utils/attribute_names.cc            |  7 --
 src/istio/utils/utils.h                       |  2 +-
 17 files changed, 158 insertions(+), 74 deletions(-)

diff --git a/include/istio/control/http/report_data.h b/include/istio/control/http/report_data.h
index 7b4968200da..71749cbde47 100644
--- a/include/istio/control/http/report_data.h
+++ b/include/istio/control/http/report_data.h
@@ -19,6 +19,8 @@
 #include <chrono>
 #include <map>
 
+#include "google/protobuf/struct.pb.h"
+
 namespace istio {
 namespace control {
 namespace http {
@@ -66,7 +68,7 @@ class ReportData {
 
   // Get dynamic metadata generated by Envoy filters.
   // Useful for logging info generated by custom codecs.
-  virtual bool GetDynamicFilterState(std::string* filter_state) const = 0;
+  virtual const ::google::protobuf::Map<std::string, ::google::protobuf::Struct >& GetDynamicFilterState() const = 0;
 };
 
 }  // namespace http
diff --git a/include/istio/control/tcp/report_data.h b/include/istio/control/tcp/report_data.h
index b8d2bc02e31..93c86609408 100644
--- a/include/istio/control/tcp/report_data.h
+++ b/include/istio/control/tcp/report_data.h
@@ -19,6 +19,8 @@
 #include <chrono>
 #include <string>
 
+#include "google/protobuf/struct.pb.h"
+
 namespace istio {
 namespace control {
 namespace tcp {
@@ -53,7 +55,7 @@ class ReportData {
 
   // Get dynamic metadata generated by Envoy filters.
   // Useful for logging info generated by custom codecs.
-  virtual bool GetDynamicFilterState(std::string* filter_state) const = 0;
+  virtual const ::google::protobuf::Map< ::std::string, ::google::protobuf::Struct >& GetDynamicFilterState() const = 0;
 };
 
 }  // namespace tcp
diff --git a/include/istio/utils/attribute_names.h b/include/istio/utils/attribute_names.h
index 4cc8b8cc336..c355e8d843e 100644
--- a/include/istio/utils/attribute_names.h
+++ b/include/istio/utils/attribute_names.h
@@ -47,9 +47,6 @@ struct AttributeName {
   static const char kRequestUserAgent[];
   static const char kRequestApiKey[];
 
-  // Dynamic state generated by various envoy http filters
-  static const char kRequestDynamicState[];
-
   static const char kResponseCode[];
   static const char kResponseDuration[];
   static const char kResponseHeaders[];
@@ -80,9 +77,6 @@ struct AttributeName {
   // Record TCP connection status: open, continue, close
   static const char kConnectionEvent[];
 
-  // Dynamic state generated by various envoy network filters
-  static const char kConnectionDynamicState[];
-
   // Context attributes
   static const char kContextProtocol[];
   static const char kContextReporterKind[];
diff --git a/include/istio/utils/attributes_builder.h b/include/istio/utils/attributes_builder.h
index 82e71e89747..edc0467f540 100644
--- a/include/istio/utils/attributes_builder.h
+++ b/include/istio/utils/attributes_builder.h
@@ -124,6 +124,21 @@ class AttributesBuilder {
     }
   }
 
+  // Serializes all the keys in a map<string, struct> and builds attributes.
+  // for example, foo.bar.com: struct {str:abc, list:[c,d,e]} will be emitted as
+  // foo.bar.com: string_map[str:abc, list: c,d,e]
+  // Only extracts strings and lists.
+  // TODO: add the ability to pack bools and nums as strings and recurse down the struct.
+  void FlattenMapOfStringToStruct(const ::google::protobuf::Map< ::std::string, ::google::protobuf::Struct >& filter_state) {
+      if (filter_state.empty()) {
+          return;
+      }
+
+      for (const auto& filter : filter_state) {
+          AddProtoStructStringMap(filter.first, filter.second);
+      }
+  }
+
   bool HasAttribute(const std::string& key) const {
     const auto& attrs_map = attributes_->attributes();
     return attrs_map.find(key) != attrs_map.end();
diff --git a/src/envoy/http/mixer/report_data.h b/src/envoy/http/mixer/report_data.h
index 338e188fbfb..65734925f9d 100644
--- a/src/envoy/http/mixer/report_data.h
+++ b/src/envoy/http/mixer/report_data.h
@@ -20,9 +20,11 @@
 #include "envoy/http/header_map.h"
 #include "envoy/stream_info/stream_info.h"
 #include "extensions/filters/http/well_known_names.h"
+#include "google/protobuf/struct.pb.h"
 #include "include/istio/control/http/controller.h"
 #include "src/envoy/utils/utils.h"
 
+
 namespace Envoy {
 namespace Http {
 namespace Mixer {
@@ -158,12 +160,8 @@ class ReportData : public ::istio::control::http::ReportData,
   }
 
   // Get attributes generated by http filters
-  bool GetDynamicFilterState(std::string *filter_state) const override {
-    if (info_.dynamicMetadata().filter_metadata_size() > 0) {
-      info_.dynamicMetadata().SerializeToString(filter_state);
-      return true;
-    }
-    return false;
+  const ::google::protobuf::Map<std::string, ::google::protobuf::Struct >& GetDynamicFilterState() const override {
+      return info_.dynamicMetadata().filter_metadata();
   }
 };
 
diff --git a/src/envoy/tcp/mixer/filter.cc b/src/envoy/tcp/mixer/filter.cc
index dee47a00982..1f1c56e66a2 100644
--- a/src/envoy/tcp/mixer/filter.cc
+++ b/src/envoy/tcp/mixer/filter.cc
@@ -177,19 +177,10 @@ bool Filter::GetDestinationUID(std::string *uid) const {
   }
   return false;
 }
-bool Filter::GetDynamicFilterState(std::string *filter_state) const {
-  if (filter_callbacks_->connection()
-          .streamInfo()
-          .dynamicMetadata()
-          .filter_metadata_size() > 0) {
-    filter_callbacks_->connection()
-        .streamInfo()
-        .dynamicMetadata()
-        .SerializeToString(filter_state);
-    return true;
-  }
-
-  return false;
+const ::google::protobuf::Map<std::string, ::google::protobuf::Struct >& Filter::GetDynamicFilterState() const {
+  return filter_callbacks_->connection()
+    .streamInfo()
+    .dynamicMetadata().filter_metadata();
 }
 void Filter::GetReportInfo(
     ::istio::control::tcp::ReportData::ReportInfo *data) const {
diff --git a/src/envoy/tcp/mixer/filter.h b/src/envoy/tcp/mixer/filter.h
index c897e038d49..a99e9254cbb 100644
--- a/src/envoy/tcp/mixer/filter.h
+++ b/src/envoy/tcp/mixer/filter.h
@@ -18,6 +18,7 @@
 #include "common/common/logger.h"
 #include "envoy/network/connection.h"
 #include "envoy/network/filter.h"
+#include "google/protobuf/struct.pb.h"
 #include "src/envoy/tcp/mixer/control.h"
 
 namespace Envoy {
@@ -57,8 +58,8 @@ class Filter : public Network::Filter,
 
   // ReportData virtual functions.
   bool GetDestinationIpPort(std::string* str_ip, int* port) const override;
-  bool GetDestinationUID(std::string* uid) const override;
-  bool GetDynamicFilterState(std::string* filter_state) const override;
+  bool GetDestinationUID(std::string* uid) const override;  
+  const ::google::protobuf::Map<std::string, ::google::protobuf::Struct >& GetDynamicFilterState() const override;
   void GetReportInfo(
       ::istio::control::tcp::ReportData::ReportInfo* data) const override;
   std::string GetConnectionId() const override;
diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc
index fce41d3a480..5e4e69fdd68 100644
--- a/src/istio/control/http/attributes_builder.cc
+++ b/src/istio/control/http/attributes_builder.cc
@@ -253,10 +253,7 @@ void AttributesBuilder::ExtractReportAttributes(ReportData *report_data) {
     }
   }
 
-  std::string filter_state;
-  if (report_data->GetDynamicFilterState(&filter_state)) {
-    builder.AddBytes(utils::AttributeName::kRequestDynamicState, filter_state);
-  }
+  builder.FlattenMapOfStringToStruct(report_data->GetDynamicFilterState());
 }
 
 }  // namespace http
diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc
index a0ceaa58f5b..e88cfd4d012 100644
--- a/src/istio/control/http/attributes_builder_test.cc
+++ b/src/istio/control/http/attributes_builder_test.cc
@@ -31,6 +31,7 @@ using ::istio::mixer::v1::Attributes_StringMap;
 
 using ::testing::_;
 using ::testing::Invoke;
+using ::testing::ReturnRef;
 
 namespace istio {
 namespace control {
@@ -407,9 +408,18 @@ attributes {
   }
 }
 attributes {
-  key: "request.dynamic_state"
+  key: "foo.bar.com"
   value {
-    bytes_value: "aeiou"
+    string_map_value {
+      entries {
+        key: "str"
+        value: "abc"
+      }
+      entries {
+        key: "list"
+        value: "a,b,c"
+      }
+    }
   }
 }
 )";
@@ -716,6 +726,22 @@ TEST(AttributesBuilderTest, TestCheckAttributes) {
 
 TEST(AttributesBuilderTest, TestReportAttributes) {
   ::testing::StrictMock<MockReportData> mock_data;
+
+  ::google::protobuf::Map<std::string, ::google::protobuf::Struct> filter_metadata;
+  ::google::protobuf::Struct struct_obj;
+  ::google::protobuf::Value strval, numval, boolval, listval;
+  strval.set_string_value("abc");
+  (*struct_obj.mutable_fields())["str"] = strval;
+  numval.set_number_value(12.3);
+  (*struct_obj.mutable_fields())["num"] = numval;
+  boolval.set_bool_value(true);
+  (*struct_obj.mutable_fields())["bool"] = boolval;
+  listval.mutable_list_value()->add_values()->set_string_value("a");
+  listval.mutable_list_value()->add_values()->set_string_value("b");
+  listval.mutable_list_value()->add_values()->set_string_value("c");
+  (*struct_obj.mutable_fields())["list"] = listval;
+  filter_metadata["foo.bar.com"]=struct_obj;
+
   EXPECT_CALL(mock_data, GetDestinationIpPort(_, _))
       .WillOnce(Invoke([](std::string *ip, int *port) -> bool {
         *ip = "1.2.3.4";
@@ -756,11 +782,8 @@ TEST(AttributesBuilderTest, TestReportAttributes) {
         report_info->permissive_policy_id = "policy-foo";
         return true;
       }));
-  EXPECT_CALL(mock_data, GetDynamicFilterState(_))
-      .WillOnce(Invoke([](std::string *dynamic_state) -> bool {
-        *dynamic_state = "aeiou";
-        return true;
-      }));
+  EXPECT_CALL(mock_data, GetDynamicFilterState())
+    .WillOnce(ReturnRef(filter_metadata));
 
   RequestContext request;
   AttributesBuilder builder(&request);
@@ -785,6 +808,22 @@ TEST(AttributesBuilderTest, TestReportAttributes) {
 
 TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) {
   ::testing::StrictMock<MockReportData> mock_data;
+
+  ::google::protobuf::Map<std::string, ::google::protobuf::Struct> filter_metadata;
+  ::google::protobuf::Struct struct_obj;
+  ::google::protobuf::Value strval, numval, boolval, listval;
+  strval.set_string_value("abc");
+  (*struct_obj.mutable_fields())["str"] = strval;
+  numval.set_number_value(12.3);
+  (*struct_obj.mutable_fields())["num"] = numval;
+  boolval.set_bool_value(true);
+  (*struct_obj.mutable_fields())["bool"] = boolval;
+  listval.mutable_list_value()->add_values()->set_string_value("a");
+  listval.mutable_list_value()->add_values()->set_string_value("b");
+  listval.mutable_list_value()->add_values()->set_string_value("c");
+  (*struct_obj.mutable_fields())["list"] = listval;
+  filter_metadata["foo.bar.com"]=struct_obj;
+
   EXPECT_CALL(mock_data, GetDestinationIpPort(_, _))
       .WillOnce(Invoke([](std::string *ip, int *port) -> bool {
         *ip = "2.3.4.5";
@@ -816,11 +855,8 @@ TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) {
         report_info->permissive_policy_id = "policy-foo";
         return true;
       }));
-  EXPECT_CALL(mock_data, GetDynamicFilterState(_))
-      .WillOnce(Invoke([](std::string *dynamic_state) -> bool {
-        *dynamic_state = "aeiou";
-        return true;
-      }));
+  EXPECT_CALL(mock_data, GetDynamicFilterState())
+    .WillOnce(ReturnRef(filter_metadata));
 
   RequestContext request;
   SetDestinationIp(&request, "1.2.3.4");
diff --git a/src/istio/control/http/mock_report_data.h b/src/istio/control/http/mock_report_data.h
index 580a1c4d074..503bb6270fc 100644
--- a/src/istio/control/http/mock_report_data.h
+++ b/src/istio/control/http/mock_report_data.h
@@ -32,7 +32,7 @@ class MockReportData : public ReportData {
   MOCK_CONST_METHOD1(GetDestinationUID, bool(std::string* ip));
   MOCK_CONST_METHOD1(GetGrpcStatus, bool(GrpcStatus* status));
   MOCK_CONST_METHOD1(GetRbacReportInfo, bool(RbacReportInfo* info));
-  MOCK_CONST_METHOD1(GetDynamicFilterState, bool(std::string* filter_state));
+  MOCK_CONST_METHOD0(GetDynamicFilterState, const ::google::protobuf::Map<std::string, ::google::protobuf::Struct>&());
 };
 
 }  // namespace http
diff --git a/src/istio/control/http/request_handler_impl_test.cc b/src/istio/control/http/request_handler_impl_test.cc
index eb5700f3dbe..74fc1d3a114 100644
--- a/src/istio/control/http/request_handler_impl_test.cc
+++ b/src/istio/control/http/request_handler_impl_test.cc
@@ -38,6 +38,7 @@ using ::istio::utils::LocalAttributes;
 
 using ::testing::_;
 using ::testing::Invoke;
+using ::testing::ReturnRef;
 
 namespace istio {
 namespace control {
@@ -464,9 +465,11 @@ TEST_F(RequestHandlerImplTest, TestDefaultApiKey) {
 TEST_F(RequestHandlerImplTest, TestHandlerReport) {
   ::testing::NiceMock<MockCheckData> mock_check;
   ::testing::NiceMock<MockReportData> mock_report;
+  ::google::protobuf::Map<std::string, ::google::protobuf::Struct> filter_metadata;
   EXPECT_CALL(mock_check, GetSourceIpPort(_, _)).Times(1);
   EXPECT_CALL(mock_report, GetResponseHeaders()).Times(1);
   EXPECT_CALL(mock_report, GetReportInfo(_)).Times(1);
+  EXPECT_CALL(mock_report, GetDynamicFilterState()).Times(1).WillOnce(ReturnRef(filter_metadata));
 
   // Report should be called.
   EXPECT_CALL(*mock_client_, Report(_)).Times(1);
@@ -485,6 +488,7 @@ TEST_F(RequestHandlerImplTest, TestHandlerDisabledReport) {
   EXPECT_CALL(mock_check, GetSourceIpPort(_, _)).Times(0);
   EXPECT_CALL(mock_report, GetResponseHeaders()).Times(0);
   EXPECT_CALL(mock_report, GetReportInfo(_)).Times(0);
+  EXPECT_CALL(mock_report, GetDynamicFilterState()).Times(0);
 
   // Report should NOT be called.
   EXPECT_CALL(*mock_client_, Report(_)).Times(0);
@@ -522,6 +526,7 @@ TEST_F(RequestHandlerImplTest, TestEmptyConfig) {
   ::testing::NiceMock<MockReportData> mock_report;
   EXPECT_CALL(mock_report, GetResponseHeaders()).Times(0);
   EXPECT_CALL(mock_report, GetReportInfo(_)).Times(0);
+  EXPECT_CALL(mock_report, GetDynamicFilterState()).Times(0);
 
   // Report should NOT be called.
   EXPECT_CALL(*mock_client_, Report(_)).Times(0);
diff --git a/src/istio/control/tcp/attributes_builder.cc b/src/istio/control/tcp/attributes_builder.cc
index d523e605f0c..0c8876e7923 100644
--- a/src/istio/control/tcp/attributes_builder.cc
+++ b/src/istio/control/tcp/attributes_builder.cc
@@ -132,11 +132,7 @@ void AttributesBuilder::ExtractReportAttributes(
     builder.AddString(utils::AttributeName::kDestinationUID, uid);
   }
 
-  std::string filter_state;
-  if (report_data->GetDynamicFilterState(&filter_state)) {
-    builder.AddBytes(utils::AttributeName::kConnectionDynamicState,
-                     filter_state);
-  }
+  builder.FlattenMapOfStringToStruct(report_data->GetDynamicFilterState());
 
   builder.AddTimestamp(utils::AttributeName::kContextTime,
                        std::chrono::system_clock::now());
diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc
index 02c96306fb4..7d6992f94cb 100644
--- a/src/istio/control/tcp/attributes_builder_test.cc
+++ b/src/istio/control/tcp/attributes_builder_test.cc
@@ -30,6 +30,7 @@ using ::google::protobuf::util::MessageDifferencer;
 using ::testing::_;
 using ::testing::Invoke;
 using ::testing::Return;
+using ::testing::ReturnRef;
 
 namespace istio {
 namespace control {
@@ -163,9 +164,18 @@ attributes {
   }
 }
 attributes {
-  key: "connection.dynamic_state"
+  key: "foo.bar.com"
   value {
-    bytes_value: "aeiou"
+    string_map_value {
+      entries {
+        key: "str"
+        value: "abc"
+      }
+      entries {
+        key: "list"
+        value: "a,b,c"
+      }
+    }
   }
 }
 )";
@@ -247,9 +257,18 @@ attributes {
   }
 }
 attributes {
-  key: "connection.dynamic_state"
+  key: "foo.bar.com"
   value {
-    bytes_value: "aeiou"
+    string_map_value {
+      entries {
+        key: "str"
+        value: "abc"
+      }
+      entries {
+        key: "list"
+        value: "a,b,c"
+      }
+    }
   }
 }
 )";
@@ -311,9 +330,18 @@ attributes {
   }
 }
 attributes {
-  key: "connection.dynamic_state"
+  key: "foo.bar.com"
   value {
-    bytes_value: "aeiou"
+    string_map_value {
+      entries {
+        key: "str"
+        value: "abc"
+      }
+      entries {
+        key: "list"
+        value: "a,b,c"
+      }
+    }
   }
 }
 )";
@@ -375,9 +403,18 @@ attributes {
   }
 }
 attributes {
-  key: "connection.dynamic_state"
+  key: "foo.bar.com"
   value {
-    bytes_value: "aeiou"
+    string_map_value {
+      entries {
+        key: "str"
+        value: "abc"
+      }
+      entries {
+        key: "list"
+        value: "a,b,c"
+      }
+    }
   }
 }
 )";
@@ -434,6 +471,23 @@ TEST(AttributesBuilderTest, TestCheckAttributes) {
 
 TEST(AttributesBuilderTest, TestReportAttributes) {
   ::testing::NiceMock<MockReportData> mock_data;
+
+  ::google::protobuf::Map<std::string, ::google::protobuf::Struct> filter_metadata;
+  ::google::protobuf::Struct struct_obj;
+  ::google::protobuf::Value strval, numval, boolval, listval;
+  strval.set_string_value("abc");
+  (*struct_obj.mutable_fields())["str"] = strval;
+  numval.set_number_value(12.3);
+  (*struct_obj.mutable_fields())["num"] = numval;
+  boolval.set_bool_value(true);
+  (*struct_obj.mutable_fields())["bool"] = boolval;
+  listval.mutable_list_value()->add_values()->set_string_value("a");
+  listval.mutable_list_value()->add_values()->set_string_value("b");
+  listval.mutable_list_value()->add_values()->set_string_value("c");
+  (*struct_obj.mutable_fields())["list"] = listval;
+  filter_metadata["foo.bar.com"]=struct_obj;
+
+
   EXPECT_CALL(mock_data, GetDestinationIpPort(_, _))
       .Times(4)
       .WillRepeatedly(Invoke([](std::string* ip, int* port) -> bool {
@@ -447,12 +501,9 @@ TEST(AttributesBuilderTest, TestReportAttributes) {
         *uid = "pod1.ns2";
         return true;
       }));
-  EXPECT_CALL(mock_data, GetDynamicFilterState(_))
-      .Times(4)
-      .WillRepeatedly(Invoke([](std::string* dynamic_state) -> bool {
-        *dynamic_state = "aeiou";
-        return true;
-      }));
+  EXPECT_CALL(mock_data, GetDynamicFilterState())
+    .Times(4)
+    .WillRepeatedly(ReturnRef(filter_metadata));
   EXPECT_CALL(mock_data, GetReportInfo(_))
       .Times(4)
       .WillOnce(Invoke([](ReportData::ReportInfo* info) {
diff --git a/src/istio/control/tcp/mock_report_data.h b/src/istio/control/tcp/mock_report_data.h
index 1631cebfae6..6113ce3c368 100644
--- a/src/istio/control/tcp/mock_report_data.h
+++ b/src/istio/control/tcp/mock_report_data.h
@@ -28,7 +28,7 @@ class MockReportData : public ReportData {
  public:
   MOCK_CONST_METHOD2(GetDestinationIpPort, bool(std::string* ip, int* port));
   MOCK_CONST_METHOD1(GetDestinationUID, bool(std::string*));
-  MOCK_CONST_METHOD1(GetDynamicFilterState, bool(std::string*));
+  MOCK_CONST_METHOD0(GetDynamicFilterState, const ::google::protobuf::Map<std::string, ::google::protobuf::Struct>&());
   MOCK_CONST_METHOD1(GetReportInfo, void(ReportInfo* info));
 };
 
diff --git a/src/istio/control/tcp/request_handler_impl_test.cc b/src/istio/control/tcp/request_handler_impl_test.cc
index eb8b9559c94..066148244a6 100644
--- a/src/istio/control/tcp/request_handler_impl_test.cc
+++ b/src/istio/control/tcp/request_handler_impl_test.cc
@@ -36,6 +36,7 @@ using ::istio::utils::LocalAttributes;
 
 using ::testing::_;
 using ::testing::Invoke;
+using ::testing::ReturnRef;
 
 namespace istio {
 namespace control {
@@ -141,9 +142,11 @@ TEST_F(RequestHandlerImplTest, TestHandlerCheck) {
 
 TEST_F(RequestHandlerImplTest, TestHandlerReport) {
   ::testing::NiceMock<MockReportData> mock_data;
+  ::google::protobuf::Map<std::string, ::google::protobuf::Struct> filter_metadata;
   EXPECT_CALL(mock_data, GetDestinationIpPort(_, _)).Times(1);
   EXPECT_CALL(mock_data, GetDestinationUID(_)).Times(1);
   EXPECT_CALL(mock_data, GetReportInfo(_)).Times(1);
+  EXPECT_CALL(mock_data, GetDynamicFilterState()).Times(1).WillOnce(ReturnRef(filter_metadata));
 
   // Report should be called.
   EXPECT_CALL(*mock_client_, Report(_)).Times(1);
diff --git a/src/istio/utils/attribute_names.cc b/src/istio/utils/attribute_names.cc
index 86c04960814..392bc1c45ef 100644
--- a/src/istio/utils/attribute_names.cc
+++ b/src/istio/utils/attribute_names.cc
@@ -39,9 +39,6 @@ const char AttributeName::kRequestTime[] = "request.time";
 const char AttributeName::kRequestUserAgent[] = "request.useragent";
 const char AttributeName::kRequestApiKey[] = "request.api_key";
 
-// Dynamic state generated by various envoy http filters
-const char AttributeName::kRequestDynamicState[] = "request.dynamic_state";
-
 const char AttributeName::kResponseCode[] = "response.code";
 const char AttributeName::kResponseDuration[] = "response.duration";
 const char AttributeName::kResponseHeaders[] = "response.headers";
@@ -75,10 +72,6 @@ const char AttributeName::kConnectionRequestedServerName[] =
 const char AttributeName::kConnectionId[] = "connection.id";
 const char AttributeName::kConnectionEvent[] = "connection.event";
 
-// Dynamic state generated by various envoy network filters
-const char AttributeName::kConnectionDynamicState[] =
-    "connection.dynamic_state";
-
 // Context attributes
 const char AttributeName::kContextProtocol[] = "context.protocol";
 const char AttributeName::kContextReporterKind[] = "context.reporter.kind";
diff --git a/src/istio/utils/utils.h b/src/istio/utils/utils.h
index 063c18ca402..0e27aa82d27 100644
--- a/src/istio/utils/utils.h
+++ b/src/istio/utils/utils.h
@@ -16,6 +16,7 @@
 #pragma once
 
 #include <string>
+#include "google/protobuf/struct.pb.h"
 
 namespace istio {
 namespace utils {
@@ -23,6 +24,5 @@ namespace utils {
 // Get source.namespace attribute from principal.
 bool GetSourceNamespace(const std::string& principal,
                         std::string* source_namespace);
-
 }  // namespace utils
 }  // namespace istio

From 0db4bc2fbcf6b94eb5fb932495a95837f15710a5 Mon Sep 17 00:00:00 2001
From: Shriram Rajagopalan <shriramr@vmware.com>
Date: Thu, 1 Nov 2018 15:32:54 +0000
Subject: [PATCH 2/4] format

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
---
 include/istio/control/http/report_data.h      |  23 ++--
 include/istio/control/tcp/report_data.h       |  20 ++--
 include/istio/utils/attributes_builder.h      | 101 +++++++++---------
 src/envoy/http/mixer/report_data.h            |  20 ++--
 src/envoy/tcp/mixer/filter.cc                 |  14 +--
 src/envoy/tcp/mixer/filter.h                  |  39 +++----
 .../control/http/attributes_builder_test.cc   |  26 ++---
 src/istio/control/http/mock_report_data.h     |  27 ++---
 .../control/http/request_handler_impl_test.cc |  75 ++++++-------
 .../control/tcp/attributes_builder_test.cc    |  40 +++----
 src/istio/control/tcp/mock_report_data.h      |  23 ++--
 .../control/tcp/request_handler_impl_test.cc  |  27 ++---
 12 files changed, 228 insertions(+), 207 deletions(-)

diff --git a/include/istio/control/http/report_data.h b/include/istio/control/http/report_data.h
index 71749cbde47..cbb15e70237 100644
--- a/include/istio/control/http/report_data.h
+++ b/include/istio/control/http/report_data.h
@@ -28,7 +28,7 @@ namespace http {
 // The interface to extract HTTP data for Mixer report.
 // Implemented by the environment (Envoy) and used by the library.
 class ReportData {
- public:
+public:
   virtual ~ReportData() {}
 
   // Get response HTTP headers.
@@ -44,35 +44,36 @@ class ReportData {
     int response_code;
     std::string response_flags;
   };
-  virtual void GetReportInfo(ReportInfo* info) const = 0;
+  virtual void GetReportInfo(ReportInfo *info) const = 0;
 
   // Get destination ip/port.
-  virtual bool GetDestinationIpPort(std::string* ip, int* port) const = 0;
+  virtual bool GetDestinationIpPort(std::string *ip, int *port) const = 0;
 
   // Get Rbac attributes.
   struct RbacReportInfo {
     std::string permissive_resp_code;
     std::string permissive_policy_id;
   };
-  virtual bool GetRbacReportInfo(RbacReportInfo* report_info) const = 0;
+  virtual bool GetRbacReportInfo(RbacReportInfo *report_info) const = 0;
 
   // Get upstream host UID. This value overrides the value in the report bag.
-  virtual bool GetDestinationUID(std::string* uid) const = 0;
+  virtual bool GetDestinationUID(std::string *uid) const = 0;
 
   // gRPC status info
   struct GrpcStatus {
     std::string status;
     std::string message;
   };
-  virtual bool GetGrpcStatus(GrpcStatus* status) const = 0;
+  virtual bool GetGrpcStatus(GrpcStatus *status) const = 0;
 
   // Get dynamic metadata generated by Envoy filters.
   // Useful for logging info generated by custom codecs.
-  virtual const ::google::protobuf::Map<std::string, ::google::protobuf::Struct >& GetDynamicFilterState() const = 0;
+  virtual const ::google::protobuf::Map<std::string, ::google::protobuf::Struct>
+      &GetDynamicFilterState() const = 0;
 };
 
-}  // namespace http
-}  // namespace control
-}  // namespace istio
+} // namespace http
+} // namespace control
+} // namespace istio
 
-#endif  // ISTIO_CONTROL_HTTP_REPORT_DATA_H
+#endif // ISTIO_CONTROL_HTTP_REPORT_DATA_H
diff --git a/include/istio/control/tcp/report_data.h b/include/istio/control/tcp/report_data.h
index 93c86609408..58c0bb34a9f 100644
--- a/include/istio/control/tcp/report_data.h
+++ b/include/istio/control/tcp/report_data.h
@@ -28,11 +28,11 @@ namespace tcp {
 // The interface to extract TCP data for Mixer report call.
 // Implemented by the environment (Envoy) and used by the library.
 class ReportData {
- public:
+public:
   virtual ~ReportData() {}
 
   // Get upstream tcp connection IP and port. IP is returned in format of bytes.
-  virtual bool GetDestinationIpPort(std::string* ip, int* port) const = 0;
+  virtual bool GetDestinationIpPort(std::string *ip, int *port) const = 0;
 
   // Get additional report data.
   struct ReportInfo {
@@ -40,10 +40,10 @@ class ReportData {
     uint64_t received_bytes;
     std::chrono::nanoseconds duration;
   };
-  virtual void GetReportInfo(ReportInfo* info) const = 0;
+  virtual void GetReportInfo(ReportInfo *info) const = 0;
 
   // Get upstream host UID. This value overrides the value in the report bag.
-  virtual bool GetDestinationUID(std::string* uid) const = 0;
+  virtual bool GetDestinationUID(std::string *uid) const = 0;
 
   // ConnectionEvent is used to indicates the tcp connection event in Report
   // call.
@@ -55,11 +55,13 @@ class ReportData {
 
   // Get dynamic metadata generated by Envoy filters.
   // Useful for logging info generated by custom codecs.
-  virtual const ::google::protobuf::Map< ::std::string, ::google::protobuf::Struct >& GetDynamicFilterState() const = 0;
+  virtual const ::google::protobuf::Map<::std::string,
+                                        ::google::protobuf::Struct> &
+  GetDynamicFilterState() const = 0;
 };
 
-}  // namespace tcp
-}  // namespace control
-}  // namespace istio
+} // namespace tcp
+} // namespace control
+} // namespace istio
 
-#endif  // ISTIO_CONTROL_TCP_REPORT_DATA_H
+#endif // ISTIO_CONTROL_TCP_REPORT_DATA_H
diff --git a/include/istio/utils/attributes_builder.h b/include/istio/utils/attributes_builder.h
index edc0467f540..e97aaad0623 100644
--- a/include/istio/utils/attributes_builder.h
+++ b/include/istio/utils/attributes_builder.h
@@ -31,33 +31,33 @@ namespace utils {
 //    builder(attribute).Add("key", value)
 //                      .Add("key2", value2);
 class AttributesBuilder {
- public:
-  AttributesBuilder(::istio::mixer::v1::Attributes* attributes)
+public:
+  AttributesBuilder(::istio::mixer::v1::Attributes *attributes)
       : attributes_(attributes) {}
 
-  void AddString(const std::string& key, const std::string& str) {
+  void AddString(const std::string &key, const std::string &str) {
     (*attributes_->mutable_attributes())[key].set_string_value(str);
   }
 
-  void AddBytes(const std::string& key, const std::string& bytes) {
+  void AddBytes(const std::string &key, const std::string &bytes) {
     (*attributes_->mutable_attributes())[key].set_bytes_value(bytes);
   }
 
-  void AddInt64(const std::string& key, int64_t value) {
+  void AddInt64(const std::string &key, int64_t value) {
     (*attributes_->mutable_attributes())[key].set_int64_value(value);
   }
 
-  void AddDouble(const std::string& key, double value) {
+  void AddDouble(const std::string &key, double value) {
     (*attributes_->mutable_attributes())[key].set_double_value(value);
   }
 
-  void AddBool(const std::string& key, bool value) {
+  void AddBool(const std::string &key, bool value) {
     (*attributes_->mutable_attributes())[key].set_bool_value(value);
   }
 
   void AddTimestamp(
-      const std::string& key,
-      const std::chrono::time_point<std::chrono::system_clock>& value) {
+      const std::string &key,
+      const std::chrono::time_point<std::chrono::system_clock> &value) {
     auto time_stamp =
         (*attributes_->mutable_attributes())[key].mutable_timestamp_value();
     long long nanos = std::chrono::duration_cast<std::chrono::nanoseconds>(
@@ -67,16 +67,16 @@ class AttributesBuilder {
     time_stamp->set_nanos(nanos % 1000000000);
   }
 
-  void AddDuration(const std::string& key,
-                   const std::chrono::nanoseconds& value) {
+  void AddDuration(const std::string &key,
+                   const std::chrono::nanoseconds &value) {
     auto duration =
         (*attributes_->mutable_attributes())[key].mutable_duration_value();
     duration->set_seconds(value.count() / 1000000000);
     duration->set_nanos(value.count() % 1000000000);
   }
 
-  void AddStringMap(const std::string& key,
-                    const std::map<std::string, std::string>& string_map) {
+  void AddStringMap(const std::string &key,
+                    const std::map<std::string, std::string> &string_map) {
     if (string_map.size() == 0) {
       return;
     }
@@ -84,13 +84,13 @@ class AttributesBuilder {
                        .mutable_string_map_value()
                        ->mutable_entries();
     entries->clear();
-    for (const auto& map_it : string_map) {
+    for (const auto &map_it : string_map) {
       (*entries)[map_it.first] = map_it.second;
     }
   }
 
-  void AddProtoStructStringMap(const std::string& key,
-                               const google::protobuf::Struct& struct_map) {
+  void AddProtoStructStringMap(const std::string &key,
+                               const google::protobuf::Struct &struct_map) {
     if (struct_map.fields().empty()) {
       return;
     }
@@ -98,28 +98,28 @@ class AttributesBuilder {
                        .mutable_string_map_value()
                        ->mutable_entries();
     entries->clear();
-    for (const auto& field : struct_map.fields()) {
+    for (const auto &field : struct_map.fields()) {
       // Ignore all fields that are not string or string list.
       switch (field.second.kind_case()) {
-        case google::protobuf::Value::kStringValue:
-          (*entries)[field.first] = field.second.string_value();
-          break;
-        case google::protobuf::Value::kListValue:
-          if (field.second.list_value().values_size() > 0) {
-            // The items in the list is converted into a
-            // comma separated string
-            std::string s;
-            for (int i = 0; i < field.second.list_value().values_size(); i++) {
-              s += field.second.list_value().values().Get(i).string_value();
-              if (i + 1 < field.second.list_value().values_size()) {
-                s += ",";
-              }
+      case google::protobuf::Value::kStringValue:
+        (*entries)[field.first] = field.second.string_value();
+        break;
+      case google::protobuf::Value::kListValue:
+        if (field.second.list_value().values_size() > 0) {
+          // The items in the list is converted into a
+          // comma separated string
+          std::string s;
+          for (int i = 0; i < field.second.list_value().values_size(); i++) {
+            s += field.second.list_value().values().Get(i).string_value();
+            if (i + 1 < field.second.list_value().values_size()) {
+              s += ",";
             }
-            (*entries)[field.first] = s;
           }
-          break;
-        default:
-          break;
+          (*entries)[field.first] = s;
+        }
+        break;
+      default:
+        break;
       }
     }
   }
@@ -128,27 +128,30 @@ class AttributesBuilder {
   // for example, foo.bar.com: struct {str:abc, list:[c,d,e]} will be emitted as
   // foo.bar.com: string_map[str:abc, list: c,d,e]
   // Only extracts strings and lists.
-  // TODO: add the ability to pack bools and nums as strings and recurse down the struct.
-  void FlattenMapOfStringToStruct(const ::google::protobuf::Map< ::std::string, ::google::protobuf::Struct >& filter_state) {
-      if (filter_state.empty()) {
-          return;
-      }
+  // TODO: add the ability to pack bools and nums as strings and recurse down
+  // the struct.
+  void FlattenMapOfStringToStruct(
+      const ::google::protobuf::Map<::std::string, ::google::protobuf::Struct>
+          &filter_state) {
+    if (filter_state.empty()) {
+      return;
+    }
 
-      for (const auto& filter : filter_state) {
-          AddProtoStructStringMap(filter.first, filter.second);
-      }
+    for (const auto &filter : filter_state) {
+      AddProtoStructStringMap(filter.first, filter.second);
+    }
   }
 
-  bool HasAttribute(const std::string& key) const {
-    const auto& attrs_map = attributes_->attributes();
+  bool HasAttribute(const std::string &key) const {
+    const auto &attrs_map = attributes_->attributes();
     return attrs_map.find(key) != attrs_map.end();
   }
 
- private:
-  ::istio::mixer::v1::Attributes* attributes_;
+private:
+  ::istio::mixer::v1::Attributes *attributes_;
 };
 
-}  // namespace utils
-}  // namespace istio
+} // namespace utils
+} // namespace istio
 
-#endif  // ISTIO_MIXERCLIENT_ATTRIBUTES_BUILDER_H
+#endif // ISTIO_MIXERCLIENT_ATTRIBUTES_BUILDER_H
diff --git a/src/envoy/http/mixer/report_data.h b/src/envoy/http/mixer/report_data.h
index 65734925f9d..0d2bbad8257 100644
--- a/src/envoy/http/mixer/report_data.h
+++ b/src/envoy/http/mixer/report_data.h
@@ -24,7 +24,6 @@
 #include "include/istio/control/http/controller.h"
 #include "src/envoy/utils/utils.h"
 
-
 namespace Envoy {
 namespace Http {
 namespace Mixer {
@@ -49,7 +48,7 @@ bool ExtractGrpcStatus(const HeaderMap *headers,
   return false;
 }
 
-}  // namespace
+} // namespace
 
 class ReportData : public ::istio::control::http::ReportData,
                    public Logger::Loggable<Logger::Id::filter> {
@@ -59,12 +58,10 @@ class ReportData : public ::istio::control::http::ReportData,
   uint64_t response_total_size_;
   uint64_t request_total_size_;
 
- public:
+public:
   ReportData(const HeaderMap *headers, const HeaderMap *response_trailers,
              const StreamInfo::StreamInfo &info, uint64_t request_total_size)
-      : headers_(headers),
-        trailers_(response_trailers),
-        info_(info),
+      : headers_(headers), trailers_(response_trailers), info_(info),
         response_total_size_(info.bytesSent()),
         request_total_size_(request_total_size) {
     if (headers != nullptr) {
@@ -160,11 +157,12 @@ class ReportData : public ::istio::control::http::ReportData,
   }
 
   // Get attributes generated by http filters
-  const ::google::protobuf::Map<std::string, ::google::protobuf::Struct >& GetDynamicFilterState() const override {
-      return info_.dynamicMetadata().filter_metadata();
+  const ::google::protobuf::Map<std::string, ::google::protobuf::Struct> &
+  GetDynamicFilterState() const override {
+    return info_.dynamicMetadata().filter_metadata();
   }
 };
 
-}  // namespace Mixer
-}  // namespace Http
-}  // namespace Envoy
+} // namespace Mixer
+} // namespace Http
+} // namespace Envoy
diff --git a/src/envoy/tcp/mixer/filter.cc b/src/envoy/tcp/mixer/filter.cc
index 1f1c56e66a2..c20842cbf46 100644
--- a/src/envoy/tcp/mixer/filter.cc
+++ b/src/envoy/tcp/mixer/filter.cc
@@ -177,10 +177,12 @@ bool Filter::GetDestinationUID(std::string *uid) const {
   }
   return false;
 }
-const ::google::protobuf::Map<std::string, ::google::protobuf::Struct >& Filter::GetDynamicFilterState() const {
+const ::google::protobuf::Map<std::string, ::google::protobuf::Struct> &
+Filter::GetDynamicFilterState() const {
   return filter_callbacks_->connection()
-    .streamInfo()
-    .dynamicMetadata().filter_metadata();
+      .streamInfo()
+      .dynamicMetadata()
+      .filter_metadata();
 }
 void Filter::GetReportInfo(
     ::istio::control::tcp::ReportData::ReportInfo *data) const {
@@ -203,6 +205,6 @@ void Filter::OnReportTimer() {
   report_timer_->enableTimer(control_.config().report_interval_ms());
 }
 
-}  // namespace Mixer
-}  // namespace Tcp
-}  // namespace Envoy
+} // namespace Mixer
+} // namespace Tcp
+} // namespace Envoy
diff --git a/src/envoy/tcp/mixer/filter.h b/src/envoy/tcp/mixer/filter.h
index a99e9254cbb..eef3687a480 100644
--- a/src/envoy/tcp/mixer/filter.h
+++ b/src/envoy/tcp/mixer/filter.h
@@ -30,18 +30,18 @@ class Filter : public Network::Filter,
                public ::istio::control::tcp::CheckData,
                public ::istio::control::tcp::ReportData,
                public Logger::Loggable<Logger::Id::filter> {
- public:
-  Filter(Control& control);
+public:
+  Filter(Control &control);
   ~Filter();
 
   void initializeReadFilterCallbacks(
-      Network::ReadFilterCallbacks& callbacks) override;
+      Network::ReadFilterCallbacks &callbacks) override;
 
   // Network::ReadFilter
-  Network::FilterStatus onData(Buffer::Instance& data, bool) override;
+  Network::FilterStatus onData(Buffer::Instance &data, bool) override;
 
   // Network::WriteFilter
-  Network::FilterStatus onWrite(Buffer::Instance& data, bool) override;
+  Network::FilterStatus onWrite(Buffer::Instance &data, bool) override;
   Network::FilterStatus onNewConnection() override;
 
   // Network::ConnectionCallbacks
@@ -51,20 +51,21 @@ class Filter : public Network::Filter,
   void onBelowWriteBufferLowWatermark() override {}
 
   // CheckData virtual functions.
-  bool GetSourceIpPort(std::string* str_ip, int* port) const override;
-  bool GetPrincipal(bool peer, std::string* user) const override;
+  bool GetSourceIpPort(std::string *str_ip, int *port) const override;
+  bool GetPrincipal(bool peer, std::string *user) const override;
   bool IsMutualTLS() const override;
-  bool GetRequestedServerName(std::string* name) const override;
+  bool GetRequestedServerName(std::string *name) const override;
 
   // ReportData virtual functions.
-  bool GetDestinationIpPort(std::string* str_ip, int* port) const override;
-  bool GetDestinationUID(std::string* uid) const override;  
-  const ::google::protobuf::Map<std::string, ::google::protobuf::Struct >& GetDynamicFilterState() const override;
+  bool GetDestinationIpPort(std::string *str_ip, int *port) const override;
+  bool GetDestinationUID(std::string *uid) const override;
+  const ::google::protobuf::Map<std::string, ::google::protobuf::Struct> &
+  GetDynamicFilterState() const override;
   void GetReportInfo(
-      ::istio::control::tcp::ReportData::ReportInfo* data) const override;
+      ::istio::control::tcp::ReportData::ReportInfo *data) const override;
   std::string GetConnectionId() const override;
 
- private:
+private:
   enum class State { NotStarted, Calling, Completed, Closed };
   // This function is invoked when timer event fires.
   // It sends periodical delta reports.
@@ -74,7 +75,7 @@ class Filter : public Network::Filter,
   void callCheck();
 
   // Called when Check is done.
-  void completeCheck(const ::google::protobuf::util::Status& status);
+  void completeCheck(const ::google::protobuf::util::Status &status);
 
   // Cancel the pending Check call.
   void cancelCheck();
@@ -82,11 +83,11 @@ class Filter : public Network::Filter,
   // the cancel check
   istio::mixerclient::CancelFunc cancel_check_;
   // the control object.
-  Control& control_;
+  Control &control_;
   // pre-request handler
   std::unique_ptr<::istio::control::tcp::RequestHandler> handler_;
   // filter callback
-  Network::ReadFilterCallbacks* filter_callbacks_{};
+  Network::ReadFilterCallbacks *filter_callbacks_{};
   // state
   State state_{State::NotStarted};
   // calling_check
@@ -102,6 +103,6 @@ class Filter : public Network::Filter,
   std::chrono::time_point<std::chrono::system_clock> start_time_;
 };
 
-}  // namespace Mixer
-}  // namespace Tcp
-}  // namespace Envoy
+} // namespace Mixer
+} // namespace Tcp
+} // namespace Envoy
diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc
index e88cfd4d012..ffa2ea0718e 100644
--- a/src/istio/control/http/attributes_builder_test.cc
+++ b/src/istio/control/http/attributes_builder_test.cc
@@ -15,14 +15,14 @@
 
 #include "src/istio/control/http/attributes_builder.h"
 
-#include "gmock/gmock.h"
 #include "google/protobuf/text_format.h"
 #include "google/protobuf/util/message_differencer.h"
-#include "gtest/gtest.h"
 #include "include/istio/utils/attribute_names.h"
 #include "include/istio/utils/attributes_builder.h"
 #include "src/istio/control/http/mock_check_data.h"
 #include "src/istio/control/http/mock_report_data.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
 
 using ::google::protobuf::TextFormat;
 using ::google::protobuf::util::MessageDifferencer;
@@ -727,7 +727,8 @@ TEST(AttributesBuilderTest, TestCheckAttributes) {
 TEST(AttributesBuilderTest, TestReportAttributes) {
   ::testing::StrictMock<MockReportData> mock_data;
 
-  ::google::protobuf::Map<std::string, ::google::protobuf::Struct> filter_metadata;
+  ::google::protobuf::Map<std::string, ::google::protobuf::Struct>
+      filter_metadata;
   ::google::protobuf::Struct struct_obj;
   ::google::protobuf::Value strval, numval, boolval, listval;
   strval.set_string_value("abc");
@@ -740,7 +741,7 @@ TEST(AttributesBuilderTest, TestReportAttributes) {
   listval.mutable_list_value()->add_values()->set_string_value("b");
   listval.mutable_list_value()->add_values()->set_string_value("c");
   (*struct_obj.mutable_fields())["list"] = listval;
-  filter_metadata["foo.bar.com"]=struct_obj;
+  filter_metadata["foo.bar.com"] = struct_obj;
 
   EXPECT_CALL(mock_data, GetDestinationIpPort(_, _))
       .WillOnce(Invoke([](std::string *ip, int *port) -> bool {
@@ -783,7 +784,7 @@ TEST(AttributesBuilderTest, TestReportAttributes) {
         return true;
       }));
   EXPECT_CALL(mock_data, GetDynamicFilterState())
-    .WillOnce(ReturnRef(filter_metadata));
+      .WillOnce(ReturnRef(filter_metadata));
 
   RequestContext request;
   AttributesBuilder builder(&request);
@@ -809,7 +810,8 @@ TEST(AttributesBuilderTest, TestReportAttributes) {
 TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) {
   ::testing::StrictMock<MockReportData> mock_data;
 
-  ::google::protobuf::Map<std::string, ::google::protobuf::Struct> filter_metadata;
+  ::google::protobuf::Map<std::string, ::google::protobuf::Struct>
+      filter_metadata;
   ::google::protobuf::Struct struct_obj;
   ::google::protobuf::Value strval, numval, boolval, listval;
   strval.set_string_value("abc");
@@ -822,7 +824,7 @@ TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) {
   listval.mutable_list_value()->add_values()->set_string_value("b");
   listval.mutable_list_value()->add_values()->set_string_value("c");
   (*struct_obj.mutable_fields())["list"] = listval;
-  filter_metadata["foo.bar.com"]=struct_obj;
+  filter_metadata["foo.bar.com"] = struct_obj;
 
   EXPECT_CALL(mock_data, GetDestinationIpPort(_, _))
       .WillOnce(Invoke([](std::string *ip, int *port) -> bool {
@@ -856,7 +858,7 @@ TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) {
         return true;
       }));
   EXPECT_CALL(mock_data, GetDynamicFilterState())
-    .WillOnce(ReturnRef(filter_metadata));
+      .WillOnce(ReturnRef(filter_metadata));
 
   RequestContext request;
   SetDestinationIp(&request, "1.2.3.4");
@@ -871,7 +873,7 @@ TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) {
   EXPECT_THAT(*request.attributes, EqualsAttribute(expected_attributes));
 }
 
-}  // namespace
-}  // namespace http
-}  // namespace control
-}  // namespace istio
+} // namespace
+} // namespace http
+} // namespace control
+} // namespace istio
diff --git a/src/istio/control/http/mock_report_data.h b/src/istio/control/http/mock_report_data.h
index 503bb6270fc..1980c058b6f 100644
--- a/src/istio/control/http/mock_report_data.h
+++ b/src/istio/control/http/mock_report_data.h
@@ -16,8 +16,8 @@
 #ifndef ISTIO_CONTROL_HTTP_MOCK_REPORT_DATA_H
 #define ISTIO_CONTROL_HTTP_MOCK_REPORT_DATA_H
 
-#include "gmock/gmock.h"
 #include "include/istio/control/http/report_data.h"
+#include "gmock/gmock.h"
 
 namespace istio {
 namespace control {
@@ -25,18 +25,21 @@ namespace http {
 
 // The mock object for ReportData interface.
 class MockReportData : public ReportData {
- public:
+public:
   MOCK_CONST_METHOD0(GetResponseHeaders, std::map<std::string, std::string>());
-  MOCK_CONST_METHOD1(GetReportInfo, void(ReportInfo* info));
-  MOCK_CONST_METHOD2(GetDestinationIpPort, bool(std::string* ip, int* port));
-  MOCK_CONST_METHOD1(GetDestinationUID, bool(std::string* ip));
-  MOCK_CONST_METHOD1(GetGrpcStatus, bool(GrpcStatus* status));
-  MOCK_CONST_METHOD1(GetRbacReportInfo, bool(RbacReportInfo* info));
-  MOCK_CONST_METHOD0(GetDynamicFilterState, const ::google::protobuf::Map<std::string, ::google::protobuf::Struct>&());
+  MOCK_CONST_METHOD1(GetReportInfo, void(ReportInfo *info));
+  MOCK_CONST_METHOD2(GetDestinationIpPort, bool(std::string *ip, int *port));
+  MOCK_CONST_METHOD1(GetDestinationUID, bool(std::string *ip));
+  MOCK_CONST_METHOD1(GetGrpcStatus, bool(GrpcStatus *status));
+  MOCK_CONST_METHOD1(GetRbacReportInfo, bool(RbacReportInfo *info));
+  MOCK_CONST_METHOD0(
+      GetDynamicFilterState,
+      const ::google::protobuf::Map<std::string, ::google::protobuf::Struct>
+          &());
 };
 
-}  // namespace http
-}  // namespace control
-}  // namespace istio
+} // namespace http
+} // namespace control
+} // namespace istio
 
-#endif  // ISTIO_CONTROL_HTTP_MOCK_REPORT_DATA_H
+#endif // ISTIO_CONTROL_HTTP_MOCK_REPORT_DATA_H
diff --git a/src/istio/control/http/request_handler_impl_test.cc b/src/istio/control/http/request_handler_impl_test.cc
index 74fc1d3a114..a2e46fb4e0e 100644
--- a/src/istio/control/http/request_handler_impl_test.cc
+++ b/src/istio/control/http/request_handler_impl_test.cc
@@ -14,13 +14,13 @@
  */
 
 #include "google/protobuf/text_format.h"
-#include "gtest/gtest.h"
 #include "include/istio/utils/attribute_names.h"
 #include "src/istio/control/http/client_context.h"
 #include "src/istio/control/http/controller_impl.h"
 #include "src/istio/control/http/mock_check_data.h"
 #include "src/istio/control/http/mock_report_data.h"
 #include "src/istio/control/mock_mixer_client.h"
+#include "gtest/gtest.h"
 
 using ::google::protobuf::TextFormat;
 using ::google::protobuf::util::Status;
@@ -127,18 +127,18 @@ forward_attributes {
 )";
 
 class RequestHandlerImplTest : public ::testing::Test {
- public:
+public:
   void SetUp() { SetUpMockController(kDefaultClientConfig); }
 
-  void SetUpMockController(const std::string& config_text) {
+  void SetUpMockController(const std::string &config_text) {
     SetUpMockController(config_text, kLocalInbound, kLocalOutbound,
                         kLocalForward);
   }
 
-  void SetUpMockController(const std::string& config_text,
-                           const std::string& local_inbound_attributes,
-                           const std::string& local_outbound_attributes,
-                           const std::string& local_forward_attributes) {
+  void SetUpMockController(const std::string &config_text,
+                           const std::string &local_inbound_attributes,
+                           const std::string &local_outbound_attributes,
+                           const std::string &local_forward_attributes) {
     ASSERT_TRUE(TextFormat::ParseFromString(config_text, &client_config_));
 
     LocalAttributes la;
@@ -159,19 +159,19 @@ class RequestHandlerImplTest : public ::testing::Test {
         std::unique_ptr<Controller>(new ControllerImpl(client_context_));
   }
 
-  void SetServiceConfig(const std::string& name, const ServiceConfig& config) {
+  void SetServiceConfig(const std::string &name, const ServiceConfig &config) {
     (*client_config_.mutable_service_configs())[name] = config;
   }
 
-  void ApplyPerRouteConfig(const ServiceConfig& service_config,
-                           Controller::PerRouteConfig* per_route) {
+  void ApplyPerRouteConfig(const ServiceConfig &service_config,
+                           Controller::PerRouteConfig *per_route) {
     per_route->service_config_id = "1111";
     controller_->AddServiceConfig(per_route->service_config_id, service_config);
   }
 
   std::shared_ptr<ClientContext> client_context_;
   HttpClientConfig client_config_;
-  ::testing::NiceMock<MockMixerClient>* mock_client_;
+  ::testing::NiceMock<MockMixerClient> *mock_client_;
   std::unique_ptr<Controller> controller_;
 };
 
@@ -211,7 +211,7 @@ TEST_F(RequestHandlerImplTest, TestHandlerDisabledCheckReport) {
 
   auto handler = controller_->CreateRequestHandler(per_route);
   handler->Check(&mock_data, &mock_header, nullptr,
-                 [](const CheckResponseInfo& info) {
+                 [](const CheckResponseInfo &info) {
                    EXPECT_TRUE(info.response_status.ok());
                  });
 }
@@ -233,7 +233,7 @@ TEST_F(RequestHandlerImplTest, TestHandlerDisabledCheck) {
 
   auto handler = controller_->CreateRequestHandler(per_route);
   handler->Check(&mock_data, &mock_header, nullptr,
-                 [](const CheckResponseInfo& info) {
+                 [](const CheckResponseInfo &info) {
                    EXPECT_TRUE(info.response_status.ok());
                  });
 }
@@ -246,8 +246,8 @@ TEST_F(RequestHandlerImplTest, TestPerRouteAttributes) {
 
   // Check should be called.
   EXPECT_CALL(*mock_client_, Check(_, _, _, _))
-      .WillOnce(Invoke([](const Attributes& attributes,
-                          const std::vector<Requirement>& quotas,
+      .WillOnce(Invoke([](const Attributes &attributes,
+                          const std::vector<Requirement> &quotas,
                           TransportCheckFunc transport,
                           CheckDoneFunc on_done) -> CancelFunc {
         auto map = attributes.attributes();
@@ -274,8 +274,8 @@ TEST_F(RequestHandlerImplTest, TestDefaultRouteAttributes) {
 
   // Check should be called.
   EXPECT_CALL(*mock_client_, Check(_, _, _, _))
-      .WillOnce(Invoke([](const Attributes& attributes,
-                          const std::vector<Requirement>& quotas,
+      .WillOnce(Invoke([](const Attributes &attributes,
+                          const std::vector<Requirement> &quotas,
                           TransportCheckFunc transport,
                           CheckDoneFunc on_done) -> CancelFunc {
         auto map = attributes.attributes();
@@ -286,7 +286,7 @@ TEST_F(RequestHandlerImplTest, TestDefaultRouteAttributes) {
 
   // Attribute is forwarded: route override
   EXPECT_CALL(mock_header, AddIstioAttributes(_))
-      .WillOnce(Invoke([](const std::string& data) {
+      .WillOnce(Invoke([](const std::string &data) {
         Attributes forwarded_attr;
         EXPECT_TRUE(forwarded_attr.ParseFromString(data));
         auto map = forwarded_attr.attributes();
@@ -313,8 +313,8 @@ TEST_F(RequestHandlerImplTest, TestRouteAttributes) {
 
   // Check should be called.
   EXPECT_CALL(*mock_client_, Check(_, _, _, _))
-      .WillOnce(Invoke([](const Attributes& attributes,
-                          const std::vector<Requirement>& quotas,
+      .WillOnce(Invoke([](const Attributes &attributes,
+                          const std::vector<Requirement> &quotas,
                           TransportCheckFunc transport,
                           CheckDoneFunc on_done) -> CancelFunc {
         auto map = attributes.attributes();
@@ -325,7 +325,7 @@ TEST_F(RequestHandlerImplTest, TestRouteAttributes) {
 
   // Attribute is forwarded: global
   EXPECT_CALL(mock_header, AddIstioAttributes(_))
-      .WillOnce(Invoke([](const std::string& data) {
+      .WillOnce(Invoke([](const std::string &data) {
         Attributes forwarded_attr;
         EXPECT_TRUE(forwarded_attr.ParseFromString(data));
         auto map = forwarded_attr.attributes();
@@ -344,8 +344,8 @@ TEST_F(RequestHandlerImplTest, TestPerRouteQuota) {
 
   // Check should be called.
   EXPECT_CALL(*mock_client_, Check(_, _, _, _))
-      .WillOnce(Invoke([](const Attributes& attributes,
-                          const std::vector<Requirement>& quotas,
+      .WillOnce(Invoke([](const Attributes &attributes,
+                          const std::vector<Requirement> &quotas,
                           TransportCheckFunc transport,
                           CheckDoneFunc on_done) -> CancelFunc {
         auto map = attributes.attributes();
@@ -372,7 +372,7 @@ TEST_F(RequestHandlerImplTest, TestPerRouteApiSpec) {
   ::testing::NiceMock<MockHeaderUpdate> mock_header;
   EXPECT_CALL(mock_data, FindHeaderByType(_, _))
       .WillRepeatedly(
-          Invoke([](CheckData::HeaderType type, std::string* value) -> bool {
+          Invoke([](CheckData::HeaderType type, std::string *value) -> bool {
             if (type == CheckData::HEADER_PATH) {
               *value = "/books/120";
               return true;
@@ -386,8 +386,8 @@ TEST_F(RequestHandlerImplTest, TestPerRouteApiSpec) {
 
   // Check should be called.
   EXPECT_CALL(*mock_client_, Check(_, _, _, _))
-      .WillOnce(Invoke([](const Attributes& attributes,
-                          const std::vector<Requirement>& quotas,
+      .WillOnce(Invoke([](const Attributes &attributes,
+                          const std::vector<Requirement> &quotas,
                           TransportCheckFunc transport,
                           CheckDoneFunc on_done) -> CancelFunc {
         auto map = attributes.attributes();
@@ -436,7 +436,7 @@ TEST_F(RequestHandlerImplTest, TestDefaultApiKey) {
   ::testing::NiceMock<MockHeaderUpdate> mock_header;
   EXPECT_CALL(mock_data, FindQueryParameter(_, _))
       .WillRepeatedly(
-          Invoke([](const std::string& name, std::string* value) -> bool {
+          Invoke([](const std::string &name, std::string *value) -> bool {
             if (name == "key") {
               *value = "test-api-key";
               return true;
@@ -446,8 +446,8 @@ TEST_F(RequestHandlerImplTest, TestDefaultApiKey) {
 
   // Check should be called.
   EXPECT_CALL(*mock_client_, Check(_, _, _, _))
-      .WillOnce(Invoke([](const Attributes& attributes,
-                          const std::vector<Requirement>& quotas,
+      .WillOnce(Invoke([](const Attributes &attributes,
+                          const std::vector<Requirement> &quotas,
                           TransportCheckFunc transport,
                           CheckDoneFunc on_done) -> CancelFunc {
         auto map = attributes.attributes();
@@ -465,11 +465,14 @@ TEST_F(RequestHandlerImplTest, TestDefaultApiKey) {
 TEST_F(RequestHandlerImplTest, TestHandlerReport) {
   ::testing::NiceMock<MockCheckData> mock_check;
   ::testing::NiceMock<MockReportData> mock_report;
-  ::google::protobuf::Map<std::string, ::google::protobuf::Struct> filter_metadata;
+  ::google::protobuf::Map<std::string, ::google::protobuf::Struct>
+      filter_metadata;
   EXPECT_CALL(mock_check, GetSourceIpPort(_, _)).Times(1);
   EXPECT_CALL(mock_report, GetResponseHeaders()).Times(1);
   EXPECT_CALL(mock_report, GetReportInfo(_)).Times(1);
-  EXPECT_CALL(mock_report, GetDynamicFilterState()).Times(1).WillOnce(ReturnRef(filter_metadata));
+  EXPECT_CALL(mock_report, GetDynamicFilterState())
+      .Times(1)
+      .WillOnce(ReturnRef(filter_metadata));
 
   // Report should be called.
   EXPECT_CALL(*mock_client_, Report(_)).Times(1);
@@ -513,7 +516,7 @@ TEST_F(RequestHandlerImplTest, TestEmptyConfig) {
 
   // Attributes is forwarded.
   EXPECT_CALL(mock_header, AddIstioAttributes(_))
-      .WillOnce(Invoke([](const std::string& data) {
+      .WillOnce(Invoke([](const std::string &data) {
         Attributes forwarded_attr;
         EXPECT_TRUE(forwarded_attr.ParseFromString(data));
         auto map = forwarded_attr.attributes();
@@ -534,12 +537,12 @@ TEST_F(RequestHandlerImplTest, TestEmptyConfig) {
   Controller::PerRouteConfig config;
   auto handler = controller_->CreateRequestHandler(config);
   handler->Check(&mock_check, &mock_header, nullptr,
-                 [](const CheckResponseInfo& info) {
+                 [](const CheckResponseInfo &info) {
                    EXPECT_TRUE(info.response_status.ok());
                  });
   handler->Report(&mock_check, &mock_report);
 }
 
-}  // namespace http
-}  // namespace control
-}  // namespace istio
+} // namespace http
+} // namespace control
+} // namespace istio
diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc
index 7d6992f94cb..711f0d3f233 100644
--- a/src/istio/control/tcp/attributes_builder_test.cc
+++ b/src/istio/control/tcp/attributes_builder_test.cc
@@ -17,12 +17,12 @@
 
 #include "google/protobuf/text_format.h"
 #include "google/protobuf/util/message_differencer.h"
-#include "gtest/gtest.h"
 #include "include/istio/utils/attribute_names.h"
 #include "include/istio/utils/attributes_builder.h"
 #include "src/istio/control/tcp/mock_check_data.h"
 #include "src/istio/control/tcp/mock_report_data.h"
 #include "src/istio/utils/utils.h"
+#include "gtest/gtest.h"
 
 using ::google::protobuf::TextFormat;
 using ::google::protobuf::util::MessageDifferencer;
@@ -419,7 +419,7 @@ attributes {
 }
 )";
 
-void ClearContextTime(RequestContext* request) {
+void ClearContextTime(RequestContext *request) {
   // Override timestamp with -
   utils::AttributesBuilder builder(request->attributes);
   std::chrono::time_point<std::chrono::system_clock> time0;
@@ -429,7 +429,7 @@ void ClearContextTime(RequestContext* request) {
 TEST(AttributesBuilderTest, TestCheckAttributes) {
   ::testing::NiceMock<MockCheckData> mock_data;
   EXPECT_CALL(mock_data, GetSourceIpPort(_, _))
-      .WillOnce(Invoke([](std::string* ip, int* port) -> bool {
+      .WillOnce(Invoke([](std::string *ip, int *port) -> bool {
         *ip = "1.2.3.4";
         *port = 8080;
         return true;
@@ -438,7 +438,7 @@ TEST(AttributesBuilderTest, TestCheckAttributes) {
     return true;
   }));
   EXPECT_CALL(mock_data, GetPrincipal(_, _))
-      .WillRepeatedly(Invoke([](bool peer, std::string* user) -> bool {
+      .WillRepeatedly(Invoke([](bool peer, std::string *user) -> bool {
         if (peer) {
           *user = "cluster.local/sa/test_user/ns/ns_ns/";
         } else {
@@ -448,7 +448,7 @@ TEST(AttributesBuilderTest, TestCheckAttributes) {
       }));
   EXPECT_CALL(mock_data, GetConnectionId()).WillOnce(Return("1234-5"));
   EXPECT_CALL(mock_data, GetRequestedServerName(_))
-      .WillOnce(Invoke([](std::string* name) -> bool {
+      .WillOnce(Invoke([](std::string *name) -> bool {
         *name = "www.google.com";
         return true;
       }));
@@ -472,7 +472,8 @@ TEST(AttributesBuilderTest, TestCheckAttributes) {
 TEST(AttributesBuilderTest, TestReportAttributes) {
   ::testing::NiceMock<MockReportData> mock_data;
 
-  ::google::protobuf::Map<std::string, ::google::protobuf::Struct> filter_metadata;
+  ::google::protobuf::Map<std::string, ::google::protobuf::Struct>
+      filter_metadata;
   ::google::protobuf::Struct struct_obj;
   ::google::protobuf::Value strval, numval, boolval, listval;
   strval.set_string_value("abc");
@@ -485,43 +486,42 @@ TEST(AttributesBuilderTest, TestReportAttributes) {
   listval.mutable_list_value()->add_values()->set_string_value("b");
   listval.mutable_list_value()->add_values()->set_string_value("c");
   (*struct_obj.mutable_fields())["list"] = listval;
-  filter_metadata["foo.bar.com"]=struct_obj;
-
+  filter_metadata["foo.bar.com"] = struct_obj;
 
   EXPECT_CALL(mock_data, GetDestinationIpPort(_, _))
       .Times(4)
-      .WillRepeatedly(Invoke([](std::string* ip, int* port) -> bool {
+      .WillRepeatedly(Invoke([](std::string *ip, int *port) -> bool {
         *ip = "1.2.3.4";
         *port = 8080;
         return true;
       }));
   EXPECT_CALL(mock_data, GetDestinationUID(_))
       .Times(4)
-      .WillRepeatedly(Invoke([](std::string* uid) -> bool {
+      .WillRepeatedly(Invoke([](std::string *uid) -> bool {
         *uid = "pod1.ns2";
         return true;
       }));
   EXPECT_CALL(mock_data, GetDynamicFilterState())
-    .Times(4)
-    .WillRepeatedly(ReturnRef(filter_metadata));
+      .Times(4)
+      .WillRepeatedly(ReturnRef(filter_metadata));
   EXPECT_CALL(mock_data, GetReportInfo(_))
       .Times(4)
-      .WillOnce(Invoke([](ReportData::ReportInfo* info) {
+      .WillOnce(Invoke([](ReportData::ReportInfo *info) {
         info->received_bytes = 0;
         info->send_bytes = 0;
         info->duration = std::chrono::nanoseconds(1);
       }))
-      .WillOnce(Invoke([](ReportData::ReportInfo* info) {
+      .WillOnce(Invoke([](ReportData::ReportInfo *info) {
         info->received_bytes = 100;
         info->send_bytes = 200;
         info->duration = std::chrono::nanoseconds(2);
       }))
-      .WillOnce(Invoke([](ReportData::ReportInfo* info) {
+      .WillOnce(Invoke([](ReportData::ReportInfo *info) {
         info->received_bytes = 201;
         info->send_bytes = 404;
         info->duration = std::chrono::nanoseconds(3);
       }))
-      .WillOnce(Invoke([](ReportData::ReportInfo* info) {
+      .WillOnce(Invoke([](ReportData::ReportInfo *info) {
         info->received_bytes = 345;
         info->send_bytes = 678;
         info->duration = std::chrono::nanoseconds(4);
@@ -600,7 +600,7 @@ TEST(AttributesBuilderTest, TestReportAttributes) {
                                          expected_final_attributes));
 }
 
-}  // namespace
-}  // namespace tcp
-}  // namespace control
-}  // namespace istio
+} // namespace
+} // namespace tcp
+} // namespace control
+} // namespace istio
diff --git a/src/istio/control/tcp/mock_report_data.h b/src/istio/control/tcp/mock_report_data.h
index 6113ce3c368..469ab4f28d1 100644
--- a/src/istio/control/tcp/mock_report_data.h
+++ b/src/istio/control/tcp/mock_report_data.h
@@ -16,8 +16,8 @@
 #ifndef ISTIO_CONTROL_TCP_MOCK_REPORT_DATA_H
 #define ISTIO_CONTROL_TCP_MOCK_REPORT_DATA_H
 
-#include "gmock/gmock.h"
 #include "include/istio/control/tcp/report_data.h"
+#include "gmock/gmock.h"
 
 namespace istio {
 namespace control {
@@ -25,15 +25,18 @@ namespace tcp {
 
 // The mock object for ReportData interface.
 class MockReportData : public ReportData {
- public:
-  MOCK_CONST_METHOD2(GetDestinationIpPort, bool(std::string* ip, int* port));
-  MOCK_CONST_METHOD1(GetDestinationUID, bool(std::string*));
-  MOCK_CONST_METHOD0(GetDynamicFilterState, const ::google::protobuf::Map<std::string, ::google::protobuf::Struct>&());
-  MOCK_CONST_METHOD1(GetReportInfo, void(ReportInfo* info));
+public:
+  MOCK_CONST_METHOD2(GetDestinationIpPort, bool(std::string *ip, int *port));
+  MOCK_CONST_METHOD1(GetDestinationUID, bool(std::string *));
+  MOCK_CONST_METHOD0(
+      GetDynamicFilterState,
+      const ::google::protobuf::Map<std::string, ::google::protobuf::Struct>
+          &());
+  MOCK_CONST_METHOD1(GetReportInfo, void(ReportInfo *info));
 };
 
-}  // namespace tcp
-}  // namespace control
-}  // namespace istio
+} // namespace tcp
+} // namespace control
+} // namespace istio
 
-#endif  // ISTIO_CONTROL_TCP_MOCK_REPORT_DATA_H
+#endif // ISTIO_CONTROL_TCP_MOCK_REPORT_DATA_H
diff --git a/src/istio/control/tcp/request_handler_impl_test.cc b/src/istio/control/tcp/request_handler_impl_test.cc
index 066148244a6..4f7c075b6bf 100644
--- a/src/istio/control/tcp/request_handler_impl_test.cc
+++ b/src/istio/control/tcp/request_handler_impl_test.cc
@@ -14,12 +14,12 @@
  */
 
 #include "google/protobuf/text_format.h"
-#include "gtest/gtest.h"
 #include "src/istio/control/mock_mixer_client.h"
 #include "src/istio/control/tcp/client_context.h"
 #include "src/istio/control/tcp/controller_impl.h"
 #include "src/istio/control/tcp/mock_check_data.h"
 #include "src/istio/control/tcp/mock_report_data.h"
+#include "gtest/gtest.h"
 
 using ::google::protobuf::TextFormat;
 using ::google::protobuf::util::Status;
@@ -70,10 +70,10 @@ attributes {
   }
 }
 )";
-}  // namespace
+} // namespace
 
 class RequestHandlerImplTest : public ::testing::Test {
- public:
+public:
   void SetUp() {
     auto map1 = client_config_.mutable_mixer_attributes()->mutable_attributes();
     (*map1)["key1"].set_string_value("value1");
@@ -98,7 +98,7 @@ class RequestHandlerImplTest : public ::testing::Test {
 
   std::shared_ptr<ClientContext> client_context_;
   TcpClientConfig client_config_;
-  ::testing::NiceMock<MockMixerClient>* mock_client_;
+  ::testing::NiceMock<MockMixerClient> *mock_client_;
   std::unique_ptr<Controller> controller_;
 };
 
@@ -112,7 +112,7 @@ TEST_F(RequestHandlerImplTest, TestHandlerDisabledCheck) {
 
   client_config_.set_disable_check_calls(true);
   auto handler = controller_->CreateRequestHandler();
-  handler->Check(&mock_data, [](const CheckResponseInfo& info) {
+  handler->Check(&mock_data, [](const CheckResponseInfo &info) {
     EXPECT_TRUE(info.response_status.ok());
   });
 }
@@ -124,8 +124,8 @@ TEST_F(RequestHandlerImplTest, TestHandlerCheck) {
 
   // Check should be called.
   EXPECT_CALL(*mock_client_, Check(_, _, _, _))
-      .WillOnce(Invoke([](const Attributes& attributes,
-                          const std::vector<Requirement>& quotas,
+      .WillOnce(Invoke([](const Attributes &attributes,
+                          const std::vector<Requirement> &quotas,
                           TransportCheckFunc transport,
                           CheckDoneFunc on_done) -> CancelFunc {
         auto map = attributes.attributes();
@@ -142,11 +142,14 @@ TEST_F(RequestHandlerImplTest, TestHandlerCheck) {
 
 TEST_F(RequestHandlerImplTest, TestHandlerReport) {
   ::testing::NiceMock<MockReportData> mock_data;
-  ::google::protobuf::Map<std::string, ::google::protobuf::Struct> filter_metadata;
+  ::google::protobuf::Map<std::string, ::google::protobuf::Struct>
+      filter_metadata;
   EXPECT_CALL(mock_data, GetDestinationIpPort(_, _)).Times(1);
   EXPECT_CALL(mock_data, GetDestinationUID(_)).Times(1);
   EXPECT_CALL(mock_data, GetReportInfo(_)).Times(1);
-  EXPECT_CALL(mock_data, GetDynamicFilterState()).Times(1).WillOnce(ReturnRef(filter_metadata));
+  EXPECT_CALL(mock_data, GetDynamicFilterState())
+      .Times(1)
+      .WillOnce(ReturnRef(filter_metadata));
 
   // Report should be called.
   EXPECT_CALL(*mock_client_, Report(_)).Times(1);
@@ -155,6 +158,6 @@ TEST_F(RequestHandlerImplTest, TestHandlerReport) {
   handler->Report(&mock_data, ReportData::CONTINUE);
 }
 
-}  // namespace tcp
-}  // namespace control
-}  // namespace istio
+} // namespace tcp
+} // namespace control
+} // namespace istio

From d7fa2891bff61583ce95707d51bdd1444248e722 Mon Sep 17 00:00:00 2001
From: Shriram Rajagopalan <shriramr@vmware.com>
Date: Thu, 1 Nov 2018 15:34:04 +0000
Subject: [PATCH 3/4] double format

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
---
 include/istio/control/http/report_data.h      | 10 ++---
 include/istio/control/tcp/report_data.h       | 14 +++---
 include/istio/utils/attributes_builder.h      | 44 +++++++++----------
 src/envoy/http/mixer/report_data.h            | 18 ++++----
 src/envoy/tcp/mixer/filter.cc                 | 10 ++---
 src/envoy/tcp/mixer/filter.h                  | 14 +++---
 .../control/http/attributes_builder_test.cc   | 12 ++---
 src/istio/control/http/mock_report_data.h     | 12 ++---
 .../control/http/request_handler_impl_test.cc | 10 ++---
 .../control/tcp/attributes_builder_test.cc    | 10 ++---
 src/istio/control/tcp/mock_report_data.h      | 12 ++---
 .../control/tcp/request_handler_impl_test.cc  | 12 ++---
 12 files changed, 90 insertions(+), 88 deletions(-)

diff --git a/include/istio/control/http/report_data.h b/include/istio/control/http/report_data.h
index cbb15e70237..8cf7326fe0f 100644
--- a/include/istio/control/http/report_data.h
+++ b/include/istio/control/http/report_data.h
@@ -28,7 +28,7 @@ namespace http {
 // The interface to extract HTTP data for Mixer report.
 // Implemented by the environment (Envoy) and used by the library.
 class ReportData {
-public:
+ public:
   virtual ~ReportData() {}
 
   // Get response HTTP headers.
@@ -72,8 +72,8 @@ class ReportData {
       &GetDynamicFilterState() const = 0;
 };
 
-} // namespace http
-} // namespace control
-} // namespace istio
+}  // namespace http
+}  // namespace control
+}  // namespace istio
 
-#endif // ISTIO_CONTROL_HTTP_REPORT_DATA_H
+#endif  // ISTIO_CONTROL_HTTP_REPORT_DATA_H
diff --git a/include/istio/control/tcp/report_data.h b/include/istio/control/tcp/report_data.h
index 58c0bb34a9f..91a6b7241c0 100644
--- a/include/istio/control/tcp/report_data.h
+++ b/include/istio/control/tcp/report_data.h
@@ -28,7 +28,7 @@ namespace tcp {
 // The interface to extract TCP data for Mixer report call.
 // Implemented by the environment (Envoy) and used by the library.
 class ReportData {
-public:
+ public:
   virtual ~ReportData() {}
 
   // Get upstream tcp connection IP and port. IP is returned in format of bytes.
@@ -56,12 +56,12 @@ class ReportData {
   // Get dynamic metadata generated by Envoy filters.
   // Useful for logging info generated by custom codecs.
   virtual const ::google::protobuf::Map<::std::string,
-                                        ::google::protobuf::Struct> &
-  GetDynamicFilterState() const = 0;
+                                        ::google::protobuf::Struct>
+      &GetDynamicFilterState() const = 0;
 };
 
-} // namespace tcp
-} // namespace control
-} // namespace istio
+}  // namespace tcp
+}  // namespace control
+}  // namespace istio
 
-#endif // ISTIO_CONTROL_TCP_REPORT_DATA_H
+#endif  // ISTIO_CONTROL_TCP_REPORT_DATA_H
diff --git a/include/istio/utils/attributes_builder.h b/include/istio/utils/attributes_builder.h
index e97aaad0623..c0714187701 100644
--- a/include/istio/utils/attributes_builder.h
+++ b/include/istio/utils/attributes_builder.h
@@ -31,7 +31,7 @@ namespace utils {
 //    builder(attribute).Add("key", value)
 //                      .Add("key2", value2);
 class AttributesBuilder {
-public:
+ public:
   AttributesBuilder(::istio::mixer::v1::Attributes *attributes)
       : attributes_(attributes) {}
 
@@ -101,25 +101,25 @@ class AttributesBuilder {
     for (const auto &field : struct_map.fields()) {
       // Ignore all fields that are not string or string list.
       switch (field.second.kind_case()) {
-      case google::protobuf::Value::kStringValue:
-        (*entries)[field.first] = field.second.string_value();
-        break;
-      case google::protobuf::Value::kListValue:
-        if (field.second.list_value().values_size() > 0) {
-          // The items in the list is converted into a
-          // comma separated string
-          std::string s;
-          for (int i = 0; i < field.second.list_value().values_size(); i++) {
-            s += field.second.list_value().values().Get(i).string_value();
-            if (i + 1 < field.second.list_value().values_size()) {
-              s += ",";
+        case google::protobuf::Value::kStringValue:
+          (*entries)[field.first] = field.second.string_value();
+          break;
+        case google::protobuf::Value::kListValue:
+          if (field.second.list_value().values_size() > 0) {
+            // The items in the list is converted into a
+            // comma separated string
+            std::string s;
+            for (int i = 0; i < field.second.list_value().values_size(); i++) {
+              s += field.second.list_value().values().Get(i).string_value();
+              if (i + 1 < field.second.list_value().values_size()) {
+                s += ",";
+              }
             }
+            (*entries)[field.first] = s;
           }
-          (*entries)[field.first] = s;
-        }
-        break;
-      default:
-        break;
+          break;
+        default:
+          break;
       }
     }
   }
@@ -147,11 +147,11 @@ class AttributesBuilder {
     return attrs_map.find(key) != attrs_map.end();
   }
 
-private:
+ private:
   ::istio::mixer::v1::Attributes *attributes_;
 };
 
-} // namespace utils
-} // namespace istio
+}  // namespace utils
+}  // namespace istio
 
-#endif // ISTIO_MIXERCLIENT_ATTRIBUTES_BUILDER_H
+#endif  // ISTIO_MIXERCLIENT_ATTRIBUTES_BUILDER_H
diff --git a/src/envoy/http/mixer/report_data.h b/src/envoy/http/mixer/report_data.h
index 0d2bbad8257..7d09992012d 100644
--- a/src/envoy/http/mixer/report_data.h
+++ b/src/envoy/http/mixer/report_data.h
@@ -48,7 +48,7 @@ bool ExtractGrpcStatus(const HeaderMap *headers,
   return false;
 }
 
-} // namespace
+}  // namespace
 
 class ReportData : public ::istio::control::http::ReportData,
                    public Logger::Loggable<Logger::Id::filter> {
@@ -58,10 +58,12 @@ class ReportData : public ::istio::control::http::ReportData,
   uint64_t response_total_size_;
   uint64_t request_total_size_;
 
-public:
+ public:
   ReportData(const HeaderMap *headers, const HeaderMap *response_trailers,
              const StreamInfo::StreamInfo &info, uint64_t request_total_size)
-      : headers_(headers), trailers_(response_trailers), info_(info),
+      : headers_(headers),
+        trailers_(response_trailers),
+        info_(info),
         response_total_size_(info.bytesSent()),
         request_total_size_(request_total_size) {
     if (headers != nullptr) {
@@ -157,12 +159,12 @@ class ReportData : public ::istio::control::http::ReportData,
   }
 
   // Get attributes generated by http filters
-  const ::google::protobuf::Map<std::string, ::google::protobuf::Struct> &
-  GetDynamicFilterState() const override {
+  const ::google::protobuf::Map<std::string, ::google::protobuf::Struct>
+      &GetDynamicFilterState() const override {
     return info_.dynamicMetadata().filter_metadata();
   }
 };
 
-} // namespace Mixer
-} // namespace Http
-} // namespace Envoy
+}  // namespace Mixer
+}  // namespace Http
+}  // namespace Envoy
diff --git a/src/envoy/tcp/mixer/filter.cc b/src/envoy/tcp/mixer/filter.cc
index c20842cbf46..5043a44d7f2 100644
--- a/src/envoy/tcp/mixer/filter.cc
+++ b/src/envoy/tcp/mixer/filter.cc
@@ -177,8 +177,8 @@ bool Filter::GetDestinationUID(std::string *uid) const {
   }
   return false;
 }
-const ::google::protobuf::Map<std::string, ::google::protobuf::Struct> &
-Filter::GetDynamicFilterState() const {
+const ::google::protobuf::Map<std::string, ::google::protobuf::Struct>
+    &Filter::GetDynamicFilterState() const {
   return filter_callbacks_->connection()
       .streamInfo()
       .dynamicMetadata()
@@ -205,6 +205,6 @@ void Filter::OnReportTimer() {
   report_timer_->enableTimer(control_.config().report_interval_ms());
 }
 
-} // namespace Mixer
-} // namespace Tcp
-} // namespace Envoy
+}  // namespace Mixer
+}  // namespace Tcp
+}  // namespace Envoy
diff --git a/src/envoy/tcp/mixer/filter.h b/src/envoy/tcp/mixer/filter.h
index eef3687a480..8bd13ca27d2 100644
--- a/src/envoy/tcp/mixer/filter.h
+++ b/src/envoy/tcp/mixer/filter.h
@@ -30,7 +30,7 @@ class Filter : public Network::Filter,
                public ::istio::control::tcp::CheckData,
                public ::istio::control::tcp::ReportData,
                public Logger::Loggable<Logger::Id::filter> {
-public:
+ public:
   Filter(Control &control);
   ~Filter();
 
@@ -59,13 +59,13 @@ class Filter : public Network::Filter,
   // ReportData virtual functions.
   bool GetDestinationIpPort(std::string *str_ip, int *port) const override;
   bool GetDestinationUID(std::string *uid) const override;
-  const ::google::protobuf::Map<std::string, ::google::protobuf::Struct> &
-  GetDynamicFilterState() const override;
+  const ::google::protobuf::Map<std::string, ::google::protobuf::Struct>
+      &GetDynamicFilterState() const override;
   void GetReportInfo(
       ::istio::control::tcp::ReportData::ReportInfo *data) const override;
   std::string GetConnectionId() const override;
 
-private:
+ private:
   enum class State { NotStarted, Calling, Completed, Closed };
   // This function is invoked when timer event fires.
   // It sends periodical delta reports.
@@ -103,6 +103,6 @@ class Filter : public Network::Filter,
   std::chrono::time_point<std::chrono::system_clock> start_time_;
 };
 
-} // namespace Mixer
-} // namespace Tcp
-} // namespace Envoy
+}  // namespace Mixer
+}  // namespace Tcp
+}  // namespace Envoy
diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc
index ffa2ea0718e..31c14b947e9 100644
--- a/src/istio/control/http/attributes_builder_test.cc
+++ b/src/istio/control/http/attributes_builder_test.cc
@@ -15,14 +15,14 @@
 
 #include "src/istio/control/http/attributes_builder.h"
 
+#include "gmock/gmock.h"
 #include "google/protobuf/text_format.h"
 #include "google/protobuf/util/message_differencer.h"
+#include "gtest/gtest.h"
 #include "include/istio/utils/attribute_names.h"
 #include "include/istio/utils/attributes_builder.h"
 #include "src/istio/control/http/mock_check_data.h"
 #include "src/istio/control/http/mock_report_data.h"
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
 
 using ::google::protobuf::TextFormat;
 using ::google::protobuf::util::MessageDifferencer;
@@ -873,7 +873,7 @@ TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) {
   EXPECT_THAT(*request.attributes, EqualsAttribute(expected_attributes));
 }
 
-} // namespace
-} // namespace http
-} // namespace control
-} // namespace istio
+}  // namespace
+}  // namespace http
+}  // namespace control
+}  // namespace istio
diff --git a/src/istio/control/http/mock_report_data.h b/src/istio/control/http/mock_report_data.h
index 1980c058b6f..64c0402ef9b 100644
--- a/src/istio/control/http/mock_report_data.h
+++ b/src/istio/control/http/mock_report_data.h
@@ -16,8 +16,8 @@
 #ifndef ISTIO_CONTROL_HTTP_MOCK_REPORT_DATA_H
 #define ISTIO_CONTROL_HTTP_MOCK_REPORT_DATA_H
 
-#include "include/istio/control/http/report_data.h"
 #include "gmock/gmock.h"
+#include "include/istio/control/http/report_data.h"
 
 namespace istio {
 namespace control {
@@ -25,7 +25,7 @@ namespace http {
 
 // The mock object for ReportData interface.
 class MockReportData : public ReportData {
-public:
+ public:
   MOCK_CONST_METHOD0(GetResponseHeaders, std::map<std::string, std::string>());
   MOCK_CONST_METHOD1(GetReportInfo, void(ReportInfo *info));
   MOCK_CONST_METHOD2(GetDestinationIpPort, bool(std::string *ip, int *port));
@@ -38,8 +38,8 @@ class MockReportData : public ReportData {
           &());
 };
 
-} // namespace http
-} // namespace control
-} // namespace istio
+}  // namespace http
+}  // namespace control
+}  // namespace istio
 
-#endif // ISTIO_CONTROL_HTTP_MOCK_REPORT_DATA_H
+#endif  // ISTIO_CONTROL_HTTP_MOCK_REPORT_DATA_H
diff --git a/src/istio/control/http/request_handler_impl_test.cc b/src/istio/control/http/request_handler_impl_test.cc
index a2e46fb4e0e..4fc87aa12f0 100644
--- a/src/istio/control/http/request_handler_impl_test.cc
+++ b/src/istio/control/http/request_handler_impl_test.cc
@@ -14,13 +14,13 @@
  */
 
 #include "google/protobuf/text_format.h"
+#include "gtest/gtest.h"
 #include "include/istio/utils/attribute_names.h"
 #include "src/istio/control/http/client_context.h"
 #include "src/istio/control/http/controller_impl.h"
 #include "src/istio/control/http/mock_check_data.h"
 #include "src/istio/control/http/mock_report_data.h"
 #include "src/istio/control/mock_mixer_client.h"
-#include "gtest/gtest.h"
 
 using ::google::protobuf::TextFormat;
 using ::google::protobuf::util::Status;
@@ -127,7 +127,7 @@ forward_attributes {
 )";
 
 class RequestHandlerImplTest : public ::testing::Test {
-public:
+ public:
   void SetUp() { SetUpMockController(kDefaultClientConfig); }
 
   void SetUpMockController(const std::string &config_text) {
@@ -543,6 +543,6 @@ TEST_F(RequestHandlerImplTest, TestEmptyConfig) {
   handler->Report(&mock_check, &mock_report);
 }
 
-} // namespace http
-} // namespace control
-} // namespace istio
+}  // namespace http
+}  // namespace control
+}  // namespace istio
diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc
index 711f0d3f233..897b7652d0e 100644
--- a/src/istio/control/tcp/attributes_builder_test.cc
+++ b/src/istio/control/tcp/attributes_builder_test.cc
@@ -17,12 +17,12 @@
 
 #include "google/protobuf/text_format.h"
 #include "google/protobuf/util/message_differencer.h"
+#include "gtest/gtest.h"
 #include "include/istio/utils/attribute_names.h"
 #include "include/istio/utils/attributes_builder.h"
 #include "src/istio/control/tcp/mock_check_data.h"
 #include "src/istio/control/tcp/mock_report_data.h"
 #include "src/istio/utils/utils.h"
-#include "gtest/gtest.h"
 
 using ::google::protobuf::TextFormat;
 using ::google::protobuf::util::MessageDifferencer;
@@ -600,7 +600,7 @@ TEST(AttributesBuilderTest, TestReportAttributes) {
                                          expected_final_attributes));
 }
 
-} // namespace
-} // namespace tcp
-} // namespace control
-} // namespace istio
+}  // namespace
+}  // namespace tcp
+}  // namespace control
+}  // namespace istio
diff --git a/src/istio/control/tcp/mock_report_data.h b/src/istio/control/tcp/mock_report_data.h
index 469ab4f28d1..85c749ff360 100644
--- a/src/istio/control/tcp/mock_report_data.h
+++ b/src/istio/control/tcp/mock_report_data.h
@@ -16,8 +16,8 @@
 #ifndef ISTIO_CONTROL_TCP_MOCK_REPORT_DATA_H
 #define ISTIO_CONTROL_TCP_MOCK_REPORT_DATA_H
 
-#include "include/istio/control/tcp/report_data.h"
 #include "gmock/gmock.h"
+#include "include/istio/control/tcp/report_data.h"
 
 namespace istio {
 namespace control {
@@ -25,7 +25,7 @@ namespace tcp {
 
 // The mock object for ReportData interface.
 class MockReportData : public ReportData {
-public:
+ public:
   MOCK_CONST_METHOD2(GetDestinationIpPort, bool(std::string *ip, int *port));
   MOCK_CONST_METHOD1(GetDestinationUID, bool(std::string *));
   MOCK_CONST_METHOD0(
@@ -35,8 +35,8 @@ class MockReportData : public ReportData {
   MOCK_CONST_METHOD1(GetReportInfo, void(ReportInfo *info));
 };
 
-} // namespace tcp
-} // namespace control
-} // namespace istio
+}  // namespace tcp
+}  // namespace control
+}  // namespace istio
 
-#endif // ISTIO_CONTROL_TCP_MOCK_REPORT_DATA_H
+#endif  // ISTIO_CONTROL_TCP_MOCK_REPORT_DATA_H
diff --git a/src/istio/control/tcp/request_handler_impl_test.cc b/src/istio/control/tcp/request_handler_impl_test.cc
index 4f7c075b6bf..c27763b3bc4 100644
--- a/src/istio/control/tcp/request_handler_impl_test.cc
+++ b/src/istio/control/tcp/request_handler_impl_test.cc
@@ -14,12 +14,12 @@
  */
 
 #include "google/protobuf/text_format.h"
+#include "gtest/gtest.h"
 #include "src/istio/control/mock_mixer_client.h"
 #include "src/istio/control/tcp/client_context.h"
 #include "src/istio/control/tcp/controller_impl.h"
 #include "src/istio/control/tcp/mock_check_data.h"
 #include "src/istio/control/tcp/mock_report_data.h"
-#include "gtest/gtest.h"
 
 using ::google::protobuf::TextFormat;
 using ::google::protobuf::util::Status;
@@ -70,10 +70,10 @@ attributes {
   }
 }
 )";
-} // namespace
+}  // namespace
 
 class RequestHandlerImplTest : public ::testing::Test {
-public:
+ public:
   void SetUp() {
     auto map1 = client_config_.mutable_mixer_attributes()->mutable_attributes();
     (*map1)["key1"].set_string_value("value1");
@@ -158,6 +158,6 @@ TEST_F(RequestHandlerImplTest, TestHandlerReport) {
   handler->Report(&mock_data, ReportData::CONTINUE);
 }
 
-} // namespace tcp
-} // namespace control
-} // namespace istio
+}  // namespace tcp
+}  // namespace control
+}  // namespace istio

From 1959ddd6dd5ef49da38ced2e446ba6dd737e72c0 Mon Sep 17 00:00:00 2001
From: Shriram Rajagopalan <shriramr@vmware.com>
Date: Thu, 1 Nov 2018 17:34:31 +0000
Subject: [PATCH 4/4] skip empty maps

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
---
 include/istio/utils/attributes_builder.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/istio/utils/attributes_builder.h b/include/istio/utils/attributes_builder.h
index c0714187701..c48d872775c 100644
--- a/include/istio/utils/attributes_builder.h
+++ b/include/istio/utils/attributes_builder.h
@@ -122,6 +122,10 @@ class AttributesBuilder {
           break;
       }
     }
+
+    if (entries->empty()) {
+      attributes_->mutable_attributes()->erase(key);
+    }
   }
 
   // Serializes all the keys in a map<string, struct> and builds attributes.