stats: Use SymbolTable API for creating and representing stat names. (e…

…nvoyproxy#6161)

* Use SymbolTable API for creating and representing stat names.

Signed-off-by: Joshua Marantz <[email protected]>
Signed-off-by: Jeff Piazza <[email protected]>

include/envoy/stats/scope.h

@@ -49,7 +49,7 @@ class Scope {
   virtual Counter& counterFromStatName(StatName name) PURE;
 
   /**
-   * TODO(jmarantz): this variant is deprecated: use counterFromStatName.
+   * TODO(#6667): this variant is deprecated: use counterFromStatName.
    * @param name The name, expressed as a string.
    * @return a counter within the scope's namespace.
    */
@@ -62,7 +62,7 @@ class Scope {
   virtual Gauge& gaugeFromStatName(StatName name) PURE;
 
   /**
-   * TODO(jmarantz): this variant is deprecated: use gaugeFromStatName.
+   * TODO(#6667): this variant is deprecated: use gaugeFromStatName.
    * @param name The name, expressed as a string.
    * @return a gauge within the scope's namespace.
    */
@@ -80,7 +80,7 @@ class Scope {
   virtual Histogram& histogramFromStatName(StatName name) PURE;
 
   /**
-   * TODO(jmarantz): this variant is deprecated: use histogramFromStatName.
+   * TODO(#6667): this variant is deprecated: use histogramFromStatName.
    * @param name The name, expressed as a string.
    * @return a histogram within the scope's namespace with a particular value type.
    */

include/envoy/stats/stat_data_allocator.h

@@ -35,8 +35,8 @@ class StatDataAllocator {
    * @return CounterSharedPtr a counter, or nullptr if allocation failed, in which case
    *     tag_extracted_name and tags are not moved.
    */
-  virtual CounterSharedPtr makeCounter(absl::string_view name, std::string&& tag_extracted_name,
-                                       std::vector<Tag>&& tags) PURE;
+  virtual CounterSharedPtr makeCounter(StatName name, absl::string_view tag_extracted_name,
+                                       const std::vector<Tag>& tags) PURE;
 
   /**
    * @param name the full name of the stat.
@@ -45,8 +45,8 @@ class StatDataAllocator {
    * @return GaugeSharedPtr a gauge, or nullptr if allocation failed, in which case
    *     tag_extracted_name and tags are not moved.
    */
-  virtual GaugeSharedPtr makeGauge(absl::string_view name, std::string&& tag_extracted_name,
-                                   std::vector<Tag>&& tags) PURE;
+  virtual GaugeSharedPtr makeGauge(StatName name, absl::string_view tag_extracted_name,
+                                   const std::vector<Tag>& tags) PURE;
 
   /**
    * Determines whether this stats allocator requires bounded stat-name size.

include/envoy/stats/stats.h

@@ -6,12 +6,14 @@
 #include <vector>
 
 #include "envoy/common/pure.h"
+#include "envoy/stats/symbol_table.h"
 
 #include "absl/strings/string_view.h"
 
 namespace Envoy {
 namespace Stats {
 
+class StatDataAllocator;
 struct Tag;
 
 /**
@@ -32,32 +34,39 @@ class Metric {
   virtual std::string name() const PURE;
 
   /**
-   * Returns the full name of the Metric as a nul-terminated string. The
-   * intention is use this as a hash-map key, so that the stat name storage
-   * is not duplicated in every map. You cannot use name() above for this,
-   * as it returns a std::string by value, as not all stat implementations
-   * contain the name as a std::string.
-   *
-   * Note that in the future, the plan is to replace this method with one that
-   * returns a reference to a symbolized representation of the elaborated string
-   * (see source/common/stats/symbol_table_impl.h).
+   * Returns the full name of the Metric as an encoded array of symbols.
    */
-  virtual const char* nameCStr() const PURE;
+  virtual StatName statName() const PURE;
 
   /**
    * Returns a vector of configurable tags to identify this Metric.
    */
-  virtual const std::vector<Tag>& tags() const PURE;
+  virtual std::vector<Tag> tags() const PURE;
+
+  /**
+   * Returns the name of the Metric with the portions designated as tags removed
+   * as a string. For example, The stat name "vhost.foo.vcluster.bar.c1" would
+   * have "foo" extracted as the value of tag "vhost" and "bar" extracted as the
+   * value of tag "vcluster". Thus the tagExtractedName is simply
+   * "vhost.vcluster.c1".
+   *
+   * @return The stat name with all tag values extracted.
+   */
+  virtual std::string tagExtractedName() const PURE;
 
   /**
-   * Returns the name of the Metric with the portions designated as tags removed.
+   * Returns the name of the Metric with the portions designated as tags
+   * removed as a StatName
    */
-  virtual const std::string& tagExtractedName() const PURE;
+  virtual StatName tagExtractedStatName() const PURE;
 
   /**
    * Indicates whether this metric has been updated since the server was started.
    */
   virtual bool used() const PURE;
+
+  virtual SymbolTable& symbolTable() PURE;
+  virtual const SymbolTable& symbolTable() const PURE;
 };
 
 /**

include/envoy/stats/tag_extractor.h

@@ -40,7 +40,7 @@ class TagExtractor {
    * @param remove_characters set of intervals of character-indices to be removed from name.
    * @return bool indicates whether a tag was found in the name.
    */
-  virtual bool extractTag(const std::string& stat_name, std::vector<Tag>& tags,
+  virtual bool extractTag(absl::string_view stat_name, std::vector<Tag>& tags,
                           IntervalSet<size_t>& remove_characters) const PURE;
 
   /**

include/envoy/stats/tag_producer.h

@@ -7,6 +7,8 @@
 #include "envoy/common/pure.h"
 #include "envoy/stats/tag.h"
 
+#include "absl/strings/string_view.h"
+
 namespace Envoy {
 namespace Stats {
 
@@ -16,12 +18,18 @@ class TagProducer {
 
   /**
    * Take a metric name and a vector then add proper tags into the vector and
-   * return an extracted metric name.
+   * return an extracted metric name. The tags array will be populated with
+   * name/value pairs extracted from the full metric name, using the regular
+   * expressions in source/common/config/well_known_names.cc. For example, the
+   * stat name "vhost.foo.vcluster.bar.c1" would have "foo" extracted as the
+   * value of tag "vhost" and "bar" extracted as the value of tag
+   * "vcluster", so this will populate tags with {"vhost", "foo"} and
+   * {"vcluster", "bar"}, and return "vhost.vcluster.c1".
+   *
    * @param metric_name std::string a name of Stats::Metric (Counter, Gauge, Histogram).
    * @param tags std::vector a set of Stats::Tag.
    */
-  virtual std::string produceTags(const std::string& metric_name,
-                                  std::vector<Tag>& tags) const PURE;
+  virtual std::string produceTags(absl::string_view metric_name, std::vector<Tag>& tags) const PURE;
 };
 
 typedef std::unique_ptr<const TagProducer> TagProducerPtr;

source/common/http/BUILD

@@ -89,6 +89,7 @@ envoy_cc_library(
         "//include/envoy/stats:stats_interface",
         "//source/common/common:enum_to_int",
         "//source/common/common:utility_lib",
+        "//source/common/stats:symbol_table_lib",
         "@envoy_api//envoy/type:http_status_cc",
     ],
 )

source/common/stats/BUILD

@@ -56,6 +56,7 @@ envoy_cc_library(
 
 envoy_cc_library(
     name = "metric_impl_lib",
+    srcs = ["metric_impl.cc"],
     hdrs = ["metric_impl.h"],
     deps = [
         ":symbol_table_lib",

source/common/stats/heap_stat_data.cc

@@ -1,35 +1,43 @@
 #include "common/stats/heap_stat_data.h"
 
 #include "common/common/lock_guard.h"
+#include "common/common/logger.h"
 #include "common/common/thread.h"
 #include "common/common/utility.h"
 
 namespace Envoy {
 namespace Stats {
 
-HeapStatData::HeapStatData(absl::string_view key) {
-  StringUtil::strlcpy(name_, key.data(), key.size() + 1);
+HeapStatDataAllocator::~HeapStatDataAllocator() { ASSERT(stats_.empty()); }
+
+HeapStatData* HeapStatData::alloc(StatName stat_name, SymbolTable& symbol_table) {
+  void* memory = ::malloc(sizeof(HeapStatData) + stat_name.size());
+  ASSERT(memory);
+  symbol_table.incRefCount(stat_name);
+  return new (memory) HeapStatData(stat_name);
 }
 
-HeapStatDataAllocator::~HeapStatDataAllocator() { ASSERT(stats_.empty()); }
+void HeapStatData::free(SymbolTable& symbol_table) {
+  symbol_table.free(statName());
+  this->~HeapStatData();
+  ::free(this); // matches malloc() call above.
+}
 
-HeapStatData* HeapStatDataAllocator::alloc(absl::string_view name) {
-  // Any expected truncation of name is done at the callsite. No truncation is
-  // required to use this allocator. Note that data must be freed by calling
-  // its free() method, and not by destruction, thus the more complex use of
-  // unique_ptr.
-  std::unique_ptr<HeapStatData, std::function<void(HeapStatData * d)>> data(
-      HeapStatData::alloc(name), [](HeapStatData* d) { d->free(); });
+HeapStatData& HeapStatDataAllocator::alloc(StatName name) {
+  using HeapStatDataFreeFn = std::function<void(HeapStatData * d)>;
+  std::unique_ptr<HeapStatData, HeapStatDataFreeFn> data_ptr(
+      HeapStatData::alloc(name, symbolTable()),
+      [this](HeapStatData* d) { d->free(symbolTable()); });
   Thread::ReleasableLockGuard lock(mutex_);
-  auto ret = stats_.insert(data.get());
+  auto ret = stats_.insert(data_ptr.get());
   HeapStatData* existing_data = *ret.first;
   lock.release();
 
   if (ret.second) {
-    return data.release();
+    return *data_ptr.release();
   }
   ++existing_data->ref_count_;
-  return existing_data;
+  return *existing_data;
 }
 
 void HeapStatDataAllocator::free(HeapStatData& data) {
@@ -44,19 +52,17 @@ void HeapStatDataAllocator::free(HeapStatData& data) {
     ASSERT(key_removed == 1);
   }
 
-  data.free();
+  data.free(symbolTable());
 }
 
-HeapStatData* HeapStatData::alloc(absl::string_view name) {
-  void* memory = ::malloc(sizeof(HeapStatData) + name.size() + 1);
-  ASSERT(memory);
-  return new (memory) HeapStatData(name);
-}
-
-void HeapStatData::free() {
-  this->~HeapStatData();
-  ::free(this); // matches malloc() call above.
+#ifndef ENVOY_CONFIG_COVERAGE
+void HeapStatDataAllocator::debugPrint() {
+  Thread::LockGuard lock(mutex_);
+  for (HeapStatData* heap_stat_data : stats_) {
+    ENVOY_LOG_MISC(info, "{}", symbolTable().toString(heap_stat_data->statName()));
+  }
 }
+#endif
 
 template class StatDataAllocatorImpl<HeapStatData>;