added histogram latency information to Hystrix dashboard stream

docs/root/operations/admin.rst

@@ -352,6 +352,7 @@ The fields are:
     In Envoy, service unavailable response will cause **outlier detection** - removing a node off the 
     load balancer pool, but requests are not rejected as a result. Therefore, this counter is always 
     set to '0'.
-  * Latency information is currently unavailable.
+  * Latency information represent cumulative data from start of the clusters operation. 
+    Mean latency is currently not available.
 
 

source/common/stats/histogram_impl.cc

@@ -19,8 +19,8 @@ HistogramStatisticsImpl::HistogramStatisticsImpl(const histogram_t* histogram_pt
 }
 
 const std::vector<double>& HistogramStatisticsImpl::supportedQuantiles() const {
-  static const std::vector<double> supported_quantiles = {0,    0.25, 0.5,   0.75, 0.90,
-                                                          0.95, 0.99, 0.999, 1};
+  static const std::vector<double> supported_quantiles = {0,    0.25, 0.5,   0.75,  0.90,
+                                                          0.95, 0.99, 0.995, 0.999, 1};
   return supported_quantiles;
 }
 

source/extensions/stat_sinks/hystrix/hystrix.cc

@@ -10,14 +10,16 @@
 #include "common/http/headers.h"
 
 #include "absl/strings/str_cat.h"
+#include "absl/strings/str_split.h"
 
 namespace Envoy {
 namespace Extensions {
 namespace StatSinks {
 namespace Hystrix {
 
 const uint64_t HystrixSink::DEFAULT_NUM_BUCKETS;
-
+static const std::vector<double> hystrix_quantiles = {0,    0.25, 0.5,   0.75, 0.90,
+                                                      0.95, 0.99, 0.995, 1};
 ClusterStatsCache::ClusterStatsCache(const std::string& cluster_name)
     : cluster_name_(cluster_name) {}
 
@@ -41,6 +43,14 @@ void ClusterStatsCache::printRollingWindow(absl::string_view name, RollingWindow
   out_str << std::endl;
 }
 
+void ClusterStatsCache::addHistogramToStream(std::stringstream& ss) {
+  bool is_first = true;
+  for (std::pair<std::string, double> element : timing_) {
+    HystrixSink::addDoubleToStream(element.first, element.second, ss, is_first);
+    is_first = false;
+  }
+}
+
 // Add new value to rolling window, in place of oldest one.
 void HystrixSink::pushNewValue(RollingWindow& rolling_window, uint64_t value) {
   if (rolling_window.empty()) {
@@ -65,7 +75,8 @@ uint64_t HystrixSink::getRollingValue(RollingWindow rolling_window) {
 }
 
 void HystrixSink::updateRollingWindowMap(const Upstream::ClusterInfo& cluster_info,
-                                         ClusterStatsCache& cluster_stats_cache) {
+                                         ClusterStatsCache& cluster_stats_cache,
+                                         std::unordered_map<std::string, double>& histogram) {
   const std::string cluster_name = cluster_info.name();
   Upstream::ClusterStats& cluster_stats = cluster_info.stats();
   Stats::Scope& cluster_stats_scope = cluster_info.statsScope();
@@ -100,6 +111,8 @@ void HystrixSink::updateRollingWindowMap(const Upstream::ClusterInfo& cluster_in
   uint64_t total = errors + timeouts + success + rejected;
   pushNewValue(cluster_stats_cache.total_, total);
 
+  cluster_stats_cache.timing_ = histogram;
+
   ENVOY_LOG(trace, "{}", printRollingWindows());
 }
 
@@ -116,6 +129,11 @@ void HystrixSink::addIntToStream(absl::string_view key, uint64_t value, std::str
   addInfoToStream(key, std::to_string(value), info, is_first);
 }
 
+void HystrixSink::addDoubleToStream(absl::string_view key, double value, std::stringstream& info,
+                                    bool is_first) {
+  addInfoToStream(key, std::to_string(value), info, is_first);
+}
+
 void HystrixSink::addInfoToStream(absl::string_view key, absl::string_view value,
                                   std::stringstream& info, bool is_first) {
   if (!is_first) {
@@ -159,7 +177,7 @@ void HystrixSink::addHystrixCommand(ClusterStatsCache& cluster_stats_cache,
   addIntToStream("rollingCountResponsesFromCache", 0, ss);
 
   // Envoy's "circuit breaker" has similar meaning to hystrix's isolation
-  // so we count upstream_rq_pending_overflow and present it as ss
+  // so we count upstream_rq_pending_overflow and present it as rollingCountSemaphoreRejected
   addIntToStream("rollingCountSemaphoreRejected", rejected, ss);
 
   // Hystrix's short circuit is not similar to Envoy's since it is triggered by 503 responses
@@ -172,11 +190,10 @@ void HystrixSink::addHystrixCommand(ClusterStatsCache& cluster_stats_cache,
   addIntToStream("rollingCountBadRequests", 0, ss);
   addIntToStream("currentConcurrentExecutionCount", 0, ss);
   addIntToStream("latencyExecute_mean", 0, ss);
-
-  // TODO trabetti : add histogram information once available by PR #2932
-  addInfoToStream(
-      "latencyExecute",
-      "{\"0\":0,\"25\":0,\"50\":0,\"75\":0,\"90\":0,\"95\":0,\"99\":0,\"99.5\":0,\"100\":0}", ss);
+  ss << ", \"latencyExecute\": {";
+  cluster_stats_cache.addHistogramToStream(ss);
+  ss << "}";
+  // addInfoToStream("latencyExecute", "{" + cluster_stats_cache.printTimingHistogram() + "}", ss);
   addIntToStream("propertyValue_circuitBreakerRequestVolumeThreshold", 0, ss);
   addIntToStream("propertyValue_circuitBreakerSleepWindowInMilliseconds", 0, ss);
   addIntToStream("propertyValue_circuitBreakerErrorThresholdPercentage", 0, ss);
@@ -304,6 +321,32 @@ void HystrixSink::flush(Stats::Source&) {
   incCounter();
   std::stringstream ss;
   Upstream::ClusterManager::ClusterInfoMap clusters = server_.clusterManager().clusters();
+
+  // Save a map of the relevant histograms per cluster in a convenient format.
+  std::unordered_map<absl::string_view, std::unordered_map<std::string, double>, StringViewHash>
+      time_histograms;
+  for (const Stats::ParentHistogramSharedPtr histogram : server_.stats().histograms()) {
+    // histogram->name() on clusters of the format "cluster.cluster_name.histogram_name"
+    // i.e. "cluster.service1.upstream_rq_time".
+    const std::vector<absl::string_view> split_name = absl::StrSplit(histogram->name(), '.');
+    if (split_name[0] == "cluster" && split_name[2] == "upstream_rq_time") {
+      std::unordered_map<std::string, double>& hist_map = time_histograms[split_name[1]];
+      for (size_t i = 0; i < histogram->cumulativeStatistics().supportedQuantiles().size(); ++i) {
+        if (std::find(hystrix_quantiles.begin(), hystrix_quantiles.end(),
+                      histogram->cumulativeStatistics().supportedQuantiles()[i]) !=
+            hystrix_quantiles.end()) {
+          if (histogram->cumulativeStatistics().supportedQuantiles()[i] == 0.995) {
+            hist_map["99.5"] = histogram->cumulativeStatistics().computedQuantiles()[i];
+          } else {
+            hist_map[std::to_string(
+                int(100 * histogram->cumulativeStatistics().supportedQuantiles()[i]))] =
+                histogram->cumulativeStatistics().computedQuantiles()[i];
+          }
+        }
+      }
+    }
+  }
+
   for (auto& cluster : clusters) {
     Upstream::ClusterInfoConstSharedPtr cluster_info = cluster.second.get().info();
 
@@ -314,7 +357,8 @@ void HystrixSink::flush(Stats::Source&) {
     }
 
     // update rolling window with cluster stats
-    updateRollingWindowMap(*cluster_info, *cluster_stats_cache_ptr);
+    updateRollingWindowMap(*cluster_info, *cluster_stats_cache_ptr,
+                           time_histograms[cluster_info->name()]);
 
     // append it to stream to be sent
     addClusterStatsToStream(

source/extensions/stat_sinks/hystrix/hystrix.h

@@ -26,6 +26,8 @@ struct ClusterStatsCache {
   void printToStream(std::stringstream& out_str);
   void printRollingWindow(absl::string_view name, RollingWindow rolling_window,
                           std::stringstream& out_str);
+  void addHistogramToStream(std::stringstream& ss);
+
   std::string cluster_name_;
 
   // Rolling windows
@@ -34,6 +36,8 @@ struct ClusterStatsCache {
   RollingWindow total_;
   RollingWindow timeouts_;
   RollingWindow rejected_;
+
+  std::unordered_map<std::string, double> timing_;
 };
 
 typedef std::unique_ptr<ClusterStatsCache> ClusterStatsCachePtr;
@@ -78,7 +82,8 @@ class HystrixSink : public Stats::Sink, public Logger::Loggable<Logger::Id::hyst
    * Calculate values needed to create the stream and write into the map.
    */
   void updateRollingWindowMap(const Upstream::ClusterInfo& cluster_info,
-                              ClusterStatsCache& cluster_stats_cache);
+                              ClusterStatsCache& cluster_stats_cache,
+                              std::unordered_map<std::string, double>& histogram);
   /**
    * Clear map.
    */
@@ -94,27 +99,34 @@ class HystrixSink : public Stats::Sink, public Logger::Loggable<Logger::Id::hyst
    */
   uint64_t getRollingValue(RollingWindow rolling_window);
 
-private:
   /**
-   * Format the given key and absl::string_view value to "key"="value", and adding to the
+   * Format the given key and value to "key"=value, and adding to the stringstream.
+   */
+  static void addInfoToStream(absl::string_view key, absl::string_view value,
+                              std::stringstream& info, bool is_first = false);
+
+  /**
+   * Format the given key and double value to "key"=<string of uint64_t>, and adding to the
    * stringstream.
    */
-  void addStringToStream(absl::string_view key, absl::string_view value, std::stringstream& info,
-                         bool is_first = false);
+  static void addDoubleToStream(absl::string_view key, double value, std::stringstream& info,
+                                bool is_first);
 
   /**
-   * Format the given key and uint64_t value to "key"=<string of uint64_t>, and adding to the
+   * Format the given key and absl::string_view value to "key"="value", and adding to the
    * stringstream.
    */
-  void addIntToStream(absl::string_view key, uint64_t value, std::stringstream& info,
-                      bool is_first = false);
+  static void addStringToStream(absl::string_view key, absl::string_view value,
+                                std::stringstream& info, bool is_first = false);
 
   /**
-   * Format the given key and value to "key"=value, and adding to the stringstream.
+   * Format the given key and uint64_t value to "key"=<string of uint64_t>, and adding to the
+   * stringstream.
    */
-  void addInfoToStream(absl::string_view key, absl::string_view value, std::stringstream& info,
-                       bool is_first = false);
+  static void addIntToStream(absl::string_view key, uint64_t value, std::stringstream& info,
+                             bool is_first = false);
 
+private:
   /**
    * Generate HystrixCommand event stream.
    */

test/common/stats/thread_local_store_test.cc

@@ -610,9 +610,9 @@ TEST_F(HistogramTest, BasicHistogramSummaryValidate) {
 
   const std::string h1_expected_summary =
       "P0: 1, P25: 1.025, P50: 1.05, P75: 1.075, P90: 1.09, P95: 1.095, "
-      "P99: 1.099, P99.9: 1.0999, P100: 1.1";
-  const std::string h2_expected_summary =
-      "P0: 0, P25: 25, P50: 50, P75: 75, P90: 90, P95: 95, P99: 99, P99.9: 99.9, P100: 100";
+      "P99: 1.099, P99.5: 1.0995, P99.9: 1.0999, P100: 1.1";
+  const std::string h2_expected_summary = "P0: 0, P25: 25, P50: 50, P75: 75, P90: 90, P95: 95, "
+                                          "P99: 99, P99.5: 99.5, P99.9: 99.9, P100: 100";
 
   for (size_t i = 0; i < 100; ++i) {
     expectCallAndAccumulate(h2, i);
@@ -639,8 +639,8 @@ TEST_F(HistogramTest, BasicHistogramMergeSummary) {
   }
   EXPECT_EQ(1, validateMerge());
 
-  const std::string expected_summary =
-      "P0: 0, P25: 25, P50: 50, P75: 75, P90: 90, P95: 95, P99: 99, P99.9: 99.9, P100: 100";
+  const std::string expected_summary = "P0: 0, P25: 25, P50: 50, P75: 75, P90: 90, P95: 95, P99: "
+                                       "99, P99.5: 99.5, P99.9: 99.9, P100: 100";
 
   NameHistogramMap name_histogram_map = makeHistogramMap(store_->histograms());
   EXPECT_EQ(expected_summary, name_histogram_map["h1"]->cumulativeStatistics().summary());

test/server/http/admin_test.cc

@@ -125,6 +125,7 @@ TEST_P(AdminStatsTest, StatsAsJson) {
                     90.0,
                     95.0,
                     99.0,
+                    99.5,
                     99.9,
                     100.0
                 ],
@@ -160,6 +161,10 @@ TEST_P(AdminStatsTest, StatsAsJson) {
                                 "interval": null,
                                 "cumulative": 109.9
                             },
+                            {
+                                "interval": null,
+                                "cumulative": 109.95
+                            },
                             {
                                 "interval": null,
                                 "cumulative": 109.99
@@ -201,6 +206,10 @@ TEST_P(AdminStatsTest, StatsAsJson) {
                                 "interval": 109.9,
                                 "cumulative": 209.8
                             },
+                            {
+                                "interval": 109.95,
+                                "cumulative": 209.9
+                            },
                             {
                                 "interval": 109.99,
                                 "cumulative": 209.98
@@ -262,6 +271,7 @@ TEST_P(AdminStatsTest, UsedOnlyStatsAsJson) {
                     90.0,
                     95.0,
                     99.0,
+                    99.5,
                     99.9,
                     100.0
                 ],
@@ -297,6 +307,10 @@ TEST_P(AdminStatsTest, UsedOnlyStatsAsJson) {
                                 "interval": 109.9,
                                 "cumulative": 209.8
                             },
+                            {
+                                "interval": 109.95,
+                                "cumulative": 209.9
+                            },
                             {
                                 "interval": 109.99,
                                 "cumulative": 209.98