envoyproxy · mattklein123 · Jun 28, 2020 · Oct 31, 2018 · Nov 2, 2018 · Nov 7, 2018
diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
@@ -107,6 +107,8 @@ New Features
 * metrics service: added added :ref:`API version <envoy_v3_api_field_config.metrics.v3.MetricsServiceConfig.transport_api_version>` to explicitly set the version of gRPC service endpoint and message to be used.
 * network filters: added a :ref:`postgres proxy filter <config_network_filters_postgres_proxy>`.
 * network filters: added a :ref:`rocketmq proxy filter <config_network_filters_rocketmq_proxy>`.
+* performance: stats symbol table implementation (enabled by default; to disable it, add
+  `--use-fake-symbol-table 1` to the command-line arguments when starting Envoy).
 * ratelimit: add support for use of dynamic metadata :ref:`dynamic_metadata <envoy_v3_api_field_config.route.v3.RateLimit.Action.dynamic_metadata>` as a ratelimit action.
 * ratelimit: added :ref:`API version <envoy_v3_api_field_config.ratelimit.v3.RateLimitServiceConfig.transport_api_version>` to explicitly set the version of gRPC service endpoint and message to be used.
 * redis: added acl support :ref:`downstream_auth_username <envoy_v3_api_field_extensions.filters.network.redis_proxy.v3.RedisProxy.downstream_auth_username>` for downstream client ACL authentication, and :ref:`auth_username <envoy_v3_api_field_extensions.filters.network.redis_proxy.v3.RedisProtocolOptions.auth_username>` to configure authentication usernames for upstream Redis 6+ server clusters with ACL enabled.

diff --git a/source/common/stats/symbol_table_creator.cc b/source/common/stats/symbol_table_creator.cc
@@ -4,7 +4,7 @@ namespace Envoy {
 namespace Stats {
 
 bool SymbolTableCreator::initialized_ = false;
-bool SymbolTableCreator::use_fake_symbol_tables_ = true;
+bool SymbolTableCreator::use_fake_symbol_tables_ = false;
 
 SymbolTablePtr SymbolTableCreator::initAndMakeSymbolTable(bool use_fake) {
   ASSERT(!initialized_ || (use_fake_symbol_tables_ == use_fake));

diff --git a/source/server/options_impl.cc b/source/server/options_impl.cc
@@ -156,7 +156,7 @@ OptionsImpl::OptionsImpl(std::vector<std::string> args,
       "", "cpuset-threads", "Get the default # of worker threads from cpuset size", cmd, false);
 
   TCLAP::ValueArg<bool> use_fake_symbol_table("", "use-fake-symbol-table",
-                                              "Use fake symbol table implementation", false, true,
+                                              "Use fake symbol table implementation", false, false,
                                               "bool", cmd);
 
   TCLAP::ValueArg<std::string> disable_extensions("", "disable-extensions",

diff --git a/test/common/stats/stat_test_utility.cc b/test/common/stats/stat_test_utility.cc
@@ -152,7 +152,8 @@ Counter& TestStore::counterFromStatNameWithTags(const StatName& stat_name,
   } else {
     // Ensures StatNames with the same string representation are specified
     // consistently using symbolic/dynamic components on every access.
-    ASSERT(counter_ref->statName() == stat_name);
+    ASSERT(counter_ref->statName() == stat_name, "Inconsistent dynamic vs symbolic "
+                                                 "stat name specification");
   }
   return *counter_ref;
 }
@@ -173,7 +174,8 @@ Gauge& TestStore::gaugeFromStatNameWithTags(const StatName& stat_name,
   if (gauge_ref == nullptr) {
     gauge_ref = &IsolatedStoreImpl::gaugeFromStatNameWithTags(stat_name, tags, mode);
   } else {
-    ASSERT(gauge_ref->statName() == stat_name);
+    ASSERT(gauge_ref->statName() == stat_name, "Inconsistent dynamic vs symbolic "
+                                               "stat name specification");
   }
   return *gauge_ref;
 }
@@ -194,7 +196,8 @@ Histogram& TestStore::histogramFromStatNameWithTags(const StatName& stat_name,
   if (histogram_ref == nullptr) {
     histogram_ref = &IsolatedStoreImpl::histogramFromStatNameWithTags(stat_name, tags, unit);
   } else {
-    ASSERT(histogram_ref->statName() == stat_name);
+    ASSERT(histogram_ref->statName() == stat_name, "Inconsistent dynamic vs symbolic "
+                                                   "stat name specification");
   }
   return *histogram_ref;
 }

diff --git a/test/extensions/filters/http/grpc_stats/config_test.cc b/test/extensions/filters/http/grpc_stats/config_test.cc
@@ -358,11 +358,18 @@ TEST_F(GrpcStatsFilterConfigTest, MessageCounts) {
                     .counterFromString(
                         "grpc.lyft.users.BadCompanions.GetBadCompanions.request_message_count")
                     .value());
-  EXPECT_EQ(0U, decoder_callbacks_.clusterInfo()
-                    ->statsScope()
-                    .counterFromString(
-                        "grpc.lyft.users.BadCompanions.GetBadCompanions.response_message_count")
-                    .value());
+
+  // Check that there is response_message_count stat yet. We use
+  // stats_store_.findCounterByString rather than looking on
+  // clusterInfo()->statsScope() because findCounterByString is not an API on
+  // Stats::Store, and there is no prefix so the names will match. We verify
+  // that by double-checking we can find the request_message_count using the
+  // same API.
+  EXPECT_FALSE(stats_store_.findCounterByString(
+      "grpc.lyft.users.BadCompanions.GetBadCompanions.response_message_count"));
+  EXPECT_TRUE(stats_store_.findCounterByString(
+      "grpc.lyft.users.BadCompanions.GetBadCompanions.request_message_count"));
+
   const auto& data = stream_info_.filterState()->getDataReadOnly<GrpcStatsObject>(
       HttpFilterNames::get().GrpcStats);
   EXPECT_EQ(2U, data.request_message_count);

diff --git a/test/integration/BUILD b/test/integration/BUILD
@@ -50,6 +50,7 @@ envoy_cc_test_library(
 
 envoy_cc_test(
     name = "ads_integration_test",
+    size = "enormous",
     srcs = ["ads_integration_test.cc"],
     tags = ["fails_on_windows"],
     deps = [

diff --git a/test/integration/http2_integration_test.cc b/test/integration/http2_integration_test.cc
@@ -1562,8 +1562,7 @@ void Http2FloodMitigationTest::floodServer(const Http2Frame& frame, const std::s
 
   EXPECT_LE(total_bytes_sent, TransmitThreshold) << "Flood mitigation is broken.";
   EXPECT_EQ(1, test_server_->counter(flood_stat)->value());
-  EXPECT_EQ(1,
-            test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value());
+  test_server_->waitForCounterGe("http.config_test.downstream_cx_delayed_close_timeout", 1);
 }
 
 // Verify that the server detects the flood using specified request parameters.

diff --git a/test/server/admin/prometheus_stats_test.cc b/test/server/admin/prometheus_stats_test.cc
@@ -36,9 +36,7 @@ class HistogramWrapper {
 
 class PrometheusStatsFormatterTest : public testing::Test {
 protected:
-  PrometheusStatsFormatterTest()
-      : symbol_table_(Stats::SymbolTableCreator::makeSymbolTable()), alloc_(*symbol_table_),
-        pool_(*symbol_table_) {}
+  PrometheusStatsFormatterTest() : alloc_(*symbol_table_), pool_(*symbol_table_) {}
 
   ~PrometheusStatsFormatterTest() override { clearStorage(); }
 
@@ -92,7 +90,7 @@ class PrometheusStatsFormatterTest : public testing::Test {
     EXPECT_EQ(0, symbol_table_->numSymbols());
   }
 
-  Stats::SymbolTablePtr symbol_table_;
+  Stats::TestSymbolTable symbol_table_;
   Stats::AllocatorImpl alloc_;
   Stats::StatNamePool pool_;
   std::vector<Stats::CounterSharedPtr> counters_;