From 719245f988c779ac7d1edb8e8a49df26c5353e70 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Thu, 22 Aug 2019 07:21:31 +0900 Subject: [PATCH] stats: Add option to switch between fake and real symbol-tables on the command-line. (#7882) * Add option to switch between fake and real symbol-tables on the command-line. Signed-off-by: Joshua Marantz --- docs/root/intro/version_history.rst | 1 + include/envoy/server/options.h | 5 + include/envoy/stats/symbol_table.h | 2 +- source/common/stats/BUILD | 12 +++ source/common/stats/isolated_store_impl.cc | 4 +- source/common/stats/isolated_store_impl.h | 2 +- source/common/stats/symbol_table_creator.cc | 24 +++++ source/common/stats/symbol_table_creator.h | 57 +++++++++++ source/exe/BUILD | 2 +- source/exe/main_common.cc | 5 +- source/exe/main_common.h | 2 +- source/server/BUILD | 1 + source/server/options_impl.cc | 8 +- source/server/options_impl.h | 5 + test/common/grpc/context_impl_test.cc | 4 +- .../grpc_client_integration_test_harness.h | 2 +- test/common/http/BUILD | 1 + test/common/http/codes_test.cc | 8 +- .../http/conn_manager_impl_fuzz_test.cc | 5 +- test/common/router/config_impl_test.cc | 2 +- test/common/stats/BUILD | 1 + test/common/stats/stat_test_utility.h | 8 ++ test/common/stats/thread_local_store_test.cc | 97 +++++++++++++------ .../http1_bridge_filter_test.cc | 2 +- .../http/grpc_web/grpc_web_filter_test.cc | 2 +- .../lightstep/lightstep_tracer_impl_test.cc | 2 +- test/integration/BUILD | 2 + test/integration/integration.cc | 4 +- test/integration/integration.h | 2 + test/integration/integration_admin_test.cc | 7 +- test/integration/server.cc | 5 +- test/integration/stats_integration_test.cc | 51 +++++++++- test/mocks/router/BUILD | 1 + test/mocks/router/mocks.h | 5 +- test/mocks/server/mocks.cc | 2 +- test/mocks/server/mocks.h | 3 +- test/mocks/stats/BUILD | 1 + test/mocks/stats/mocks.cc | 5 +- test/mocks/stats/mocks.h | 35 +++++-- test/mocks/upstream/host.h | 4 +- .../config_validation/cluster_manager_test.cc | 1 - test/server/http/BUILD | 1 + test/server/http/admin_test.cc | 15 +-- test/server/options_impl_test.cc | 7 +- test/tools/router_check/router.h | 2 +- 45 files changed, 334 insertions(+), 83 deletions(-) create mode 100644 source/common/stats/symbol_table_creator.cc create mode 100644 source/common/stats/symbol_table_creator.h diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index c40a3454e4b1..2a049c36c740 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -28,6 +28,7 @@ Version history * http: added the ability to :ref:`merge adjacent slashes` in the path. * listeners: added :ref:`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 `. +* 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 ` to allow reading from redis replicas for Redis Cluster deployments. * rbac: added support for DNS SAN as :ref:`principal_name `. * lua: extended `httpCall()` and `respond()` APIs to accept headers with entry values that can be a string or table of strings. diff --git a/include/envoy/server/options.h b/include/envoy/server/options.h index 8ecc14f0c555..32b277321790 100644 --- a/include/envoy/server/options.h +++ b/include/envoy/server/options.h @@ -184,6 +184,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. */ diff --git a/include/envoy/stats/symbol_table.h b/include/envoy/stats/symbol_table.h index 8f15df4534da..a4346d69397b 100644 --- a/include/envoy/stats/symbol_table.h +++ b/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; +using SymbolTablePtr = std::unique_ptr; } // namespace Stats } // namespace Envoy diff --git a/source/common/stats/BUILD b/source/common/stats/BUILD index cb88ed2dc976..7bc419766cdc 100644 --- a/source/common/stats/BUILD +++ b/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"], diff --git a/source/common/stats/isolated_store_impl.cc b/source/common/stats/isolated_store_impl.cc index 6f2da0f899af..aa58676ae59a 100644 --- a/source/common/stats/isolated_store_impl.cc +++ b/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()) {} +IsolatedStoreImpl::IsolatedStoreImpl() : IsolatedStoreImpl(SymbolTableCreator::makeSymbolTable()) {} IsolatedStoreImpl::IsolatedStoreImpl(std::unique_ptr&& symbol_table) : IsolatedStoreImpl(*symbol_table) { diff --git a/source/common/stats/isolated_store_impl.h b/source/common/stats/isolated_store_impl.h index 6a11ceec2a7c..cce741211df1 100644 --- a/source/common/stats/isolated_store_impl.h +++ b/source/common/stats/isolated_store_impl.h @@ -133,7 +133,7 @@ class IsolatedStoreImpl : public StoreImpl { private: IsolatedStoreImpl(std::unique_ptr&& symbol_table); - std::unique_ptr symbol_table_storage_; + SymbolTablePtr symbol_table_storage_; AllocatorImpl alloc_; IsolatedStatsCache counters_; IsolatedStatsCache gauges_; diff --git a/source/common/stats/symbol_table_creator.cc b/source/common/stats/symbol_table_creator.cc new file mode 100644 index 000000000000..8b29313130b5 --- /dev/null +++ b/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)); + use_fake_symbol_tables_ = use_fake; + return makeSymbolTable(); +} + +SymbolTablePtr SymbolTableCreator::makeSymbolTable() { + initialized_ = true; + if (use_fake_symbol_tables_) { + return std::make_unique(); + } + return std::make_unique(); +} + +} // namespace Stats +} // namespace Envoy diff --git a/source/common/stats/symbol_table_creator.h b/source/common/stats/symbol_table_creator.h new file mode 100644 index 000000000000..4b51468890ce --- /dev/null +++ b/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 diff --git a/source/exe/BUILD b/source/exe/BUILD index 4117a4bb83ca..d48f99ce33b8 100644 --- a/source/exe/BUILD +++ b/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", diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc index 84d57abfc5ac..ed76b4381847 100644 --- a/source/exe/main_common.cc +++ b/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 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: { diff --git a/source/exe/main_common.h b/source/exe/main_common.h index 5faf31b547cc..a0a4796de18e 100644 --- a/source/exe/main_common.h +++ b/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 tls_; diff --git a/source/server/BUILD b/source/server/BUILD index 2e83ab4671c6..bd9c72b56e63 100644 --- a/source/server/BUILD +++ b/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", diff --git a/source/server/options_impl.cc b/source/server/options_impl.cc index c3fd28734149..2beca59d6f3c 100644 --- a/source/server/options_impl.cc +++ b/source/server/options_impl.cc @@ -112,7 +112,9 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv, TCLAP::ValueArg use_libevent_buffer("", "use-libevent-buffers", "Use the original libevent buffer implementation", false, false, "bool", cmd); - + TCLAP::ValueArg 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); @@ -136,6 +138,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; @@ -300,6 +303,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 diff --git a/source/server/options_impl.h b/source/server/options_impl.h index 7fea3a2546a1..a06365c64e6c 100644 --- a/source/server/options_impl.h +++ b/source/server/options_impl.h @@ -81,6 +81,9 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable cluster; - Envoy::Test::Global 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 symbol_table; + Stats::TestSymbolTable symbol_table; ContextImpl context(*symbol_table); absl::optional request_names = context.resolveServiceAndMethod(&path); EXPECT_TRUE(request_names); diff --git a/test/common/grpc/grpc_client_integration_test_harness.h b/test/common/grpc/grpc_client_integration_test_harness.h index 997bae8b6bb8..1d02a8e81de6 100644 --- a/test/common/grpc/grpc_client_integration_test_harness.h +++ b/test/common/grpc/grpc_client_integration_test_harness.h @@ -416,7 +416,7 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { FakeHttpConnectionPtr fake_connection_; std::vector fake_streams_; const Protobuf::MethodDescriptor* method_descriptor_; - Envoy::Test::Global symbol_table_; + Stats::TestSymbolTable symbol_table_; Stats::IsolatedStoreImpl* stats_store_ = new Stats::IsolatedStoreImpl(*symbol_table_); Api::ApiPtr api_; Event::DispatcherPtr dispatcher_; diff --git a/test/common/http/BUILD b/test/common/http/BUILD index 56eefc2b25a6..8e13f1625db3 100644 --- a/test/common/http/BUILD +++ b/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", diff --git a/test/common/http/codes_test.cc b/test/common/http/codes_test.cc index 1bdd1f3202e4..de05c4178e94 100644 --- a/test/common/http/codes_test.cc +++ b/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 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_; }; diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index 5c2bf01fe1ab..47f151b5b1db 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/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 drain_close; NiceMock 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; NiceMock local_info; NiceMock cluster_manager; diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 5af81cbc1ff9..6b7c5743dc90 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -109,7 +109,7 @@ class ConfigImplTestBase { return factory_context_.scope().symbolTable().toString(name); } - Test::Global symbol_table_; + Stats::TestSymbolTable symbol_table_; Api::ApiPtr api_; NiceMock factory_context_; }; diff --git a/test/common/stats/BUILD b/test/common/stats/BUILD index 6ab96a754d42..78157bdee706 100644 --- a/test/common/stats/BUILD +++ b/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", ], ) diff --git a/test/common/stats/stat_test_utility.h b/test/common/stats/stat_test_utility.h index afc97bb9c6cc..c10fe4032065 100644 --- a/test/common/stats/stat_test_utility.h +++ b/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 diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc index 711d1c459e46..be0b87058ced 100644 --- a/test/common/stats/thread_local_store_test.cc +++ b/test/common/stats/thread_local_store_test.cc @@ -845,46 +845,85 @@ TEST_F(StatsThreadLocalStoreTest, NonHotRestartNoTruncation) { tls_.shutdownThread(); } -// Tests how much memory is consumed allocating 100k stats. -TEST(StatsThreadLocalStoreTestNoFixture, MemoryWithoutTls) { - MockSink sink; - Stats::FakeSymbolTableImpl symbol_table; - AllocatorImpl alloc(symbol_table); - ThreadLocalStoreImpl store(alloc); - store.addSink(sink); +class StatsThreadLocalStoreTestNoFixture : public testing::Test { +protected: + StatsThreadLocalStoreTestNoFixture() + : save_use_fakes_(SymbolTableCreator::useFakeSymbolTables()) {} + ~StatsThreadLocalStoreTestNoFixture() override { + TestUtil::SymbolTableCreatorTestPeer::setUseFakeSymbolTables(save_use_fakes_); + if (threading_enabled_) { + store_->shutdownThreading(); + tls_.shutdownThread(); + } + } - // Use a tag producer that will produce tags. - envoy::config::metrics::v2::StatsConfig stats_config; - store.setTagProducer(std::make_unique(stats_config)); + void init(bool use_fakes) { + TestUtil::SymbolTableCreatorTestPeer::setUseFakeSymbolTables(use_fakes); + symbol_table_ = SymbolTableCreator::makeSymbolTable(); + alloc_ = std::make_unique(*symbol_table_); + store_ = std::make_unique(*alloc_); + store_->addSink(sink_); + + // Use a tag producer that will produce tags. + envoy::config::metrics::v2::StatsConfig stats_config; + store_->setTagProducer(std::make_unique(stats_config)); + } + + void initThreading() { + threading_enabled_ = true; + store_->initializeThreading(main_thread_dispatcher_, tls_); + } + + static constexpr size_t million_ = 1000 * 1000; + + MockSink sink_; + SymbolTablePtr symbol_table_; + std::unique_ptr alloc_; + std::unique_ptr store_; + NiceMock main_thread_dispatcher_; + NiceMock tls_; + const bool save_use_fakes_; + bool threading_enabled_{false}; +}; +// Tests how much memory is consumed allocating 100k stats. +TEST_F(StatsThreadLocalStoreTestNoFixture, MemoryWithoutTlsFakeSymbolTable) { + init(true); TestUtil::MemoryTest memory_test; TestUtil::forEachSampleStat( - 1000, [&store](absl::string_view name) { store.counter(std::string(name)); }); - const size_t million = 1000 * 1000; + 1000, [this](absl::string_view name) { store_->counter(std::string(name)); }); EXPECT_MEMORY_EQ(memory_test.consumedBytes(), 15268336); // June 30, 2019 - EXPECT_MEMORY_LE(memory_test.consumedBytes(), 16 * million); + EXPECT_MEMORY_LE(memory_test.consumedBytes(), 16 * million_); } -TEST(StatsThreadLocalStoreTestNoFixture, MemoryWithTls) { - Stats::FakeSymbolTableImpl symbol_table; - AllocatorImpl alloc(symbol_table); - NiceMock main_thread_dispatcher; - NiceMock tls; - ThreadLocalStoreImpl store(alloc); +TEST_F(StatsThreadLocalStoreTestNoFixture, MemoryWithTlsFakeSymbolTable) { + init(true); + initThreading(); + TestUtil::MemoryTest memory_test; + TestUtil::forEachSampleStat( + 1000, [this](absl::string_view name) { store_->counter(std::string(name)); }); + EXPECT_MEMORY_EQ(memory_test.consumedBytes(), 17496848); // June 30, 2019 + EXPECT_MEMORY_LE(memory_test.consumedBytes(), 18 * million_); +} - // Use a tag producer that will produce tags. - envoy::config::metrics::v2::StatsConfig stats_config; - store.setTagProducer(std::make_unique(stats_config)); +// Tests how much memory is consumed allocating 100k stats. +TEST_F(StatsThreadLocalStoreTestNoFixture, MemoryWithoutTlsRealSymbolTable) { + init(false); + TestUtil::MemoryTest memory_test; + TestUtil::forEachSampleStat( + 1000, [this](absl::string_view name) { store_->counter(std::string(name)); }); + EXPECT_MEMORY_EQ(memory_test.consumedBytes(), 9139120); // Aug 9, 2019 + EXPECT_MEMORY_LE(memory_test.consumedBytes(), 10 * million_); +} - store.initializeThreading(main_thread_dispatcher, tls); +TEST_F(StatsThreadLocalStoreTestNoFixture, MemoryWithTlsRealSymbolTable) { + init(false); + initThreading(); TestUtil::MemoryTest memory_test; TestUtil::forEachSampleStat( - 1000, [&store](absl::string_view name) { store.counter(std::string(name)); }); - const size_t million = 1000 * 1000; - EXPECT_MEMORY_EQ(memory_test.consumedBytes(), 17496848); // June 30, 2019 - EXPECT_MEMORY_LE(memory_test.consumedBytes(), 18 * million); - store.shutdownThreading(); - tls.shutdownThread(); + 1000, [this](absl::string_view name) { store_->counter(std::string(name)); }); + EXPECT_MEMORY_EQ(memory_test.consumedBytes(), 11367632); // Aug 9, 2019 + EXPECT_MEMORY_LE(memory_test.consumedBytes(), 12 * million_); } TEST_F(StatsThreadLocalStoreTest, ShuttingDown) { diff --git a/test/extensions/filters/http/grpc_http1_bridge/http1_bridge_filter_test.cc b/test/extensions/filters/http/grpc_http1_bridge/http1_bridge_filter_test.cc index aedebdfa6f0b..aafdb5bdeaa2 100644 --- a/test/extensions/filters/http/grpc_http1_bridge/http1_bridge_filter_test.cc +++ b/test/extensions/filters/http/grpc_http1_bridge/http1_bridge_filter_test.cc @@ -36,7 +36,7 @@ class GrpcHttp1BridgeFilterTest : public testing::Test { ~GrpcHttp1BridgeFilterTest() override { filter_.onDestroy(); } - Envoy::Test::Global symbol_table_; + Stats::TestSymbolTable symbol_table_; Grpc::ContextImpl context_; Http1BridgeFilter filter_; NiceMock decoder_callbacks_; diff --git a/test/extensions/filters/http/grpc_web/grpc_web_filter_test.cc b/test/extensions/filters/http/grpc_web/grpc_web_filter_test.cc index e0a501f48d69..b20f517ee5e9 100644 --- a/test/extensions/filters/http/grpc_web/grpc_web_filter_test.cc +++ b/test/extensions/filters/http/grpc_web/grpc_web_filter_test.cc @@ -106,7 +106,7 @@ class GrpcWebFilterTest : public testing::TestWithParamvalue().getStringView()); } - Envoy::Test::Global symbol_table_; + Stats::TestSymbolTable symbol_table_; Grpc::ContextImpl grpc_context_; GrpcWebFilter filter_; NiceMock decoder_callbacks_; diff --git a/test/extensions/tracers/lightstep/lightstep_tracer_impl_test.cc b/test/extensions/tracers/lightstep/lightstep_tracer_impl_test.cc index 046bd9596c4b..cf9e44e1b9f3 100644 --- a/test/extensions/tracers/lightstep/lightstep_tracer_impl_test.cc +++ b/test/extensions/tracers/lightstep/lightstep_tracer_impl_test.cc @@ -89,7 +89,7 @@ class LightStepDriverTest : public testing::Test { SystemTime start_time_; StreamInfo::MockStreamInfo stream_info_; - Envoy::Test::Global symbol_table_; + Stats::TestSymbolTable symbol_table_; Grpc::ContextImpl grpc_context_; NiceMock tls_; Stats::IsolatedStoreImpl stats_; diff --git a/test/integration/BUILD b/test/integration/BUILD index 393eadbfa2e8..60ababd92f17 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -434,6 +434,7 @@ envoy_cc_test_library( "//source/common/network:utility_lib", "//source/common/runtime:runtime_lib", "//source/common/stats:isolated_store_lib", + "//source/common/stats:symbol_table_creator_lib", "//source/common/stats:thread_local_store_lib", "//source/common/thread_local:thread_local_lib", "//source/common/upstream:upstream_includes", @@ -548,6 +549,7 @@ envoy_cc_test( deps = [ ":integration_lib", "//source/common/memory:stats_lib", + "//source/common/stats:symbol_table_creator_lib", "//source/extensions/filters/http/router:config", "//source/extensions/filters/network/http_connection_manager:config", "//test/common/stats:stat_test_utility_lib", diff --git a/test/integration/integration.cc b/test/integration/integration.cc index e0c124bd3c6b..f8fe1910abf1 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -524,9 +524,9 @@ void BaseIntegrationTest::createXdsUpstream() { auto cfg = std::make_unique( tls_context, factory_context_); - static Stats::Scope* upstream_stats_store = new Stats::TestIsolatedStoreImpl(); + upstream_stats_store_ = std::make_unique(); auto context = std::make_unique( - std::move(cfg), context_manager_, *upstream_stats_store, std::vector{}); + std::move(cfg), context_manager_, *upstream_stats_store_, std::vector{}); fake_upstreams_.emplace_back(new FakeUpstream( std::move(context), 0, FakeHttpConnection::Type::HTTP2, version_, timeSystem())); } diff --git a/test/integration/integration.h b/test/integration/integration.h index db32e8a8cfc6..c61dfb0fc006 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -327,6 +327,8 @@ class BaseIntegrationTest : Logger::Loggable { bool initialized() const { return initialized_; } + std::unique_ptr upstream_stats_store_; + // The IpVersion (IPv4, IPv6) to use. Network::Address::IpVersion version_; // IP Address to use when binding sockets on upstreams. diff --git a/test/integration/integration_admin_test.cc b/test/integration/integration_admin_test.cc index 2af1c1f069df..43c3a867bfd6 100644 --- a/test/integration/integration_admin_test.cc +++ b/test/integration/integration_admin_test.cc @@ -500,9 +500,14 @@ TEST_F(IntegrationAdminIpv4Ipv6Test, Ipv4Ipv6Listen) { // Testing the behavior of StatsMatcher, which allows/denies the instantiation of stats based on // restrictions on their names. +// +// Note: using 'Event::TestUsingSimulatedTime' appears to conflict with LDS in +// StatsMatcherIntegrationTest.IncludeExact, which manifests in a coverage test +// crash, which is really difficult to debug. See #7215. It's possible this is +// due to a bad interaction between the wait-for constructs in the integration +// test framework with sim-time. class StatsMatcherIntegrationTest : public testing::Test, - public Event::TestUsingSimulatedTime, public HttpIntegrationTest, public testing::WithParamInterface { public: diff --git a/test/integration/server.cc b/test/integration/server.cc index 6161eeedf8b4..83777e19db1e 100644 --- a/test/integration/server.cc +++ b/test/integration/server.cc @@ -8,6 +8,7 @@ #include "common/common/thread.h" #include "common/local_info/local_info_impl.h" #include "common/network/utility.h" +#include "common/stats/symbol_table_creator.h" #include "common/stats/thread_local_store.h" #include "common/thread_local/thread_local_impl.h" @@ -187,10 +188,10 @@ void IntegrationTestServerImpl::createAndRunEnvoyServer( Runtime::RandomGeneratorPtr&& random_generator, absl::optional> process_object) { { - Stats::FakeSymbolTableImpl symbol_table; + Stats::SymbolTablePtr symbol_table = Stats::SymbolTableCreator::makeSymbolTable(); Server::HotRestartNopImpl restarter; ThreadLocal::InstanceImpl tls; - Stats::AllocatorImpl stats_allocator(symbol_table); + Stats::AllocatorImpl stats_allocator(*symbol_table); Stats::ThreadLocalStoreImpl stat_store(stats_allocator); std::unique_ptr process_context; if (process_object.has_value()) { diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index ceadf07bacd7..746baf523926 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -7,6 +7,7 @@ #include "common/config/well_known_names.h" #include "common/memory/stats.h" +#include "common/stats/symbol_table_creator.h" #include "test/common/stats/stat_test_utility.h" #include "test/config/utility.h" @@ -172,13 +173,24 @@ class ClusterMemoryTestHelper : public BaseIntegrationTest { return memory_test.consumedBytes(); } }; -class ClusterMemoryTestRunner : public testing::TestWithParam {}; +class ClusterMemoryTestRunner : public testing::TestWithParam { +protected: + ClusterMemoryTestRunner() : save_use_fakes_(Stats::SymbolTableCreator::useFakeSymbolTables()) {} + ~ClusterMemoryTestRunner() override { + Stats::TestUtil::SymbolTableCreatorTestPeer::setUseFakeSymbolTables(save_use_fakes_); + } + +private: + const bool save_use_fakes_; +}; INSTANTIATE_TEST_SUITE_P(IpVersions, ClusterMemoryTestRunner, testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), TestUtility::ipTestParamsToString); -TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithStats) { +TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { + Stats::TestUtil::SymbolTableCreatorTestPeer::setUseFakeSymbolTables(true); + // A unique instance of ClusterMemoryTest allows for multiple runs of Envoy with // differing configuration. This is necessary for measuring the memory consumption // between the different instances within the same test. @@ -226,5 +238,40 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithStats) { EXPECT_MEMORY_LE(m_per_cluster, 44000); } +TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { + Stats::TestUtil::SymbolTableCreatorTestPeer::setUseFakeSymbolTables(false); + + // A unique instance of ClusterMemoryTest allows for multiple runs of Envoy with + // differing configuration. This is necessary for measuring the memory consumption + // between the different instances within the same test. + const size_t m1 = ClusterMemoryTestHelper::computeMemory(1); + const size_t m1001 = ClusterMemoryTestHelper::computeMemory(1001); + const size_t m_per_cluster = (m1001 - m1) / 1000; + + // Note: if you are increasing this golden value because you are adding a + // stat, please confirm that this will be generally useful to most Envoy + // users. Otherwise you are adding to the per-cluster memory overhead, which + // will be significant for Envoy installations that are massively + // multi-tenant. + // + // History of golden values: + // + // Date PR Bytes Per Cluster Notes + // exact upper-bound + // ---------- ----- ----------------- ----- + // 2019/08/09 7882 35489 36000 Initial version + + // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI + // 'release' builds, where we control the platform and tool-chain. So you + // will need to find the correct value only after failing CI and looking + // at the logs. + // + // On a local clang8/libstdc++/linux flow, the memory usage was observed in + // June 2019 to be 64 bytes higher than it is in CI/release. Your mileage may + // vary. + EXPECT_MEMORY_EQ(m_per_cluster, 35489); // 104 bytes higher than a debug build. + EXPECT_MEMORY_LE(m_per_cluster, 36000); +} + } // namespace } // namespace Envoy diff --git a/test/mocks/router/BUILD b/test/mocks/router/BUILD index eead78a0d5fc..d5d8729cccad 100644 --- a/test/mocks/router/BUILD +++ b/test/mocks/router/BUILD @@ -27,5 +27,6 @@ envoy_cc_mock( "//include/envoy/upstream:cluster_manager_interface", "//source/common/stats:fake_symbol_table_lib", "//test/mocks:common_lib", + "//test/mocks/stats:stats_mocks", ], ) diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index aeda21400f98..7e6ce48606ea 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -24,6 +24,7 @@ #include "common/stats/fake_symbol_table_impl.h" +#include "test/mocks/stats/mocks.h" #include "test/test_common/global.h" #include "gmock/gmock.h" @@ -199,7 +200,7 @@ class TestVirtualCluster : public VirtualCluster { // Router::VirtualCluster Stats::StatName statName() const override { return stat_name_.statName(); } - Test::Global symbol_table_; + Stats::TestSymbolTable symbol_table_; Stats::StatNameManagedStorage stat_name_{"fake_virtual_cluster", *symbol_table_}; }; @@ -223,7 +224,7 @@ class MockVirtualHost : public VirtualHost { return stat_name_->statName(); } - mutable Test::Global symbol_table_; + mutable Stats::TestSymbolTable symbol_table_; std::string name_{"fake_vhost"}; mutable std::unique_ptr stat_name_; testing::NiceMock rate_limit_policy_; diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index 9d2d49f52750..42bcd95947a8 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -77,7 +77,7 @@ MockGuardDog::MockGuardDog() : watch_dog_(new NiceMock()) { } MockGuardDog::~MockGuardDog() = default; -MockHotRestart::MockHotRestart() : stats_allocator_(symbol_table_.get()) { +MockHotRestart::MockHotRestart() : stats_allocator_(*symbol_table_) { ON_CALL(*this, logLock()).WillByDefault(ReturnRef(log_lock_)); ON_CALL(*this, accessLogLock()).WillByDefault(ReturnRef(access_log_lock_)); ON_CALL(*this, statsAllocator()).WillByDefault(ReturnRef(stats_allocator_)); diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index 4ba0920f5695..fd0ee9b27ef6 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -85,6 +85,7 @@ class MockOptions : public Options { MOCK_CONST_METHOD0(signalHandlingEnabled, bool()); MOCK_CONST_METHOD0(mutexTracingEnabled, bool()); MOCK_CONST_METHOD0(libeventBufferEnabled, bool()); + MOCK_CONST_METHOD0(fakeSymbolTableEnabled, bool()); MOCK_CONST_METHOD0(cpusetThreadsEnabled, bool()); MOCK_CONST_METHOD0(toCommandLineOptions, Server::CommandLineOptionsPtr()); @@ -218,7 +219,7 @@ class MockHotRestart : public HotRestart { MOCK_METHOD0(statsAllocator, Stats::Allocator&()); private: - Test::Global symbol_table_; + Stats::TestSymbolTable symbol_table_; Thread::MutexBasicLockable log_lock_; Thread::MutexBasicLockable access_log_lock_; Stats::AllocatorImpl stats_allocator_; diff --git a/test/mocks/stats/BUILD b/test/mocks/stats/BUILD index 6539e818467b..bf9048b25366 100644 --- a/test/mocks/stats/BUILD +++ b/test/mocks/stats/BUILD @@ -22,6 +22,7 @@ envoy_cc_mock( "//source/common/stats:isolated_store_lib", "//source/common/stats:stats_lib", "//source/common/stats:store_impl_lib", + "//source/common/stats:symbol_table_creator_lib", "//test/mocks:common_lib", "//test/test_common:global_lib", ], diff --git a/test/mocks/stats/mocks.cc b/test/mocks/stats/mocks.cc index 56dfbaf84d40..ef5eeebeb488 100644 --- a/test/mocks/stats/mocks.cc +++ b/test/mocks/stats/mocks.cc @@ -63,7 +63,7 @@ MockMetricSnapshot::~MockMetricSnapshot() = default; MockSink::MockSink() = default; MockSink::~MockSink() = default; -MockStore::MockStore() : StoreImpl(*fake_symbol_table_) { +MockStore::MockStore() : StoreImpl(*global_symbol_table_) { ON_CALL(*this, counter(_)).WillByDefault(ReturnRef(counter_)); ON_CALL(*this, histogram(_)).WillByDefault(Invoke([this](const std::string& name) -> Histogram& { auto* histogram = new NiceMock(); // symbol_table_); @@ -75,8 +75,7 @@ MockStore::MockStore() : StoreImpl(*fake_symbol_table_) { } MockStore::~MockStore() = default; -MockIsolatedStatsStore::MockIsolatedStatsStore() - : IsolatedStoreImpl(Test::Global::get()) {} +MockIsolatedStatsStore::MockIsolatedStatsStore() : IsolatedStoreImpl(*global_symbol_table_) {} MockIsolatedStatsStore::~MockIsolatedStatsStore() = default; MockStatsMatcher::MockStatsMatcher() = default; diff --git a/test/mocks/stats/mocks.h b/test/mocks/stats/mocks.h index d418ad430f2b..d5ad120ffcae 100644 --- a/test/mocks/stats/mocks.h +++ b/test/mocks/stats/mocks.h @@ -18,6 +18,7 @@ #include "common/stats/histogram_impl.h" #include "common/stats/isolated_store_impl.h" #include "common/stats/store_impl.h" +#include "common/stats/symbol_table_creator.h" #include "test/test_common/global.h" @@ -26,6 +27,25 @@ namespace Envoy { namespace Stats { +class TestSymbolTableHelper { +public: + TestSymbolTableHelper() : symbol_table_(SymbolTableCreator::makeSymbolTable()) {} + SymbolTable& symbolTable() { return *symbol_table_; } + const SymbolTable& constSymbolTable() const { return *symbol_table_; } + +private: + SymbolTablePtr symbol_table_; +}; + +class TestSymbolTable { +public: + SymbolTable& operator*() { return global_.get().symbolTable(); } + const SymbolTable& operator*() const { return global_.get().constSymbolTable(); } + SymbolTable* operator->() { return &global_.get().symbolTable(); } + const SymbolTable* operator->() const { return &global_.get().constSymbolTable(); } + Envoy::Test::Global global_; +}; + template class MockMetric : public BaseClass { public: MockMetric() : name_(*this), tag_pool_(*symbol_table_) {} @@ -39,7 +59,7 @@ template class MockMetric : public BaseClass { explicit MetricName(MockMetric& mock_metric) : mock_metric_(mock_metric) {} ~MetricName() { if (stat_name_storage_ != nullptr) { - stat_name_storage_->free(*mock_metric_.symbol_table_); + stat_name_storage_->free(mock_metric_.symbolTable()); } } @@ -57,8 +77,8 @@ template class MockMetric : public BaseClass { std::unique_ptr stat_name_storage_; }; - SymbolTable& symbolTable() override { return symbol_table_.get(); } - const SymbolTable& constSymbolTable() const override { return symbol_table_.get(); } + SymbolTable& symbolTable() override { return *symbol_table_; } + const SymbolTable& constSymbolTable() const override { return *symbol_table_; } // Note: cannot be mocked because it is accessed as a Property in a gmock EXPECT_CALL. This // creates a deadlock in gmock and is an unintended use of mock functions. @@ -90,7 +110,7 @@ template class MockMetric : public BaseClass { } } - Test::Global symbol_table_; // Must outlive name_. + TestSymbolTable symbol_table_; // Must outlive name_. MetricName name_; void setTags(const std::vector& tags) { @@ -251,7 +271,7 @@ class MockSink : public Sink { class SymbolTableProvider { public: - Test::Global fake_symbol_table_; + TestSymbolTable global_symbol_table_; }; class MockStore : public SymbolTableProvider, public StoreImpl { @@ -285,7 +305,7 @@ class MockStore : public SymbolTableProvider, public StoreImpl { return histogram(symbol_table_->toString(name)); } - Test::Global symbol_table_; + TestSymbolTable symbol_table_; testing::NiceMock counter_; std::vector> histograms_; }; @@ -294,8 +314,7 @@ class MockStore : public SymbolTableProvider, public StoreImpl { * With IsolatedStoreImpl it's hard to test timing stats. * MockIsolatedStatsStore mocks only deliverHistogramToSinks for better testing. */ -class MockIsolatedStatsStore : private Test::Global, - public IsolatedStoreImpl { +class MockIsolatedStatsStore : public SymbolTableProvider, public IsolatedStoreImpl { public: MockIsolatedStatsStore(); ~MockIsolatedStatsStore() override; diff --git a/test/mocks/upstream/host.h b/test/mocks/upstream/host.h index 095c67c1ae88..6fea5dec1f2e 100644 --- a/test/mocks/upstream/host.h +++ b/test/mocks/upstream/host.h @@ -108,7 +108,7 @@ class MockHostDescription : public HostDescription { testing::NiceMock cluster_; testing::NiceMock stats_store_; HostStats stats_{ALL_HOST_STATS(POOL_COUNTER(stats_store_), POOL_GAUGE(stats_store_))}; - mutable Test::Global symbol_table_; + mutable Stats::TestSymbolTable symbol_table_; mutable std::unique_ptr locality_zone_stat_name_; }; @@ -186,7 +186,7 @@ class MockHost : public Host { testing::NiceMock outlier_detector_; NiceMock stats_store_; HostStats stats_{ALL_HOST_STATS(POOL_COUNTER(stats_store_), POOL_GAUGE(stats_store_))}; - mutable Test::Global symbol_table_; + mutable Stats::TestSymbolTable symbol_table_; mutable std::unique_ptr locality_zone_stat_name_; }; diff --git a/test/server/config_validation/cluster_manager_test.cc b/test/server/config_validation/cluster_manager_test.cc index 0d3639d4860b..0653c0cc8ae7 100644 --- a/test/server/config_validation/cluster_manager_test.cc +++ b/test/server/config_validation/cluster_manager_test.cc @@ -51,7 +51,6 @@ TEST(ValidationClusterManagerTest, MockedMethods) { log_manager, singleton_manager, time_system); const envoy::config::bootstrap::v2::Bootstrap bootstrap; - Stats::FakeSymbolTableImpl symbol_table; ClusterManagerPtr cluster_manager = factory.clusterManagerFromProto(bootstrap); EXPECT_EQ(nullptr, cluster_manager->httpConnPoolForCluster("cluster", ResourcePriority::Default, Http::Protocol::Http11, nullptr)); diff --git a/test/server/http/BUILD b/test/server/http/BUILD index f513226fd1a5..20df4a917b1b 100644 --- a/test/server/http/BUILD +++ b/test/server/http/BUILD @@ -19,6 +19,7 @@ envoy_cc_test( "//source/common/profiler:profiler_lib", "//source/common/protobuf", "//source/common/protobuf:utility_lib", + "//source/common/stats:symbol_table_creator_lib", "//source/common/stats:thread_local_store_lib", "//source/extensions/transport_sockets/tls:context_config_lib", "//source/server/http:admin_lib", diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index 481a1bbfd533..2c9e933ed881 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -14,6 +14,7 @@ #include "common/profiler/profiler.h" #include "common/protobuf/protobuf.h" #include "common/protobuf/utility.h" +#include "common/stats/symbol_table_creator.h" #include "common/stats/thread_local_store.h" #include "server/http/admin.h" @@ -50,7 +51,8 @@ namespace Server { class AdminStatsTest : public testing::TestWithParam { public: - AdminStatsTest() : alloc_(symbol_table_) { + AdminStatsTest() + : symbol_table_(Stats::SymbolTableCreator::makeSymbolTable()), alloc_(*symbol_table_) { store_ = std::make_unique(alloc_); store_->addSink(sink_); } @@ -63,7 +65,7 @@ class AdminStatsTest : public testing::TestWithParam main_thread_dispatcher_; NiceMock tls_; Stats::AllocatorImpl alloc_; @@ -1374,15 +1376,16 @@ class HistogramWrapper { class PrometheusStatsFormatterTest : public testing::Test { protected: - PrometheusStatsFormatterTest() : alloc_(symbol_table_) {} + PrometheusStatsFormatterTest() + : symbol_table_(Stats::SymbolTableCreator::makeSymbolTable()), alloc_(*symbol_table_) {} void addCounter(const std::string& name, std::vector cluster_tags) { - Stats::StatNameManagedStorage storage(name, symbol_table_); + Stats::StatNameManagedStorage storage(name, *symbol_table_); counters_.push_back(alloc_.makeCounter(storage.statName(), name, cluster_tags)); } void addGauge(const std::string& name, std::vector cluster_tags) { - Stats::StatNameManagedStorage storage(name, symbol_table_); + Stats::StatNameManagedStorage storage(name, *symbol_table_); gauges_.push_back(alloc_.makeGauge(storage.statName(), name, cluster_tags, Stats::Gauge::ImportMode::Accumulate)); } @@ -1396,7 +1399,7 @@ class PrometheusStatsFormatterTest : public testing::Test { return MockHistogramSharedPtr(new NiceMock()); } - Stats::FakeSymbolTableImpl symbol_table_; + Stats::SymbolTablePtr symbol_table_; Stats::AllocatorImpl alloc_; std::vector counters_; std::vector gauges_; diff --git a/test/server/options_impl_test.cc b/test/server/options_impl_test.cc index 71662bb36bd6..bc4cc16f7fa7 100644 --- a/test/server/options_impl_test.cc +++ b/test/server/options_impl_test.cc @@ -99,6 +99,7 @@ TEST_F(OptionsImplTest, All) { EXPECT_TRUE(options->cpusetThreadsEnabled()); EXPECT_TRUE(options->allowUnknownStaticFields()); EXPECT_TRUE(options->rejectUnknownDynamicFields()); + EXPECT_TRUE(options->fakeSymbolTableEnabled()); options = createOptionsImpl("envoy --mode init_only"); EXPECT_EQ(Server::Mode::InitOnly, options->mode()); @@ -129,6 +130,7 @@ TEST_F(OptionsImplTest, SetAll) { bool hot_restart_disabled = options->hotRestartDisabled(); bool signal_handling_enabled = options->signalHandlingEnabled(); bool cpuset_threads_enabled = options->cpusetThreadsEnabled(); + bool fake_symbol_table_enabled = options->fakeSymbolTableEnabled(); options->setBaseId(109876); options->setConcurrency(42); @@ -155,6 +157,7 @@ TEST_F(OptionsImplTest, SetAll) { options->setCpusetThreads(!options->cpusetThreadsEnabled()); options->setAllowUnkownFields(true); options->setRejectUnknownFieldsDynamic(true); + options->setFakeSymbolTableEnabled(!options->fakeSymbolTableEnabled()); EXPECT_EQ(109876, options->baseId()); EXPECT_EQ(42U, options->concurrency()); @@ -181,6 +184,7 @@ TEST_F(OptionsImplTest, SetAll) { EXPECT_EQ(!cpuset_threads_enabled, options->cpusetThreadsEnabled()); EXPECT_TRUE(options->allowUnknownStaticFields()); EXPECT_TRUE(options->rejectUnknownDynamicFields()); + EXPECT_EQ(!fake_symbol_table_enabled, options->fakeSymbolTableEnabled()); // Validate that CommandLineOptions is constructed correctly. Server::CommandLineOptionsPtr command_line_options = options->toCommandLineOptions(); @@ -247,7 +251,8 @@ TEST_F(OptionsImplTest, OptionsAreInSyncWithProto) { // 3. ignore_rest - default TCLAP argument. // 4. use-libevent-buffers - short-term override for rollout of new buffer implementation. // 5. allow-unknown-fields - deprecated alias of allow-unknown-static-fields. - EXPECT_EQ(options->count() - 5, command_line_options->GetDescriptor()->field_count()); + // 6. use-fake-symbol-table - short-term override for rollout of real symbol-table implementation. + EXPECT_EQ(options->count() - 6, command_line_options->GetDescriptor()->field_count()); } TEST_F(OptionsImplTest, BadCliOption) { diff --git a/test/tools/router_check/router.h b/test/tools/router_check/router.h index a3395de2ad87..7c81835da7ae 100644 --- a/test/tools/router_check/router.h +++ b/test/tools/router_check/router.h @@ -54,7 +54,7 @@ struct ToolConfig { private: ToolConfig(std::unique_ptr headers, int random_value); - Test::Global symbol_table_; + Stats::TestSymbolTable symbol_table_; }; /**