stats: add mechanism to create dynamic tokens without locks or sharing (

envoyproxy#9355)

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

include/envoy/stats/symbol_table.h

@@ -182,6 +182,7 @@ class SymbolTable {
 
 private:
   friend struct HeapStatData;
+  friend class StatNameDynamicStorage;
   friend class StatNameStorage;
   friend class StatNameList;
   friend class StatNameSet;
@@ -222,12 +223,7 @@ class SymbolTable {
    */
   virtual StoragePtr encode(absl::string_view name) PURE;
 
-  /**
-   * Called by StatNameSet's destructor.
-   *
-   * @param stat_name_set the set.
-   */
-  virtual void forgetSet(StatNameSet& stat_name_set) PURE;
+  virtual StoragePtr makeDynamicStorage(absl::string_view name) PURE;
 };
 
 using SymbolTablePtr = std::unique_ptr<SymbolTable>;

source/common/http/codes.cc

@@ -158,13 +158,6 @@ void CodeStatsImpl::chargeResponseTiming(const ResponseTimingInfo& info) const {
   }
 }
 
-absl::string_view CodeStatsImpl::stripTrailingDot(absl::string_view str) {
-  if (absl::EndsWith(str, ".")) {
-    str.remove_suffix(1);
-  }
-  return str;
-}
-
 Stats::StatName CodeStatsImpl::upstreamRqGroup(Code response_code) const {
   switch (enumToInt(response_code) / 100) {
   case 1:
@@ -187,7 +180,7 @@ Stats::StatName CodeStatsImpl::upstreamRqStatName(Code response_code) const {
   if (rc_index >= NumHttpCodes) {
     return upstream_rq_unknown_;
   }
-  std::atomic<uint8_t*>& atomic_ref = rc_stat_names_[rc_index];
+  std::atomic<const uint8_t*>& atomic_ref = rc_stat_names_[rc_index];
   if (atomic_ref.load() == nullptr) {
     absl::MutexLock lock(&mutex_);
 

source/common/http/codes.h

@@ -59,7 +59,6 @@ class CodeStatsImpl : public CodeStats {
   void recordHistogram(Stats::Scope& scope, const Stats::StatNameVec& names,
                        Stats::Histogram::Unit unit, uint64_t count) const;
 
-  static absl::string_view stripTrailingDot(absl::string_view prefix);
   Stats::StatName upstreamRqGroup(Code response_code) const;
   Stats::StatName upstreamRqStatName(Code response_code) const;
 
@@ -109,7 +108,7 @@ class CodeStatsImpl : public CodeStats {
 
   static constexpr uint32_t NumHttpCodes = 500;
   static constexpr uint32_t HttpCodeOffset = 100; // code 100 is at index 0.
-  mutable std::atomic<uint8_t*> rc_stat_names_[NumHttpCodes];
+  mutable std::atomic<const uint8_t*> rc_stat_names_[NumHttpCodes];
 };
 
 /**

source/common/router/router.cc

@@ -493,11 +493,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e
 
   const Http::HeaderEntry* request_alt_name = headers.EnvoyUpstreamAltStatName();
   if (request_alt_name) {
-    // TODO(#7003): converting this header value into a StatName requires
-    // taking a global symbol-table lock. This is not a frequently used feature,
-    // but may not be the only occurrence of this pattern, where it's difficult
-    // or impossible to pre-compute a StatName for a component of a stat name.
-    alt_stat_prefix_ = std::make_unique<Stats::StatNameManagedStorage>(
+    alt_stat_prefix_ = std::make_unique<Stats::StatNameDynamicStorage>(
         request_alt_name->value().getStringView(), config_.scope_.symbolTable());
     headers.removeEnvoyUpstreamAltStatName();
   }

source/common/router/router.h

@@ -565,7 +565,7 @@ class Filter : Logger::Loggable<Logger::Id::router>,
   RouteConstSharedPtr route_;
   const RouteEntry* route_entry_{};
   Upstream::ClusterInfoConstSharedPtr cluster_;
-  std::unique_ptr<Stats::StatNameManagedStorage> alt_stat_prefix_;
+  std::unique_ptr<Stats::StatNameDynamicStorage> alt_stat_prefix_;
   const VirtualCluster* request_vcluster_;
   Event::TimerPtr response_timeout_;
   FilterUtility::TimeoutData timeout_;

source/common/stats/fake_symbol_table_impl.h

@@ -75,12 +75,11 @@ class FakeSymbolTableImpl : public SymbolTable {
     MemBlockBuilder<uint8_t> mem_block(total_size_bytes);
     mem_block.appendOne(num_names);
     for (uint32_t i = 0; i < num_names; ++i) {
-      auto& name = names[i];
-      size_t sz = name.size();
+      absl::string_view name = names[i];
+      const size_t sz = name.size();
       SymbolTableImpl::Encoding::appendEncoding(sz, mem_block);
       if (!name.empty()) {
-        mem_block.appendData(
-            absl::MakeConstSpan(reinterpret_cast<const uint8_t*>(name.data()), sz));
+        mem_block.appendData(absl::MakeSpan(reinterpret_cast<const uint8_t*>(name.data()), sz));
       }
     }
 
@@ -102,6 +101,7 @@ class FakeSymbolTableImpl : public SymbolTable {
   void free(const StatName&) override {}
   void incRefCount(const StatName&) override {}
   StoragePtr encode(absl::string_view name) override { return encodeHelper(name); }
+  StoragePtr makeDynamicStorage(absl::string_view name) override { return encodeHelper(name); }
   SymbolTable::StoragePtr join(const std::vector<StatName>& names) const override {
     std::vector<absl::string_view> strings;
     for (StatName name : names) {
@@ -125,7 +125,6 @@ class FakeSymbolTableImpl : public SymbolTable {
     // make_unique does not work with private ctor, even though FakeSymbolTableImpl is a friend.
     return StatNameSetPtr(new StatNameSet(*this, name));
   }
-  void forgetSet(StatNameSet&) override {}
   uint64_t getRecentLookups(const RecentLookupsFn&) const override { return 0; }
   void clearRecentLookups() override {}
   void setRecentLookupCapacity(uint64_t) override {}
@@ -142,7 +141,8 @@ class FakeSymbolTableImpl : public SymbolTable {
     MemBlockBuilder<uint8_t> mem_block(SymbolTableImpl::Encoding::totalSizeBytes(name.size()));
     SymbolTableImpl::Encoding::appendEncoding(name.size(), mem_block);
     mem_block.appendData(
-        absl::MakeConstSpan(reinterpret_cast<const uint8_t*>(name.data()), name.size()));
+        absl::MakeSpan(reinterpret_cast<const uint8_t*>(name.data()), name.size()));
+    ASSERT(mem_block.capacityRemaining() == 0);
     return mem_block.release();
   }
 };