diff --git a/include/envoy/server/configuration.h b/include/envoy/server/configuration.h index 09ec8d85e55d..6b0456984f36 100644 --- a/include/envoy/server/configuration.h +++ b/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 ListenerPtr; diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc index 5f63b66afcbe..859dfb51896c 100644 --- a/source/server/configuration_impl.cc +++ b/source/server/configuration_impl.cc @@ -131,22 +131,24 @@ void MainImpl::initializeTracers(const Json::Object& configuration) { const std::list& 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); diff --git a/source/server/configuration_impl.h b/source/server/configuration_impl.h index 58327481d5aa..3e430b745048 100644 --- a/source/server/configuration_impl.h +++ b/source/server/configuration_impl.h @@ -176,6 +176,7 @@ class MainImpl : Logger::Loggable, 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, 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, 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_{}; diff --git a/source/server/worker.cc b/source/server/worker.cc index 176e9fe91df1..ee20665bbdb4 100644 --- a/source/server/worker.cc +++ b/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); } } diff --git a/test/server/configuration_impl_test.cc b/test/server/configuration_impl_test.cc index 433fce0ac4c3..d0d1c7bd73ec 100644 --- a/test/server/configuration_impl_test.cc +++ b/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 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