stats: Plumb through symbol tables everywhere. Pulled from #6161. (#6299

) * Plumb through symbol tables everywhere. Pulled from #6161. Signed-off-by: Joshua Marantz <[email protected]>
envoyproxy · Mar 19, 2019 · 8593e6a · 8593e6a
1 parent e8b4d0f
commit 8593e6a
Show file tree

Hide file tree

Showing 52 changed files with 199 additions and 76 deletions.
diff --git a/include/envoy/stats/BUILD b/include/envoy/stats/BUILD
@@ -25,12 +25,18 @@ envoy_cc_library(
         "tag_extractor.h",
         "tag_producer.h",
     ],
-    deps = ["//include/envoy/common:interval_set_interface"],
+    deps = [
+        ":symbol_table_interface",
+        "//include/envoy/common:interval_set_interface",
+    ],
 )
 
 envoy_cc_library(
     name = "symbol_table_interface",
     hdrs = ["symbol_table.h"],
+    deps = [
+        "//source/common/common:hash_lib",
+    ],
 )
 
 envoy_cc_library(

diff --git a/include/envoy/stats/scope.h b/include/envoy/stats/scope.h
@@ -2,11 +2,11 @@
 
 #include <cstdint>
 #include <memory>
-#include <string>
 
 #include "envoy/common/pure.h"
 #include "envoy/stats/histogram.h"
 #include "envoy/stats/stats_options.h"
+#include "envoy/stats/symbol_table.h"
 
 namespace Envoy {
 namespace Stats {
@@ -61,6 +61,12 @@ class Scope {
    * maximum allowable object name length and stat suffix length.
    */
   virtual const Stats::StatsOptions& statsOptions() const PURE;
+
+  /**
+   * @return a reference to the symbol table.
+   */
+  virtual const SymbolTable& symbolTable() const PURE;
+  virtual SymbolTable& symbolTable() PURE;
 };
 
 } // namespace Stats

diff --git a/include/envoy/stats/stat_data_allocator.h b/include/envoy/stats/stat_data_allocator.h
@@ -10,6 +10,7 @@
 
 #include "envoy/common/pure.h"
 #include "envoy/stats/stats.h"
+#include "envoy/stats/symbol_table.h"
 #include "envoy/stats/tag.h"
 
 #include "absl/strings/string_view.h"
@@ -52,6 +53,9 @@ class StatDataAllocator {
    */
   virtual bool requiresBoundedStatNameSize() const PURE;
 
+  virtual const SymbolTable& symbolTable() const PURE;
+  virtual SymbolTable& symbolTable() PURE;
+
   // TODO(jmarantz): create a parallel mechanism to instantiate histograms. At
   // the moment, histograms don't fit the same pattern of counters and gauges
   // as they are not actually created in the context of a stats allocator.

diff --git a/source/common/stats/BUILD b/source/common/stats/BUILD
@@ -13,6 +13,7 @@ envoy_cc_library(
     srcs = ["heap_stat_data.cc"],
     hdrs = ["heap_stat_data.h"],
     deps = [
+        ":metric_impl_lib",
         ":stat_data_allocator_lib",
         "//source/common/common:assert_lib",
         "//source/common/common:hash_lib",
@@ -42,6 +43,7 @@ envoy_cc_library(
     srcs = ["isolated_store_impl.cc"],
     hdrs = ["isolated_store_impl.h"],
     deps = [
+        ":fake_symbol_table_lib",
         ":histogram_lib",
         ":stats_lib",
         ":stats_options_lib",
@@ -54,6 +56,7 @@ envoy_cc_library(
     name = "metric_impl_lib",
     hdrs = ["metric_impl.h"],
     deps = [
+        ":symbol_table_lib",
         "//include/envoy/stats:stats_interface",
         "//source/common/common:assert_lib",
     ],
@@ -130,6 +133,7 @@ envoy_cc_library(
         "//include/envoy/stats:symbol_table_interface",
         "//source/common/common:assert_lib",
         "//source/common/common:logger_lib",
+        "//source/common/common:stack_array",
         "//source/common/common:thread_lib",
         "//source/common/common:utility_lib",
     ],
@@ -202,4 +206,5 @@ envoy_cc_library(
     name = "utility_lib",
     srcs = ["utility.cc"],
     hdrs = ["utility.h"],
+    deps = [":symbol_table_lib"],
 )
diff --git a/source/common/stats/heap_stat_data.cc b/source/common/stats/heap_stat_data.cc
@@ -11,8 +11,6 @@ HeapStatData::HeapStatData(absl::string_view key) {
   StringUtil::strlcpy(name_, key.data(), key.size() + 1);
 }
 
-HeapStatDataAllocator::HeapStatDataAllocator() {}
-
 HeapStatDataAllocator::~HeapStatDataAllocator() { ASSERT(stats_.empty()); }
 
 HeapStatData* HeapStatDataAllocator::alloc(absl::string_view name) {

diff --git a/source/common/stats/heap_stat_data.h b/source/common/stats/heap_stat_data.h
@@ -4,6 +4,8 @@
 #include <string>
 #include <unordered_set>
 
+#include "envoy/stats/symbol_table.h"
+
 #include "common/common/hash.h"
 #include "common/common/thread.h"
 #include "common/common/thread_annotations.h"
@@ -53,8 +55,8 @@ struct HeapStatData {
  */
 class HeapStatDataAllocator : public StatDataAllocatorImpl<HeapStatData> {
 public:
-  HeapStatDataAllocator();
-  ~HeapStatDataAllocator();
+  HeapStatDataAllocator(SymbolTable& symbol_table) : StatDataAllocatorImpl(symbol_table) {}
+  ~HeapStatDataAllocator() override;
 
   // StatDataAllocatorImpl
   HeapStatData* alloc(absl::string_view name) override;

diff --git a/source/common/stats/isolated_store_impl.cc b/source/common/stats/isolated_store_impl.cc
@@ -6,14 +6,24 @@
 #include <string>
 
 #include "common/common/utility.h"
+#include "common/stats/fake_symbol_table_impl.h"
 #include "common/stats/histogram_impl.h"
 #include "common/stats/utility.h"
 
 namespace Envoy {
 namespace Stats {
 
 IsolatedStoreImpl::IsolatedStoreImpl()
-    : counters_([this](const std::string& name) -> CounterSharedPtr {
+    : IsolatedStoreImpl(std::make_unique<FakeSymbolTableImpl>()) {}
+
+IsolatedStoreImpl::IsolatedStoreImpl(std::unique_ptr<SymbolTable>&& symbol_table)
+    : IsolatedStoreImpl(*symbol_table) {
+  symbol_table_storage_ = std::move(symbol_table);
+}
+
+IsolatedStoreImpl::IsolatedStoreImpl(SymbolTable& symbol_table)
+    : symbol_table_(symbol_table), alloc_(symbol_table_),
+      counters_([this](const std::string& name) -> CounterSharedPtr {
         std::string tag_extracted_name = name;
         std::vector<Tag> tags;
         return alloc_.makeCounter(name, std::move(tag_extracted_name), std::move(tags));
@@ -42,6 +52,8 @@ struct IsolatedScopeImpl : public Scope {
     return parent_.histogram(prefix_ + name);
   }
   const Stats::StatsOptions& statsOptions() const override { return parent_.statsOptions(); }
+  const SymbolTable& symbolTable() const override { return parent_.symbolTable(); }
+  SymbolTable& symbolTable() override { return parent_.symbolTable(); }
 
   IsolatedStoreImpl& parent_;
   const std::string prefix_;

diff --git a/source/common/stats/isolated_store_impl.h b/source/common/stats/isolated_store_impl.h
@@ -12,8 +12,11 @@
 #include "common/common/utility.h"
 #include "common/stats/heap_stat_data.h"
 #include "common/stats/stats_options_impl.h"
+#include "common/stats/symbol_table_impl.h"
 #include "common/stats/utility.h"
 
+#include "absl/container/flat_hash_map.h"
+
 namespace Envoy {
 namespace Stats {
 
@@ -22,7 +25,7 @@ namespace Stats {
  */
 template <class Base> class IsolatedStatsCache {
 public:
-  typedef std::function<std::shared_ptr<Base>(const std::string& name)> Allocator;
+  using Allocator = std::function<std::shared_ptr<Base>(const std::string& name)>;
 
   IsolatedStatsCache(Allocator alloc) : alloc_(alloc) {}
 
@@ -48,13 +51,14 @@ template <class Base> class IsolatedStatsCache {
   }
 
 private:
-  std::unordered_map<std::string, std::shared_ptr<Base>> stats_;
+  absl::flat_hash_map<std::string, std::shared_ptr<Base>> stats_;
   Allocator alloc_;
 };
 
 class IsolatedStoreImpl : public Store {
 public:
   IsolatedStoreImpl();
+  explicit IsolatedStoreImpl(SymbolTable& symbol_table);
 
   // Stats::Scope
   Counter& counter(const std::string& name) override { return counters_.get(name); }
@@ -66,6 +70,8 @@ class IsolatedStoreImpl : public Store {
     return histogram;
   }
   const Stats::StatsOptions& statsOptions() const override { return stats_options_; }
+  const SymbolTable& symbolTable() const override { return symbol_table_; }
+  virtual SymbolTable& symbolTable() override { return symbol_table_; }
 
   // Stats::Store
   std::vector<CounterSharedPtr> counters() const override { return counters_.toVector(); }
@@ -75,6 +81,10 @@ class IsolatedStoreImpl : public Store {
   }
 
 private:
+  IsolatedStoreImpl(std::unique_ptr<SymbolTable>&& symbol_table);
+
+  std::unique_ptr<SymbolTable> symbol_table_storage_;
+  SymbolTable& symbol_table_;
   HeapStatDataAllocator alloc_;
   IsolatedStatsCache<Counter> counters_;
   IsolatedStatsCache<Gauge> gauges_;

diff --git a/source/common/stats/raw_stat_data.h b/source/common/stats/raw_stat_data.h
@@ -13,6 +13,7 @@
 
 #include "envoy/stats/stat_data_allocator.h"
 #include "envoy/stats/stats_options.h"
+#include "envoy/stats/symbol_table.h"
 
 #include "common/common/assert.h"
 #include "common/common/block_memory_hash_set.h"
@@ -97,8 +98,9 @@ using RawStatDataSet = BlockMemoryHashSet<Stats::RawStatData>;
 class RawStatDataAllocator : public StatDataAllocatorImpl<RawStatData> {
 public:
   RawStatDataAllocator(Thread::BasicLockable& mutex, RawStatDataSet& stats_set,
-                       const StatsOptions& options)
-      : mutex_(mutex), stats_set_(stats_set), options_(options) {}
+                       const StatsOptions& options, SymbolTable& symbol_table)
+      : StatDataAllocatorImpl(symbol_table), mutex_(mutex), stats_set_(stats_set),
+        options_(options) {}
 
   // StatDataAllocator
   bool requiresBoundedStatNameSize() const override { return true; }

diff --git a/source/common/stats/stat_data_allocator_impl.h b/source/common/stats/stat_data_allocator_impl.h
@@ -5,6 +5,7 @@
 
 #include "envoy/stats/stat_data_allocator.h"
 #include "envoy/stats/stats.h"
+#include "envoy/stats/symbol_table.h"
 
 #include "common/common/assert.h"
 #include "common/stats/metric_impl.h"
@@ -29,6 +30,8 @@ namespace Stats {
 // available. This could be resolved with placed new, or another nesting level.
 template <class StatData> class StatDataAllocatorImpl : public StatDataAllocator {
 public:
+  explicit StatDataAllocatorImpl(SymbolTable& symbol_table) : symbol_table_(symbol_table) {}
+
   // StatDataAllocator
   CounterSharedPtr makeCounter(absl::string_view name, std::string&& tag_extracted_name,
                                std::vector<Tag>&& tags) override;
@@ -50,6 +53,12 @@ template <class StatData> class StatDataAllocatorImpl : public StatDataAllocator
    * @param data the data returned by alloc().
    */
   virtual void free(StatData& data) PURE;
+
+  SymbolTable& symbolTable() override { return symbol_table_; }
+  const SymbolTable& symbolTable() const override { return symbol_table_; }
+
+private:
+  SymbolTable& symbol_table_;
 };
 
 /**

diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc
@@ -26,7 +26,8 @@ ThreadLocalStoreImpl::ThreadLocalStoreImpl(const StatsOptions& stats_options,
     : stats_options_(stats_options), alloc_(alloc), default_scope_(createScope("")),
       tag_producer_(std::make_unique<TagProducerImpl>()),
       stats_matcher_(std::make_unique<StatsMatcherImpl>()),
-      num_last_resort_stats_(default_scope_->counter("stats.overflow")), source_(*this) {}
+      num_last_resort_stats_(default_scope_->counter("stats.overflow")),
+      heap_allocator_(alloc.symbolTable()), source_(*this) {}
 
 ThreadLocalStoreImpl::~ThreadLocalStoreImpl() {
   ASSERT(shutting_down_);

diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h
@@ -148,6 +148,8 @@ class ThreadLocalStoreImpl : Logger::Loggable<Logger::Id::stats>, public StoreRo
   Histogram& histogram(const std::string& name) override {
     return default_scope_->histogram(name);
   };
+  const SymbolTable& symbolTable() const override { return alloc_.symbolTable(); }
+  SymbolTable& symbolTable() override { return alloc_.symbolTable(); }
 
   // Stats::Store
   std::vector<CounterSharedPtr> counters() const override;
@@ -206,6 +208,8 @@ class ThreadLocalStoreImpl : Logger::Loggable<Logger::Id::stats>, public StoreRo
     ScopePtr createScope(const std::string& name) override {
       return parent_.createScope(prefix_ + name);
     }
+    const SymbolTable& symbolTable() const override { return parent_.symbolTable(); }
+    SymbolTable& symbolTable() override { return parent_.symbolTable(); }
     void deliverHistogramToSinks(const Histogram& histogram, uint64_t value) override;
     Gauge& gauge(const std::string& name) override;
     Histogram& histogram(const std::string& name) override;

diff --git a/source/exe/BUILD b/source/exe/BUILD
@@ -70,6 +70,7 @@ envoy_cc_library(
         "//source/common/common:compiler_requirements_lib",
         "//source/common/http/http2:codec_lib",
         "//source/common/common:perf_annotation_lib",
+        "//source/common/stats:fake_symbol_table_lib",
         "//source/common/thread:thread_factory_singleton_lib",
         "//source/server:hot_restart_lib",
         "//source/server:hot_restart_nop_lib",

diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc
@@ -58,11 +58,11 @@ MainCommonBase::MainCommonBase(const OptionsImpl& options, Event::TimeSystem& ti
   case Server::Mode::Serve: {
 #ifdef ENVOY_HOT_RESTART
     if (!options.hotRestartDisabled()) {
-      restarter_ = std::make_unique<Server::HotRestartImpl>(options_);
+      restarter_ = std::make_unique<Server::HotRestartImpl>(options_, symbol_table_);
     }
 #endif
     if (restarter_ == nullptr) {
-      restarter_ = std::make_unique<Server::HotRestartNopImpl>();
+      restarter_ = std::make_unique<Server::HotRestartNopImpl>(symbol_table_);
     }
 
     tls_ = std::make_unique<ThreadLocal::InstanceImpl>();
@@ -88,7 +88,7 @@ MainCommonBase::MainCommonBase(const OptionsImpl& options, Event::TimeSystem& ti
     break;
   }
   case Server::Mode::Validate:
-    restarter_ = std::make_unique<Server::HotRestartNopImpl>();
+    restarter_ = std::make_unique<Server::HotRestartNopImpl>(symbol_table_);
     logging_context_ = std::make_unique<Logger::Context>(options_.logLevel(), options_.logFormat(),
                                                          restarter_->logLock());
     break;

diff --git a/source/exe/main_common.h b/source/exe/main_common.h
@@ -5,6 +5,7 @@
 
 #include "common/common/thread.h"
 #include "common/event/real_time_system.h"
+#include "common/stats/fake_symbol_table_impl.h"
 #include "common/stats/thread_local_store.h"
 #include "common/thread_local/thread_local_impl.h"
 
@@ -64,7 +65,7 @@ class MainCommonBase {
 
 protected:
   const Envoy::OptionsImpl& options_;
-
+  Stats::FakeSymbolTableImpl symbol_table_;
   Server::ComponentFactory& component_factory_;
   Thread::ThreadFactory& thread_factory_;
 

diff --git a/source/server/hot_restart_impl.cc b/source/server/hot_restart_impl.cc
@@ -122,7 +122,7 @@ std::string SharedMemory::version(uint64_t max_num_stats,
                      stats_options.maxNameLength());
 }
 
-HotRestartImpl::HotRestartImpl(const Options& options)
+HotRestartImpl::HotRestartImpl(const Options& options, Stats::SymbolTable& symbol_table)
     : options_(options), stats_set_options_(blockMemHashOptions(options.maxStats())),
       shmem_(SharedMemory::initialize(
           Stats::RawStatDataSet::numBytes(stats_set_options_, options_.statsOptions()), options_)),
@@ -136,8 +136,8 @@ HotRestartImpl::HotRestartImpl(const Options& options)
         std::make_unique<Stats::RawStatDataSet>(stats_set_options_, options.restartEpoch() == 0,
                                                 shmem_.stats_set_data_, options_.statsOptions());
   }
-  stats_allocator_ = std::make_unique<Stats::RawStatDataAllocator>(stat_lock_, *stats_set_,
-                                                                   options_.statsOptions());
+  stats_allocator_ = std::make_unique<Stats::RawStatDataAllocator>(
+      stat_lock_, *stats_set_, options_.statsOptions(), symbol_table);
   my_domain_socket_ = bindDomainSocket(options.restartEpoch());
   child_address_ = createDomainSocketAddress((options.restartEpoch() + 1));
   initDomainSocketAddress(&parent_address_);

diff --git a/source/server/hot_restart_impl.h b/source/server/hot_restart_impl.h
@@ -114,7 +114,7 @@ class ProcessSharedMutex : public Thread::BasicLockable {
  */
 class HotRestartImpl : public HotRestart, Logger::Loggable<Logger::Id::main> {
 public:
-  HotRestartImpl(const Options& options);
+  HotRestartImpl(const Options& options, Stats::SymbolTable& symbol_table);
 
   // Server::HotRestart
   void drainParentListeners() override;

diff --git a/source/server/hot_restart_nop_impl.h b/source/server/hot_restart_nop_impl.h
@@ -15,7 +15,7 @@ namespace Server {
  */
 class HotRestartNopImpl : public Server::HotRestart {
 public:
-  HotRestartNopImpl() {}
+  explicit HotRestartNopImpl(Stats::SymbolTable& symbol_table) : stats_allocator_(symbol_table) {}
 
   // Server::HotRestart
   void drainParentListeners() override {}

diff --git a/test/common/grpc/BUILD b/test/common/grpc/BUILD
@@ -108,6 +108,7 @@ envoy_cc_test_library(
         "//test/mocks/local_info:local_info_mocks",
         "//test/mocks/server:server_mocks",
         "//test/proto:helloworld_proto_cc",
+        "//test/test_common:global_lib",
         "//test/test_common:test_time_lib",
         "//test/test_common:utility_lib",
     ],