stats: migrate the simple scope creation to new API

source/common/grpc/BUILD

@@ -56,6 +56,7 @@ envoy_cc_library(
         "//envoy/singleton:manager_interface",
         "//envoy/thread_local:thread_local_interface",
         "//envoy/upstream:cluster_manager_interface",
+        "//source/common/config:well_known_names",
         "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
     ] + envoy_select_google_grpc([":google_async_client_lib"]),
 )

source/common/grpc/async_client_manager_impl.cc

@@ -6,6 +6,7 @@
 #include "envoy/stats/scope.h"
 
 #include "source/common/common/base64.h"
+#include "source/common/config/well_known_names.h"
 #include "source/common/grpc/async_client_impl.h"
 #include "source/common/protobuf/utility.h"
 
@@ -84,7 +85,12 @@ GoogleAsyncClientFactoryImpl::GoogleAsyncClientFactoryImpl(
     Stats::Scope& scope, Server::Configuration::CommonFactoryContext& context,
     const StatNames& stat_names, absl::Status& creation_status)
     : google_tls_slot_(google_tls_slot),
-      scope_(scope.createScope(fmt::format("grpc.{}.", config.google_grpc().stat_prefix()))),
+      // grpc.(<stat_prefix>).**
+      scope_(scope.createScopeWithTaggedName(
+          "grpc",
+          {Stats::TagStringView{Envoy::Config::TagNames::get().GOOGLE_GRPC_CLIENT_PREFIX,
+                                config.google_grpc().stat_prefix()}},
+          fmt::format("grpc.{}.", config.google_grpc().stat_prefix()))),
       config_(config), factory_context_(context), stat_names_(stat_names) {
 #ifndef ENVOY_GOOGLE_GRPC
   UNREFERENCED_PARAMETER(google_tls_slot_);

source/common/listener_manager/listener_impl.cc

@@ -289,9 +289,22 @@ generateListenerStatsScope(const envoy::config::listener::v3::Listener& config,
     }
   }
 
-  Stats::ScopeSharedPtr scope = stats.createScope("", false, {}, scope_matcher);
-  Stats::ScopeSharedPtr listener_scope = stats.createScope(
-      fmt::format("listener.{}.", listenerStatsScope(config)), false, {}, std::move(scope_matcher));
+  Stats::ScopeSharedPtr scope =
+      stats.createScopeWithTaggedName({}, {}, {}, false, {}, scope_matcher);
+
+  // listener.(<address|stat_prefix>.)*, but specifically excluding "admin"
+  const std::string observability_name = listenerStatsScope(config);
+  Stats::ScopeSharedPtr listener_scope;
+  if (observability_name != "admin") {
+    // TODO(wbpcode): for the 'admin' listener, we won't extract the listener address as a tag.
+    listener_scope = stats.createScopeWithTaggedName(
+        "listener",
+        {Stats::TagStringView{Config::TagNames::get().LISTENER_ADDRESS, observability_name}},
+        fmt::format("listener.{}.", observability_name), false, {}, std::move(scope_matcher));
+  } else {
+    listener_scope = stats.createScopeWithTaggedName("listener.admin.", {}, {}, false, {},
+                                                     std::move(scope_matcher));
+  }
 
   return std::make_pair(std::move(scope), std::move(listener_scope));
 }

source/common/secret/BUILD

@@ -61,6 +61,7 @@ envoy_cc_library(
         "//source/common/config:resource_type_helper_lib",
         "//source/common/config:utility_lib",
         "//source/common/config:watched_directory_lib",
+        "//source/common/config:well_known_names",
         "//source/common/init:target_lib",
         "//source/common/protobuf:utility_lib",
         "//source/common/ssl:certificate_validation_context_config_impl_lib",

source/common/secret/sds_api.cc

@@ -6,6 +6,7 @@
 
 #include "source/common/common/assert.h"
 #include "source/common/config/api_version.h"
+#include "source/common/config/well_known_names.h"
 #include "source/common/grpc/common.h"
 #include "source/common/protobuf/utility.h"
 
@@ -23,7 +24,11 @@ SdsApi::SdsApi(envoy::config::core::v3::ConfigSource sds_config, absl::string_vi
                bool warm)
     : init_target_(fmt::format("SdsApi {}", sds_config_name), [this, warm] { initialize(warm); }),
       dispatcher_(dispatcher), api_(api),
-      scope_(stats.createScope(absl::StrCat("sds.", sds_config_name, "."))),
+      // sds.[<resource_name>.]**
+      scope_(stats.createScopeWithTaggedName(
+          "sds",
+          {Stats::TagStringView{Envoy::Config::TagNames::get().XDS_RESOURCE_NAME, sds_config_name}},
+          absl::StrCat("sds.", sds_config_name, "."))),
       sds_api_stats_(generateStats(*scope_)), resource_type_helper_(validation_visitor, "name"),
       sds_config_(std::move(sds_config)), sds_config_name_(sds_config_name),
       clean_up_(std::move(destructor_cb)), subscription_factory_(subscription_factory),

source/common/tcp_proxy/tcp_proxy.cc

@@ -134,7 +134,11 @@ OnDemandStats OnDemandConfig::generateStats(Stats::Scope& scope) {
 Config::SharedConfig::SharedConfig(
     const envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy& config,
     Server::Configuration::FactoryContext& context)
-    : stats_scope_(context.scope().createScope(fmt::format("tcp.{}", config.stat_prefix()))),
+    : // tcp.(<stat_prefix>.)*
+      stats_scope_(context.scope().createScopeWithTaggedName(
+          "tcp",
+          {Stats::TagStringView{Envoy::Config::TagNames::get().TCP_PREFIX, config.stat_prefix()}},
+          fmt::format("tcp.{}", config.stat_prefix()))),
       stats_(generateStats(*stats_scope_)),
       flush_access_log_on_start_(config.access_log_options().flush_access_log_on_start()),
       proxy_protocol_tlv_merge_policy_(config.proxy_protocol_tlv_merge_policy()) {

source/common/upstream/upstream_impl.cc

@@ -42,6 +42,7 @@
 #include "source/common/common/fmt.h"
 #include "source/common/common/utility.h"
 #include "source/common/config/utility.h"
+#include "source/common/config/well_known_names.h"
 #include "source/common/http/http1/codec_stats.h"
 #include "source/common/http/http2/codec_stats.h"
 #include "source/common/http/utility.h"
@@ -465,10 +466,13 @@ generateStatsScope(const envoy::config::cluster::v3::Cluster& config,
     }
   }
 
-  return stats.createScope(fmt::format("cluster.{}.", (!config.alt_stat_name().empty())
-                                                          ? config.alt_stat_name()
-                                                          : config.name()),
-                           false, {}, std::move(scope_matcher));
+  absl::string_view observability_name =
+      !config.alt_stat_name().empty() ? config.alt_stat_name() : config.name();
+
+  // cluster.(<cluster_name>.)*
+  return stats.createScopeWithTaggedName(
+      "cluster", {Stats::TagStringView{Config::TagNames::get().CLUSTER_NAME, observability_name}},
+      fmt::format("cluster.{}.", observability_name), false, {}, std::move(scope_matcher));
 }
 
 // TODO(pianiststickman): this implementation takes a lock on the hot path and puts a copy of the

source/extensions/filters/network/redis_proxy/BUILD

@@ -161,6 +161,7 @@ envoy_cc_extension(
         ":proxy_filter_lib",
         ":router_lib",
         "//envoy/upstream:upstream_interface",
+        "//source/common/config:well_known_names",
         "//source/extensions/common/dynamic_forward_proxy:dns_cache_manager_impl",
         "//source/extensions/common/redis:cluster_refresh_manager_lib",
         "//source/extensions/filters/network:well_known_names",

source/extensions/filters/network/redis_proxy/config.cc

@@ -4,6 +4,7 @@
 #include "envoy/extensions/filters/network/redis_proxy/v3/redis_proxy.pb.h"
 #include "envoy/extensions/filters/network/redis_proxy/v3/redis_proxy.pb.validate.h"
 
+#include "source/common/config/well_known_names.h"
 #include "source/extensions/common/dynamic_forward_proxy/dns_cache_manager_impl.h"
 #include "source/extensions/common/redis/cluster_refresh_manager_impl.h"
 #include "source/extensions/filters/network/common/redis/aws_iam_authenticator_impl.h"
@@ -100,8 +101,11 @@ Network::FilterFactoryCb RedisProxyFilterConfigFactory::createFilterFactoryFromP
       }
     }
 
-    Stats::ScopeSharedPtr stats_scope =
-        context.scope().createScope(fmt::format("cluster.{}.redis_cluster", cluster));
+    // cluster.(<cluster_name>.)*
+    Stats::ScopeSharedPtr stats_scope = context.scope().createScopeWithTaggedName(
+        "cluster.redis_cluster",
+        {Stats::TagStringView{Envoy::Config::TagNames::get().CLUSTER_NAME, cluster}},
+        fmt::format("cluster.{}.redis_cluster", cluster));
     // Get zone from LocalInfo for fallback when client_zone not explicitly configured
     const std::string& local_zone = server_context.localInfo().zoneName();
     auto conn_pool_ptr = std::make_shared<ConnPool::InstanceImpl>(

test/common/grpc/async_client_manager_impl_test.cc

@@ -363,7 +363,7 @@ TEST_F(AsyncClientManagerImplTest, EnvoyGrpcInvalid) {
 
 TEST_F(AsyncClientManagerImplTest, GoogleGrpc) {
   initialize();
-  EXPECT_CALL(scope_, createScope_("grpc.foo."));
+  EXPECT_CALL(scope_, createScope_("grpc", "grpc.foo."));
   envoy::config::core::v3::GrpcService grpc_service;
   grpc_service.mutable_google_grpc()->set_stat_prefix("foo");
 
@@ -379,7 +379,7 @@ TEST_F(AsyncClientManagerImplTest, GoogleGrpc) {
 
 TEST_F(AsyncClientManagerImplTest, GoogleGrpcIllegalCharsInKey) {
   initialize();
-  EXPECT_CALL(scope_, createScope_("grpc.foo."));
+  EXPECT_CALL(scope_, createScope_("grpc", "grpc.foo."));
   envoy::config::core::v3::GrpcService grpc_service;
   grpc_service.mutable_google_grpc()->set_stat_prefix("foo");
 
@@ -400,7 +400,7 @@ TEST_F(AsyncClientManagerImplTest, GoogleGrpcIllegalCharsInKey) {
 
 TEST_F(AsyncClientManagerImplTest, LegalGoogleGrpcChar) {
   initialize();
-  EXPECT_CALL(scope_, createScope_("grpc.foo."));
+  EXPECT_CALL(scope_, createScope_("grpc", "grpc.foo."));
   envoy::config::core::v3::GrpcService grpc_service;
   grpc_service.mutable_google_grpc()->set_stat_prefix("foo");
 
@@ -420,7 +420,7 @@ TEST_F(AsyncClientManagerImplTest, LegalGoogleGrpcChar) {
 
 TEST_F(AsyncClientManagerImplTest, GoogleGrpcIllegalCharsInValue) {
   initialize();
-  EXPECT_CALL(scope_, createScope_("grpc.foo."));
+  EXPECT_CALL(scope_, createScope_("grpc", "grpc.foo."));
   envoy::config::core::v3::GrpcService grpc_service;
   grpc_service.mutable_google_grpc()->set_stat_prefix("foo");
 

test/extensions/access_loggers/stats/scope_provider_singleton_test.cc

@@ -52,8 +52,8 @@ TEST_F(ScopeProviderSingletonTest, GetScopeCachesAndReturnsSameScope) {
 
   std::shared_ptr<Stats::MockScope> newly_created_scope;
 
-  EXPECT_CALL(mock_store_.mockScope(), createScope_(_))
-      .WillOnce(Invoke([&](const std::string& name) {
+  EXPECT_CALL(mock_store_.mockScope(), createScope_(_, _))
+      .WillOnce(Invoke([&](const std::string& name, const std::string&) {
         auto scope_name_storage =
             std::make_unique<Stats::StatNameDynamicStorage>(name, mock_store_.symbolTable());
         newly_created_scope = std::shared_ptr<Stats::MockScope>(
@@ -84,8 +84,8 @@ TEST_F(ScopeProviderSingletonTest, GetScopeCachesAndReturnsSameScope) {
   scope_config3.mutable_max_histograms()->set_value(30);
 
   std::shared_ptr<Stats::MockScope> newly_created_scope3;
-  EXPECT_CALL(mock_store_.mockScope(), createScope_(_))
-      .WillOnce(Invoke([&](const std::string& name) {
+  EXPECT_CALL(mock_store_.mockScope(), createScope_(_, _))
+      .WillOnce(Invoke([&](const std::string& name, const std::string&) {
         auto scope_name_storage =
             std::make_unique<Stats::StatNameDynamicStorage>(name, mock_store_.symbolTable());
         newly_created_scope3 = std::shared_ptr<Stats::MockScope>(
@@ -111,9 +111,9 @@ TEST_F(ScopeProviderSingletonTest, CleansUpExpiredScopesOnNextGetScope) {
   EXPECT_CALL(factory_context_, statsScope()).WillRepeatedly(ReturnRef(mock_store_.mockScope()));
 
   // First request should trigger createScope_
-  EXPECT_CALL(mock_store_.mockScope(), createScope_(_))
+  EXPECT_CALL(mock_store_.mockScope(), createScope_(_, _))
       .Times(1)
-      .WillOnce(Invoke([&](const std::string& name) {
+      .WillOnce(Invoke([&](const std::string& name, const std::string&) {
         auto scope_name_storage =
             std::make_unique<Stats::StatNameDynamicStorage>(name, mock_store_.symbolTable());
         return std::shared_ptr<Stats::Scope>(
@@ -124,7 +124,7 @@ TEST_F(ScopeProviderSingletonTest, CleansUpExpiredScopesOnNextGetScope) {
   EXPECT_NE(returned_scope, nullptr);
 
   // While returned_scope is alive, requesting again should not trigger createScope_
-  EXPECT_CALL(mock_store_.mockScope(), createScope_(_)).Times(0);
+  EXPECT_CALL(mock_store_.mockScope(), createScope_(_, _)).Times(0);
   auto returned_scope2 = Stats::ScopeProviderSingleton::getScope(factory_context_, scope_config);
   EXPECT_EQ(returned_scope, returned_scope2);
 
@@ -136,9 +136,9 @@ TEST_F(ScopeProviderSingletonTest, CleansUpExpiredScopesOnNextGetScope) {
   returned_scope2.reset();
 
   // Next request should trigger a new createScope_ because it was deleted
-  EXPECT_CALL(mock_store_.mockScope(), createScope_(_))
+  EXPECT_CALL(mock_store_.mockScope(), createScope_(_, _))
       .Times(1)
-      .WillOnce(Invoke([&](const std::string& name) {
+      .WillOnce(Invoke([&](const std::string& name, const std::string&) {
         auto scope_name_storage =
             std::make_unique<Stats::StatNameDynamicStorage>(name, mock_store_.symbolTable());
         return std::shared_ptr<Stats::Scope>(
@@ -159,9 +159,9 @@ TEST_F(ScopeProviderSingletonTest, GetScopeWithSharingDisabledDoesNotCache) {
   EXPECT_CALL(factory_context_, statsScope()).WillRepeatedly(ReturnRef(mock_store_.mockScope()));
 
   // First request should create scope
-  EXPECT_CALL(mock_store_.mockScope(), createScope_(_))
+  EXPECT_CALL(mock_store_.mockScope(), createScope_(_, _))
       .Times(1)
-      .WillOnce(Invoke([&](const std::string& name) {
+      .WillOnce(Invoke([&](const std::string& name, const std::string&) {
         auto scope_name_storage =
             std::make_unique<Stats::StatNameDynamicStorage>(name, mock_store_.symbolTable());
         return std::shared_ptr<Stats::Scope>(
@@ -172,9 +172,9 @@ TEST_F(ScopeProviderSingletonTest, GetScopeWithSharingDisabledDoesNotCache) {
   EXPECT_NE(returned_scope1, nullptr);
 
   // Second request should also create scope since sharing is disabled
-  EXPECT_CALL(mock_store_.mockScope(), createScope_(_))
+  EXPECT_CALL(mock_store_.mockScope(), createScope_(_, _))
       .Times(1)
-      .WillOnce(Invoke([&](const std::string& name) {
+      .WillOnce(Invoke([&](const std::string& name, const std::string&) {
         auto scope_name_storage =
             std::make_unique<Stats::StatNameDynamicStorage>(name, mock_store_.symbolTable());
         return std::shared_ptr<Stats::Scope>(
@@ -197,8 +197,8 @@ TEST_F(ScopeProviderSingletonTest, SupportsGetSharedAndCopying) {
   EXPECT_CALL(factory_context_.server_context_, mainThreadDispatcher())
       .WillRepeatedly(ReturnRef(factory_context_.server_context_.dispatcher_));
 
-  EXPECT_CALL(mock_store_.mockScope(), createScope_(_))
-      .WillOnce(Invoke([&](const std::string& name) {
+  EXPECT_CALL(mock_store_.mockScope(), createScope_(_, _))
+      .WillOnce(Invoke([&](const std::string& name, const std::string&) {
         auto scope_name_storage =
             std::make_unique<Stats::StatNameDynamicStorage>(name, mock_store_.symbolTable());
         return std::shared_ptr<Stats::Scope>(

test/extensions/access_loggers/stats/stats_test.cc

@@ -129,8 +129,8 @@ class StatsAccessLoggerTest : public testing::Test {
         .WillByDefault(testing::ReturnRef(store_.mockScope()));
     ON_CALL(context_, serverScope()).WillByDefault(testing::ReturnRef(store_.mockScope()));
 
-    EXPECT_CALL(store_.mockScope(), createScope_(_))
-        .WillRepeatedly(Invoke([this](const std::string& name) {
+    EXPECT_CALL(store_.mockScope(), createScope_(_, _))
+        .WillRepeatedly(Invoke([this](const std::string& name, const std::string&) {
           auto scope_name_storage =
               std::make_unique<Stats::StatNameDynamicStorage>(name, context_.store_.symbolTable());
           auto scope = std::make_shared<NiceMock<MockScopeWithGauge>>(

test/mocks/stats/mocks.h

@@ -298,21 +298,23 @@ class MockScope : public TestUtil::TestScope {
   MockScope(StatName prefix, MockStore& store);
 
   ScopeSharedPtr createScopeWithTaggedName(absl::string_view base_name, TagStringViewSpan,
-                                           absl::string_view, bool evictable,
+                                           absl::string_view tagged_name, bool evictable,
                                            const ScopeStatsLimitSettings& limits,
                                            StatsMatcherSharedPtr) override {
     checkCreateScopeArgs(evictable, limits);
-    return ScopeSharedPtr(createScope_(std::string(base_name)));
+    return ScopeSharedPtr(createScope_(std::string(base_name), std::string(tagged_name)));
   }
-  ScopeSharedPtr scopeFromTaggedName(StatName base_name, StatNameTagSpan, StatName, bool evictable,
-                                     const ScopeStatsLimitSettings& limits,
+  ScopeSharedPtr scopeFromTaggedName(StatName base_name, StatNameTagSpan, StatName tagged_name,
+                                     bool evictable, const ScopeStatsLimitSettings& limits,
                                      StatsMatcherSharedPtr) override {
     checkCreateScopeArgs(evictable, limits);
-    return createScope_(symbolTable().toString(base_name));
+    return createScope_(symbolTable().toString(base_name),
+                        tagged_name.empty() ? std::string() : symbolTable().toString(tagged_name));
   }
 
   MOCK_METHOD(void, checkCreateScopeArgs, (bool, const ScopeStatsLimitSettings&));
-  MOCK_METHOD(ScopeSharedPtr, createScope_, (const std::string& name));
+  MOCK_METHOD(ScopeSharedPtr, createScope_,
+              (const std::string& base_name, const std::string& tagged_name));
   MOCK_METHOD(CounterOptConstRef, findCounter, (StatName), (const));
   MOCK_METHOD(GaugeOptConstRef, findGauge, (StatName), (const));
   MOCK_METHOD(HistogramOptConstRef, findHistogram, (StatName), (const));