diff --git a/repositories.bzl b/repositories.bzl index 719413dab4a..fb8d3782fa4 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -113,7 +113,7 @@ cc_library( actual = "@googletest_git//:googletest_prod", ) -ISTIO_API = "214c7598afb74f7f4dea49f77e45832c49382a15" +ISTIO_API = "62c345bd6d6e4c2047dd2dee128b7413231be7b4" def mixerapi_repositories(bind=True): BUILD = """ diff --git a/src/istio/control/client_context_base.cc b/src/istio/control/client_context_base.cc index e489faf18a2..c2ff0ef551e 100644 --- a/src/istio/control/client_context_base.cc +++ b/src/istio/control/client_context_base.cc @@ -90,7 +90,7 @@ CancelFunc ClientContextBase::SendCheck(TransportCheckFunc transport, // save the check status code request->check_status = check_response_info.response_status; - utils::AttributesBuilder builder(&request->attributes); + utils::AttributesBuilder builder(request->attributes); builder.AddBool(utils::AttributeName::kCheckCacheHit, check_response_info.is_check_cache_hit); builder.AddBool(utils::AttributeName::kQuotaCacheHit, @@ -100,16 +100,16 @@ CancelFunc ClientContextBase::SendCheck(TransportCheckFunc transport, // TODO: add debug message // GOOGLE_LOG(INFO) << "Check attributes: " << - // request->attributes.DebugString(); - return mixer_client_->Check(request->attributes, request->quotas, transport, + // request->attributes->DebugString(); + return mixer_client_->Check(*request->attributes, request->quotas, transport, local_on_done); } void ClientContextBase::SendReport(const RequestContext& request) { // TODO: add debug message // GOOGLE_LOG(INFO) << "Report attributes: " << - // request.attributes.DebugString(); - mixer_client_->Report(request.attributes); + // request.attributes->DebugString(); + mixer_client_->Report(*request.attributes); } void ClientContextBase::GetStatistics(Statistics* stat) const { diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc index ac1e00fc92a..2c0babcbabf 100644 --- a/src/istio/control/http/attributes_builder.cc +++ b/src/istio/control/http/attributes_builder.cc @@ -35,7 +35,7 @@ const std::set kGrpcContentTypes{ } // namespace void AttributesBuilder::ExtractRequestHeaderAttributes(CheckData *check_data) { - utils::AttributesBuilder builder(&request_->attributes); + utils::AttributesBuilder builder(request_->attributes); std::map headers = check_data->GetRequestHeaders(); builder.AddStringMap(utils::AttributeName::kRequestHeaders, headers); @@ -79,7 +79,7 @@ void AttributesBuilder::ExtractRequestHeaderAttributes(CheckData *check_data) { } void AttributesBuilder::ExtractAuthAttributes(CheckData *check_data) { - utils::AttributesBuilder builder(&request_->attributes); + utils::AttributesBuilder builder(request_->attributes); std::string destination_principal; if (check_data->GetPrincipal(false, &destination_principal)) { @@ -132,7 +132,7 @@ void AttributesBuilder::ExtractForwardedAttributes(CheckData *check_data) { } Attributes v2_format; if (v2_format.ParseFromString(forwarded_data)) { - request_->attributes.MergeFrom(v2_format); + request_->attributes->MergeFrom(v2_format); return; } } @@ -141,7 +141,7 @@ void AttributesBuilder::ExtractCheckAttributes(CheckData *check_data) { ExtractRequestHeaderAttributes(check_data); ExtractAuthAttributes(check_data); - utils::AttributesBuilder builder(&request_->attributes); + utils::AttributesBuilder builder(request_->attributes); // connection remote IP is always reported as origin IP std::string source_ip; @@ -181,7 +181,7 @@ void AttributesBuilder::ForwardAttributes(const Attributes &forward_attributes, } void AttributesBuilder::ExtractReportAttributes(ReportData *report_data) { - utils::AttributesBuilder builder(&request_->attributes); + utils::AttributesBuilder builder(request_->attributes); std::string dest_ip; int dest_port; diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index 5f2d2429b3b..f14c32cb98d 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -517,13 +517,13 @@ fields { void ClearContextTime(const std::string &name, RequestContext *request) { // Override timestamp with - - utils::AttributesBuilder builder(&request->attributes); + utils::AttributesBuilder builder(request->attributes); std::chrono::time_point time0; builder.AddTimestamp(name, time0); } void SetDestinationIp(RequestContext *request, const std::string &ip) { - utils::AttributesBuilder builder(&request->attributes); + utils::AttributesBuilder builder(request->attributes); builder.AddBytes(utils::AttributeName::kDestinationIp, ip); } @@ -541,7 +541,7 @@ TEST(AttributesBuilderTest, TestExtractForwardedAttributes) { RequestContext request; AttributesBuilder builder(&request); builder.ExtractForwardedAttributes(&mock_data); - EXPECT_THAT(request.attributes, EqualsAttribute(attr)); + EXPECT_THAT(*request.attributes, EqualsAttribute(attr)); } TEST(AttributesBuilderTest, TestForwardAttributes) { @@ -631,7 +631,7 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithoutAuthnFilter) { Attributes expected_attributes; ASSERT_TRUE(TextFormat::ParseFromString(kCheckAttributesWithoutAuthnFilter, &expected_attributes)); - EXPECT_THAT(request.attributes, EqualsAttribute(expected_attributes)); + EXPECT_THAT(*request.attributes, EqualsAttribute(expected_attributes)); } TEST(AttributesBuilderTest, TestCheckAttributes) { @@ -705,7 +705,7 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { Attributes expected_attributes; ASSERT_TRUE( TextFormat::ParseFromString(kCheckAttributes, &expected_attributes)); - EXPECT_THAT(request.attributes, EqualsAttribute(expected_attributes)); + EXPECT_THAT(*request.attributes, EqualsAttribute(expected_attributes)); } TEST(AttributesBuilderTest, TestReportAttributes) { @@ -769,7 +769,7 @@ TEST(AttributesBuilderTest, TestReportAttributes) { (*expected_attributes .mutable_attributes())[utils::AttributeName::kResponseGrpcMessage] .set_string_value("grpc-message"); - EXPECT_THAT(request.attributes, EqualsAttribute(expected_attributes)); + EXPECT_THAT(*request.attributes, EqualsAttribute(expected_attributes)); } TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) { @@ -816,7 +816,7 @@ TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) { Attributes expected_attributes; ASSERT_TRUE( TextFormat::ParseFromString(kReportAttributes, &expected_attributes)); - EXPECT_THAT(request.attributes, EqualsAttribute(expected_attributes)); + EXPECT_THAT(*request.attributes, EqualsAttribute(expected_attributes)); } } // namespace diff --git a/src/istio/control/http/service_context.cc b/src/istio/control/http/service_context.cc index 1a3c4b67839..dc1332db698 100644 --- a/src/istio/control/http/service_context.cc +++ b/src/istio/control/http/service_context.cc @@ -52,13 +52,14 @@ void ServiceContext::BuildParsers() { // Add static mixer attributes. void ServiceContext::AddStaticAttributes(RequestContext *request) const { - client_context_->AddLocalNodeAttributes(&request->attributes); + client_context_->AddLocalNodeAttributes(request->attributes); if (client_context_->config().has_mixer_attributes()) { - request->attributes.MergeFrom(client_context_->config().mixer_attributes()); + request->attributes->MergeFrom( + client_context_->config().mixer_attributes()); } if (service_config_ && service_config_->has_mixer_attributes()) { - request->attributes.MergeFrom(service_config_->mixer_attributes()); + request->attributes->MergeFrom(service_config_->mixer_attributes()); } } @@ -90,13 +91,13 @@ void ServiceContext::AddApiAttributes(CheckData *check_data, std::string path; if (check_data->FindHeaderByType(CheckData::HEADER_METHOD, &http_method) && check_data->FindHeaderByType(CheckData::HEADER_PATH, &path)) { - api_spec_parser_->AddAttributes(http_method, path, &request->attributes); + api_spec_parser_->AddAttributes(http_method, path, request->attributes); } std::string api_key; if (api_spec_parser_->ExtractApiKey(check_data, &api_key)) { (*request->attributes - .mutable_attributes())[utils::AttributeName::kRequestApiKey] + ->mutable_attributes())[utils::AttributeName::kRequestApiKey] .set_string_value(api_key); } } @@ -104,7 +105,7 @@ void ServiceContext::AddApiAttributes(CheckData *check_data, // Add quota requirements from quota configs. void ServiceContext::AddQuotas(RequestContext *request) const { for (const auto &parser : quota_parsers_) { - parser->GetRequirements(request->attributes, &request->quotas); + parser->GetRequirements(*request->attributes, &request->quotas); } } diff --git a/src/istio/control/request_context.h b/src/istio/control/request_context.h index 55dd4b3fa4f..5e655abbba0 100644 --- a/src/istio/control/request_context.h +++ b/src/istio/control/request_context.h @@ -16,6 +16,7 @@ #ifndef ISTIO_CONTROL_REQUEST_CONTEXT_H #define ISTIO_CONTROL_REQUEST_CONTEXT_H +#include "google/protobuf/arena.h" #include "google/protobuf/stubs/status.h" #include "include/istio/quota_config/requirement.h" #include "mixer/v1/attributes.pb.h" @@ -27,8 +28,15 @@ namespace control { // The context to hold request data for both HTTP and TCP. struct RequestContext { + RequestContext() { + attributes = + google::protobuf::Arena::CreateMessage<::istio::mixer::v1::Attributes>( + &arena_); + } + // protobuf arena + google::protobuf::Arena arena_; // The attributes for both Check and Report. - ::istio::mixer::v1::Attributes attributes; + ::istio::mixer::v1::Attributes* attributes; // The quota requirements std::vector<::istio::quota_config::Requirement> quotas; // The check status. diff --git a/src/istio/control/tcp/attributes_builder.cc b/src/istio/control/tcp/attributes_builder.cc index 595a5d9d7a6..0e5617d101d 100644 --- a/src/istio/control/tcp/attributes_builder.cc +++ b/src/istio/control/tcp/attributes_builder.cc @@ -30,7 +30,7 @@ const std::string kConnectionClose("close"); } // namespace void AttributesBuilder::ExtractCheckAttributes(CheckData* check_data) { - utils::AttributesBuilder builder(&request_->attributes); + utils::AttributesBuilder builder(request_->attributes); std::string source_ip; int source_port; @@ -83,7 +83,7 @@ void AttributesBuilder::ExtractCheckAttributes(CheckData* check_data) { void AttributesBuilder::ExtractReportAttributes( ReportData* report_data, ReportData::ConnectionEvent event, ReportData::ReportInfo* last_report_info) { - utils::AttributesBuilder builder(&request_->attributes); + utils::AttributesBuilder builder(request_->attributes); ReportData::ReportInfo info; report_data->GetReportInfo(&info); diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc index faa78ccc06e..1ce0afe0aa5 100644 --- a/src/istio/control/tcp/attributes_builder_test.cc +++ b/src/istio/control/tcp/attributes_builder_test.cc @@ -360,7 +360,7 @@ attributes { void ClearContextTime(RequestContext* request) { // Override timestamp with - - utils::AttributesBuilder builder(&request->attributes); + utils::AttributesBuilder builder(request->attributes); std::chrono::time_point time0; builder.AddTimestamp(utils::AttributeName::kContextTime, time0); } @@ -398,14 +398,14 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { ClearContextTime(&request); std::string out_str; - TextFormat::PrintToString(request.attributes, &out_str); + TextFormat::PrintToString(*request.attributes, &out_str); GOOGLE_LOG(INFO) << "===" << out_str << "==="; ::istio::mixer::v1::Attributes expected_attributes; ASSERT_TRUE( TextFormat::ParseFromString(kCheckAttributes, &expected_attributes)); EXPECT_TRUE( - MessageDifferencer::Equals(request.attributes, expected_attributes)); + MessageDifferencer::Equals(*request.attributes, expected_attributes)); } TEST(AttributesBuilderTest, TestReportAttributes) { @@ -459,14 +459,14 @@ TEST(AttributesBuilderTest, TestReportAttributes) { ClearContextTime(&request); std::string out_str; - TextFormat::PrintToString(request.attributes, &out_str); + TextFormat::PrintToString(*request.attributes, &out_str); GOOGLE_LOG(INFO) << "===" << out_str << "==="; ::istio::mixer::v1::Attributes expected_open_attributes; ASSERT_TRUE(TextFormat::ParseFromString(kFirstReportAttributes, &expected_open_attributes)); - EXPECT_TRUE( - MessageDifferencer::Equals(request.attributes, expected_open_attributes)); + EXPECT_TRUE(MessageDifferencer::Equals(*request.attributes, + expected_open_attributes)); EXPECT_EQ(0, last_report_info.received_bytes); EXPECT_EQ(0, last_report_info.send_bytes); @@ -475,13 +475,13 @@ TEST(AttributesBuilderTest, TestReportAttributes) { &mock_data, ReportData::ConnectionEvent::CONTINUE, &last_report_info); ClearContextTime(&request); - TextFormat::PrintToString(request.attributes, &out_str); + TextFormat::PrintToString(*request.attributes, &out_str); GOOGLE_LOG(INFO) << "===" << out_str << "==="; ::istio::mixer::v1::Attributes expected_delta_attributes; ASSERT_TRUE(TextFormat::ParseFromString(kDeltaOneReportAttributes, &expected_delta_attributes)); - EXPECT_TRUE(MessageDifferencer::Equals(request.attributes, + EXPECT_TRUE(MessageDifferencer::Equals(*request.attributes, expected_delta_attributes)); EXPECT_EQ(100, last_report_info.received_bytes); EXPECT_EQ(200, last_report_info.send_bytes); @@ -492,13 +492,13 @@ TEST(AttributesBuilderTest, TestReportAttributes) { ClearContextTime(&request); out_str.clear(); - TextFormat::PrintToString(request.attributes, &out_str); + TextFormat::PrintToString(*request.attributes, &out_str); GOOGLE_LOG(INFO) << "===" << out_str << "==="; expected_delta_attributes.Clear(); ASSERT_TRUE(TextFormat::ParseFromString(kDeltaTwoReportAttributes, &expected_delta_attributes)); - EXPECT_TRUE(MessageDifferencer::Equals(request.attributes, + EXPECT_TRUE(MessageDifferencer::Equals(*request.attributes, expected_delta_attributes)); EXPECT_EQ(201, last_report_info.received_bytes); EXPECT_EQ(404, last_report_info.send_bytes); @@ -509,13 +509,13 @@ TEST(AttributesBuilderTest, TestReportAttributes) { ClearContextTime(&request); out_str.clear(); - TextFormat::PrintToString(request.attributes, &out_str); + TextFormat::PrintToString(*request.attributes, &out_str); GOOGLE_LOG(INFO) << "===" << out_str << "==="; ::istio::mixer::v1::Attributes expected_final_attributes; ASSERT_TRUE(TextFormat::ParseFromString(kReportAttributes, &expected_final_attributes)); - EXPECT_TRUE(MessageDifferencer::Equals(request.attributes, + EXPECT_TRUE(MessageDifferencer::Equals(*request.attributes, expected_final_attributes)); } diff --git a/src/istio/control/tcp/client_context.h b/src/istio/control/tcp/client_context.h index 86296d3f1d9..38072694aed 100644 --- a/src/istio/control/tcp/client_context.h +++ b/src/istio/control/tcp/client_context.h @@ -52,17 +52,17 @@ class ClientContext : public ClientContextBase { // Add static mixer attributes. void AddStaticAttributes(RequestContext* request) const { - AddLocalNodeAttributes(&request->attributes); + AddLocalNodeAttributes(request->attributes); if (config_.has_mixer_attributes()) { - request->attributes.MergeFrom(config_.mixer_attributes()); + request->attributes->MergeFrom(config_.mixer_attributes()); } } // Add quota requirements from quota configs. void AddQuotas(RequestContext* request) const { if (quota_parser_) { - quota_parser_->GetRequirements(request->attributes, &request->quotas); + quota_parser_->GetRequirements(*request->attributes, &request->quotas); } } diff --git a/src/istio/mixerclient/attribute_compressor.cc b/src/istio/mixerclient/attribute_compressor.cc index 5b9f04bead0..1a49622204b 100644 --- a/src/istio/mixerclient/attribute_compressor.cc +++ b/src/istio/mixerclient/attribute_compressor.cc @@ -14,6 +14,7 @@ */ #include "src/istio/mixerclient/attribute_compressor.h" +#include "google/protobuf/arena.h" #include "include/istio/utils/protobuf.h" #include "src/istio/mixerclient/global_dictionary.h" @@ -126,31 +127,36 @@ void CompressByDict(const Attributes& attributes, MessageDictionary& dict, class BatchCompressorImpl : public BatchCompressor { public: BatchCompressorImpl(const GlobalDictionary& global_dict) - : global_dict_(global_dict), dict_(global_dict) {} + : global_dict_(global_dict), dict_(global_dict) { + report_ = google::protobuf::Arena::CreateMessage< + ::istio::mixer::v1::ReportRequest>(&arena_); + } void Add(const Attributes& attributes) override { - CompressByDict(attributes, dict_, report_.add_attributes()); + CompressByDict(attributes, dict_, report_->add_attributes()); } - int size() const override { return report_.attributes_size(); } + int size() const override { return report_->attributes_size(); } const ::istio::mixer::v1::ReportRequest& Finish() override { for (const std::string& word : dict_.GetWords()) { - report_.add_default_words(word); + report_->add_default_words(word); } - report_.set_global_word_count(global_dict_.size()); - return report_; + report_->set_global_word_count(global_dict_.size()); + return *report_; } void Clear() override { dict_.Clear(); - report_.Clear(); + report_->Clear(); } private: const GlobalDictionary& global_dict_; + // protobuf arena + google::protobuf::Arena arena_; MessageDictionary dict_; - ::istio::mixer::v1::ReportRequest report_; + ::istio::mixer::v1::ReportRequest* report_; }; } // namespace diff --git a/src/istio/mixerclient/client_impl.cc b/src/istio/mixerclient/client_impl.cc index b80ce6a8c83..2a563039f41 100644 --- a/src/istio/mixerclient/client_impl.cc +++ b/src/istio/mixerclient/client_impl.cc @@ -13,6 +13,7 @@ * limitations under the License. */ #include "src/istio/mixerclient/client_impl.h" +#include #include "include/istio/mixerclient/check_response.h" #include "include/istio/utils/protobuf.h" @@ -82,26 +83,32 @@ CancelFunc MixerClientImpl::Check( quota_cache_->Check(attributes, quotas, check_result->IsCacheHit(), quota_result.get()); - CheckRequest request; - bool quota_call = quota_result->BuildRequest(&request); + auto arena = new google::protobuf::Arena; + CheckRequest *request = + google::protobuf::Arena::CreateMessage(arena); + bool quota_call = quota_result->BuildRequest(request); check_response_info.is_quota_cache_hit = quota_result->IsCacheHit(); check_response_info.response_status = quota_result->status(); if (check_result->IsCacheHit() && quota_result->IsCacheHit()) { on_done(check_response_info); on_done = nullptr; if (!quota_call) { + delete arena; return nullptr; } } - compressor_.Compress(attributes, request.mutable_attributes()); - request.set_global_word_count(compressor_.global_word_count()); - request.set_deduplication_id(deduplication_id_base_ + - std::to_string(deduplication_id_.fetch_add(1))); + compressor_.Compress(attributes, request->mutable_attributes()); + request->set_global_word_count(compressor_.global_word_count()); + request->set_deduplication_id(deduplication_id_base_ + + std::to_string(deduplication_id_.fetch_add(1))); // Need to make a copy for processing the response for check cache. - Attributes *request_copy = new Attributes(attributes); - auto response = new CheckResponse; + Attributes *attributes_copy = + google::protobuf::Arena::CreateMessage(arena); + CheckResponse *response = + google::protobuf::Arena::CreateMessage(arena); + *attributes_copy = attributes; // Lambda capture could not pass unique_ptr, use raw pointer. CheckCache::CheckResult *raw_check_result = check_result.release(); QuotaCache::CheckResult *raw_quota_result = quota_result.release(); @@ -121,11 +128,11 @@ CancelFunc MixerClientImpl::Check( } return transport( - request, response, - [this, request_copy, response, raw_check_result, raw_quota_result, - on_done](const Status &status) { - raw_check_result->SetResponse(status, *request_copy, *response); - raw_quota_result->SetResponse(status, *request_copy, *response); + *request, response, + [this, attributes_copy, response, raw_check_result, raw_quota_result, + on_done, arena](const Status &status) { + raw_check_result->SetResponse(status, *attributes_copy, *response); + raw_quota_result->SetResponse(status, *attributes_copy, *response); CheckResponseInfo check_response_info; if (on_done) { if (!raw_check_result->status().ok()) { @@ -139,8 +146,7 @@ CancelFunc MixerClientImpl::Check( } delete raw_check_result; delete raw_quota_result; - delete request_copy; - delete response; + delete arena; if (utils::InvalidDictionaryStatus(status)) { compressor_.ShrinkGlobalDictionary();