stats: add unit support to histogram (envoyproxy#8484)

Signed-off-by: Taras Roshko <[email protected]>

Author: troshko111

.gitignore

@@ -11,6 +11,7 @@ BROWSE
 compile_commands.json
 cscope.*
 .deps
+.devcontainer.json
 /docs/landing_source/.bundle
 /generated
 .idea/

docs/root/intro/version_history.rst

@@ -82,6 +82,7 @@ Version history
 * server: added :ref:`per-handler listener stats <config_listener_stats_per_handler>` and
   :ref:`per-worker watchdog stats <operations_performance_watchdog>` to help diagnosing event
   loop imbalance and general performance issues.
+* stats: added unit support to histogram.
 * thrift_proxy: fix crashing bug on invalid transport/protocol framing
 * tls: added verification of IP address SAN fields in certificates against configured SANs in the
 * tracing: added support to the Zipkin reporter for sending list of spans as Zipkin JSON v2 and protobuf message over HTTP.

include/envoy/event/dispatcher.h

@@ -30,8 +30,8 @@ namespace Event {
  */
 // clang-format off
 #define ALL_DISPATCHER_STATS(HISTOGRAM)                                                            \
-  HISTOGRAM(loop_duration_us)                                                                      \
-  HISTOGRAM(poll_delay_us)
+  HISTOGRAM(loop_duration_us, Microseconds)                                                        \
+  HISTOGRAM(poll_delay_us, Microseconds)
 // clang-format on
 
 /**

include/envoy/stats/BUILD

@@ -56,12 +56,8 @@ envoy_cc_library(
 )
 
 envoy_cc_library(
-    name = "timespan",
+    name = "timespan_interface",
     hdrs = ["timespan.h"],
-    deps = [
-        ":stats_interface",
-        "//include/envoy/common:time_interface",
-    ],
 )
 
 envoy_cc_library(

include/envoy/stats/histogram.h

@@ -67,17 +67,37 @@ class HistogramStatistics {
 
 /**
  * A histogram that records values one at a time.
- * Note: Histograms now incorporate what used to be timers because the only difference between the
- * two stat types was the units being represented. It is assumed that no downstream user of this
- * class (Sinks, in particular) will need to explicitly differentiate between histograms
- * representing durations and histograms representing other types of data.
+ * Note: Histograms now incorporate what used to be timers because the only
+ * difference between the two stat types was the units being represented.
  */
 class Histogram : public Metric {
 public:
+  /**
+   * Histogram values represent scalar quantity like time, length, mass,
+   * distance, or in general anything which has only magnitude and no other
+   * characteristics. These are often accompanied by a unit of measurement.
+   * This enum defines units for commonly measured quantities. Base units
+   * are preferred unless they are not granular enough to be useful as an
+   * integer.
+   */
+  enum class Unit {
+    Null,        // The histogram has been rejected, i.e. it's a null histogram and is not recording
+                 // anything.
+    Unspecified, // Measured quantity does not require a unit, e.g. "items".
+    Bytes,
+    Microseconds,
+    Milliseconds,
+  };
+
   ~Histogram() override = default;
 
   /**
-   * Records an unsigned value. If a timer, values are in units of milliseconds.
+   * @return the unit of measurement for values recorded by the histogram.
+   */
+  virtual Unit unit() const PURE;
+
+  /**
+   * Records an unsigned value in the unit specified during the construction.
    */
   virtual void recordValue(uint64_t value) PURE;
 };

include/envoy/stats/scope.h

@@ -81,16 +81,18 @@ class Scope {
 
   /**
    * @param name The name of the stat, obtained from the SymbolTable.
+   * @param unit The unit of measurement.
    * @return a histogram within the scope's namespace with a particular value type.
    */
-  virtual Histogram& histogramFromStatName(StatName name) PURE;
+  virtual Histogram& histogramFromStatName(StatName name, Histogram::Unit unit) PURE;
 
   /**
    * TODO(#6667): this variant is deprecated: use histogramFromStatName.
    * @param name The name, expressed as a string.
+   * @param unit The unit of measurement.
    * @return a histogram within the scope's namespace with a particular value type.
    */
-  virtual Histogram& histogram(const std::string& name) PURE;
+  virtual Histogram& histogram(const std::string& name, Histogram::Unit unit) PURE;
 
   /**
    * @param The name of the stat, obtained from the SymbolTable.

include/envoy/stats/stats_macros.h

@@ -17,7 +17,7 @@ namespace Envoy {
  *   #define MY_COOL_STATS(COUNTER, GAUGE, HISTOGRAM)     \
  *     COUNTER(counter1)                                  \
  *     GAUGE(gauge1, mode)                                \
- *     HISTOGRAM(histogram1)
+ *     HISTOGRAM(histogram1, unit)
  *     ...
  *
  * By convention, starting with #7083, we sort the lines of this macro block, so
@@ -38,10 +38,11 @@ namespace Envoy {
 // Fully-qualified for use in external callsites.
 #define GENERATE_COUNTER_STRUCT(NAME) Envoy::Stats::Counter& NAME##_;
 #define GENERATE_GAUGE_STRUCT(NAME, MODE) Envoy::Stats::Gauge& NAME##_;
-#define GENERATE_HISTOGRAM_STRUCT(NAME) Envoy::Stats::Histogram& NAME##_;
+#define GENERATE_HISTOGRAM_STRUCT(NAME, UNIT) Envoy::Stats::Histogram& NAME##_;
 
 #define FINISH_STAT_DECL_(X) #X)),
 #define FINISH_STAT_DECL_MODE_(X, MODE) #X), Envoy::Stats::Gauge::ImportMode::MODE),
+#define FINISH_STAT_DECL_UNIT_(X, UNIT) #X), Envoy::Stats::Histogram::Unit::UNIT),
 
 static inline std::string statPrefixJoin(absl::string_view prefix, absl::string_view token) {
   if (prefix.empty()) {
@@ -55,7 +56,7 @@ static inline std::string statPrefixJoin(absl::string_view prefix, absl::string_
 
 #define POOL_COUNTER_PREFIX(POOL, PREFIX) (POOL).counter(Envoy::statPrefixJoin(PREFIX, FINISH_STAT_DECL_
 #define POOL_GAUGE_PREFIX(POOL, PREFIX) (POOL).gauge(Envoy::statPrefixJoin(PREFIX, FINISH_STAT_DECL_MODE_
-#define POOL_HISTOGRAM_PREFIX(POOL, PREFIX) (POOL).histogram(Envoy::statPrefixJoin(PREFIX, FINISH_STAT_DECL_
+#define POOL_HISTOGRAM_PREFIX(POOL, PREFIX) (POOL).histogram(Envoy::statPrefixJoin(PREFIX, FINISH_STAT_DECL_UNIT_
 
 #define POOL_COUNTER(POOL) POOL_COUNTER_PREFIX(POOL, "")
 #define POOL_GAUGE(POOL) POOL_GAUGE_PREFIX(POOL, "")

include/envoy/stats/timespan.h

@@ -3,9 +3,7 @@
 #include <chrono>
 #include <memory>
 
-#include "envoy/common/time.h"
-#include "envoy/stats/histogram.h"
-#include "envoy/stats/stats.h"
+#include "envoy/common/pure.h"
 
 namespace Envoy {
 namespace Stats {
@@ -18,43 +16,18 @@ class CompletableTimespan {
   virtual ~CompletableTimespan() = default;
 
   /**
-   * Complete the timespan.
-   */
-  virtual void complete() PURE;
-};
-
-/**
- * An individual timespan that flushes its measured value in time unit (e.g
- * std::chrono::milliseconds). The initial time is captured on construction. A timespan must be
- * completed via complete() for it to be stored. If the timespan is deleted this will be treated as
- * a cancellation.
- */
-template <class TimeUnit> class TimespanWithUnit : public CompletableTimespan {
-public:
-  TimespanWithUnit(Histogram& histogram, TimeSource& time_source)
-      : time_source_(time_source), histogram_(histogram), start_(time_source.monotonicTime()) {}
-
-  /**
-   * Complete the timespan and send the time to the histogram.
+   * Time elapsed since the creation of the timespan.
    */
-  void complete() override { histogram_.recordValue(getRawDuration().count()); }
+  virtual std::chrono::milliseconds elapsed() const PURE;
 
   /**
-   * Get duration in the time unit since the creation of the span.
+   * Complete the timespan.
    */
-  TimeUnit getRawDuration() {
-    return std::chrono::duration_cast<TimeUnit>(time_source_.monotonicTime() - start_);
-  }
-
-private:
-  TimeSource& time_source_;
-  Histogram& histogram_;
-  const MonotonicTime start_;
+  virtual void complete() PURE;
 };
 
-using Timespan = TimespanWithUnit<std::chrono::milliseconds>;
+using Timespan = CompletableTimespan;
 using TimespanPtr = std::unique_ptr<Timespan>;
-using CompletableTimespanPtr = std::unique_ptr<CompletableTimespan>;
 
 } // namespace Stats
 } // namespace Envoy

include/envoy/upstream/upstream.h

@@ -586,8 +586,8 @@ class PrioritySet {
   GAUGE(upstream_rq_active, Accumulate)                                                            \
   GAUGE(upstream_rq_pending_active, Accumulate)                                                    \
   GAUGE(version, NeverImport)                                                                      \
-  HISTOGRAM(upstream_cx_connect_ms)                                                                \
-  HISTOGRAM(upstream_cx_length_ms)
+  HISTOGRAM(upstream_cx_connect_ms, Milliseconds)                                                  \
+  HISTOGRAM(upstream_cx_length_ms, Milliseconds)
 
 /**
  * All cluster load report stats. These are only use for EDS load reporting and not sent to the

source/common/http/BUILD

@@ -169,7 +169,7 @@ envoy_cc_library(
         "//include/envoy/ssl:connection_interface",
         "//include/envoy/stats:stats_interface",
         "//include/envoy/stats:stats_macros",
-        "//include/envoy/stats:timespan",
+        "//include/envoy/stats:timespan_interface",
         "//include/envoy/upstream:upstream_interface",
         "//source/common/access_log:access_log_formatter_lib",
         "//source/common/buffer:buffer_lib",
@@ -186,6 +186,7 @@ envoy_cc_library(
         "//source/common/network:utility_lib",
         "//source/common/router:config_lib",
         "//source/common/runtime:uuid_util_lib",
+        "//source/common/stats:timespan_lib",
         "//source/common/stream_info:stream_info_lib",
         "//source/common/tracing:http_tracer_lib",
     ],
@@ -289,7 +290,7 @@ envoy_cc_library(
         "//include/envoy/network:connection_interface",
         "//include/envoy/stats:stats_interface",
         "//include/envoy/stats:stats_macros",
-        "//include/envoy/stats:timespan",
+        "//include/envoy/stats:timespan_interface",
     ],
 )