envoyproxy · jmarantz · Jun 27, 2023 · Oct 17, 2022 · Oct 17, 2022 · Oct 19, 2022
diff --git a/api/envoy/config/bootstrap/v3/bootstrap.proto b/api/envoy/config/bootstrap/v3/bootstrap.proto
@@ -199,7 +199,6 @@ message Bootstrap {
   repeated metrics.v3.StatsSink stats_sinks = 6;
 
   // Options to control behaviors of deferred creation compatible stats.
-  // [#not-implemented-hide:]
   DeferredStatOptions deferred_stat_options = 39;
 
   // Configuration for internal processing of stats.

diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -459,3 +459,8 @@ deprecated:
   change: |
     deprecated the :ref:`HealthCheck event_log_path <envoy_v3_api_field_config.core.v3.HealthCheck.event_log_path>` in favor of
     :ref:`HealthCheck event_logger extension <envoy_v3_api_field_config.core.v3.HealthCheck.event_logger>`.
+- area: stats
+  change: |
+    added :ref:`enable_deferred_creation_stats
+    <envoy_v3_api_field_config.bootstrap.v3.Bootstrap.DeferredStatOptions.enable_deferred_creation_stats>`.
+    support for ClusterTrafficStats.
diff --git a/envoy/upstream/upstream.h b/envoy/upstream/upstream.h
@@ -789,12 +789,8 @@ MAKE_STATS_STRUCT(ClusterLbStats, ClusterLbStatNames, ALL_CLUSTER_LB_STATS);
  */
 MAKE_STAT_NAMES_STRUCT(ClusterTrafficStatNames, ALL_CLUSTER_TRAFFIC_STATS);
 MAKE_STATS_STRUCT(ClusterTrafficStats, ClusterTrafficStatNames, ALL_CLUSTER_TRAFFIC_STATS);
-/*
- * NOTE: LazyClusterTrafficStats for now is an alias of "std::unique_ptr<ClusterTrafficStats>",
- * this is to make way for future lazy-init on trafficStats(). See
- * https://github.com/envoyproxy/envoy/pull/23921#issuecomment-1335239116 for more context.
- */
-using LazyClusterTrafficStats = std::unique_ptr<ClusterTrafficStats>;
+using DeferredCreationCompatibleClusterTrafficStats =
+    Stats::DeferredCreationCompatibleStats<ClusterTrafficStats>;
 
 MAKE_STAT_NAMES_STRUCT(ClusterLoadReportStatNames, ALL_CLUSTER_LOAD_REPORT_STATS);
 MAKE_STATS_STRUCT(ClusterLoadReportStats, ClusterLoadReportStatNames,
@@ -1097,8 +1093,7 @@ class ClusterInfo : public Http::FilterChainFactory {
   /**
    * @return  all traffic related stats for this cluster.
    */
-  virtual LazyClusterTrafficStats& trafficStats() const PURE;
-
+  virtual DeferredCreationCompatibleClusterTrafficStats& trafficStats() const PURE;
   /**
    * @return the stats scope that contains all cluster stats. This can be used to produce dynamic
    *         stats that will be freed when the cluster is removed.

diff --git a/source/common/stats/deferred_creation.h b/source/common/stats/deferred_creation.h
@@ -105,8 +105,8 @@ template <typename StatsStructType>
 DeferredCreationCompatibleStats<StatsStructType>
 createDeferredCompatibleStats(Stats::ScopeSharedPtr scope,
                               const typename StatsStructType::StatNameType& stat_names,
-                              bool deferred_creation) {
-  if (deferred_creation) {
+                              bool defer_creation) {
+  if (defer_creation) {
     return DeferredCreationCompatibleStats<StatsStructType>(
         std::make_unique<DeferredStats<StatsStructType>>(stat_names, scope));
   } else {

diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD
@@ -79,14 +79,12 @@ envoy_cc_library(
     srcs = ["cluster_manager_impl.cc"],
     hdrs = ["cluster_manager_impl.h"],
     deps = [
-        "//source/extensions/filters/network/http_connection_manager:config",
-        "//source/common/config:null_grpc_mux_lib",
         ":cds_api_lib",
         ":cluster_discovery_manager_lib",
+        ":host_utility_lib",
         ":load_balancer_lib",
         ":load_stats_reporter_lib",
         ":od_cds_api_lib",
-        ":host_utility_lib",
         "//envoy/api:api_interface",
         "//envoy/config:xds_resources_delegate_interface",
         "//envoy/event:dispatcher_interface",
@@ -102,6 +100,7 @@ envoy_cc_library(
         "//source/common/common:enum_to_int",
         "//source/common/common:utility_lib",
         "//source/common/config:custom_config_validators_lib",
+        "//source/common/config:null_grpc_mux_lib",
         "//source/common/config:subscription_factory_lib",
         "//source/common/config:utility_lib",
         "//source/common/config:xds_resource_lib",
@@ -114,14 +113,15 @@ envoy_cc_library(
         "//source/common/network:resolver_lib",
         "//source/common/network:utility_lib",
         "//source/common/protobuf:utility_lib",
+        "//source/common/quic:quic_stat_names_lib",
         "//source/common/router:context_lib",
         "//source/common/router:shadow_writer_lib",
         "//source/common/shared_pool:shared_pool_lib",
-        "//source/common/tcp:conn_pool_lib",
         "//source/common/tcp:async_tcp_client_lib",
+        "//source/common/tcp:conn_pool_lib",
         "//source/common/upstream:priority_conn_pool_map_impl_lib",
         "//source/common/upstream:upstream_lib",
-        "//source/common/quic:quic_stat_names_lib",
+        "//source/extensions/filters/network/http_connection_manager:config",
         "//source/server:factory_context_base_impl_lib",
         "@envoy_api//envoy/admin/v3:pkg_cc_proto",
         "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
@@ -399,6 +399,7 @@ envoy_cc_library(
         ":transport_socket_match_lib",
         "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
+        "//source/common/stats:deferred_creation",
         "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto",
         "@envoy_api//envoy/extensions/filters/http/upstream_codec/v3:pkg_cc_proto",
         "@envoy_api//envoy/extensions/transport_sockets/raw_buffer/v3:pkg_cc_proto",
@@ -479,6 +480,7 @@ envoy_cc_library(
         "//source/common/http/http3:codec_stats_lib",
         "//source/common/init:manager_lib",
         "//source/common/shared_pool:shared_pool_lib",
+        "//source/common/stats:deferred_creation",
         "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stats_lib",
         "//source/extensions/filters/network/http_connection_manager:config",

diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc
@@ -48,6 +48,7 @@
 #include "source/common/router/config_utility.h"
 #include "source/common/runtime/runtime_features.h"
 #include "source/common/runtime/runtime_impl.h"
+#include "source/common/stats/deferred_creation.h"
 #include "source/common/upstream/cluster_factory_impl.h"
 #include "source/common/upstream/health_checker_impl.h"
 #include "source/extensions/filters/network/http_connection_manager/config.h"
@@ -863,9 +864,11 @@ void MainPrioritySetImpl::updateCrossPriorityHostMap(const HostVector& hosts_add
   }
 }
 
-LazyClusterTrafficStats ClusterInfoImpl::generateStats(Stats::Scope& scope,
-                                                       const ClusterTrafficStatNames& stat_names) {
-  return std::make_unique<ClusterTrafficStats>(stat_names, scope);
+DeferredCreationCompatibleClusterTrafficStats
+ClusterInfoImpl::generateStats(Stats::ScopeSharedPtr scope,
+                               const ClusterTrafficStatNames& stat_names, bool defer_creation) {
+  return Stats::createDeferredCompatibleStats<ClusterTrafficStats>(scope, stat_names,
+                                                                   defer_creation);
 }
 
 ClusterRequestResponseSizeStats ClusterInfoImpl::generateRequestResponseSizeStats(
@@ -1042,8 +1045,9 @@ ClusterInfoImpl::ClusterInfoImpl(
       peekahead_ratio_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config.preconnect_policy(),
                                                        predictive_preconnect_ratio, 0)),
       socket_matcher_(std::move(socket_matcher)), stats_scope_(std::move(stats_scope)),
-      traffic_stats_(
-          generateStats(*stats_scope_, factory_context.clusterManager().clusterStatNames())),
+      traffic_stats_(generateStats(stats_scope_,
+                                   factory_context.clusterManager().clusterStatNames(),
+                                   server_context.statsConfig().enableDeferredCreationStats())),
       config_update_stats_(factory_context.clusterManager().clusterConfigUpdateStatNames(),
                            *stats_scope_),
       lb_stats_(factory_context.clusterManager().clusterLbStatNames(), *stats_scope_),

diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h
@@ -789,8 +789,9 @@ class ClusterInfoImpl : public ClusterInfo,
                   Stats::ScopeSharedPtr&& stats_scope, bool added_via_api,
                   Server::Configuration::TransportSocketFactoryContext&);
 
-  static LazyClusterTrafficStats generateStats(Stats::Scope& scope,
-                                               const ClusterTrafficStatNames& cluster_stat_names);
+  static DeferredCreationCompatibleClusterTrafficStats
+  generateStats(Stats::ScopeSharedPtr scope, const ClusterTrafficStatNames& cluster_stat_names,
+                bool defer_creation);
   static ClusterLoadReportStats
   generateLoadReportStats(Stats::Scope& scope, const ClusterLoadReportStatNames& stat_names);
   static ClusterCircuitBreakersStats
@@ -911,7 +912,9 @@ class ClusterInfoImpl : public ClusterInfo,
   }
   ResourceManager& resourceManager(ResourcePriority priority) const override;
   TransportSocketMatcher& transportSocketMatcher() const override { return *socket_matcher_; }
-  LazyClusterTrafficStats& trafficStats() const override { return traffic_stats_; }
+  DeferredCreationCompatibleClusterTrafficStats& trafficStats() const override {
+    return traffic_stats_;
+  }
   ClusterConfigUpdateStats& configUpdateStats() const override { return config_update_stats_; }
   ClusterLbStats& lbStats() const override { return lb_stats_; }
   ClusterEndpointStats& endpointStats() const override { return endpoint_stats_; }
@@ -1056,7 +1059,7 @@ class ClusterInfoImpl : public ClusterInfo,
   const float peekahead_ratio_;
   TransportSocketMatcherPtr socket_matcher_;
   Stats::ScopeSharedPtr stats_scope_;
-  mutable LazyClusterTrafficStats traffic_stats_;
+  mutable DeferredCreationCompatibleClusterTrafficStats traffic_stats_;
   mutable ClusterConfigUpdateStats config_update_stats_;
   mutable ClusterLbStats lb_stats_;
   mutable ClusterEndpointStats endpoint_stats_;

@@ -34,8 +34,8 @@ class AggregateClusterTest : public Event::TestUsingSimulatedTime, public testin
 public:
   AggregateClusterTest()
       : stat_names_(server_context_.store_.symbolTable()),
-        stats_(Upstream::ClusterInfoImpl::generateStats(*server_context_.store_.rootScope(),
-                                                        stat_names_)) {
+        traffic_stats_(Upstream::ClusterInfoImpl::generateStats(server_context_.store_.rootScope(),
+                                                                stat_names_, false)) {
     ON_CALL(*primary_info_, name()).WillByDefault(ReturnRef(primary_name));
     ON_CALL(*secondary_info_, name()).WillByDefault(ReturnRef(secondary_name));
 
@@ -138,7 +138,7 @@ class AggregateClusterTest : public Event::TestUsingSimulatedTime, public testin
   Upstream::LoadBalancerFactorySharedPtr lb_factory_;
   Upstream::LoadBalancerPtr lb_;
   Upstream::ClusterTrafficStatNames stat_names_;
-  Upstream::LazyClusterTrafficStats stats_;
+  Upstream::DeferredCreationCompatibleClusterTrafficStats traffic_stats_;
   std::shared_ptr<Upstream::MockClusterInfo> primary_info_{
       new NiceMock<Upstream::MockClusterInfo>()};
   std::shared_ptr<Upstream::MockClusterInfo> secondary_info_{

@@ -280,8 +280,9 @@ TEST_F(ProxyFilterTest, CircuitBreakerOverflowWithDnsCacheResourceManager) {
             filter2->decodeHeaders(request_headers_, false));
 
   // Cluster circuit breaker overflow counter won't be incremented.
-  EXPECT_EQ(0, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_
-                   ->traffic_stats_->upstream_rq_pending_overflow_.value());
+  EXPECT_EQ(0,
+            factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->trafficStats()
+                ->upstream_rq_pending_overflow_.value());
   filter2->onDestroy();
   EXPECT_CALL(*handle, onDestroy());
   filter_->onDestroy();