From 3cc76171a91d77c5ec7e7896d8b6ac79a075c963 Mon Sep 17 00:00:00 2001
From: Yousuk Seung <ysseung@google.com>
Date: Thu, 16 Mar 2023 13:30:04 -0700
Subject: [PATCH] Merge per-request and per-server named metrics field-wise
 (#32634)

We currently take named metrics recorded per-request only. Instead we
should merge field-wise.

<!--

If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the
appropriate
lang label.

-->
---
 src/cpp/server/backend_metric_recorder.cc  |  8 ++++++--
 test/cpp/end2end/client_lb_end2end_test.cc | 24 ++++++++++++++++------
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/src/cpp/server/backend_metric_recorder.cc b/src/cpp/server/backend_metric_recorder.cc
index cbe6945f819..4d7fff47960 100644
--- a/src/cpp/server/backend_metric_recorder.cc
+++ b/src/cpp/server/backend_metric_recorder.cc
@@ -295,8 +295,12 @@ BackendMetricData BackendMetricState::GetBackendMetricData() {
   }
   {
     internal::MutexLock lock(&mu_);
-    data.utilization = std::move(utilization_);
-    data.request_cost = std::move(request_cost_);
+    for (const auto& u : utilization_) {
+      data.utilization[u.first] = u.second;
+    }
+    for (const auto& r : request_cost_) {
+      data.request_cost[r.first] = r.second;
+    }
   }
   if (GRPC_TRACE_FLAG_ENABLED(grpc_backend_metric_trace)) {
     gpr_log(GPR_INFO,
diff --git a/test/cpp/end2end/client_lb_end2end_test.cc b/test/cpp/end2end/client_lb_end2end_test.cc
index 01197db8053..c11793d734a 100644
--- a/test/cpp/end2end/client_lb_end2end_test.cc
+++ b/test/cpp/end2end/client_lb_end2end_test.cc
@@ -2571,20 +2571,32 @@ TEST_F(ClientLbInterceptTrailingMetadataTest, BackendMetricData) {
   const int kNumServers = 1;
   const int kNumRpcs = 10;
   StartServers(kNumServers);
+  servers_[0]->server_metric_recorder_->SetCpuUtilization(0.99);
+  servers_[0]->server_metric_recorder_->SetMemoryUtilization(0.99);
+  servers_[0]->server_metric_recorder_->SetQps(0.5);
+  servers_[0]->server_metric_recorder_->SetNamedUtilization("foo", 0.99);
+  servers_[0]->server_metric_recorder_->SetNamedUtilization("bar", 0.1);
+  servers_[0]->server_metric_recorder_->SetNamedUtilization("baz", 0.2);
   xds::data::orca::v3::OrcaLoadReport load_report;
+  // Metrics recorded per-call override ones reorded per-server above.
   load_report.set_cpu_utilization(0.5);
   load_report.set_mem_utilization(0.75);
-  load_report.set_rps_fractional(0.25);
   auto* request_cost = load_report.mutable_request_cost();
   (*request_cost)["foo"] = 0.8;
   (*request_cost)["bar"] = 1.4;
   auto* utilization = load_report.mutable_utilization();
-  (*utilization)["baz"] = 1.0;
-  (*utilization)["quux"] = 0.9;
-  // This will be rejected.
-  (*utilization)["out_of_range_invalid"] = 1.1;
+  (*utilization)["foo"] = 1.0;
+  (*utilization)["bar"] = -1.0;
+  (*utilization)["invalid"] = 1.1;
   auto expected = load_report;
-  expected.mutable_utilization()->erase("out_of_range_invalid");
+  // Per-call utilization["bar"] and ["invalid"] are invalid and won't override
+  // per-server values (if present).
+  (*expected.mutable_utilization())["bar"] = 0.1;
+  expected.mutable_utilization()->erase("invalid");
+  // No per-call values for utilization["baz"] and QPS, so keep per-server
+  // values.
+  (*expected.mutable_utilization())["baz"] = 0.2;
+  expected.set_rps_fractional(0.5);
   auto response_generator = BuildResolverResponseGenerator();
   auto channel =
       BuildChannel("intercept_trailing_metadata_lb", response_generator);