stats: Add option to switch between fake and real symbol-tables on the command-line.

docs/root/intro/version_history.rst

@@ -21,6 +21,7 @@ Version history
 * http: added the ability to :ref:`merge adjacent slashes<envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.merge_slashes>` in the path.
 * listeners: added :ref:`continue_on_listener_filters_timeout <envoy_api_field_Listener.continue_on_listener_filters_timeout>` to configure whether a listener will still create a connection when listener filters time out.
 * listeners: added :ref:`HTTP inspector listener filter <config_listener_filters_http_inspector>`.
+* performance: stats symbol table implementation (disabled by default; to test it, add "--use-fake-symbol-table 0" to the command-line arguments when starting Envoy).
 * redis: added :ref:`read_policy <envoy_api_field_config.filter.network.redis_proxy.v2.RedisProxy.ConnPoolSettings.read_policy>` to allow reading from redis replicas for Redis Cluster deployments.
 * rbac: added support for DNS SAN as :ref:`principal_name <envoy_api_field_config.rbac.v2.Principal.Authenticated.principal_name>`.
 * lua: extended `httpCall()` and `respond()` APIs to accept headers with entry values that can be a string or table of strings.

include/envoy/server/options.h

@@ -179,6 +179,11 @@ class Options {
  */
  virtual bool libeventBufferEnabled() const PURE;
 
+ /**
+ * @return whether to use the fake symbol table implementation.
+ */
+ virtual bool fakeSymbolTableEnabled() const PURE;
+
  /**
  * @return bool indicating whether cpuset size should determine the number of worker threads.
  */

include/envoy/stats/symbol_table.h

@@ -185,7 +185,7 @@ class SymbolTable {
  virtual StoragePtr encode(absl::string_view name) PURE;
 };
 
-using SharedSymbolTable = std::shared_ptr<SymbolTable>;
+using SymbolTablePtr = std::unique_ptr<SymbolTable>;
 
 } // namespace Stats
 } // namespace Envoy

source/common/stats/BUILD

@@ -50,6 +50,7 @@ envoy_cc_library(
  ":scope_prefixer_lib",
  ":stats_lib",
  ":store_impl_lib",
+ ":symbol_table_creator_lib",
  "//include/envoy/stats:stats_macros",
  "//source/common/stats:allocator_lib",
  ],
@@ -155,6 +156,17 @@ envoy_cc_library(
  ],
 )
 
+envoy_cc_library(
+ name = "symbol_table_creator_lib",
+ srcs = ["symbol_table_creator.cc"],
+ hdrs = ["symbol_table_creator.h"],
+ external_deps = ["abseil_base"],
+ deps = [
+ ":fake_symbol_table_lib",
+ ":symbol_table_lib",
+ ],
+)
+
 envoy_cc_library(
  name = "fake_symbol_table_lib",
  hdrs = ["fake_symbol_table_impl.h"],

source/common/stats/isolated_store_impl.cc

@@ -8,13 +8,13 @@
 #include "common/stats/fake_symbol_table_impl.h"
 #include "common/stats/histogram_impl.h"
 #include "common/stats/scope_prefixer.h"
+#include "common/stats/symbol_table_creator.h"
 #include "common/stats/utility.h"
 
 namespace Envoy {
 namespace Stats {
 
-IsolatedStoreImpl::IsolatedStoreImpl()
- : IsolatedStoreImpl(std::make_unique<FakeSymbolTableImpl>()) {}
+IsolatedStoreImpl::IsolatedStoreImpl() : IsolatedStoreImpl(SymbolTableCreator::makeSymbolTable()) {}
 
 IsolatedStoreImpl::IsolatedStoreImpl(std::unique_ptr<SymbolTable>&& symbol_table)
  : IsolatedStoreImpl(*symbol_table) {

source/common/stats/isolated_store_impl.h

@@ -133,7 +133,7 @@ class IsolatedStoreImpl : public StoreImpl {
 private:
  IsolatedStoreImpl(std::unique_ptr<SymbolTable>&& symbol_table);
 
- std::unique_ptr<SymbolTable> symbol_table_storage_;
+ SymbolTablePtr symbol_table_storage_;
  AllocatorImpl alloc_;
  IsolatedStatsCache<Counter> counters_;
  IsolatedStatsCache<Gauge> gauges_;

source/common/stats/symbol_table_creator.cc

@@ -0,0 +1,24 @@
+#include "common/stats/symbol_table_creator.h"
+
+namespace Envoy {
+namespace Stats {
+
+bool SymbolTableCreator::initialized_ = false;
+bool SymbolTableCreator::use_fake_symbol_tables_ = true;
+
+SymbolTablePtr SymbolTableCreator::initAndMakeSymbolTable(bool use_fake) {
+ ASSERT(!initialized_ || (use_fake_symbol_tables_ == use_fake));
+ initialized_ = true;
+ use_fake_symbol_tables_ = use_fake;
+ return makeSymbolTable();
+}
+
+SymbolTablePtr SymbolTableCreator::makeSymbolTable() {
+ if (use_fake_symbol_tables_) {
+ return std::make_unique<FakeSymbolTableImpl>();
+ }
+ return std::make_unique<SymbolTableImpl>();
+}
+
+} // namespace Stats
+} // namespace Envoy

source/common/stats/symbol_table_creator.h

@@ -0,0 +1,57 @@
+#pragma once
+
+#include "common/stats/fake_symbol_table_impl.h"
+#include "common/stats/symbol_table_impl.h"
+
+namespace Envoy {
+namespace Stats {
+
+namespace TestUtil {
+class SymbolTableCreatorTestPeer;
+}
+
+class SymbolTableCreator {
+public:
+ /**
+ * Initializes the symbol-table creation system. Once this is called, it is a
+ * runtime assertion to call this again in production code, changing the
+ * use_fakes setting. However, tests can change the setting via
+ * TestUtil::SymbolTableCreatorTestPeer::setUseFakeSymbolTables(use_fakes).
+ *
+ * @param use_fakes Whether to use fake symbol tables; typically from a command-line option.
+ * @return a SymbolTable.
+ */
+ static SymbolTablePtr initAndMakeSymbolTable(bool use_fakes);
+
+ /**
+ * Factory method to create SymbolTables. This is needed to help make it
+ * possible to flag-flip use of real symbol tables, and ultimately should be
+ * removed.
+ *
+ * @return a SymbolTable.
+ */
+ static SymbolTablePtr makeSymbolTable();
+
+ /**
+ * @return whether the system is initialized to use fake symbol tables.
+ */
+ static bool useFakeSymbolTables() { return use_fake_symbol_tables_; }
+
+private:
+ friend class TestUtil::SymbolTableCreatorTestPeer;
+
+ /**
+ * Sets whether fake or real symbol tables should be used. Tests that alter
+ * this should restore previous value at the end of the test. This must be
+ * called via TestUtil::SymbolTableCreatorTestPeer.
+ *
+ * *param use_fakes whether to use fake symbol tables.
+ */
+ static void setUseFakeSymbolTables(bool use_fakes) { use_fake_symbol_tables_ = use_fakes; }
+
+ static bool initialized_;
+ static bool use_fake_symbol_tables_;
+};
+
+} // namespace Stats
+} // namespace Envoy

source/exe/BUILD

@@ -70,7 +70,7 @@ envoy_cc_library(
  "//source/common/api:os_sys_calls_lib",
  "//source/common/common:compiler_requirements_lib",
  "//source/common/common:perf_annotation_lib",
- "//source/common/stats:fake_symbol_table_lib",
+ "//source/common/stats:symbol_table_creator_lib",
  "//source/server:hot_restart_lib",
  "//source/server:hot_restart_nop_lib",
  "//source/server/config_validation:server_lib",

source/exe/main_common.cc

@@ -7,6 +7,7 @@
 #include "common/common/compiler_requirements.h"
 #include "common/common/perf_annotation.h"
 #include "common/network/utility.h"
+#include "common/stats/symbol_table_creator.h"
 #include "common/stats/thread_local_store.h"
 
 #include "server/config_validation/server.h"
@@ -45,7 +46,9 @@ MainCommonBase::MainCommonBase(const OptionsImpl& options, Event::TimeSystem& ti
  Filesystem::Instance& file_system,
  std::unique_ptr<ProcessContext> process_context)
  : options_(options), component_factory_(component_factory), thread_factory_(thread_factory),
- file_system_(file_system), stats_allocator_(symbol_table_) {
+ file_system_(file_system), symbol_table_(Stats::SymbolTableCreator::initAndMakeSymbolTable(
+ options_.fakeSymbolTableEnabled())),
+ stats_allocator_(*symbol_table_) {
  switch (options_.mode()) {
  case Server::Mode::InitOnly:
  case Server::Mode::Serve: {

source/exe/main_common.h

@@ -67,10 +67,10 @@ class MainCommonBase {
 protected:
  ProcessWide process_wide_; // Process-wide state setup/teardown.
  const Envoy::OptionsImpl& options_;
- Stats::FakeSymbolTableImpl symbol_table_;
  Server::ComponentFactory& component_factory_;
  Thread::ThreadFactory& thread_factory_;
  Filesystem::Instance& file_system_;
+ Stats::SymbolTablePtr symbol_table_;
  Stats::AllocatorImpl stats_allocator_;
 
  std::unique_ptr<ThreadLocal::InstanceImpl> tls_;

source/server/BUILD

@@ -378,6 +378,7 @@ envoy_cc_library(
  "//source/common/runtime:runtime_lib",
  "//source/common/secret:secret_manager_impl_lib",
  "//source/common/singleton:manager_impl_lib",
+ "//source/common/stats:symbol_table_creator_lib",
  "//source/common/stats:thread_local_store_lib",
  "//source/common/upstream:cluster_manager_lib",
  "//source/common/upstream:health_discovery_service_lib",

source/server/options_impl.cc

@@ -105,7 +105,9 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv,
  TCLAP::ValueArg<bool> use_libevent_buffer("", "use-libevent-buffers",
  "Use the original libevent buffer implementation",
  false, false, "bool", cmd);
-
+ TCLAP::ValueArg<bool> use_fake_symbol_table("", "use-fake-symbol-table",
+ "Use fake symbol table implementation", false, true,
+ "bool", cmd);
  cmd.setExceptionHandling(false);
  try {
  cmd.parse(argc, argv);
@@ -129,6 +131,7 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv,
  mutex_tracing_enabled_ = enable_mutex_tracing.getValue();
 
  libevent_buffer_enabled_ = use_libevent_buffer.getValue();
+ fake_symbol_table_enabled_ = use_fake_symbol_table.getValue();
  cpuset_threads_ = cpuset_threads.getValue();
 
  log_level_ = default_log_level;
@@ -286,6 +289,7 @@ OptionsImpl::OptionsImpl(const std::string& service_cluster, const std::string&
  service_cluster_(service_cluster), service_node_(service_node), service_zone_(service_zone),
  file_flush_interval_msec_(10000), drain_time_(600), parent_shutdown_time_(900),
  mode_(Server::Mode::Serve), hot_restart_disabled_(false), signal_handling_enabled_(true),
- mutex_tracing_enabled_(false), cpuset_threads_(false), libevent_buffer_enabled_(false) {}
+ mutex_tracing_enabled_(false), cpuset_threads_(false), libevent_buffer_enabled_(false),
+ fake_symbol_table_enabled_(false) {}
 
 } // namespace Envoy

source/server/options_impl.h

@@ -75,6 +75,9 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable<Logger::I
  signal_handling_enabled_ = signal_handling_enabled;
  }
  void setCpusetThreads(bool cpuset_threads_enabled) { cpuset_threads_ = cpuset_threads_enabled; }
+ void setFakeSymbolTableEnabled(bool fake_symbol_table_enabled) {
+ fake_symbol_table_enabled_ = fake_symbol_table_enabled;
+ }
 
  // Server::Options
  uint64_t baseId() const override { return base_id_; }
@@ -110,6 +113,7 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable<Logger::I
  bool signalHandlingEnabled() const override { return signal_handling_enabled_; }
  bool mutexTracingEnabled() const override { return mutex_tracing_enabled_; }
  bool libeventBufferEnabled() const override { return libevent_buffer_enabled_; }
+ bool fakeSymbolTableEnabled() const override { return fake_symbol_table_enabled_; }
  Server::CommandLineOptionsPtr toCommandLineOptions() const override;
  void parseComponentLogLevels(const std::string& component_log_levels);
  bool cpusetThreadsEnabled() const override { return cpuset_threads_; }
@@ -144,6 +148,7 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable<Logger::I
  bool mutex_tracing_enabled_;
  bool cpuset_threads_;
  bool libevent_buffer_enabled_;
+ bool fake_symbol_table_enabled_;
  uint32_t count_;
 };
 

test/common/grpc/context_impl_test.cc

@@ -18,7 +18,7 @@ namespace Grpc {
 
 TEST(GrpcContextTest, ChargeStats) {
  NiceMock<Upstream::MockClusterInfo> cluster;
- Envoy::Test::Global<Stats::FakeSymbolTableImpl> symbol_table_;
+ Stats::TestSymbolTable symbol_table_;
  Stats::StatNamePool pool(*symbol_table_);
  const Stats::StatName service = pool.add("service");
  const Stats::StatName method = pool.add("method");
@@ -58,7 +58,7 @@ TEST(GrpcContextTest, ResolveServiceAndMethod) {
  Http::HeaderMapImpl headers;
  Http::HeaderEntry& path = headers.insertPath();
  path.value(std::string("/service_name/method_name"));
- Envoy::Test::Global<Stats::FakeSymbolTableImpl> symbol_table;
+ Stats::TestSymbolTable symbol_table;
  ContextImpl context(*symbol_table);
  absl::optional<Context::RequestNames> request_names = context.resolveServiceAndMethod(&path);
  EXPECT_TRUE(request_names);

test/common/grpc/grpc_client_integration_test_harness.h

@@ -416,7 +416,7 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest {
  FakeHttpConnectionPtr fake_connection_;
  std::vector<FakeStreamPtr> fake_streams_;
  const Protobuf::MethodDescriptor* method_descriptor_;
- Envoy::Test::Global<Stats::FakeSymbolTableImpl> symbol_table_;
+ Stats::TestSymbolTable symbol_table_;
  Stats::IsolatedStoreImpl* stats_store_ = new Stats::IsolatedStoreImpl(*symbol_table_);
  Api::ApiPtr api_;
  Event::DispatcherPtr dispatcher_;

test/common/http/BUILD

@@ -161,6 +161,7 @@ envoy_cc_fuzz_test(
  "//source/common/http:date_provider_lib",
  "//source/common/network:address_lib",
  "//source/common/network:utility_lib",
+ "//source/common/stats:symbol_table_creator_lib",
  "//test/fuzz:utility_lib",
  "//test/mocks/access_log:access_log_mocks",
  "//test/mocks/http:http_mocks",

test/common/http/codes_test.cc

@@ -8,6 +8,7 @@
 #include "common/common/empty_string.h"
 #include "common/http/codes.h"
 #include "common/http/header_map_impl.h"
+#include "common/stats/symbol_table_creator.h"
 
 #include "test/mocks/stats/mocks.h"
 #include "test/test_common/printers.h"
@@ -45,7 +46,7 @@ class CodeUtilityTest : public testing::Test {
  code_stats_.chargeResponseStat(info);
  }
 
- Envoy::Test::Global<Stats::FakeSymbolTableImpl> symbol_table_;
+ Stats::TestSymbolTable symbol_table_;
  Stats::IsolatedStoreImpl global_store_;
  Stats::IsolatedStoreImpl cluster_scope_;
  Http::CodeStatsImpl code_stats_;
@@ -276,13 +277,14 @@ TEST_F(CodeUtilityTest, ResponseTimingTest) {
 
 class CodeStatsTest : public testing::Test {
 protected:
- CodeStatsTest() : code_stats_(symbol_table_) {}
+ CodeStatsTest()
+ : symbol_table_(Stats::SymbolTableCreator::makeSymbolTable()), code_stats_(*symbol_table_) {}
 
  absl::string_view stripTrailingDot(absl::string_view prefix) {
  return CodeStatsImpl::stripTrailingDot(prefix);
  }
 
- Stats::FakeSymbolTableImpl symbol_table_;
+ Stats::SymbolTablePtr symbol_table_;
  CodeStatsImpl code_stats_;
 };
 

test/common/http/conn_manager_impl_fuzz_test.cc

@@ -19,6 +19,7 @@
 #include "common/http/exception.h"
 #include "common/network/address_impl.h"
 #include "common/network/utility.h"
+#include "common/stats/symbol_table_creator.h"
 
 #include "test/common/http/conn_manager_impl_common.h"
 #include "test/common/http/conn_manager_impl_fuzz.pb.h"
@@ -382,8 +383,8 @@ DEFINE_PROTO_FUZZER(const test::common::http::ConnManagerImplTestCase& input) {
  FuzzConfig config;
  NiceMock<Network::MockDrainDecision> drain_close;
  NiceMock<Runtime::MockRandomGenerator> random;
- Stats::FakeSymbolTableImpl symbol_table;
- Http::ContextImpl http_context(symbol_table);
+ Stats::SymbolTablePtr symbol_table(Stats::SymbolTableCreator::makeSymbolTable());
+ Http::ContextImpl http_context(*symbol_table);
  NiceMock<Runtime::MockLoader> runtime;
  NiceMock<LocalInfo::MockLocalInfo> local_info;
  NiceMock<Upstream::MockClusterManager> cluster_manager;

test/common/router/config_impl_test.cc

@@ -109,7 +109,7 @@ class ConfigImplTestBase {
  return factory_context_.scope().symbolTable().toString(name);
  }
 
- Test::Global<Stats::FakeSymbolTableImpl> symbol_table_;
+ Stats::TestSymbolTable symbol_table_;
  Api::ApiPtr api_;
  NiceMock<Server::Configuration::MockFactoryContext> factory_context_;
 };

test/common/stats/BUILD

@@ -61,6 +61,7 @@ envoy_cc_test_library(
  deps = [
  "//source/common/common:assert_lib",
  "//source/common/memory:stats_lib",
+ "//source/common/stats:symbol_table_creator_lib",
  ],
 )
 

test/common/stats/stat_test_utility.h

@@ -2,6 +2,7 @@
 
 #include "common/common/logger.h"
 #include "common/memory/stats.h"
+#include "common/stats/symbol_table_creator.h"
 
 #include "absl/strings/str_join.h"
 #include "absl/strings/string_view.h"
@@ -104,6 +105,13 @@ class MemoryTest {
  } \
  } while (false)
 
+class SymbolTableCreatorTestPeer {
+public:
+ static void setUseFakeSymbolTables(bool use_fakes) {
+ SymbolTableCreator::setUseFakeSymbolTables(use_fakes);
+ }
+};
+
 } // namespace TestUtil
 } // namespace Stats
 } // namespace Envoy