stats: unrevert and fix histograms (envoyproxy#3262)

This change unreverts:
Also fixes envoyproxy#3223

Please see the 2nd commit for the actual changes. The changes are:

Bring in a new histogram library version with the fix (and more debug
checking).
Fix a small issue with scope iteration during merging.
Remove de-dup for histograms until we iterate to shared
storage for overlapping scope histograms. Personally, I found
this very confusing when debugging and I think the way I changed
it is better for now given the code we have.

Signed-off-by: Matt Klein <[email protected]>

bazel/external/libcircllhist.BUILD

@@ -0,0 +1,9 @@
+cc_library(
+    name = "libcircllhist",
+    srcs = ["src/circllhist.c"],
+    hdrs = [
+        "src/circllhist.h",
+    ],
+    includes = ["src"],
+    visibility = ["//visibility:public"],
+)

bazel/repositories.bzl

@@ -226,6 +226,7 @@ def envoy_dependencies(path = "@envoy_deps//", skip_targets = []):
     _boringssl()
     _com_google_absl()
     _com_github_bombela_backward()
+    _com_github_circonus_labs_libcircllhist()
     _com_github_cyan4973_xxhash()
     _com_github_eile_tclap()
     _com_github_fmtlib_fmt()
@@ -265,6 +266,16 @@ def _com_github_bombela_backward():
         actual = "@com_github_bombela_backward//:backward",
     )
 
+def _com_github_circonus_labs_libcircllhist():
+    _repository_impl(
+        name = "com_github_circonus_labs_libcircllhist",
+        build_file = "@envoy//bazel/external:libcircllhist.BUILD",
+    )
+    native.bind(
+        name = "libcircllhist",
+        actual = "@com_github_circonus_labs_libcircllhist//:libcircllhist",
+    )
+
 def _com_github_cyan4973_xxhash():
     _repository_impl(
         name = "com_github_cyan4973_xxhash",

bazel/repository_locations.bzl

@@ -12,6 +12,10 @@ REPOSITORY_LOCATIONS = dict(
         commit = "44ae9609e860e3428cd057f7052e505b4819eb84",  # 2018-02-06
         remote = "https://github.com/bombela/backward-cpp",
     ),
+    com_github_circonus_labs_libcircllhist = dict(
+        commit = "0c44450723e34c9d8768e69b11bf919be83fd2ed",  # 2018-04-30
+        remote = "https://github.com/circonus-labs/libcircllhist",
+    ),
     com_github_cyan4973_xxhash = dict(
         commit = "7cc9639699f64b750c0b82333dced9ea77e8436e",  # v0.6.5
         remote = "https://github.com/Cyan4973/xxHash",

docs/root/intro/version_history.rst

@@ -64,6 +64,7 @@ Version history
   <envoy_api_field_Listener.transparent>`.
 * sockets: added `SO_KEEPALIVE` socket option for upstream connections
   :ref:`per cluster <envoy_api_field_Cluster.upstream_connection_options>`.
+* stats: added support for histograms.
 * tracing: the sampling decision is now delegated to the tracers, allowing the tracer to decide when and if
   to use it. For example, if the :ref:`x-b3-sampled <config_http_conn_man_headers_x-b3-sampled>` header
   is supplied with the client request, its value will override any sampling decision made by the Envoy proxy.

docs/root/operations/admin.rst

@@ -181,10 +181,12 @@ The fields are:
 
 .. http:get:: /stats
 
-  Outputs all statistics on demand. This includes only counters and gauges. Histograms are not
-  output as Envoy currently has no built in histogram support and relies on statsd for
-  aggregation. This command is very useful for local debugging. See :ref:`here <operations_stats>`
-  for more information.
+  Outputs all statistics on demand. This command is very useful for local debugging.
+  Histograms will output the computed quantiles i.e P0,P25,P50,P75,P90,P99,P99.9 and P100. 
+  The output for each quantile will be in the form of (interval,cumulative) where interval value 
+  represents the summary since last flush interval and cumulative value represents the 
+  summary since the start of envoy instance.
+  See :ref:`here <operations_stats>` for more information.
 
   .. http:get:: /stats?format=json
 

include/envoy/stats/stats.h

@@ -2,6 +2,7 @@
 
 #include <chrono>
 #include <cstdint>
+#include <functional>
 #include <list>
 #include <memory>
 #include <string>
@@ -114,6 +115,11 @@ class Metric {
    * Returns the name of the Metric with the portions designated as tags removed.
    */
   virtual const std::string& tagExtractedName() const PURE;
+
+  /**
+   * Indicates whether this metric has been updated since the server was started.
+   */
+  virtual bool used() const PURE;
 };
 
 /**
@@ -128,7 +134,6 @@ class Counter : public virtual Metric {
   virtual void inc() PURE;
   virtual uint64_t latch() PURE;
   virtual void reset() PURE;
-  virtual bool used() const PURE;
   virtual uint64_t value() const PURE;
 };
 
@@ -146,12 +151,34 @@ class Gauge : public virtual Metric {
   virtual void inc() PURE;
   virtual void set(uint64_t value) PURE;
   virtual void sub(uint64_t amount) PURE;
-  virtual bool used() const PURE;
   virtual uint64_t value() const PURE;
 };
 
 typedef std::shared_ptr<Gauge> GaugeSharedPtr;
 
+/**
+ * Holds the computed statistics for a histogram.
+ */
+class HistogramStatistics {
+public:
+  virtual ~HistogramStatistics() {}
+
+  /**
+   * Returns summary representation of the histogram.
+   */
+  virtual std::string summary() const PURE;
+
+  /**
+   * Returns supported quantiles.
+   */
+  virtual const std::vector<double>& supportedQuantiles() const PURE;
+
+  /**
+   * Returns computed quantile values during the period.
+   */
+  virtual const std::vector<double>& computedQuantiles() const PURE;
+};
+
 /**
  * A histogram that records values one at a time.
  * Note: Histograms now incorporate what used to be timers because the only difference between the
@@ -171,6 +198,32 @@ class Histogram : public virtual Metric {
 
 typedef std::shared_ptr<Histogram> HistogramSharedPtr;
 
+/**
+ * A histogram that is stored in main thread and provides summary view of the histogram.
+ */
+class ParentHistogram : public virtual Histogram {
+public:
+  virtual ~ParentHistogram() {}
+
+  /**
+   * This method is called during the main stats flush process for each of the histograms and used
+   * to merge the histogram values.
+   */
+  virtual void merge() PURE;
+
+  /**
+   * Returns the interval histogram summary statistics for the flush interval.
+   */
+  virtual const HistogramStatistics& intervalStatistics() const PURE;
+
+  /**
+   * Returns the cumulative histogram summary statistics.
+   */
+  virtual const HistogramStatistics& cumulativeStatistics() const PURE;
+};
+
+typedef std::shared_ptr<ParentHistogram> ParentHistogramSharedPtr;
+
 /**
  * A sink for stats. Each sink is responsible for writing stats to a backing store.
  */
@@ -194,6 +247,11 @@ class Sink {
    */
   virtual void flushGauge(const Gauge& gauge, uint64_t value) PURE;
 
+  /**
+   * Flush a histogram.
+   */
+  virtual void flushHistogram(const ParentHistogram& histogram) PURE;
+
   /**
    * This will be called after beginFlush(), some number of flushCounter(), and some number of
    * flushGauge(). Sinks can use this to optimize writing if desired.
@@ -263,10 +321,20 @@ class Store : public Scope {
    * @return a list of all known gauges.
    */
   virtual std::list<GaugeSharedPtr> gauges() const PURE;
+
+  /**
+   * @return a list of all known histograms.
+   */
+  virtual std::list<ParentHistogramSharedPtr> histograms() const PURE;
 };
 
 typedef std::unique_ptr<Store> StorePtr;
 
+/**
+ * Callback invoked when a store's mergeHistogram() runs.
+ */
+typedef std::function<void()> PostMergeCb;
+
 /**
  * The root of the stat store.
  */
@@ -294,6 +362,15 @@ class StoreRoot : public Store {
    * down.
    */
   virtual void shutdownThreading() PURE;
+
+  /**
+   * Called during the flush process to merge all the thread local histograms. The passed in
+   * callback will be called on the main thread, but it will happen after the method returns
+   * which means that the actual flush process will happen on the main thread after this method
+   * returns. It is expected that only one merge runs at any time and concurrent calls to this
+   * method would be asserted.
+   */
+  virtual void mergeHistograms(PostMergeCb merge_complete_cb) PURE;
 };
 
 typedef std::unique_ptr<StoreRoot> StoreRootPtr;

include/envoy/thread_local/thread_local.h

@@ -46,6 +46,15 @@ class Slot {
    */
   virtual void runOnAllThreads(Event::PostCb cb) PURE;
 
+  /**
+   * Run a callback on all registered threads with a barrier. A shutdown initiated during the
+   * running of the PostCBs may prevent all_threads_complete_cb from being called.
+   * @param cb supplies the callback to run on each thread.
+   * @param all_threads_complete_cb supplies the callback to run on main thread after cb has
+   * been run on all registered threads.
+   */
+  virtual void runOnAllThreads(Event::PostCb cb, Event::PostCb all_threads_complete_cb) PURE;
+
   /**
    * Set thread local data on all threads previously registered via registerThread().
    * @param initializeCb supplies the functor that will be called *on each thread*. The functor

source/common/common/logger.h

@@ -41,7 +41,9 @@ namespace Logger {
   FUNCTION(testing)              \
   FUNCTION(tracing)              \
   FUNCTION(upstream)             \
-  FUNCTION(grpc)
+  FUNCTION(grpc)                 \
+  FUNCTION(stats)
+
 
 enum class Id {
   ALL_LOGGER_IDS(GENERATE_ENUM)

source/common/stats/BUILD

@@ -12,12 +12,17 @@ envoy_cc_library(
     name = "stats_lib",
     srcs = ["stats_impl.cc"],
     hdrs = ["stats_impl.h"],
+    external_deps = [
+        "abseil_optional",
+        "libcircllhist",
+    ],
     deps = [
         "//include/envoy/common:time_interface",
         "//include/envoy/server:options_interface",
         "//include/envoy/stats:stats_interface",
         "//source/common/common:assert_lib",
         "//source/common/common:hash_lib",
+        "//source/common/common:non_copyable",
         "//source/common/common:perf_annotation_lib",
         "//source/common/common:utility_lib",
         "//source/common/config:well_known_names",

source/common/stats/stats_impl.cc

@@ -277,5 +277,38 @@ void RawStatData::initialize(absl::string_view key) {
   name_[xfer_size] = '\0';
 }
 
+HistogramStatisticsImpl::HistogramStatisticsImpl(const histogram_t* histogram_ptr)
+    : computed_quantiles_(supportedQuantiles().size(), 0.0) {
+  hist_approx_quantile(histogram_ptr, supportedQuantiles().data(), supportedQuantiles().size(),
+                       computed_quantiles_.data());
+}
+
+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};
+  return supported_quantiles;
+}
+
+std::string HistogramStatisticsImpl::summary() const {
+  std::vector<std::string> summary;
+  const std::vector<double>& supported_quantiles_ref = supportedQuantiles();
+  summary.reserve(supported_quantiles_ref.size());
+  for (size_t i = 0; i < supported_quantiles_ref.size(); ++i) {
+    summary.push_back(
+        fmt::format("P{}: {}", 100 * supported_quantiles_ref[i], computed_quantiles_[i]));
+  }
+  return absl::StrJoin(summary, ", ");
+}
+
+/**
+ * Clears the old computed values and refreshes it with values computed from passed histogram.
+ */
+void HistogramStatisticsImpl::refresh(const histogram_t* new_histogram_ptr) {
+  std::fill(computed_quantiles_.begin(), computed_quantiles_.end(), 0.0);
+  ASSERT(supportedQuantiles().size() == computed_quantiles_.size());
+  hist_approx_quantile(new_histogram_ptr, supportedQuantiles().data(), supportedQuantiles().size(),
+                       computed_quantiles_.data());
+}
+
 } // namespace Stats
 } // namespace Envoy