stats: fix listener/filter scope prefixes

include/envoy/server/configuration.h

@@ -68,7 +68,7 @@ class Listener {
   /**
    * @return Stats::Scope& the stats scope to use for all listener specific stats.
    */
-  virtual Stats::Scope& scope() PURE;
+  virtual Stats::Scope& listenerScope() PURE;
 };
 
 typedef std::unique_ptr<Listener> ListenerPtr;

source/server/configuration_impl.cc

@@ -131,22 +131,24 @@ void MainImpl::initializeTracers(const Json::Object& configuration) {
 
 const std::list<ListenerPtr>& MainImpl::listeners() { return listeners_; }
 
-MainImpl::ListenerConfig::ListenerConfig(Instance& server, Json::Object& json) : server_(server) {
-  address_ = Network::Utility::resolveUrl(json.getString("address"));
+MainImpl::ListenerConfig::ListenerConfig(Instance& server, Json::Object& json)
+    : server_(server), address_(Network::Utility::resolveUrl(json.getString("address"))),
+      global_scope_(server.stats().createScope("")) {
 
   // ':' is a reserved char in statsd. Do the translation here to avoid costly inline translations
   // later.
   std::string final_stat_name = fmt::format("listener.{}.", address_->asString());
   std::replace(final_stat_name.begin(), final_stat_name.end(), ':', '_');
 
-  scope_ = server.stats().createScope(final_stat_name);
+  listener_scope_ = server.stats().createScope(final_stat_name);
   LOG(info, "  address={}", address_->asString());
 
   json.validateSchema(Json::Schema::LISTENER_SCHEMA);
 
   if (json.hasObject("ssl_context")) {
     Ssl::ContextConfigImpl context_config(*json.getObject("ssl_context"));
-    ssl_context_ = server.sslContextManager().createSslServerContext(*scope_, context_config);
+    ssl_context_ =
+        server.sslContextManager().createSslServerContext(*listener_scope_, context_config);
   }
 
   bind_to_port_ = json.getBoolean("bind_to_port", true);

source/server/configuration_impl.h

@@ -176,6 +176,7 @@ class MainImpl : Logger::Loggable<Logger::Id::config>, public Main {
     bool useProxyProto() override { return use_proxy_proto_; }
     bool useOriginalDst() override { return use_original_dst_; }
     uint32_t perConnectionBufferLimitBytes() override { return per_connection_buffer_limit_bytes_; }
+    Stats::Scope& listenerScope() override { return *listener_scope_; }
 
     // Server::Configuration::FactoryContext
     AccessLog::AccessLogManager& accessLogManager() override { return server_.accessLogManager(); }
@@ -193,7 +194,7 @@ class MainImpl : Logger::Loggable<Logger::Id::config>, public Main {
     }
     Envoy::Runtime::Loader& runtime() override { return server_.runtime(); }
     Instance& server() override { return server_; }
-    Stats::Scope& scope() override { return *scope_; }
+    Stats::Scope& scope() override { return *global_scope_; }
     ThreadLocal::Instance& threadLocal() override { return server_.threadLocal(); }
 
     // Network::FilterChainFactory
@@ -203,7 +204,8 @@ class MainImpl : Logger::Loggable<Logger::Id::config>, public Main {
     Instance& server_;
     Network::Address::InstanceConstSharedPtr address_;
     bool bind_to_port_{};
-    Stats::ScopePtr scope_;
+    Stats::ScopePtr global_scope_;   // Stats with global named scope, but needed for LDS cleanup.
+    Stats::ScopePtr listener_scope_; // Stats with listener named scope.
     Ssl::ServerContextPtr ssl_context_;
     bool use_proxy_proto_{};
     bool use_original_dst_{};

source/server/worker.cc

@@ -29,10 +29,11 @@ void Worker::initializeConfiguration(Server::Configuration::Main& config,
         .per_connection_buffer_limit_bytes_ = listener->perConnectionBufferLimitBytes()};
     if (listener->sslContext()) {
       handler_->addSslListener(listener->filterChainFactory(), *listener->sslContext(),
-                               *socket_map.at(listener.get()), listener->scope(), listener_options);
+                               *socket_map.at(listener.get()), listener->listenerScope(),
+                               listener_options);
     } else {
       handler_->addListener(listener->filterChainFactory(), *socket_map.at(listener.get()),
-                            listener->scope(), listener_options);
+                            listener->listenerScope(), listener_options);
     }
   }
 

test/server/configuration_impl_test.cc

@@ -459,6 +459,50 @@ TEST_F(ConfigurationImplTest, ConfigurationFailsWhenInvalidTracerSpecified) {
                             "No HttpTracerFactory found for type: invalid");
 }
 
+class TestStatsConfigFactory : public NamedNetworkFilterConfigFactory {
+public:
+  // NetworkFilterConfigFactory
+  NetworkFilterFactoryCb createFilterFactory(const Json::Object&,
+                                             FactoryContext& context) override {
+    context.scope().counter("bar").inc();
+    return [](Network::FilterManager&) -> void {};
+  }
+  std::string name() override { return "stats_test"; }
+  NetworkFilterType type() override { return NetworkFilterType::Read; }
+};
+
+TEST_F(ConfigurationImplTest, StatsScopeTest) {
+  RegisterNamedNetworkFilterConfigFactory<TestStatsConfigFactory> registered;
+
+  std::string json = R"EOF(
+  {
+    "listeners" : [
+      {
+        "address": "tcp://127.0.0.1:1234",
+        "filters": [
+          {
+            "type" : "read",
+            "name" : "stats_test",
+            "config" : {}
+          }
+        ]
+      }
+    ],
+    "cluster_manager": {
+      "clusters": []
+    }
+  }
+  )EOF";
+
+  Json::ObjectSharedPtr loader = Json::Factory::loadFromString(json);
+  MainImpl config(server_, cluster_manager_factory_);
+  config.initialize(*loader);
+  config.listeners().front()->listenerScope().counter("foo").inc();
+
+  EXPECT_EQ(1UL, server_.stats_store_.counter("bar").value());
+  EXPECT_EQ(1UL, server_.stats_store_.counter("listener.127.0.0.1_1234.foo").value());
+}
+
 /**
  * Config registration for the echo filter using the deprecated registration class.
  */
@@ -507,6 +551,7 @@ TEST_F(ConfigurationImplTest, DeprecatedFilterConfigFactoryRegistrationTest) {
   MainImpl config(server_, cluster_manager_factory_);
   config.initialize(*loader);
 }
+
 } // Configuration
 } // Server
 } // Envoy