use SymbolTableCreator rather than fakes in a few stray places. (#8006)

stats: use SymbolTableCreator rather than fakes in a few stray places. (#8006)

Signed-off-by: Joshua Marantz <jmarantz@google.com>

test/common/stats/BUILD

@@ -15,7 +15,7 @@ envoy_cc_test(
     srcs = ["allocator_impl_test.cc"],
     deps = [
         "//source/common/stats:allocator_lib",
-        "//source/common/stats:fake_symbol_table_lib",
+        "//source/common/stats:symbol_table_creator_lib",
         "//test/test_common:logging_lib",
     ],
 )
@@ -33,7 +33,7 @@ envoy_cc_test(
     srcs = ["metric_impl_test.cc"],
     deps = [
         "//source/common/stats:allocator_lib",
-        "//source/common/stats:fake_symbol_table_lib",
+        "//source/common/stats:symbol_table_creator_lib",
         "//source/common/stats:utility_lib",
         "//test/test_common:logging_lib",
     ],
@@ -43,9 +43,9 @@ envoy_cc_test(
     name = "stat_merger_test",
     srcs = ["stat_merger_test.cc"],
     deps = [
-        "//source/common/stats:fake_symbol_table_lib",
         "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stat_merger_lib",
+        "//source/common/stats:symbol_table_creator_lib",
         "//source/common/stats:thread_local_store_lib",
         "//test/test_common:utility_lib",
     ],

test/common/stats/allocator_impl_test.cc

@@ -1,7 +1,7 @@
 #include <string>
 
 #include "common/stats/allocator_impl.h"
-#include "common/stats/fake_symbol_table_impl.h"
+#include "common/stats/symbol_table_creator.h"
 
 #include "test/test_common/logging.h"
 
@@ -13,21 +13,23 @@ namespace {
 
 class AllocatorImplTest : public testing::Test {
 protected:
-  AllocatorImplTest() : alloc_(symbol_table_), pool_(symbol_table_) {}
+  AllocatorImplTest()
+      : symbol_table_(SymbolTableCreator::makeSymbolTable()), alloc_(*symbol_table_),
+        pool_(*symbol_table_) {}
   ~AllocatorImplTest() override { clearStorage(); }
 
   StatNameStorage makeStatStorage(absl::string_view name) {
-    return StatNameStorage(name, symbol_table_);
+    return StatNameStorage(name, *symbol_table_);
   }
 
   StatName makeStat(absl::string_view name) { return pool_.add(name); }
 
   void clearStorage() {
     pool_.clear();
-    EXPECT_EQ(0, symbol_table_.numSymbols());
+    EXPECT_EQ(0, symbol_table_->numSymbols());
   }
 
-  FakeSymbolTableImpl symbol_table_;
+  SymbolTablePtr symbol_table_;
   AllocatorImpl alloc_;
   StatNamePool pool_;
 };

test/common/stats/metric_impl_test.cc

@@ -1,7 +1,7 @@
 #include <string>
 
 #include "common/stats/allocator_impl.h"
-#include "common/stats/fake_symbol_table_impl.h"
+#include "common/stats/symbol_table_creator.h"
 #include "common/stats/utility.h"
 
 #include "test/test_common/logging.h"
@@ -14,17 +14,19 @@ namespace {
 
 class MetricImplTest : public testing::Test {
 protected:
-  MetricImplTest() : alloc_(symbol_table_), pool_(symbol_table_) {}
+  MetricImplTest()
+      : symbol_table_(SymbolTableCreator::makeSymbolTable()), alloc_(*symbol_table_),
+        pool_(*symbol_table_) {}
   ~MetricImplTest() override { clearStorage(); }
 
   StatName makeStat(absl::string_view name) { return pool_.add(name); }
 
   void clearStorage() {
     pool_.clear();
-    EXPECT_EQ(0, symbol_table_.numSymbols());
+    EXPECT_EQ(0, symbol_table_->numSymbols());
   }
 
-  FakeSymbolTableImpl symbol_table_;
+  SymbolTablePtr symbol_table_;
   AllocatorImpl alloc_;
   StatNamePool pool_;
 };

test/common/stats/stat_merger_test.cc

@@ -1,8 +1,8 @@
 #include <memory>
 
-#include "common/stats/fake_symbol_table_impl.h"
 #include "common/stats/isolated_store_impl.h"
 #include "common/stats/stat_merger.h"
+#include "common/stats/symbol_table_creator.h"
 #include "common/stats/thread_local_store.h"
 
 #include "test/test_common/utility.h"
@@ -182,8 +182,8 @@ TEST_F(StatMergerTest, gaugeMergeImportMode) {
 
 class StatMergerThreadLocalTest : public testing::Test {
 protected:
-  FakeSymbolTableImpl symbol_table_;
-  AllocatorImpl alloc_{symbol_table_};
+  SymbolTablePtr symbol_table_{SymbolTableCreator::makeSymbolTable()};
+  AllocatorImpl alloc_{*symbol_table_};
   ThreadLocalStoreImpl store_{alloc_};
 };
 

test/common/stats/thread_local_store_speed_test.cc

@@ -22,18 +22,18 @@ namespace Envoy {
 class ThreadLocalStorePerf {
 public:
   ThreadLocalStorePerf()
-      : heap_alloc_(symbol_table_), store_(heap_alloc_),
-        api_(Api::createApiForTest(store_, time_system_)) {
+      : symbol_table_(Stats::SymbolTableCreator::makeSymbolTable()), heap_alloc_(*symbol_table_),
+        store_(heap_alloc_), api_(Api::createApiForTest(store_, time_system_)) {
     store_.setTagProducer(std::make_unique<Stats::TagProducerImpl>(stats_config_));
 
     Stats::TestUtil::forEachSampleStat(1000, [this](absl::string_view name) {
-      stat_names_.push_back(std::make_unique<Stats::StatNameStorage>(name, symbol_table_));
+      stat_names_.push_back(std::make_unique<Stats::StatNameStorage>(name, *symbol_table_));
     });
   }
 
   ~ThreadLocalStorePerf() {
     for (auto& stat_name_storage : stat_names_) {
-      stat_name_storage->free(symbol_table_);
+      stat_name_storage->free(*symbol_table_);
     }
     store_.shutdownThreading();
     if (tls_) {
@@ -54,7 +54,7 @@ class ThreadLocalStorePerf {
   }
 
 private:
-  Stats::FakeSymbolTableImpl symbol_table_;
+  Stats::SymbolTablePtr symbol_table_;
   Event::SimulatedTimeSystem time_system_;
   Stats::AllocatorImpl heap_alloc_;
   Stats::ThreadLocalStoreImpl store_;

test/common/stats/thread_local_store_test.cc

@@ -39,7 +39,8 @@ const uint64_t MaxStatNameLength = 127;
 class StatsThreadLocalStoreTest : public testing::Test {
 public:
   StatsThreadLocalStoreTest()
-      : alloc_(symbol_table_), store_(std::make_unique<ThreadLocalStoreImpl>(alloc_)) {
+      : symbol_table_(SymbolTableCreator::makeSymbolTable()), alloc_(*symbol_table_),
+        store_(std::make_unique<ThreadLocalStoreImpl>(alloc_)) {
     store_->addSink(sink_);
   }
 
@@ -48,7 +49,7 @@ class StatsThreadLocalStoreTest : public testing::Test {
     store_->addSink(sink_);
   }
 
-  Stats::FakeSymbolTableImpl symbol_table_;
+  SymbolTablePtr symbol_table_;
   NiceMock<Event::MockDispatcher> main_thread_dispatcher_;
   NiceMock<ThreadLocal::MockInstance> tls_;
   AllocatorImpl alloc_;
@@ -78,7 +79,7 @@ class HistogramTest : public testing::Test {
 public:
   using NameHistogramMap = std::map<std::string, ParentHistogramSharedPtr>;
 
-  HistogramTest() : alloc_(symbol_table_) {}
+  HistogramTest() : symbol_table_(SymbolTableCreator::makeSymbolTable()), alloc_(*symbol_table_) {}
 
   void SetUp() override {
     store_ = std::make_unique<ThreadLocalStoreImpl>(alloc_);
@@ -166,7 +167,7 @@ class HistogramTest : public testing::Test {
     }
   }
 
-  FakeSymbolTableImpl symbol_table_;
+  SymbolTablePtr symbol_table_;
   NiceMock<Event::MockDispatcher> main_thread_dispatcher_;
   NiceMock<ThreadLocal::MockInstance> tls_;
   AllocatorImpl alloc_;
@@ -182,7 +183,7 @@ TEST_F(StatsThreadLocalStoreTest, NoTls) {
 
   Counter& c1 = store_->counter("c1");
   EXPECT_EQ(&c1, &store_->counter("c1"));
-  StatNameManagedStorage c1_name("c1", symbol_table_);
+  StatNameManagedStorage c1_name("c1", *symbol_table_);
   c1.add(100);
   auto found_counter = store_->findCounter(c1_name.statName());
   ASSERT_TRUE(found_counter.has_value());
@@ -193,7 +194,7 @@ TEST_F(StatsThreadLocalStoreTest, NoTls) {
 
   Gauge& g1 = store_->gauge("g1", Gauge::ImportMode::Accumulate);
   EXPECT_EQ(&g1, &store_->gauge("g1", Gauge::ImportMode::Accumulate));
-  StatNameManagedStorage g1_name("g1", symbol_table_);
+  StatNameManagedStorage g1_name("g1", *symbol_table_);
   g1.set(100);
   auto found_gauge = store_->findGauge(g1_name.statName());
   ASSERT_TRUE(found_gauge.has_value());
@@ -204,7 +205,7 @@ TEST_F(StatsThreadLocalStoreTest, NoTls) {
 
   Histogram& h1 = store_->histogram("h1");
   EXPECT_EQ(&h1, &store_->histogram("h1"));
-  StatNameManagedStorage h1_name("h1", symbol_table_);
+  StatNameManagedStorage h1_name("h1", *symbol_table_);
   auto found_histogram = store_->findHistogram(h1_name.statName());
   ASSERT_TRUE(found_histogram.has_value());
   EXPECT_EQ(&h1, &found_histogram->get());
@@ -230,7 +231,7 @@ TEST_F(StatsThreadLocalStoreTest, Tls) {
 
   Counter& c1 = store_->counter("c1");
   EXPECT_EQ(&c1, &store_->counter("c1"));
-  StatNameManagedStorage c1_name("c1", symbol_table_);
+  StatNameManagedStorage c1_name("c1", *symbol_table_);
   c1.add(100);
   auto found_counter = store_->findCounter(c1_name.statName());
   ASSERT_TRUE(found_counter.has_value());
@@ -241,7 +242,7 @@ TEST_F(StatsThreadLocalStoreTest, Tls) {
 
   Gauge& g1 = store_->gauge("g1", Gauge::ImportMode::Accumulate);
   EXPECT_EQ(&g1, &store_->gauge("g1", Gauge::ImportMode::Accumulate));
-  StatNameManagedStorage g1_name("g1", symbol_table_);
+  StatNameManagedStorage g1_name("g1", *symbol_table_);
   g1.set(100);
   auto found_gauge = store_->findGauge(g1_name.statName());
   ASSERT_TRUE(found_gauge.has_value());
@@ -252,7 +253,7 @@ TEST_F(StatsThreadLocalStoreTest, Tls) {
 
   Histogram& h1 = store_->histogram("h1");
   EXPECT_EQ(&h1, &store_->histogram("h1"));
-  StatNameManagedStorage h1_name("h1", symbol_table_);
+  StatNameManagedStorage h1_name("h1", *symbol_table_);
   auto found_histogram = store_->findHistogram(h1_name.statName());
   ASSERT_TRUE(found_histogram.has_value());
   EXPECT_EQ(&h1, &found_histogram->get());
@@ -284,11 +285,11 @@ TEST_F(StatsThreadLocalStoreTest, BasicScope) {
   Counter& c2 = scope1->counter("c2");
   EXPECT_EQ("c1", c1.name());
   EXPECT_EQ("scope1.c2", c2.name());
-  StatNameManagedStorage c1_name("c1", symbol_table_);
+  StatNameManagedStorage c1_name("c1", *symbol_table_);
   auto found_counter = store_->findCounter(c1_name.statName());
   ASSERT_TRUE(found_counter.has_value());
   EXPECT_EQ(&c1, &found_counter->get());
-  StatNameManagedStorage c2_name("scope1.c2", symbol_table_);
+  StatNameManagedStorage c2_name("scope1.c2", *symbol_table_);
   auto found_counter2 = store_->findCounter(c2_name.statName());
   ASSERT_TRUE(found_counter2.has_value());
   EXPECT_EQ(&c2, &found_counter2->get());
@@ -297,11 +298,11 @@ TEST_F(StatsThreadLocalStoreTest, BasicScope) {
   Gauge& g2 = scope1->gauge("g2", Gauge::ImportMode::Accumulate);
   EXPECT_EQ("g1", g1.name());
   EXPECT_EQ("scope1.g2", g2.name());
-  StatNameManagedStorage g1_name("g1", symbol_table_);
+  StatNameManagedStorage g1_name("g1", *symbol_table_);
   auto found_gauge = store_->findGauge(g1_name.statName());
   ASSERT_TRUE(found_gauge.has_value());
   EXPECT_EQ(&g1, &found_gauge->get());
-  StatNameManagedStorage g2_name("scope1.g2", symbol_table_);
+  StatNameManagedStorage g2_name("scope1.g2", *symbol_table_);
   auto found_gauge2 = store_->findGauge(g2_name.statName());
   ASSERT_TRUE(found_gauge2.has_value());
   EXPECT_EQ(&g2, &found_gauge2->get());
@@ -314,11 +315,11 @@ TEST_F(StatsThreadLocalStoreTest, BasicScope) {
   h1.recordValue(100);
   EXPECT_CALL(sink_, onHistogramComplete(Ref(h2), 200));
   h2.recordValue(200);
-  StatNameManagedStorage h1_name("h1", symbol_table_);
+  StatNameManagedStorage h1_name("h1", *symbol_table_);
   auto found_histogram = store_->findHistogram(h1_name.statName());
   ASSERT_TRUE(found_histogram.has_value());
   EXPECT_EQ(&h1, &found_histogram->get());
-  StatNameManagedStorage h2_name("scope1.h2", symbol_table_);
+  StatNameManagedStorage h2_name("scope1.h2", *symbol_table_);
   auto found_histogram2 = store_->findHistogram(h2_name.statName());
   ASSERT_TRUE(found_histogram2.has_value());
   EXPECT_EQ(&h2, &found_histogram2->get());
@@ -380,7 +381,7 @@ TEST_F(StatsThreadLocalStoreTest, NestedScopes) {
   ScopePtr scope1 = store_->createScope("scope1.");
   Counter& c1 = scope1->counter("foo.bar");
   EXPECT_EQ("scope1.foo.bar", c1.name());
-  StatNameManagedStorage c1_name("scope1.foo.bar", symbol_table_);
+  StatNameManagedStorage c1_name("scope1.foo.bar", *symbol_table_);
   auto found_counter = store_->findCounter(c1_name.statName());
   ASSERT_TRUE(found_counter.has_value());
   EXPECT_EQ(&c1, &found_counter->get());
@@ -389,7 +390,7 @@ TEST_F(StatsThreadLocalStoreTest, NestedScopes) {
   Counter& c2 = scope2->counter("bar");
   EXPECT_EQ(&c1, &c2);
   EXPECT_EQ("scope1.foo.bar", c2.name());
-  StatNameManagedStorage c2_name("scope1.foo.bar", symbol_table_);
+  StatNameManagedStorage c2_name("scope1.foo.bar", *symbol_table_);
   auto found_counter2 = store_->findCounter(c2_name.statName());
   ASSERT_TRUE(found_counter2.has_value());
 
@@ -455,12 +456,14 @@ TEST_F(StatsThreadLocalStoreTest, OverlappingScopes) {
 
 class LookupWithStatNameTest : public testing::Test {
 public:
-  LookupWithStatNameTest() : alloc_(symbol_table_), store_(alloc_), pool_(symbol_table_) {}
+  LookupWithStatNameTest()
+      : symbol_table_(SymbolTableCreator::makeSymbolTable()), alloc_(*symbol_table_),
+        store_(alloc_), pool_(*symbol_table_) {}
   ~LookupWithStatNameTest() override { store_.shutdownThreading(); }
 
   StatName makeStatName(absl::string_view name) { return pool_.add(name); }
 
-  Stats::FakeSymbolTableImpl symbol_table_;
+  SymbolTablePtr symbol_table_;
   AllocatorImpl alloc_;
   ThreadLocalStoreImpl store_;
   StatNamePool pool_;
@@ -664,7 +667,8 @@ TEST_F(StatsMatcherTLSTest, TestExclusionRegex) {
 class RememberStatsMatcherTest : public testing::TestWithParam<bool> {
 public:
   RememberStatsMatcherTest()
-      : heap_alloc_(symbol_table_), store_(heap_alloc_), scope_(store_.createScope("scope.")) {
+      : symbol_table_(SymbolTableCreator::makeSymbolTable()), heap_alloc_(*symbol_table_),
+        store_(heap_alloc_), scope_(store_.createScope("scope.")) {
     if (GetParam()) {
       store_.initializeThreading(main_thread_dispatcher_, tls_);
     }
@@ -755,7 +759,7 @@ class RememberStatsMatcherTest : public testing::TestWithParam<bool> {
     };
   }
 
-  Stats::FakeSymbolTableImpl symbol_table_;
+  Stats::SymbolTablePtr symbol_table_;
   NiceMock<Event::MockDispatcher> main_thread_dispatcher_;
   NiceMock<ThreadLocal::MockInstance> tls_;
   AllocatorImpl heap_alloc_;
@@ -968,11 +972,11 @@ TEST_F(StatsThreadLocalStoreTest, MergeDuringShutDown) {
 }
 
 TEST(ThreadLocalStoreThreadTest, ConstructDestruct) {
-  Stats::FakeSymbolTableImpl symbol_table;
+  SymbolTablePtr symbol_table(SymbolTableCreator::makeSymbolTable());
   Api::ApiPtr api = Api::createApiForTest();
   Event::DispatcherPtr dispatcher = api->allocateDispatcher();
   NiceMock<ThreadLocal::MockInstance> tls;
-  AllocatorImpl alloc(symbol_table);
+  AllocatorImpl alloc(*symbol_table);
   ThreadLocalStoreImpl store(alloc);
 
   store.initializeThreading(*dispatcher, tls);