admin: order stats in clusters json admin (envoyproxy#4306)

Currently host level stats in clusters proto uses map so they are outputted in random order. This PR changes it to list so that the order is predictable. Risk Level: Low Testing: Added automated tests Docs Changes: N/A Release Notes: N/A Signed-off-by: Rama <[email protected]>
rshriram · Sep 4, 2018 · 69474b3 · 69474b3
1 parent 2d155f9
commit 69474b3
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 12 deletions.
diff --git a/api/envoy/admin/v2alpha/clusters.proto b/api/envoy/admin/v2alpha/clusters.proto
@@ -46,8 +46,8 @@ message HostStatus {
   // Address of this host.
   envoy.api.v2.core.Address address = 1;
 
-  // Mapping from the name of the statistic to the current value.
-  map<string, SimpleMetric> stats = 2;
+  // List of stats specific to this host.
+  repeated SimpleMetric stats = 2;
 
   // The host's current health status.
   HostHealthStatus health_status = 3;

diff --git a/api/envoy/admin/v2alpha/metrics.proto b/api/envoy/admin/v2alpha/metrics.proto
@@ -11,9 +11,12 @@ message SimpleMetric {
     GAUGE = 1;
   }
 
-  // Type of metric represented.
+  // Type of the metric represented.
   Type type = 1;
 
   // Current metric value.
   uint64 value = 2;
+
+  // Name of the metric.
+  string name = 3;
 }
diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc
@@ -284,17 +284,37 @@ void AdminImpl::writeClustersAsJson(Buffer::Instance& response) {
         envoy::admin::v2alpha::HostStatus& host_status = *cluster_status.add_host_statuses();
         Network::Utility::addressToProtobufAddress(*host->address(),
                                                    *host_status.mutable_address());
-
+        std::vector<Stats::CounterSharedPtr> sorted_counters;
         for (const Stats::CounterSharedPtr& counter : host->counters()) {
-          auto& metric = (*host_status.mutable_stats())[counter->name()];
-          metric.set_type(envoy::admin::v2alpha::SimpleMetric::COUNTER);
+          sorted_counters.push_back(counter);
+        }
+        std::sort(
+            sorted_counters.begin(), sorted_counters.end(),
+            [](const Stats::CounterSharedPtr& counter1, const Stats::CounterSharedPtr& counter2) {
+              return counter1->name() < counter2->name();
+            });
+
+        for (const Stats::CounterSharedPtr& counter : sorted_counters) {
+          auto& metric = *host_status.add_stats();
+          metric.set_name(counter->name());
           metric.set_value(counter->value());
+          metric.set_type(envoy::admin::v2alpha::SimpleMetric::COUNTER);
         }
 
+        std::vector<Stats::GaugeSharedPtr> sorted_gauges;
         for (const Stats::GaugeSharedPtr& gauge : host->gauges()) {
-          auto& metric = (*host_status.mutable_stats())[gauge->name()];
-          metric.set_type(envoy::admin::v2alpha::SimpleMetric::GAUGE);
+          sorted_gauges.push_back(gauge);
+        }
+        std::sort(sorted_gauges.begin(), sorted_gauges.end(),
+                  [](const Stats::GaugeSharedPtr& gauge1, const Stats::GaugeSharedPtr& gauge2) {
+                    return gauge1->name() < gauge2->name();
+                  });
+
+        for (const Stats::GaugeSharedPtr& gauge : sorted_gauges) {
+          auto& metric = *host_status.add_stats();
+          metric.set_name(gauge->name());
           metric.set_value(gauge->value());
+          metric.set_type(envoy::admin::v2alpha::SimpleMetric::GAUGE);
         }
 
         envoy::admin::v2alpha::HostHealthStatus& health_status =

diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc
@@ -737,9 +737,13 @@ TEST_P(AdminInstanceTest, ClustersJson) {
       Network::Utility::resolveUrl("tcp://1.2.3.4:80");
   ON_CALL(*host, address()).WillByDefault(Return(address));
 
+  // Add stats in random order and validate that they come in order.
   Stats::IsolatedStoreImpl store;
   store.counter("test_counter").add(10);
+  store.counter("rest_counter").add(10);
+  store.counter("arest_counter").add(5);
   store.gauge("test_gauge").set(11);
+  store.gauge("atest_gauge").set(10);
   ON_CALL(*host, gauges()).WillByDefault(Invoke([&store]() { return store.gauges(); }));
   ON_CALL(*host, counters()).WillByDefault(Invoke([&store]() { return store.counters(); }));
 
@@ -776,16 +780,33 @@ TEST_P(AdminInstanceTest, ClustersJson) {
        "port_value": 80
       }
      },
-     "stats": {
-      "test_counter": {
+     "stats": [
+       {
+       "name": "arest_counter",
+       "value": "5",
+       "type": "COUNTER"
+       },
+       {
+       "name": "rest_counter",
+       "value": "10",
+       "type": "COUNTER"
+      },
+      {
+       "name": "test_counter",
        "value": "10",
        "type": "COUNTER"
       },
-      "test_gauge": {
+      {
+       "name": "atest_gauge",
+       "value": "10",
+       "type": "GAUGE"
+      },
+      {
+       "name": "test_gauge",
        "value": "11",
        "type": "GAUGE"
       },
-     },
+     ],
      "health_status": {
       "eds_health_status": "HEALTHY",
       "failed_active_health_check": true,