From fba6b7ad5da92fb10700ac6d5f48ad2bf52db186 Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Tue, 6 Dec 2022 20:11:39 -0500 Subject: [PATCH 01/12] make ClusterInfo::traffic_stats_ a unique_ptr, so that later we can LazyInit it Signed-off-by: Xin Zhuang --- api/envoy/config/metrics/v3/stats.proto | 2 + envoy/stats/stats_macros.h | 4 +- envoy/upstream/upstream.h | 4 +- source/common/conn_pool/conn_pool_base.cc | 44 +++---- source/common/http/codec_client.cc | 2 +- source/common/http/codec_client.h | 2 +- source/common/http/conn_pool_base.cc | 8 +- source/common/http/conn_pool_base.h | 9 +- source/common/http/conn_pool_grid.cc | 2 +- source/common/http/http1/conn_pool.cc | 4 +- source/common/http/http2/conn_pool.cc | 2 +- source/common/http/http3/conn_pool.cc | 2 +- source/common/router/retry_state_impl.cc | 12 +- source/common/router/router.cc | 14 +-- source/common/router/upstream_codec_filter.cc | 4 +- source/common/router/upstream_request.cc | 12 +- source/common/tcp/conn_pool.cc | 11 +- source/common/tcp_proxy/tcp_proxy.cc | 10 +- .../common/upstream/cluster_manager_impl.cc | 6 +- source/common/upstream/conn_pool_map_impl.h | 2 +- .../upstream/health_checker_base_impl.cc | 2 +- .../common/upstream/thread_aware_lb_impl.cc | 2 +- source/common/upstream/upstream_impl.cc | 23 ++-- source/common/upstream/upstream_impl.h | 8 +- .../original_dst/original_dst_cluster.cc | 2 +- .../http/grpc_stats/grpc_stats_filter.cc | 8 +- .../network/common/redis/client_impl.cc | 24 ++-- .../network/redis_proxy/conn_pool_impl.cc | 12 +- .../filters/udp/udp_proxy/udp_proxy_filter.cc | 10 +- .../stat_sinks/common/statsd/statsd.cc | 15 ++- .../extensions/stat_sinks/hystrix/hystrix.cc | 2 +- test/common/conn_pool/conn_pool_base_test.cc | 20 +-- test/common/http/codec_client_test.cc | 6 +- test/common/http/http1/conn_pool_test.cc | 48 +++---- test/common/http/http2/conn_pool_test.cc | 82 ++++++------ test/common/router/retry_state_impl_test.cc | 58 ++++----- test/common/stats/BUILD | 17 ++- test/common/stats/thread_local_store_test.cc | 63 +--------- test/common/tcp/conn_pool_test.cc | 20 +-- .../upstream/conn_pool_map_impl_test.cc | 6 +- test/common/upstream/health_check_fuzz.cc | 6 +- .../upstream/health_checker_impl_test.cc | 106 ++++++++-------- test/common/upstream/upstream_impl_test.cc | 6 +- .../clusters/aggregate/cluster_test.cc | 2 +- .../original_dst/original_dst_cluster_test.cc | 5 +- .../proxy_filter_test.cc | 4 +- .../gcp_authn_filter_integration_test.cc | 4 +- .../network/common/redis/client_impl_test.cc | 117 ++++++++++-------- .../udp/udp_proxy/udp_proxy_filter_test.cc | 40 +++--- .../stats_sinks/common/statsd/statsd_test.cc | 6 +- .../stats_sinks/hystrix/hystrix_test.cc | 6 +- test/integration/base_integration_test.h | 26 ++++ test/integration/cds_integration_test.cc | 2 + test/integration/integration_admin_test.cc | 1 + test/integration/integration_test.cc | 8 +- test/integration/xfcc_integration_test.cc | 2 +- test/mocks/upstream/cluster_info.cc | 6 +- test/mocks/upstream/cluster_info.h | 6 +- test/test_common/real_threads_test_helper.h | 2 + tools/spelling/spelling_dictionary.txt | 1 + 60 files changed, 475 insertions(+), 465 deletions(-) diff --git a/api/envoy/config/metrics/v3/stats.proto b/api/envoy/config/metrics/v3/stats.proto index fb73e91f8f998..388ad1275fb75 100644 --- a/api/envoy/config/metrics/v3/stats.proto +++ b/api/envoy/config/metrics/v3/stats.proto @@ -109,6 +109,8 @@ message StatsConfig { // 3600000 // ] repeated HistogramBucketSettings histogram_bucket_settings = 4; + // When true, enable lazy init feature for stats that's of Stats::LazyInit type. + bool enable_lazy_init = 5; } // Configuration for disabling stat instantiation. diff --git a/envoy/stats/stats_macros.h b/envoy/stats/stats_macros.h index 6ffca3ea0d118..dd36d903ff5db 100644 --- a/envoy/stats/stats_macros.h +++ b/envoy/stats/stats_macros.h @@ -152,6 +152,7 @@ static inline std::string statPrefixJoin(absl::string_view prefix, absl::string_ */ #define MAKE_STATS_STRUCT(StatsStruct, StatNamesStruct, ALL_STATS) \ struct StatsStruct { \ + using StatNameType = StatNamesStruct; \ StatsStruct(const StatNamesStruct& stat_names, Envoy::Stats::Scope& scope, \ Envoy::Stats::StatName prefix = Envoy::Stats::StatName()) \ : stat_names_(stat_names) \ @@ -159,9 +160,8 @@ static inline std::string statPrefixJoin(absl::string_view prefix, absl::string_ MAKE_STATS_STRUCT_HISTOGRAM_HELPER_, \ MAKE_STATS_STRUCT_TEXT_READOUT_HELPER_, \ MAKE_STATS_STRUCT_STATNAME_HELPER_) {} \ - const StatNamesStruct& stat_names_; \ + const StatNameType& stat_names_; \ ALL_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT, GENERATE_HISTOGRAM_STRUCT, \ GENERATE_TEXT_READOUT_STRUCT, GENERATE_STATNAME_STRUCT) \ } - } // namespace Envoy diff --git a/envoy/upstream/upstream.h b/envoy/upstream/upstream.h index 521e78e40d084..2e81a6191b17a 100644 --- a/envoy/upstream/upstream.h +++ b/envoy/upstream/upstream.h @@ -1060,9 +1060,9 @@ class ClusterInfo : public Http::FilterChainFactory { virtual ClusterEndpointStats& endpointStats() const PURE; /** - * @return ClusterTrafficStats& all traffic related stats for this cluster. + * @return std::unique_ptr& all traffic related stats for this cluster. */ - virtual ClusterTrafficStats& trafficStats() const PURE; + virtual std::unique_ptr& trafficStats() const PURE; /** * @return the stats scope that contains all cluster stats. This can be used to produce dynamic diff --git a/source/common/conn_pool/conn_pool_base.cc b/source/common/conn_pool/conn_pool_base.cc index 31ee3f3aff524..e2d308ca4ad89 100644 --- a/source/common/conn_pool/conn_pool_base.cc +++ b/source/common/conn_pool/conn_pool_base.cc @@ -135,7 +135,7 @@ ConnPoolImplBase::tryCreateNewConnection(float global_preconnect_ratio) { const bool can_create_connection = host_->canCreateConnection(priority_); if (!can_create_connection) { - host_->cluster().trafficStats().upstream_cx_overflow_.inc(); + host_->cluster().trafficStats()->upstream_cx_overflow_.inc(); } // If we are at the connection circuit-breaker limit due to other upstreams having // too many open connections, and this upstream has no connections, always create one, to @@ -168,14 +168,14 @@ void ConnPoolImplBase::attachStreamToClient(Envoy::ConnectionPool::ActiveClient& ASSERT(client.readyForStream()); if (client.state() == Envoy::ConnectionPool::ActiveClient::State::ReadyForEarlyData) { - host_->cluster().trafficStats().upstream_rq_0rtt_.inc(); + host_->cluster().trafficStats()->upstream_rq_0rtt_.inc(); } if (enforceMaxRequests() && !host_->cluster().resourceManager(priority_).requests().canCreate()) { ENVOY_LOG(debug, "max streams overflow"); onPoolFailure(client.real_host_description_, absl::string_view(), ConnectionPool::PoolFailureReason::Overflow, context); - host_->cluster().trafficStats().upstream_rq_pending_overflow_.inc(); + host_->cluster().trafficStats()->upstream_rq_pending_overflow_.inc(); return; } ENVOY_CONN_LOG(debug, "creating stream", client); @@ -185,7 +185,7 @@ void ConnPoolImplBase::attachStreamToClient(Envoy::ConnectionPool::ActiveClient& client.remaining_streams_--; if (client.remaining_streams_ == 0) { ENVOY_CONN_LOG(debug, "maximum streams per connection, start draining", client); - host_->cluster().trafficStats().upstream_cx_max_requests_.inc(); + host_->cluster().trafficStats()->upstream_cx_max_requests_.inc(); transitionActiveClientState(client, Envoy::ConnectionPool::ActiveClient::State::Draining); } else if (capacity == 1) { // As soon as the new stream is created, the client will be maxed out. @@ -202,8 +202,8 @@ void ConnPoolImplBase::attachStreamToClient(Envoy::ConnectionPool::ActiveClient& num_active_streams_++; host_->stats().rq_total_.inc(); host_->stats().rq_active_.inc(); - host_->cluster().trafficStats().upstream_rq_total_.inc(); - host_->cluster().trafficStats().upstream_rq_active_.inc(); + host_->cluster().trafficStats()->upstream_rq_total_.inc(); + host_->cluster().trafficStats()->upstream_rq_active_.inc(); host_->cluster().resourceManager(priority_).requests().inc(); onPoolReady(client, context); @@ -216,7 +216,7 @@ void ConnPoolImplBase::onStreamClosed(Envoy::ConnectionPool::ActiveClient& clien state_.decrActiveStreams(1); num_active_streams_--; host_->stats().rq_active_.dec(); - host_->cluster().trafficStats().upstream_rq_active_.dec(); + host_->cluster().trafficStats()->upstream_rq_active_.dec(); host_->cluster().resourceManager(priority_).requests().dec(); // We don't update the capacity for HTTP/3 as the stream count should only // increase when a MAX_STREAMS frame is received. @@ -282,7 +282,7 @@ ConnectionPool::Cancellable* ConnPoolImplBase::newStreamImpl(AttachContext& cont ENVOY_LOG(debug, "max pending streams overflow"); onPoolFailure(nullptr, absl::string_view(), ConnectionPool::PoolFailureReason::Overflow, context); - host_->cluster().trafficStats().upstream_rq_pending_overflow_.inc(); + host_->cluster().trafficStats()->upstream_rq_pending_overflow_.inc(); return nullptr; } @@ -490,7 +490,7 @@ void ConnPoolImplBase::onConnectionEvent(ActiveClient& client, absl::string_view if (!client.hasHandshakeCompleted()) { client.has_handshake_completed_ = true; - host_->cluster().trafficStats().upstream_cx_connect_fail_.inc(); + host_->cluster().trafficStats()->upstream_cx_connect_fail_.inc(); host_->stats().cx_connect_fail_.inc(); onConnectFailed(client); @@ -595,7 +595,7 @@ void ConnPoolImplBase::onConnectionEvent(ActiveClient& client, absl::string_view client.currentUnusedCapacity()); // No need to update connecting capacity and connect_timer_ as the client is still connecting. ASSERT(client.state() == ActiveClient::State::Connecting); - host()->cluster().trafficStats().upstream_cx_connect_with_0_rtt_.inc(); + host()->cluster().trafficStats()->upstream_cx_connect_with_0_rtt_.inc(); transitionActiveClientState(client, (client.currentUnusedCapacity() > 0 ? ActiveClient::State::ReadyForEarlyData : ActiveClient::State::Busy)); @@ -606,13 +606,13 @@ void ConnPoolImplBase::onConnectionEvent(ActiveClient& client, absl::string_view PendingStream::PendingStream(ConnPoolImplBase& parent, bool can_send_early_data) : parent_(parent), can_send_early_data_(can_send_early_data) { - parent_.host()->cluster().trafficStats().upstream_rq_pending_total_.inc(); - parent_.host()->cluster().trafficStats().upstream_rq_pending_active_.inc(); + parent_.host()->cluster().trafficStats()->upstream_rq_pending_total_.inc(); + parent_.host()->cluster().trafficStats()->upstream_rq_pending_active_.inc(); parent_.host()->cluster().resourceManager(parent_.priority()).pendingRequests().inc(); } PendingStream::~PendingStream() { - parent_.host()->cluster().trafficStats().upstream_rq_pending_active_.dec(); + parent_.host()->cluster().trafficStats()->upstream_rq_pending_active_.dec(); parent_.host()->cluster().resourceManager(parent_.priority()).pendingRequests().dec(); } @@ -630,7 +630,7 @@ void ConnPoolImplBase::purgePendingStreams( while (!pending_streams_to_purge_.empty()) { PendingStreamPtr stream = pending_streams_to_purge_.front()->removeFromList(pending_streams_to_purge_); - host_->cluster().trafficStats().upstream_rq_pending_failure_eject_.inc(); + host_->cluster().trafficStats()->upstream_rq_pending_failure_eject_.inc(); onPoolFailure(host_description, failure_reason, reason, stream->context()); } } @@ -683,7 +683,7 @@ void ConnPoolImplBase::onPendingStreamCancel(PendingStream& stream, } } - host_->cluster().trafficStats().upstream_rq_cancelled_.inc(); + host_->cluster().trafficStats()->upstream_rq_cancelled_.inc(); checkForIdleAndCloseIdleConnsIfDraining(); } @@ -757,16 +757,16 @@ ActiveClient::ActiveClient(ConnPoolImplBase& parent, uint32_t lifetime_stream_li concurrent_stream_limit_(translateZeroToUnlimited(concurrent_stream_limit)), connect_timer_(parent_.dispatcher().createTimer([this]() { onConnectTimeout(); })) { conn_connect_ms_ = std::make_unique( - parent_.host()->cluster().trafficStats().upstream_cx_connect_ms_, + parent_.host()->cluster().trafficStats()->upstream_cx_connect_ms_, parent_.dispatcher().timeSource()); conn_length_ = std::make_unique( - parent_.host()->cluster().trafficStats().upstream_cx_length_ms_, + parent_.host()->cluster().trafficStats()->upstream_cx_length_ms_, parent_.dispatcher().timeSource()); connect_timer_->enableTimer(parent_.host()->cluster().connectTimeout()); parent_.host()->stats().cx_total_.inc(); parent_.host()->stats().cx_active_.inc(); - parent_.host()->cluster().trafficStats().upstream_cx_total_.inc(); - parent_.host()->cluster().trafficStats().upstream_cx_active_.inc(); + parent_.host()->cluster().trafficStats()->upstream_cx_total_.inc(); + parent_.host()->cluster().trafficStats()->upstream_cx_active_.inc(); parent_.host()->cluster().resourceManager(parent_.priority()).connections().inc(); } @@ -778,7 +778,7 @@ void ActiveClient::releaseResourcesBase() { conn_length_->complete(); - parent_.host()->cluster().trafficStats().upstream_cx_active_.dec(); + parent_.host()->cluster().trafficStats()->upstream_cx_active_.dec(); parent_.host()->stats().cx_active_.dec(); parent_.host()->cluster().resourceManager(parent_.priority()).connections().dec(); } @@ -786,7 +786,7 @@ void ActiveClient::releaseResourcesBase() { void ActiveClient::onConnectTimeout() { ENVOY_CONN_LOG(debug, "connect timeout", *this); - parent_.host()->cluster().trafficStats().upstream_cx_connect_timeout_.inc(); + parent_.host()->cluster().trafficStats()->upstream_cx_connect_timeout_.inc(); timed_out_ = true; close(); } @@ -811,7 +811,7 @@ void ActiveClient::onConnectionDurationTimeout() { } ENVOY_CONN_LOG(debug, "max connection duration reached, start draining", *this); - parent_.host()->cluster().trafficStats().upstream_cx_max_duration_reached_.inc(); + parent_.host()->cluster().trafficStats()->upstream_cx_max_duration_reached_.inc(); parent_.transitionActiveClientState(*this, Envoy::ConnectionPool::ActiveClient::State::Draining); // Close out the draining client if we no longer have active streams. diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc index 19c11106187bf..4369f94909094 100644 --- a/source/common/http/codec_client.cc +++ b/source/common/http/codec_client.cc @@ -171,7 +171,7 @@ void CodecClient::onData(Buffer::Instance& data) { if (!isPrematureResponseError(status) || (!active_requests_.empty() || getPrematureResponseHttpCode(status) != Code::RequestTimeout)) { - host_->cluster().trafficStats().upstream_cx_protocol_error_.inc(); + host_->cluster().trafficStats()->upstream_cx_protocol_error_.inc(); protocol_error_ = true; } close(); diff --git a/source/common/http/codec_client.h b/source/common/http/codec_client.h index 6dee7081f4ac9..65c3d9715f849 100644 --- a/source/common/http/codec_client.h +++ b/source/common/http/codec_client.h @@ -164,7 +164,7 @@ class CodecClient : protected Logger::Loggable, } void onIdleTimeout() { - host_->cluster().trafficStats().upstream_cx_idle_timeout_.inc(); + host_->cluster().trafficStats()->upstream_cx_idle_timeout_.inc(); close(); } diff --git a/source/common/http/conn_pool_base.cc b/source/common/http/conn_pool_base.cc index 0ed8e5ef1c2f7..afec14e51bf9b 100644 --- a/source/common/http/conn_pool_base.cc +++ b/source/common/http/conn_pool_base.cc @@ -99,7 +99,7 @@ static const uint64_t DEFAULT_MAX_STREAMS = (1 << 29); void MultiplexedActiveClientBase::onGoAway(Http::GoAwayErrorCode) { ENVOY_CONN_LOG(debug, "remote goaway", *codec_client_); - parent_.host()->cluster().trafficStats().upstream_cx_close_notify_.inc(); + parent_.host()->cluster().trafficStats()->upstream_cx_close_notify_.inc(); if (state() != ActiveClient::State::Draining) { if (codec_client_->numActiveRequests() == 0) { codec_client_->close(); @@ -160,16 +160,16 @@ void MultiplexedActiveClientBase::onStreamReset(Http::StreamResetReason reason) switch (reason) { case StreamResetReason::ConnectionTermination: case StreamResetReason::ConnectionFailure: - parent_.host()->cluster().trafficStats().upstream_rq_pending_failure_eject_.inc(); + parent_.host()->cluster().trafficStats()->upstream_rq_pending_failure_eject_.inc(); closed_with_active_rq_ = true; break; case StreamResetReason::LocalReset: case StreamResetReason::ProtocolError: case StreamResetReason::OverloadManager: - parent_.host()->cluster().trafficStats().upstream_rq_tx_reset_.inc(); + parent_.host()->cluster().trafficStats()->upstream_rq_tx_reset_.inc(); break; case StreamResetReason::RemoteReset: - parent_.host()->cluster().trafficStats().upstream_rq_rx_reset_.inc(); + parent_.host()->cluster().trafficStats()->upstream_rq_rx_reset_.inc(); break; case StreamResetReason::LocalRefusedStreamReset: case StreamResetReason::RemoteRefusedStreamReset: diff --git a/source/common/http/conn_pool_base.h b/source/common/http/conn_pool_base.h index 33e2faabf1df1..6222abe6ec36e 100644 --- a/source/common/http/conn_pool_base.h +++ b/source/common/http/conn_pool_base.h @@ -128,12 +128,11 @@ class ActiveClient : public Envoy::ConnectionPool::ActiveClient { real_host_description_ = data.host_description_; codec_client_ = parent.createCodecClient(data); codec_client_->addConnectionCallbacks(*this); + Upstream::ClusterTrafficStats& traffic_stats = *parent_.host()->cluster().trafficStats(); codec_client_->setConnectionStats( - {parent_.host()->cluster().trafficStats().upstream_cx_rx_bytes_total_, - parent_.host()->cluster().trafficStats().upstream_cx_rx_bytes_buffered_, - parent_.host()->cluster().trafficStats().upstream_cx_tx_bytes_total_, - parent_.host()->cluster().trafficStats().upstream_cx_tx_bytes_buffered_, - &parent_.host()->cluster().trafficStats().bind_errors_, nullptr}); + {traffic_stats.upstream_cx_rx_bytes_total_, traffic_stats.upstream_cx_rx_bytes_buffered_, + traffic_stats.upstream_cx_tx_bytes_total_, traffic_stats.upstream_cx_tx_bytes_buffered_, + &traffic_stats.bind_errors_, nullptr}); } absl::optional protocol() const override { return codec_client_->protocol(); } diff --git a/source/common/http/conn_pool_grid.cc b/source/common/http/conn_pool_grid.cc index 15db9536b94fb..1e6e6cf3419d0 100644 --- a/source/common/http/conn_pool_grid.cc +++ b/source/common/http/conn_pool_grid.cc @@ -382,7 +382,7 @@ HttpServerPropertiesCache::Http3StatusTracker& ConnectivityGrid::getHttp3StatusT bool ConnectivityGrid::isHttp3Broken() const { return getHttp3StatusTracker().isHttp3Broken(); } void ConnectivityGrid::markHttp3Broken() { - host_->cluster().trafficStats().upstream_http3_broken_.inc(); + host_->cluster().trafficStats()->upstream_http3_broken_.inc(); getHttp3StatusTracker().markHttp3Broken(); } diff --git a/source/common/http/http1/conn_pool.cc b/source/common/http/http1/conn_pool.cc index 77cf7ebcdd54b..0439641c472f2 100644 --- a/source/common/http/http1/conn_pool.cc +++ b/source/common/http/http1/conn_pool.cc @@ -42,7 +42,7 @@ void ActiveClient::StreamWrapper::decodeHeaders(ResponseHeaderMapPtr&& headers, close_connection_ = HeaderUtility::shouldCloseConnection(parent_.codec_client_->protocol(), *headers); if (close_connection_) { - parent_.parent().host()->cluster().trafficStats().upstream_cx_close_notify_.inc(); + parent_.parent().host()->cluster().trafficStats()->upstream_cx_close_notify_.inc(); } ResponseDecoderWrapper::decodeHeaders(std::move(headers), end_stream); } @@ -76,7 +76,7 @@ ActiveClient::ActiveClient(HttpConnPoolImplBase& parent, : Envoy::Http::ActiveClient(parent, parent.host()->cluster().maxRequestsPerConnection(), /* effective_concurrent_stream_limit */ 1, /* configured_concurrent_stream_limit */ 1, data) { - parent.host()->cluster().trafficStats().upstream_cx_http1_total_.inc(); + parent.host()->cluster().trafficStats()->upstream_cx_http1_total_.inc(); } ActiveClient::~ActiveClient() { ASSERT(!stream_wrapper_.get()); } diff --git a/source/common/http/http2/conn_pool.cc b/source/common/http/http2/conn_pool.cc index 67761b3889e23..865f0f0d6dcc2 100644 --- a/source/common/http/http2/conn_pool.cc +++ b/source/common/http/http2/conn_pool.cc @@ -45,7 +45,7 @@ ActiveClient::ActiveClient(HttpConnPoolImplBase& parent, : MultiplexedActiveClientBase( parent, calculateInitialStreamsLimit(parent.cache(), parent.origin(), parent.host()), parent.host()->cluster().http2Options().max_concurrent_streams().value(), - parent.host()->cluster().trafficStats().upstream_cx_http2_total_, data) {} + parent.host()->cluster().trafficStats()->upstream_cx_http2_total_, data) {} ConnectionPool::InstancePtr allocateConnPool(Event::Dispatcher& dispatcher, Random::RandomGenerator& random_generator, diff --git a/source/common/http/http3/conn_pool.cc b/source/common/http/http3/conn_pool.cc index 061f71b364871..ea7d365601386 100644 --- a/source/common/http/http3/conn_pool.cc +++ b/source/common/http/http3/conn_pool.cc @@ -41,7 +41,7 @@ ActiveClient::ActiveClient(Envoy::Http::HttpConnPoolImplBase& parent, Upstream::Host::CreateConnectionData& data) : MultiplexedActiveClientBase( parent, getMaxStreams(parent.host()->cluster()), getMaxStreams(parent.host()->cluster()), - parent.host()->cluster().trafficStats().upstream_cx_http3_total_, data), + parent.host()->cluster().trafficStats()->upstream_cx_http3_total_, data), async_connect_callback_(parent_.dispatcher().createSchedulableCallback([this]() { if (state() != Envoy::ConnectionPool::ActiveClient::State::Connecting) { return; diff --git a/source/common/router/retry_state_impl.cc b/source/common/router/retry_state_impl.cc index f4d3a868c749d..f45144832f629 100644 --- a/source/common/router/retry_state_impl.cc +++ b/source/common/router/retry_state_impl.cc @@ -185,13 +185,13 @@ void RetryStateImpl::enableBackoffTimer() { // be reused. ratelimited_backoff_strategy_.reset(); - cluster_.trafficStats().upstream_rq_retry_backoff_ratelimited_.inc(); + cluster_.trafficStats()->upstream_rq_retry_backoff_ratelimited_.inc(); } else { // Otherwise we use a fully jittered exponential backoff algorithm. retry_timer_->enableTimer(std::chrono::milliseconds(backoff_strategy_->nextBackOffMs())); - cluster_.trafficStats().upstream_rq_retry_backoff_exponential_.inc(); + cluster_.trafficStats()->upstream_rq_retry_backoff_exponential_.inc(); } } @@ -277,7 +277,7 @@ RetryStatus RetryStateImpl::shouldRetry(RetryDecision would_retry, DoRetryCallba // retry this particular request, we can infer that we did a retry earlier // and it was successful. if ((backoff_callback_ || next_loop_callback_) && would_retry == RetryDecision::NoRetry) { - cluster_.trafficStats().upstream_rq_retry_success_.inc(); + cluster_.trafficStats()->upstream_rq_retry_success_.inc(); if (vcluster_) { vcluster_->stats().upstream_rq_retry_success_.inc(); } @@ -295,7 +295,7 @@ RetryStatus RetryStateImpl::shouldRetry(RetryDecision would_retry, DoRetryCallba // The request has exhausted the number of retries allotted to it by the retry policy configured // (or the x-envoy-max-retries header). if (retries_remaining_ == 0) { - cluster_.trafficStats().upstream_rq_retry_limit_exceeded_.inc(); + cluster_.trafficStats()->upstream_rq_retry_limit_exceeded_.inc(); if (vcluster_) { vcluster_->stats().upstream_rq_retry_limit_exceeded_.inc(); } @@ -308,7 +308,7 @@ RetryStatus RetryStateImpl::shouldRetry(RetryDecision would_retry, DoRetryCallba retries_remaining_--; if (!cluster_.resourceManager(priority_).retries().canCreate()) { - cluster_.trafficStats().upstream_rq_retry_overflow_.inc(); + cluster_.trafficStats()->upstream_rq_retry_overflow_.inc(); if (vcluster_) { vcluster_->stats().upstream_rq_retry_overflow_.inc(); } @@ -324,7 +324,7 @@ RetryStatus RetryStateImpl::shouldRetry(RetryDecision would_retry, DoRetryCallba ASSERT(!backoff_callback_ && !next_loop_callback_); cluster_.resourceManager(priority_).retries().inc(); - cluster_.trafficStats().upstream_rq_retry_.inc(); + cluster_.trafficStats()->upstream_rq_retry_.inc(); if (vcluster_) { vcluster_->stats().upstream_rq_retry_.inc(); } diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 76972034a0631..3693e13bcfa7d 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -509,7 +509,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, modify_headers(headers); }, absl::nullopt, StreamInfo::ResponseCodeDetails::get().MaintenanceMode); - cluster_->trafficStats().upstream_rq_maintenance_mode_.inc(); + cluster_->trafficStats()->upstream_rq_maintenance_mode_.inc(); return Http::FilterHeadersStatus::StopIteration; } @@ -754,7 +754,7 @@ Http::FilterDataStatus Filter::decodeData(Buffer::Instance& data, bool end_strea "The request payload has at least {} bytes data which exceeds buffer limit {}. Give " "up on the retry/shadow.", getLength(callbacks_->decodingBuffer()) + data.length(), retry_shadow_buffer_limit_); - cluster_->trafficStats().retry_or_shadow_abandoned_.inc(); + cluster_->trafficStats()->retry_or_shadow_abandoned_.inc(); retry_state_.reset(); buffering = false; active_shadow_policies_.clear(); @@ -950,7 +950,7 @@ void Filter::onResponseTimeout() { // We want to record the upstream timeouts and increase the stats counters in all the cases. // For example, we also want to record the stats in the case of BiDi streaming APIs where we // might have already seen the headers. - cluster_->trafficStats().upstream_rq_timeout_.inc(); + cluster_->trafficStats()->upstream_rq_timeout_.inc(); if (request_vcluster_) { request_vcluster_->stats().upstream_rq_timeout_.inc(); } @@ -1024,12 +1024,12 @@ void Filter::onSoftPerTryTimeout(UpstreamRequest& upstream_request) { void Filter::onPerTryIdleTimeout(UpstreamRequest& upstream_request) { onPerTryTimeoutCommon(upstream_request, - cluster_->trafficStats().upstream_rq_per_try_idle_timeout_, + cluster_->trafficStats()->upstream_rq_per_try_idle_timeout_, StreamInfo::ResponseCodeDetails::get().UpstreamPerTryIdleTimeout); } void Filter::onPerTryTimeout(UpstreamRequest& upstream_request) { - onPerTryTimeoutCommon(upstream_request, cluster_->trafficStats().upstream_rq_per_try_timeout_, + onPerTryTimeoutCommon(upstream_request, cluster_->trafficStats()->upstream_rq_per_try_timeout_, StreamInfo::ResponseCodeDetails::get().UpstreamPerTryTimeout); } @@ -1618,7 +1618,7 @@ bool Filter::setupRedirect(const Http::ResponseHeaderMap& headers) { convertRequestHeadersForInternalRedirect(*downstream_headers_, *location, status_code) && callbacks_->recreateStream(&headers)) { ENVOY_STREAM_LOG(debug, "Internal redirect succeeded", *callbacks_); - cluster_->trafficStats().upstream_internal_redirect_succeeded_total_.inc(); + cluster_->trafficStats()->upstream_internal_redirect_succeeded_total_.inc(); return true; } // convertRequestHeadersForInternalRedirect logs failure reasons but log @@ -1631,7 +1631,7 @@ bool Filter::setupRedirect(const Http::ResponseHeaderMap& headers) { ENVOY_STREAM_LOG(trace, "Internal redirect failed: missing location header", *callbacks_); } - cluster_->trafficStats().upstream_internal_redirect_failed_total_.inc(); + cluster_->trafficStats()->upstream_internal_redirect_failed_total_.inc(); return false; } diff --git a/source/common/router/upstream_codec_filter.cc b/source/common/router/upstream_codec_filter.cc index 33994cbdce26d..df00ef8813da6 100644 --- a/source/common/router/upstream_codec_filter.cc +++ b/source/common/router/upstream_codec_filter.cc @@ -28,12 +28,12 @@ namespace Envoy { namespace Router { void UpstreamCodecFilter::onBelowWriteBufferLowWatermark() { - callbacks_->clusterInfo()->trafficStats().upstream_flow_control_resumed_reading_total_.inc(); + callbacks_->clusterInfo()->trafficStats()->upstream_flow_control_resumed_reading_total_.inc(); callbacks_->upstreamCallbacks()->upstream()->readDisable(false); } void UpstreamCodecFilter::onAboveWriteBufferHighWatermark() { - callbacks_->clusterInfo()->trafficStats().upstream_flow_control_paused_reading_total_.inc(); + callbacks_->clusterInfo()->trafficStats()->upstream_flow_control_paused_reading_total_.inc(); callbacks_->upstreamCallbacks()->upstream()->readDisable(true); } diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index 0d680148c5de9..6c41b82e3c1d3 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -208,7 +208,7 @@ void UpstreamRequest::cleanUp() { while (downstream_data_disabled_ != 0) { parent_.callbacks()->onDecoderFilterBelowWriteBufferLowWatermark(); - parent_.cluster()->trafficStats().upstream_flow_control_drained_total_.inc(); + parent_.cluster()->trafficStats()->upstream_flow_control_drained_total_.inc(); --downstream_data_disabled_; } if (allow_upstream_filters_) { @@ -764,7 +764,7 @@ UpstreamToDownstream& UpstreamRequest::upstreamToDownstream() { } void UpstreamRequest::onStreamMaxDurationReached() { - upstream_host_->cluster().trafficStats().upstream_rq_max_duration_reached_.inc(); + upstream_host_->cluster().trafficStats()->upstream_rq_max_duration_reached_.inc(); // The upstream had closed then try to retry along with retry policy. parent_.onStreamMaxDurationReached(*this); @@ -791,7 +791,7 @@ void UpstreamRequest::DownstreamWatermarkManager::onAboveWriteBufferHighWatermar // The downstream connection is overrun. Pause reads from upstream. // If there are multiple calls to readDisable either the codec (H2) or the underlying // Network::Connection (H1) will handle reference counting. - parent_.parent_.cluster()->trafficStats().upstream_flow_control_paused_reading_total_.inc(); + parent_.parent_.cluster()->trafficStats()->upstream_flow_control_paused_reading_total_.inc(); parent_.upstream_->readDisable(true); } @@ -800,7 +800,7 @@ void UpstreamRequest::DownstreamWatermarkManager::onBelowWriteBufferLowWatermark // One source of connection blockage has buffer available. Pass this on to the stream, which // will resume reads if this was the last remaining high watermark. - parent_.parent_.cluster()->trafficStats().upstream_flow_control_resumed_reading_total_.inc(); + parent_.parent_.cluster()->trafficStats()->upstream_flow_control_resumed_reading_total_.inc(); parent_.upstream_->readDisable(false); } @@ -815,7 +815,7 @@ void UpstreamRequest::disableDataFromDownstreamForFlowControl() { // the per try timeout timer is started only after downstream_end_stream_ // is true. ASSERT(parent_.upstreamRequests().size() == 1 || parent_.downstreamEndStream()); - parent_.cluster()->trafficStats().upstream_flow_control_backed_up_total_.inc(); + parent_.cluster()->trafficStats()->upstream_flow_control_backed_up_total_.inc(); parent_.callbacks()->onDecoderFilterAboveWriteBufferHighWatermark(); ++downstream_data_disabled_; } @@ -831,7 +831,7 @@ void UpstreamRequest::enableDataFromDownstreamForFlowControl() { // the per try timeout timer is started only after downstream_end_stream_ // is true. ASSERT(parent_.upstreamRequests().size() == 1 || parent_.downstreamEndStream()); - parent_.cluster()->trafficStats().upstream_flow_control_drained_total_.inc(); + parent_.cluster()->trafficStats()->upstream_flow_control_drained_total_.inc(); parent_.callbacks()->onDecoderFilterBelowWriteBufferLowWatermark(); ASSERT(downstream_data_disabled_ != 0); if (downstream_data_disabled_ > 0) { diff --git a/source/common/tcp/conn_pool.cc b/source/common/tcp/conn_pool.cc index 036315cc02922..35afe2d6e8e18 100644 --- a/source/common/tcp/conn_pool.cc +++ b/source/common/tcp/conn_pool.cc @@ -25,11 +25,12 @@ ActiveTcpClient::ActiveTcpClient(Envoy::ConnectionPool::ConnPoolImplBase& parent connection_->addConnectionCallbacks(*this); read_filter_handle_ = std::make_shared(*this); connection_->addReadFilter(read_filter_handle_); - connection_->setConnectionStats({host->cluster().trafficStats().upstream_cx_rx_bytes_total_, - host->cluster().trafficStats().upstream_cx_rx_bytes_buffered_, - host->cluster().trafficStats().upstream_cx_tx_bytes_total_, - host->cluster().trafficStats().upstream_cx_tx_bytes_buffered_, - &host->cluster().trafficStats().bind_errors_, nullptr}); + Upstream::ClusterTrafficStats& cluster_traffic_stats = *host->cluster().trafficStats(); + connection_->setConnectionStats({cluster_traffic_stats.upstream_cx_rx_bytes_total_, + cluster_traffic_stats.upstream_cx_rx_bytes_buffered_, + cluster_traffic_stats.upstream_cx_tx_bytes_total_, + cluster_traffic_stats.upstream_cx_tx_bytes_buffered_, + &cluster_traffic_stats.bind_errors_, nullptr}); connection_->noDelay(true); connection_->connect(); } diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index fc22377d4cff6..584e04a86dcb0 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -247,12 +247,12 @@ void Filter::readDisableUpstream(bool disable) { read_callbacks_->upstreamHost() ->cluster() .trafficStats() - .upstream_flow_control_paused_reading_total_.inc(); + ->upstream_flow_control_paused_reading_total_.inc(); } else { read_callbacks_->upstreamHost() ->cluster() .trafficStats() - .upstream_flow_control_resumed_reading_total_.inc(); + ->upstream_flow_control_resumed_reading_total_.inc(); } } @@ -389,7 +389,7 @@ Network::FilterStatus Filter::establishUpstreamConnection() { // will never be released. if (!cluster->resourceManager(Upstream::ResourcePriority::Default).connections().canCreate()) { getStreamInfo().setResponseFlag(StreamInfo::ResponseFlag::UpstreamOverflow); - cluster->trafficStats().upstream_cx_overflow_.inc(); + cluster->trafficStats()->upstream_cx_overflow_.inc(); onInitFailure(UpstreamFailureReason::ResourceLimitExceeded); return Network::FilterStatus::StopIteration; } @@ -397,7 +397,7 @@ Network::FilterStatus Filter::establishUpstreamConnection() { const uint32_t max_connect_attempts = config_->maxConnectAttempts(); if (connect_attempts_ >= max_connect_attempts) { getStreamInfo().setResponseFlag(StreamInfo::ResponseFlag::UpstreamRetryLimitExceeded); - cluster->trafficStats().upstream_cx_connect_attempts_exceeded_.inc(); + cluster->trafficStats()->upstream_cx_connect_attempts_exceeded_.inc(); onInitFailure(UpstreamFailureReason::ConnectFailed); return Network::FilterStatus::StopIteration; } @@ -429,7 +429,7 @@ Network::FilterStatus Filter::establishUpstreamConnection() { if (!maybeTunnel(*thread_local_cluster)) { // Either cluster is unknown or there are no healthy hosts. tcpConnPool() increments - // cluster->trafficStats().upstream_cx_none_healthy in the latter case. + // cluster->trafficStats()->upstream_cx_none_healthy in the latter case. getStreamInfo().setResponseFlag(StreamInfo::ResponseFlag::NoHealthyUpstream); onInitFailure(UpstreamFailureReason::NoHealthyUpstream); } diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 0bf01bcdafa3d..fa2effc16fa9c 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -1156,7 +1156,7 @@ Host::CreateConnectionData ClusterManagerImpl::ThreadLocalClusterManagerImpl::Cl } return conn_info; } else { - cluster_info_->trafficStats().upstream_cx_none_healthy_.inc(); + cluster_info_->trafficStats()->upstream_cx_none_healthy_.inc(); return {nullptr, nullptr}; } } @@ -1628,7 +1628,7 @@ ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::httpConnPoolImp if (!host) { if (!peek) { ENVOY_LOG(debug, "no healthy host for HTTP connection pool"); - cluster_info_->trafficStats().upstream_cx_none_healthy_.inc(); + cluster_info_->trafficStats()->upstream_cx_none_healthy_.inc(); } return nullptr; } @@ -1758,7 +1758,7 @@ ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::tcpConnPoolImpl if (!host) { if (!peek) { ENVOY_LOG(debug, "no healthy host for TCP connection pool"); - cluster_info_->trafficStats().upstream_cx_none_healthy_.inc(); + cluster_info_->trafficStats()->upstream_cx_none_healthy_.inc(); } return nullptr; } diff --git a/source/common/upstream/conn_pool_map_impl.h b/source/common/upstream/conn_pool_map_impl.h index e30a73bfd2fe5..d4360858b58d1 100644 --- a/source/common/upstream/conn_pool_map_impl.h +++ b/source/common/upstream/conn_pool_map_impl.h @@ -35,7 +35,7 @@ ConnPoolMap::getPool(const KEY_TYPE& key, const PoolFactory if (!connPoolResource.canCreate()) { // We're full. Try to free up a pool. If we can't, bail out. if (!freeOnePool()) { - host_->cluster().trafficStats().upstream_cx_pool_overflow_.inc(); + host_->cluster().trafficStats()->upstream_cx_pool_overflow_.inc(); return absl::nullopt; } diff --git a/source/common/upstream/health_checker_base_impl.cc b/source/common/upstream/health_checker_base_impl.cc index 88b2dc0c239c5..1fac61208920b 100644 --- a/source/common/upstream/health_checker_base_impl.cc +++ b/source/common/upstream/health_checker_base_impl.cc @@ -105,7 +105,7 @@ std::chrono::milliseconds HealthCheckerImplBase::interval(HealthState state, // If a connection has been established, we choose an interval based on the host's health. Please // refer to the HealthCheck API documentation for more details. uint64_t base_time_ms; - if (cluster_.info()->trafficStats().upstream_cx_total_.used()) { + if (cluster_.info()->trafficStats()->upstream_cx_total_.used()) { // When healthy/unhealthy threshold is configured the health transition of a host will be // delayed. In this situation Envoy should use the edge interval settings between health checks. // diff --git a/source/common/upstream/thread_aware_lb_impl.cc b/source/common/upstream/thread_aware_lb_impl.cc index 17c83d315bf79..a2d7e17c8cac0 100644 --- a/source/common/upstream/thread_aware_lb_impl.cc +++ b/source/common/upstream/thread_aware_lb_impl.cc @@ -190,7 +190,7 @@ double ThreadAwareLoadBalancerBase::BoundedLoadHashingLoadBalancer::hostOverload // TODO(scheler): This will not work if rq_active cluster stat is disabled, need to detect // and alert the user if that's the case. - const uint32_t overall_active = host.cluster().trafficStats().upstream_rq_active_.value(); + const uint32_t overall_active = host.cluster().trafficStats()->upstream_rq_active_.value(); const uint32_t host_active = host.stats().rq_active_.value(); const uint32_t total_slots = ((overall_active + 1) * hash_balance_factor_ + 99) / 100; diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 866bc26f3e971..a1a7d8bd8b9b0 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -846,9 +846,9 @@ void MainPrioritySetImpl::updateCrossPriorityHostMap(const HostVector& hosts_add } } -ClusterTrafficStats ClusterInfoImpl::generateStats(Stats::Scope& scope, - const ClusterTrafficStatNames& stat_names) { - return {stat_names, scope}; +std::unique_ptr +ClusterInfoImpl::generateStats(Stats::Scope& scope, const ClusterTrafficStatNames& stat_names) { + return std::make_unique(stat_names, scope); } ClusterRequestResponseSizeStats ClusterInfoImpl::generateRequestResponseSizeStats( @@ -992,7 +992,8 @@ ClusterInfoImpl::ClusterInfoImpl( per_connection_buffer_limit_bytes_( PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, per_connection_buffer_limit_bytes, 1024 * 1024)), socket_matcher_(std::move(socket_matcher)), stats_scope_(std::move(stats_scope)), - stats_(generateStats(*stats_scope_, factory_context.clusterManager().clusterStatNames())), + traffic_stats_( + generateStats(*stats_scope_, factory_context.clusterManager().clusterStatNames())), config_update_stats_(factory_context.clusterManager().clusterConfigUpdateStatNames(), *stats_scope_), lb_stats_(factory_context.clusterManager().clusterLbStatNames(), *stats_scope_), @@ -2101,21 +2102,23 @@ getDnsLookupFamilyFromCluster(const envoy::config::cluster::v3::Cluster& cluster void reportUpstreamCxDestroy(const Upstream::HostDescriptionConstSharedPtr& host, Network::ConnectionEvent event) { - host->cluster().trafficStats().upstream_cx_destroy_.inc(); + Upstream::ClusterTrafficStats& stats = *host->cluster().trafficStats(); + stats.upstream_cx_destroy_.inc(); if (event == Network::ConnectionEvent::RemoteClose) { - host->cluster().trafficStats().upstream_cx_destroy_remote_.inc(); + stats.upstream_cx_destroy_remote_.inc(); } else { - host->cluster().trafficStats().upstream_cx_destroy_local_.inc(); + stats.upstream_cx_destroy_local_.inc(); } } void reportUpstreamCxDestroyActiveRequest(const Upstream::HostDescriptionConstSharedPtr& host, Network::ConnectionEvent event) { - host->cluster().trafficStats().upstream_cx_destroy_with_active_rq_.inc(); + Upstream::ClusterTrafficStats& stats = *host->cluster().trafficStats(); + stats.upstream_cx_destroy_with_active_rq_.inc(); if (event == Network::ConnectionEvent::RemoteClose) { - host->cluster().trafficStats().upstream_cx_destroy_remote_with_active_rq_.inc(); + stats.upstream_cx_destroy_remote_with_active_rq_.inc(); } else { - host->cluster().trafficStats().upstream_cx_destroy_local_with_active_rq_.inc(); + stats.upstream_cx_destroy_local_with_active_rq_.inc(); } } diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 4003ceeef63bf..1607612e6d195 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -702,8 +702,8 @@ class ClusterInfoImpl : public ClusterInfo, Stats::ScopeSharedPtr&& stats_scope, bool added_via_api, Server::Configuration::TransportSocketFactoryContext&); - static ClusterTrafficStats generateStats(Stats::Scope& scope, - const ClusterTrafficStatNames& cluster_stat_names); + static std::unique_ptr + generateStats(Stats::Scope& scope, const ClusterTrafficStatNames& cluster_stat_names); static ClusterLoadReportStats generateLoadReportStats(Stats::Scope& scope, const ClusterLoadReportStatNames& stat_names); static ClusterCircuitBreakersStats @@ -790,7 +790,7 @@ class ClusterInfoImpl : public ClusterInfo, const std::string& observabilityName() const override { return observability_name_; } ResourceManager& resourceManager(ResourcePriority priority) const override; TransportSocketMatcher& transportSocketMatcher() const override { return *socket_matcher_; } - ClusterTrafficStats& trafficStats() const override { return stats_; } + std::unique_ptr& 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_; } @@ -912,7 +912,7 @@ class ClusterInfoImpl : public ClusterInfo, const uint32_t per_connection_buffer_limit_bytes_; TransportSocketMatcherPtr socket_matcher_; Stats::ScopeSharedPtr stats_scope_; - mutable ClusterTrafficStats stats_; + mutable std::unique_ptr traffic_stats_; mutable ClusterConfigUpdateStats config_update_stats_; mutable ClusterLbStats lb_stats_; mutable ClusterEndpointStats endpoint_stats_; diff --git a/source/extensions/clusters/original_dst/original_dst_cluster.cc b/source/extensions/clusters/original_dst/original_dst_cluster.cc index 5f9f636df1e81..fe10a85649a5f 100644 --- a/source/extensions/clusters/original_dst/original_dst_cluster.cc +++ b/source/extensions/clusters/original_dst/original_dst_cluster.cc @@ -124,7 +124,7 @@ OriginalDstCluster::LoadBalancer::requestOverrideHost(LoadBalancerContext* conte if (request_host == nullptr) { ENVOY_LOG(debug, "original_dst_load_balancer: invalid override header value. {}", request_override_host); - parent_->info()->trafficStats().original_dst_host_invalid_.inc(); + parent_->info()->trafficStats()->original_dst_host_invalid_.inc(); return nullptr; } ENVOY_LOG(debug, "Using request override host {}.", request_override_host); diff --git a/source/extensions/filters/http/grpc_stats/grpc_stats_filter.cc b/source/extensions/filters/http/grpc_stats/grpc_stats_filter.cc index 5f0d2200fa2dc..b35f382d0b0af 100644 --- a/source/extensions/filters/http/grpc_stats/grpc_stats_filter.cc +++ b/source/extensions/filters/http/grpc_stats/grpc_stats_filter.cc @@ -93,7 +93,7 @@ struct Config { Config(const envoy::extensions::filters::http::grpc_stats::v3::FilterConfig& proto_config, Server::Configuration::FactoryContext& context) : context_(context.grpcContext()), emit_filter_state_(proto_config.emit_filter_state()), - enable_upstream_stats_(proto_config.enable_upstream_stats()), + enable_traffic_stats_(proto_config.enable_upstream_stats()), replace_dots_in_grpc_service_name_(proto_config.replace_dots_in_grpc_service_name()) { switch (proto_config.per_method_stat_specifier_case()) { @@ -137,7 +137,7 @@ struct Config { } Grpc::Context& context_; const bool emit_filter_state_; - const bool enable_upstream_stats_; + const bool enable_traffic_stats_; const bool replace_dots_in_grpc_service_name_; bool stats_for_all_methods_{false}; absl::optional allowlist_; @@ -262,11 +262,11 @@ class GrpcStatsFilter : public Http::PassThroughFilter { } void maybeChargeUpstreamStat() { - if (!config_->enable_upstream_stats_) { + if (!config_->enable_traffic_stats_) { return; } StreamInfo::TimingUtility timing(decoder_callbacks_->streamInfo()); - if (config_->enable_upstream_stats_ && timing.lastUpstreamTxByteSent().has_value() && + if (config_->enable_traffic_stats_ && timing.lastUpstreamTxByteSent().has_value() && timing.lastUpstreamRxByteReceived().has_value()) { std::chrono::milliseconds chrono_duration = std::chrono::duration_cast( diff --git a/source/extensions/filters/network/common/redis/client_impl.cc b/source/extensions/filters/network/common/redis/client_impl.cc index c2958cbfbb912..5bf5b690d082f 100644 --- a/source/extensions/filters/network/common/redis/client_impl.cc +++ b/source/extensions/filters/network/common/redis/client_impl.cc @@ -77,9 +77,9 @@ ClientImpl::ClientImpl(Upstream::HostConstSharedPtr host, Event::Dispatcher& dis flush_timer_(dispatcher.createTimer([this]() { flushBufferAndResetTimer(); })), time_source_(dispatcher.timeSource()), redis_command_stats_(redis_command_stats), scope_(scope), is_transaction_client_(is_transaction_client) { - host->cluster().trafficStats().upstream_cx_total_.inc(); + host->cluster().trafficStats()->upstream_cx_total_.inc(); host->stats().cx_total_.inc(); - host->cluster().trafficStats().upstream_cx_active_.inc(); + host->cluster().trafficStats()->upstream_cx_active_.inc(); host->stats().cx_active_.inc(); connect_or_op_timer_->enableTimer(host->cluster().connectTimeout()); } @@ -87,7 +87,7 @@ ClientImpl::ClientImpl(Upstream::HostConstSharedPtr host, Event::Dispatcher& dis ClientImpl::~ClientImpl() { ASSERT(pending_requests_.empty()); ASSERT(connection_->state() == Network::Connection::State::Closed); - host_->cluster().trafficStats().upstream_cx_active_.dec(); + host_->cluster().trafficStats()->upstream_cx_active_.dec(); host_->stats().cx_active_.dec(); } @@ -141,10 +141,10 @@ PoolRequest* ClientImpl::makeRequest(const RespValue& request, ClientCallbacks& void ClientImpl::onConnectOrOpTimeout() { putOutlierEvent(Upstream::Outlier::Result::LocalOriginTimeout); if (connected_) { - host_->cluster().trafficStats().upstream_rq_timeout_.inc(); + host_->cluster().trafficStats()->upstream_rq_timeout_.inc(); host_->stats().rq_timeout_.inc(); } else { - host_->cluster().trafficStats().upstream_cx_connect_timeout_.inc(); + host_->cluster().trafficStats()->upstream_cx_connect_timeout_.inc(); host_->stats().cx_connect_fail_.inc(); } @@ -156,7 +156,7 @@ void ClientImpl::onData(Buffer::Instance& data) { decoder_->decode(data); } catch (ProtocolError&) { putOutlierEvent(Upstream::Outlier::Result::ExtOriginRequestFailed); - host_->cluster().trafficStats().upstream_cx_protocol_error_.inc(); + host_->cluster().trafficStats()->upstream_cx_protocol_error_.inc(); host_->stats().rq_error_.inc(); connection_->close(Network::ConnectionCloseType::NoFlush); } @@ -185,7 +185,7 @@ void ClientImpl::onEvent(Network::ConnectionEvent event) { if (!request.canceled_) { request.callbacks_.onFailure(); } else { - host_->cluster().trafficStats().upstream_rq_cancelled_.inc(); + host_->cluster().trafficStats()->upstream_rq_cancelled_.inc(); } pending_requests_.pop_front(); } @@ -198,7 +198,7 @@ void ClientImpl::onEvent(Network::ConnectionEvent event) { } if (event == Network::ConnectionEvent::RemoteClose && !connected_) { - host_->cluster().trafficStats().upstream_cx_connect_fail_.inc(); + host_->cluster().trafficStats()->upstream_cx_connect_fail_.inc(); host_->stats().cx_connect_fail_.inc(); } } @@ -221,7 +221,7 @@ void ClientImpl::onRespValue(RespValuePtr&& value) { // result in closing the connection. pending_requests_.pop_front(); if (canceled) { - host_->cluster().trafficStats().upstream_rq_cancelled_.inc(); + host_->cluster().trafficStats()->upstream_rq_cancelled_.inc(); } else if (config_.enableRedirection() && !is_transaction_client_ && (value->type() == Common::Redis::RespType::Error)) { std::vector err = StringUtil::splitToken(value->asString(), " ", false); @@ -263,14 +263,14 @@ ClientImpl::PendingRequest::PendingRequest(ClientImpl& parent, ClientCallbacks& command_request_timer_ = parent_.redis_command_stats_->createCommandTimer( parent_.scope_, command_, parent_.time_source_); } - parent.host_->cluster().trafficStats().upstream_rq_total_.inc(); + parent.host_->cluster().trafficStats()->upstream_rq_total_.inc(); parent.host_->stats().rq_total_.inc(); - parent.host_->cluster().trafficStats().upstream_rq_active_.inc(); + parent.host_->cluster().trafficStats()->upstream_rq_active_.inc(); parent.host_->stats().rq_active_.inc(); } ClientImpl::PendingRequest::~PendingRequest() { - parent_.host_->cluster().trafficStats().upstream_rq_active_.dec(); + parent_.host_->cluster().trafficStats()->upstream_rq_active_.dec(); parent_.host_->stats().rq_active_.dec(); } diff --git a/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc b/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc index 4795ab4fec625..fb04dfbc585e3 100644 --- a/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc +++ b/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc @@ -454,7 +454,7 @@ void InstanceImpl::PendingRequest::onRedirection(Common::Redis::RespValuePtr&& v host_address); auto host = host_; onResponse(std::move(resp_value_)); - host->cluster().trafficStats().upstream_internal_redirect_failed_total_.inc(); + host->cluster().trafficStats()->upstream_internal_redirect_failed_total_.inc(); } else { doRedirection(std::move(resp_value_), formatAddress(*result.host_info_.value()->address()->ip()), ask_redirection_); @@ -470,7 +470,7 @@ void InstanceImpl::PendingRequest::onRedirection(Common::Redis::RespValuePtr&& v host_address); auto host = host_; onResponse(std::move(resp_value_)); - host->cluster().trafficStats().upstream_internal_redirect_failed_total_.inc(); + host->cluster().trafficStats()->upstream_internal_redirect_failed_total_.inc(); return; } PANIC_DUE_TO_CORRUPT_ENUM; @@ -487,7 +487,7 @@ void InstanceImpl::PendingRequest::onLoadDnsCacheComplete( ENVOY_LOG(debug, "DNS lookup failed"); auto host = host_; onResponse(std::move(resp_value_)); - host->cluster().trafficStats().upstream_internal_redirect_failed_total_.inc(); + host->cluster().trafficStats()->upstream_internal_redirect_failed_total_.inc(); } else { doRedirection(std::move(resp_value_), formatAddress(*host_info->address()->ip()), ask_redirection_); @@ -509,16 +509,16 @@ void InstanceImpl::PendingRequest::doRedirection(Common::Redis::RespValuePtr&& v !parent_.makeRequestToHost(host_address, Common::Redis::Utility::AskingRequest::instance(), null_client_callbacks)) { onResponse(std::move(value)); - host->cluster().trafficStats().upstream_internal_redirect_failed_total_.inc(); + host->cluster().trafficStats()->upstream_internal_redirect_failed_total_.inc(); } else { request_handler_ = parent_.makeRequestToHost(host_address, getRequest(incoming_request_), *this); if (!request_handler_) { onResponse(std::move(value)); - host->cluster().trafficStats().upstream_internal_redirect_failed_total_.inc(); + host->cluster().trafficStats()->upstream_internal_redirect_failed_total_.inc(); } else { parent_.refresh_manager_->onRedirection(parent_.cluster_name_); - host->cluster().trafficStats().upstream_internal_redirect_succeeded_total_.inc(); + host->cluster().trafficStats()->upstream_internal_redirect_succeeded_total_.inc(); } } } diff --git a/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.cc b/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.cc index 19ab2d997b0a5..cb0480018b109 100644 --- a/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.cc +++ b/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.cc @@ -128,7 +128,7 @@ UdpProxyFilter::ClusterInfo::createSession(Network::UdpRecvData::LocalPeerAddres .connections() .canCreate()) { ENVOY_LOG(debug, "cannot create new connection."); - cluster_.info()->trafficStats().upstream_cx_overflow_.inc(); + cluster_.info()->trafficStats()->upstream_cx_overflow_.inc(); return nullptr; } @@ -139,7 +139,7 @@ UdpProxyFilter::ClusterInfo::createSession(Network::UdpRecvData::LocalPeerAddres auto host = chooseHost(addresses.peer_); if (host == nullptr) { ENVOY_LOG(debug, "cannot find any valid host."); - cluster_.info()->trafficStats().upstream_cx_none_healthy_.inc(); + cluster_.info()->trafficStats()->upstream_cx_none_healthy_.inc(); return nullptr; } return createSessionWithHost(std::move(addresses), host); @@ -212,7 +212,7 @@ UdpProxyFilter::PerPacketLoadBalancingClusterInfo::onData(Network::UdpRecvData& auto host = chooseHost(data.addresses_.peer_); if (host == nullptr) { ENVOY_LOG(debug, "cannot find any valid host."); - cluster_.info()->trafficStats().upstream_cx_none_healthy_.inc(); + cluster_.info()->trafficStats()->upstream_cx_none_healthy_.inc(); return Network::FilterStatus::StopIteration; } @@ -407,7 +407,7 @@ void UdpProxyFilter::ActiveSession::write(const Buffer::Instance& buffer) { cluster_.cluster_stats_.sess_tx_errors_.inc(); } else { cluster_.cluster_stats_.sess_tx_datagrams_.inc(); - cluster_.cluster_.info()->trafficStats().upstream_cx_tx_bytes_total_.add(buffer_length); + cluster_.cluster_.info()->trafficStats()->upstream_cx_tx_bytes_total_.add(buffer_length); } } @@ -420,7 +420,7 @@ void UdpProxyFilter::ActiveSession::processPacket(Network::Address::InstanceCons const uint64_t buffer_length = buffer->length(); cluster_.cluster_stats_.sess_rx_datagrams_.inc(); - cluster_.cluster_.info()->trafficStats().upstream_cx_rx_bytes_total_.add(buffer_length); + cluster_.cluster_.info()->trafficStats()->upstream_cx_rx_bytes_total_.add(buffer_length); Network::UdpSendData data{addresses_.local_->ip(), *addresses_.peer_, *buffer}; const Api::IoCallUint64Result rc = cluster_.filter_.read_callbacks_->udpListener().send(data); diff --git a/source/extensions/stat_sinks/common/statsd/statsd.cc b/source/extensions/stat_sinks/common/statsd/statsd.cc index 47eef9a1d22ab..062fdfd01fca9 100644 --- a/source/extensions/stat_sinks/common/statsd/statsd.cc +++ b/source/extensions/stat_sinks/common/statsd/statsd.cc @@ -330,8 +330,8 @@ void TcpStatsdSink::TlsSink::write(Buffer::Instance& buffer) { // since if we stay over, the other threads will eventually kill their connections too. // TODO(mattklein123): The use of the stat is somewhat of a hack, and should be replaced with // real flow control callbacks once they are available. - if (parent_.cluster_info_->trafficStats().upstream_cx_tx_bytes_buffered_.value() > - MAX_BUFFERED_STATS_BYTES) { + Upstream::ClusterTrafficStats& cluster_traffic_stats = *parent_.cluster_info_->trafficStats(); + if (cluster_traffic_stats.upstream_cx_tx_bytes_buffered_.value() > MAX_BUFFERED_STATS_BYTES) { if (connection_) { connection_->close(Network::ConnectionCloseType::NoFlush); } @@ -354,12 +354,11 @@ void TcpStatsdSink::TlsSink::write(Buffer::Instance& buffer) { connection_ = std::move(info.connection_); connection_->addConnectionCallbacks(*this); - connection_->setConnectionStats( - {parent_.cluster_info_->trafficStats().upstream_cx_rx_bytes_total_, - parent_.cluster_info_->trafficStats().upstream_cx_rx_bytes_buffered_, - parent_.cluster_info_->trafficStats().upstream_cx_tx_bytes_total_, - parent_.cluster_info_->trafficStats().upstream_cx_tx_bytes_buffered_, - &parent_.cluster_info_->trafficStats().bind_errors_, nullptr}); + connection_->setConnectionStats({cluster_traffic_stats.upstream_cx_rx_bytes_total_, + cluster_traffic_stats.upstream_cx_rx_bytes_buffered_, + cluster_traffic_stats.upstream_cx_tx_bytes_total_, + cluster_traffic_stats.upstream_cx_tx_bytes_buffered_, + &cluster_traffic_stats.bind_errors_, nullptr}); connection_->connect(); } diff --git a/source/extensions/stat_sinks/hystrix/hystrix.cc b/source/extensions/stat_sinks/hystrix/hystrix.cc index dac8610db6ae7..25e81763ca6f6 100644 --- a/source/extensions/stat_sinks/hystrix/hystrix.cc +++ b/source/extensions/stat_sinks/hystrix/hystrix.cc @@ -90,7 +90,7 @@ uint64_t HystrixSink::getRollingValue(RollingWindow rolling_window) { void HystrixSink::updateRollingWindowMap(const Upstream::ClusterInfo& cluster_info, ClusterStatsCache& cluster_stats_cache) { - Upstream::ClusterTrafficStats& cluster_stats = cluster_info.trafficStats(); + Upstream::ClusterTrafficStats& cluster_stats = *cluster_info.trafficStats(); Stats::Scope& cluster_stats_scope = cluster_info.statsScope(); // Combining timeouts+retries - retries are counted as separate requests diff --git a/test/common/conn_pool/conn_pool_base_test.cc b/test/common/conn_pool/conn_pool_base_test.cc index fb53487b214f5..1313c5d38d69e 100644 --- a/test/common/conn_pool/conn_pool_base_test.cc +++ b/test/common/conn_pool/conn_pool_base_test.cc @@ -374,13 +374,13 @@ TEST_F(ConnPoolImplDispatcherBaseTest, MaxConnectionDurationBusy) { // Verify that advancing to just before the connection duration timeout doesn't drain the // connection. advanceTimeAndRun(max_connection_duration_ - 1); - EXPECT_EQ(0, pool_.host()->cluster().trafficStats().upstream_cx_max_duration_reached_.value()); + EXPECT_EQ(0, pool_.host()->cluster().trafficStats()->upstream_cx_max_duration_reached_.value()); EXPECT_EQ(ActiveClient::State::Busy, clients_.back()->state()); // Verify that advancing past the connection duration timeout drains the connection, // because there's a busy client. advanceTimeAndRun(2); - EXPECT_EQ(1, pool_.host()->cluster().trafficStats().upstream_cx_max_duration_reached_.value()); + EXPECT_EQ(1, pool_.host()->cluster().trafficStats()->upstream_cx_max_duration_reached_.value()); EXPECT_EQ(ActiveClient::State::Draining, clients_.back()->state()); closeStream(); } @@ -395,13 +395,13 @@ TEST_F(ConnPoolImplDispatcherBaseTest, MaxConnectionDurationReady) { // Verify that advancing to just before the connection duration timeout doesn't close the // connection. advanceTimeAndRun(max_connection_duration_ - 1); - EXPECT_EQ(0, pool_.host()->cluster().trafficStats().upstream_cx_max_duration_reached_.value()); + EXPECT_EQ(0, pool_.host()->cluster().trafficStats()->upstream_cx_max_duration_reached_.value()); EXPECT_EQ(ActiveClient::State::Ready, clients_.back()->state()); // Verify that advancing past the connection duration timeout closes the connection, // because there's nothing to drain. advanceTimeAndRun(2); - EXPECT_EQ(1, pool_.host()->cluster().trafficStats().upstream_cx_max_duration_reached_.value()); + EXPECT_EQ(1, pool_.host()->cluster().trafficStats()->upstream_cx_max_duration_reached_.value()); } TEST_F(ConnPoolImplDispatcherBaseTest, MaxConnectionDurationAlreadyDraining) { @@ -411,7 +411,7 @@ TEST_F(ConnPoolImplDispatcherBaseTest, MaxConnectionDurationAlreadyDraining) { // Verify that advancing past the connection duration timeout does nothing to an active client // that is already draining. advanceTimeAndRun(max_connection_duration_ + 1); - EXPECT_EQ(0, pool_.host()->cluster().trafficStats().upstream_cx_max_duration_reached_.value()); + EXPECT_EQ(0, pool_.host()->cluster().trafficStats()->upstream_cx_max_duration_reached_.value()); EXPECT_EQ(ActiveClient::State::Draining, clients_.back()->state()); closeStream(); } @@ -423,7 +423,7 @@ TEST_F(ConnPoolImplDispatcherBaseTest, MaxConnectionDurationAlreadyClosed) { // Verify that advancing past the connection duration timeout does nothing to the active // client that is already closed. advanceTimeAndRun(max_connection_duration_ + 1); - EXPECT_EQ(0, pool_.host()->cluster().trafficStats().upstream_cx_max_duration_reached_.value()); + EXPECT_EQ(0, pool_.host()->cluster().trafficStats()->upstream_cx_max_duration_reached_.value()); } TEST_F(ConnPoolImplDispatcherBaseTest, MaxConnectionDurationCallbackWhileClosedBug) { @@ -565,7 +565,7 @@ TEST_F(ConnPoolImplDispatcherBaseTest, ConnectedZeroRttSendsEarlyData) { pool_.onUpstreamReadyForEarlyData(client_ref); CHECK_STATE(1 /*active*/, 0 /*pending*/, concurrent_streams_ - 1 /*connecting capacity*/); - EXPECT_EQ(1, pool_.host()->cluster().trafficStats().upstream_rq_0rtt_.value()); + EXPECT_EQ(1, pool_.host()->cluster().trafficStats()->upstream_rq_0rtt_.value()); EXPECT_NE(nullptr, pool_.newStreamImpl(context_, /*can_send_early_data=*/false)); CHECK_STATE(1 /*active*/, 1 /*pending*/, concurrent_streams_ - 1 /*connecting capacity*/); @@ -574,7 +574,7 @@ TEST_F(ConnPoolImplDispatcherBaseTest, ConnectedZeroRttSendsEarlyData) { clients_.back()->onEvent(Network::ConnectionEvent::Connected); CHECK_STATE(2 /*active*/, 0 /*pending*/, 0 /*connecting capacity*/); - EXPECT_EQ(1, pool_.host()->cluster().trafficStats().upstream_rq_0rtt_.value()); + EXPECT_EQ(1, pool_.host()->cluster().trafficStats()->upstream_rq_0rtt_.value()); // Clean up. closeStreamAndDrainClient(); @@ -596,12 +596,12 @@ TEST_F(ConnPoolImplDispatcherBaseTest, EarlyDataStreamsReachConcurrentStreamLimi pool_.onUpstreamReadyForEarlyData(client_ref); CHECK_STATE(1 /*active*/, 0 /*pending*/, concurrent_streams_ - 1 /*connecting capacity*/); - EXPECT_EQ(1, pool_.host()->cluster().trafficStats().upstream_rq_0rtt_.value()); + EXPECT_EQ(1, pool_.host()->cluster().trafficStats()->upstream_rq_0rtt_.value()); EXPECT_CALL(pool_, onPoolReady); EXPECT_EQ(nullptr, pool_.newStreamImpl(context_, /*can_send_early_data=*/true)); CHECK_STATE(2 /*active*/, 0 /*pending*/, concurrent_streams_ - 2 /*connecting capacity*/); - EXPECT_EQ(2, pool_.host()->cluster().trafficStats().upstream_rq_0rtt_.value()); + EXPECT_EQ(2, pool_.host()->cluster().trafficStats()->upstream_rq_0rtt_.value()); EXPECT_EQ(ActiveClient::State::Busy, clients_.back()->state()); // After 1 stream gets closed, the client should transit to ReadyForEarlyData. diff --git a/test/common/http/codec_client_test.cc b/test/common/http/codec_client_test.cc index 910f1995cf5b9..4c46a755c8d00 100644 --- a/test/common/http/codec_client_test.cc +++ b/test/common/http/codec_client_test.cc @@ -258,7 +258,7 @@ TEST_F(CodecClientTest, ProtocolError) { Buffer::OwnedImpl data; filter_->onData(data, false); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_protocol_error_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_protocol_error_.value()); } TEST_F(CodecClientTest, 408Response) { @@ -270,7 +270,7 @@ TEST_F(CodecClientTest, 408Response) { Buffer::OwnedImpl data; filter_->onData(data, false); - EXPECT_EQ(0U, cluster_->stats_.upstream_cx_protocol_error_.value()); + EXPECT_EQ(0U, cluster_->traffic_stats_->upstream_cx_protocol_error_.value()); } TEST_F(CodecClientTest, PrematureResponse) { @@ -281,7 +281,7 @@ TEST_F(CodecClientTest, PrematureResponse) { Buffer::OwnedImpl data; filter_->onData(data, false); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_protocol_error_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_protocol_error_.value()); } TEST_F(CodecClientTest, WatermarkPassthrough) { diff --git a/test/common/http/http1/conn_pool_test.cc b/test/common/http/http1/conn_pool_test.cc index 52ae8c044597c..30a13e46dce90 100644 --- a/test/common/http/http1/conn_pool_test.cc +++ b/test/common/http/http1/conn_pool_test.cc @@ -175,7 +175,7 @@ struct ActiveTestRequest { : parent_(parent), client_index_(client_index) { uint64_t active_rq_observed = parent_.cluster_->resourceManager(Upstream::ResourcePriority::Default).requests().count(); - uint64_t current_rq_total = parent_.cluster_->stats_.upstream_rq_total_.value(); + uint64_t current_rq_total = parent_.cluster_->traffic_stats_->upstream_rq_total_.value(); if (type == Type::CreateConnection) { parent.conn_pool_->expectClientCreate(); } @@ -199,7 +199,7 @@ struct ActiveTestRequest { Network::ConnectionEvent::Connected); } if (type != Type::Pending) { - EXPECT_EQ(current_rq_total + 1, parent_.cluster_->stats_.upstream_rq_total_.value()); + EXPECT_EQ(current_rq_total + 1, parent_.cluster_->traffic_stats_->upstream_rq_total_.value()); EXPECT_EQ(active_rq_observed + 1, parent_.cluster_->resourceManager(Upstream::ResourcePriority::Default) .requests() @@ -440,7 +440,7 @@ TEST_F(Http1ConnPoolImplTest, MaxPendingRequests) { conn_pool_->test_clients_[0].connection_->raiseEvent(Network::ConnectionEvent::RemoteClose); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(1U, cluster_->stats_.upstream_rq_pending_overflow_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_rq_pending_overflow_.value()); } /** @@ -464,8 +464,8 @@ TEST_F(Http1ConnPoolImplTest, ConnectFailure) { EXPECT_CALL(*conn_pool_, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_connect_fail_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_rq_pending_failure_eject_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_connect_fail_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_rq_pending_failure_eject_.value()); } /** @@ -553,9 +553,9 @@ TEST_F(Http1ConnPoolImplTest, ConnectTimeout) { EXPECT_CALL(*conn_pool_, onClientDestroy()).Times(2); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(0U, cluster_->stats_.upstream_rq_total_.value()); - EXPECT_EQ(2U, cluster_->stats_.upstream_cx_connect_fail_.value()); - EXPECT_EQ(2U, cluster_->stats_.upstream_cx_connect_timeout_.value()); + EXPECT_EQ(0U, cluster_->traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(2U, cluster_->traffic_stats_->upstream_cx_connect_fail_.value()); + EXPECT_EQ(2U, cluster_->traffic_stats_->upstream_cx_connect_timeout_.value()); } /** @@ -655,7 +655,7 @@ TEST_F(Http1ConnPoolImplTest, MaxConnections) { NiceMock outer_decoder2; ConnPoolCallbacks callbacks2; handle = conn_pool_->newStream(outer_decoder2, callbacks2, {false, true}); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_overflow_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_overflow_.value()); EXPECT_EQ(1U, cluster_->circuit_breakers_stats_.cx_open_.value()); EXPECT_NE(nullptr, handle); @@ -719,7 +719,7 @@ TEST_F(Http1ConnPoolImplTest, ConnectionCloseWithoutHeader) { NiceMock outer_decoder2; ConnPoolCallbacks callbacks2; handle = conn_pool_->newStream(outer_decoder2, callbacks2, {false, true}); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_overflow_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_overflow_.value()); EXPECT_NE(nullptr, handle); @@ -803,7 +803,7 @@ TEST_F(Http1ConnPoolImplTest, ConnectionCloseHeader) { inner_decoder->decodeHeaders(std::move(response_headers), true); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(0U, cluster_->stats_.upstream_cx_destroy_with_active_rq_.value()); + EXPECT_EQ(0U, cluster_->traffic_stats_->upstream_cx_destroy_with_active_rq_.value()); } /** @@ -841,7 +841,7 @@ TEST_F(Http1ConnPoolImplTest, ProxyConnectionCloseHeader) { inner_decoder->decodeHeaders(std::move(response_headers), true); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(0U, cluster_->stats_.upstream_cx_destroy_with_active_rq_.value()); + EXPECT_EQ(0U, cluster_->traffic_stats_->upstream_cx_destroy_with_active_rq_.value()); } /** @@ -878,7 +878,7 @@ TEST_F(Http1ConnPoolImplTest, Http10NoConnectionKeepAlive) { inner_decoder->decodeHeaders(std::move(response_headers), true); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(0U, cluster_->stats_.upstream_cx_destroy_with_active_rq_.value()); + EXPECT_EQ(0U, cluster_->traffic_stats_->upstream_cx_destroy_with_active_rq_.value()); } /** @@ -920,8 +920,8 @@ TEST_F(Http1ConnPoolImplTest, MaxRequestsPerConnection) { dispatcher_.clearDeferredDeleteList(); CHECK_STATE(0 /*active*/, 0 /*pending*/, 0 /*capacity*/); - EXPECT_EQ(0U, cluster_->stats_.upstream_cx_destroy_with_active_rq_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_max_requests_.value()); + EXPECT_EQ(0U, cluster_->traffic_stats_->upstream_cx_destroy_with_active_rq_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_max_requests_.value()); } TEST_F(Http1ConnPoolImplTest, ConcurrentConnections) { @@ -943,7 +943,7 @@ TEST_F(Http1ConnPoolImplTest, ConcurrentConnections) { r1.completeResponse(false); conn_pool_->expectAndRunUpstreamReady(); r3.startRequest(); - EXPECT_EQ(3U, cluster_->stats_.upstream_rq_total_.value()); + EXPECT_EQ(3U, cluster_->traffic_stats_->upstream_rq_total_.value()); conn_pool_->expectEnableUpstreamReady(); r2.completeResponse(false); @@ -956,8 +956,8 @@ TEST_F(Http1ConnPoolImplTest, ConcurrentConnections) { conn_pool_->test_clients_[0].connection_->raiseEvent(Network::ConnectionEvent::RemoteClose); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(2U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(2U, cluster_->stats_.upstream_cx_destroy_remote_.value()); + EXPECT_EQ(2U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(2U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); } TEST_F(Http1ConnPoolImplTest, DrainCallback) { @@ -971,7 +971,7 @@ TEST_F(Http1ConnPoolImplTest, DrainCallback) { conn_pool_->drainConnections(Envoy::ConnectionPool::DrainBehavior::DrainAndDelete); r2.handle_->cancel(Envoy::ConnectionPool::CancelPolicy::Default); - EXPECT_EQ(1U, cluster_->stats_.upstream_rq_total_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_rq_total_.value()); conn_pool_->expectEnableUpstreamReady(); EXPECT_CALL(drained, ready()).Times(AtLeast(1)); @@ -1048,8 +1048,8 @@ TEST_F(Http1ConnPoolImplTest, RemoteCloseToCompleteResponse) { conn_pool_->test_clients_[0].connection_->raiseEvent(Network::ConnectionEvent::RemoteClose); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_remote_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); } TEST_F(Http1ConnPoolImplTest, NoActiveConnectionsByDefault) { @@ -1093,13 +1093,13 @@ TEST_F(Http1ConnPoolImplTest, PendingRequestIsConsideredActive) { EXPECT_CALL(*conn_pool_, onClientDestroy()); r1.handle_->cancel(Envoy::ConnectionPool::CancelPolicy::Default); - EXPECT_EQ(0U, cluster_->stats_.upstream_rq_total_.value()); + EXPECT_EQ(0U, cluster_->traffic_stats_->upstream_rq_total_.value()); conn_pool_->drainConnections(Envoy::ConnectionPool::DrainBehavior::DrainExistingConnections); conn_pool_->test_clients_[0].connection_->raiseEvent(Network::ConnectionEvent::RemoteClose); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_local_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_local_.value()); } // Schedulable callback that can track it's destruction. diff --git a/test/common/http/http2/conn_pool_test.cc b/test/common/http/http2/conn_pool_test.cc index 2050ea59b7710..17823ad3ef516 100644 --- a/test/common/http/http2/conn_pool_test.cc +++ b/test/common/http/http2/conn_pool_test.cc @@ -634,8 +634,8 @@ TEST_F(Http2ConnPoolImplTest, DrainConnections) { EXPECT_CALL(*this, onClientDestroy()).Times(2); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(2U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(2U, cluster_->stats_.upstream_cx_destroy_remote_.value()); + EXPECT_EQ(2U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(2U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); } // Test that cluster.http2_protocol_options.max_concurrent_streams limits @@ -688,7 +688,7 @@ TEST_F(Http2ConnPoolImplTest, MaxConcurrentRequestsPerStream) { EXPECT_CALL(*this, onClientDestroy()).Times(2); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(2U, cluster_->stats_.upstream_cx_total_.value()); + EXPECT_EQ(2U, cluster_->traffic_stats_->upstream_cx_total_.value()); } // Verifies that requests are queued up in the conn pool until the connection becomes ready. @@ -732,8 +732,8 @@ TEST_F(Http2ConnPoolImplTest, PendingStreams) { EXPECT_CALL(*this, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_remote_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); } // Verifies that the correct number of CONNECTING connections are created for @@ -853,8 +853,8 @@ TEST_F(Http2ConnPoolImplTest, PendingStreamsFailure) { EXPECT_CALL(*this, onClientDestroy()).Times(2); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(2U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(2U, cluster_->stats_.upstream_cx_destroy_remote_.value()); + EXPECT_EQ(2U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(2U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); } // Verifies resets due to local connection closes are tracked correctly. @@ -909,8 +909,8 @@ TEST_F(Http2ConnPoolImplTest, PendingStreamsRequestOverflow) { EXPECT_CALL(*this, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_remote_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); } // Verifies that we honor the max pending requests circuit breaker. @@ -947,8 +947,8 @@ TEST_F(Http2ConnPoolImplTest, PendingStreamsMaxPendingCircuitBreaker) { EXPECT_CALL(*this, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_remote_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); } TEST_F(Http2ConnPoolImplTest, VerifyConnectionTimingStats) { @@ -973,8 +973,8 @@ TEST_F(Http2ConnPoolImplTest, VerifyConnectionTimingStats) { EXPECT_CALL(*this, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_remote_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); } /** @@ -1004,8 +1004,8 @@ TEST_F(Http2ConnPoolImplTest, VerifyBufferLimits) { dispatcher_.clearDeferredDeleteList(); CHECK_STATE(0 /*active*/, 0 /*pending*/, 0 /*capacity*/); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_remote_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); } TEST_F(Http2ConnPoolImplTest, RequestAndResponse) { @@ -1019,7 +1019,7 @@ TEST_F(Http2ConnPoolImplTest, RequestAndResponse) { r1.callbacks_.outer_encoder_ ->encodeHeaders(TestRequestHeaderMapImpl{{":path", "/"}, {":method", "GET"}}, true) .ok()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_active_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_active_.value()); EXPECT_CALL(r1.decoder_, decodeHeaders_(_, true)); r1.inner_decoder_->decodeHeaders( ResponseHeaderMapPtr{new TestResponseHeaderMapImpl{{":status", "200"}}}, true); @@ -1038,9 +1038,9 @@ TEST_F(Http2ConnPoolImplTest, RequestAndResponse) { EXPECT_CALL(*this, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(0U, cluster_->stats_.upstream_cx_active_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_remote_.value()); + EXPECT_EQ(0U, cluster_->traffic_stats_->upstream_cx_active_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); } TEST_F(Http2ConnPoolImplTest, LocalReset) { @@ -1059,11 +1059,11 @@ TEST_F(Http2ConnPoolImplTest, LocalReset) { test_clients_[0].connection_->raiseEvent(Network::ConnectionEvent::RemoteClose); EXPECT_CALL(*this, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_remote_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_rq_tx_reset_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_rq_tx_reset_.value()); EXPECT_EQ(0U, cluster_->circuit_breakers_stats_.rq_open_.value()); - EXPECT_EQ(0U, cluster_->stats_.upstream_cx_active_.value()); + EXPECT_EQ(0U, cluster_->traffic_stats_->upstream_cx_active_.value()); } TEST_F(Http2ConnPoolImplTest, RemoteReset) { @@ -1082,11 +1082,11 @@ TEST_F(Http2ConnPoolImplTest, RemoteReset) { test_clients_[0].connection_->raiseEvent(Network::ConnectionEvent::RemoteClose); EXPECT_CALL(*this, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_remote_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_rq_rx_reset_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_rq_rx_reset_.value()); EXPECT_EQ(0U, cluster_->circuit_breakers_stats_.rq_open_.value()); - EXPECT_EQ(0U, cluster_->stats_.upstream_cx_active_.value()); + EXPECT_EQ(0U, cluster_->traffic_stats_->upstream_cx_active_.value()); } TEST_F(Http2ConnPoolImplTest, DrainDisconnectWithActiveRequest) { @@ -1111,8 +1111,8 @@ TEST_F(Http2ConnPoolImplTest, DrainDisconnectWithActiveRequest) { EXPECT_CALL(*this, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_remote_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); } TEST_F(Http2ConnPoolImplTest, DrainDisconnectDrainingWithActiveRequest) { @@ -1155,8 +1155,8 @@ TEST_F(Http2ConnPoolImplTest, DrainDisconnectDrainingWithActiveRequest) { EXPECT_CALL(*this, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(2U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_remote_.value()); + EXPECT_EQ(2U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); } TEST_F(Http2ConnPoolImplTest, DrainPrimary) { @@ -1278,13 +1278,13 @@ TEST_F(Http2ConnPoolImplTest, ConnectTimeout) { EXPECT_CALL(*this, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(1U, cluster_->stats_.upstream_rq_total_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_connect_fail_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_connect_timeout_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_rq_pending_failure_eject_.value()); - EXPECT_EQ(2U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_local_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_remote_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_connect_fail_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_connect_timeout_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_rq_pending_failure_eject_.value()); + EXPECT_EQ(2U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_local_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); } TEST_F(Http2ConnPoolImplTest, MaxGlobalRequests) { @@ -1310,8 +1310,8 @@ TEST_F(Http2ConnPoolImplTest, MaxGlobalRequests) { EXPECT_CALL(*this, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_destroy_remote_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_destroy_remote_.value()); } TEST_F(Http2ConnPoolImplTest, GoAway) { @@ -1348,7 +1348,7 @@ TEST_F(Http2ConnPoolImplTest, GoAway) { EXPECT_CALL(*this, onClientDestroy()).Times(2); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_close_notify_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_close_notify_.value()); } TEST_F(Http2ConnPoolImplTest, NoActiveConnectionsByDefault) { diff --git a/test/common/router/retry_state_impl_test.cc b/test/common/router/retry_state_impl_test.cc index 6abd42db62059..f772846071634 100644 --- a/test/common/router/retry_state_impl_test.cc +++ b/test/common/router/retry_state_impl_test.cc @@ -157,9 +157,9 @@ class RouterRetryStateImplTest : public testing::Test { EXPECT_EQ(RetryStatus::NoRetryLimitExceeded, state_->shouldRetryHeaders(response_headers, request_headers, header_callback_)); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_limit_exceeded_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_limit_exceeded_.value()); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_.value()); } @@ -217,10 +217,10 @@ TEST_F(RouterRetryStateImplTest, PolicyRefusedStream) { state_->shouldRetryReset(remote_refused_stream_reset_, RetryState::Http3Used::No, reset_callback_)); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_limit_exceeded_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_.value()); } @@ -253,10 +253,10 @@ TEST_F(RouterRetryStateImplTest, PolicyAltProtocolPostHandshakeFailure) { state_->shouldRetryReset(remote_refused_stream_reset_, RetryState::Http3Used::No, reset_callback_)); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_limit_exceeded_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_.value()); } @@ -300,10 +300,10 @@ TEST_F(RouterRetryStateImplTest, Policy5xxRemoteReset) { EXPECT_EQ(RetryStatus::NoRetryLimitExceeded, state_->shouldRetryReset(remote_reset_, RetryState::Http3Used::No, reset_callback_)); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_limit_exceeded_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); } @@ -390,10 +390,10 @@ TEST_F(RouterRetryStateImplTest, PolicyGatewayErrorRemoteReset) { EXPECT_EQ(RetryStatus::NoRetryLimitExceeded, state_->shouldRetryReset(remote_reset_, RetryState::Http3Used::No, reset_callback_)); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_limit_exceeded_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); } @@ -437,10 +437,10 @@ TEST_F(RouterRetryStateImplTest, Policy5xxRemote200RemoteReset) { EXPECT_EQ(RetryStatus::NoRetryLimitExceeded, state_->shouldRetryReset(remote_reset_, RetryState::Http3Used::No, reset_callback_)); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_limit_exceeded_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); } @@ -532,7 +532,7 @@ TEST_F(RouterRetryStateImplTest, NoRetryUponTooEarlyStatusCodeWithDownstreamEarl EXPECT_EQ(RetryStatus::No, state_->shouldRetryHeaders(response_headers, request_headers, header_callback_)); - EXPECT_EQ(0UL, cluster_.trafficStats().upstream_rq_retry_.value()); + EXPECT_EQ(0UL, cluster_.trafficStats()->upstream_rq_retry_.value()); EXPECT_EQ(0UL, virtual_cluster_.stats().upstream_rq_retry_.value()); EXPECT_EQ(0UL, route_stats_context_.stats().upstream_rq_retry_.value()); } @@ -828,10 +828,10 @@ TEST_F(RouterRetryStateImplTest, PolicyResetRemoteReset) { EXPECT_EQ(RetryStatus::NoRetryLimitExceeded, state_->shouldRetryReset(remote_reset_, RetryState::Http3Used::No, reset_callback_)); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_limit_exceeded_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); } @@ -915,10 +915,10 @@ TEST_F(RouterRetryStateImplTest, RouteConfigNoRetriesAllowed) { RetryStatus::NoRetryLimitExceeded, state_->shouldRetryReset(connect_failure_, RetryState::Http3Used::Unknown, reset_callback_)); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_limit_exceeded_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); - EXPECT_EQ(0UL, cluster_.trafficStats().upstream_rq_retry_.value()); + EXPECT_EQ(0UL, cluster_.trafficStats()->upstream_rq_retry_.value()); EXPECT_EQ(0UL, virtual_cluster_.stats().upstream_rq_retry_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); } @@ -948,7 +948,7 @@ TEST_F(RouterRetryStateImplTest, NoAvailableRetries) { EXPECT_EQ( RetryStatus::NoOverflow, state_->shouldRetryReset(connect_failure_, RetryState::Http3Used::Unknown, reset_callback_)); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_overflow_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_overflow_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_overflow_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_overflow_.value()); } @@ -988,9 +988,9 @@ TEST_F(RouterRetryStateImplTest, MaxRetriesHeader) { RetryStatus::NoRetryLimitExceeded, state_->shouldRetryReset(connect_failure_, RetryState::Http3Used::Unknown, reset_callback_)); - EXPECT_EQ(3UL, cluster_.trafficStats().upstream_rq_retry_.value()); - EXPECT_EQ(0UL, cluster_.trafficStats().upstream_rq_retry_success_.value()); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_limit_exceeded_.value()); + EXPECT_EQ(3UL, cluster_.trafficStats()->upstream_rq_retry_.value()); + EXPECT_EQ(0UL, cluster_.trafficStats()->upstream_rq_retry_success_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(3UL, virtual_cluster_.stats().upstream_rq_retry_.value()); EXPECT_EQ(0UL, virtual_cluster_.stats().upstream_rq_retry_success_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_limit_exceeded_.value()); @@ -1062,8 +1062,8 @@ TEST_F(RouterRetryStateImplTest, Backoff) { EXPECT_EQ(RetryStatus::No, state_->shouldRetryHeaders(response_headers, request_headers, header_callback_)); - EXPECT_EQ(5UL, cluster_.trafficStats().upstream_rq_retry_.value()); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_success_.value()); + EXPECT_EQ(5UL, cluster_.trafficStats()->upstream_rq_retry_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_success_.value()); EXPECT_EQ(5UL, virtual_cluster_.stats().upstream_rq_retry_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_success_.value()); EXPECT_EQ(5UL, route_stats_context_.stats().upstream_rq_retry_.value()); @@ -1306,8 +1306,8 @@ TEST_F(RouterRetryStateImplTest, RateLimitedRetryBackoffStrategy) { RetryStatus::NoRetryLimitExceeded, state_->shouldRetryHeaders(response_headers_reset_2, request_headers, header_callback_)); - EXPECT_EQ(2UL, cluster_.trafficStats().upstream_rq_retry_backoff_ratelimited_.value()); - EXPECT_EQ(2UL, cluster_.trafficStats().upstream_rq_retry_backoff_exponential_.value()); + EXPECT_EQ(2UL, cluster_.trafficStats()->upstream_rq_retry_backoff_ratelimited_.value()); + EXPECT_EQ(2UL, cluster_.trafficStats()->upstream_rq_retry_backoff_exponential_.value()); } TEST_F(RouterRetryStateImplTest, HostSelectionAttempts) { @@ -1343,10 +1343,10 @@ TEST_F(RouterRetryStateImplTest, ZeroMaxRetriesHeader) { RetryStatus::NoRetryLimitExceeded, state_->shouldRetryReset(connect_failure_, RetryState::Http3Used::Unknown, reset_callback_)); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_limit_exceeded_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(1UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); - EXPECT_EQ(0UL, cluster_.trafficStats().upstream_rq_retry_.value()); + EXPECT_EQ(0UL, cluster_.trafficStats()->upstream_rq_retry_.value()); EXPECT_EQ(0UL, virtual_cluster_.stats().upstream_rq_retry_.value()); EXPECT_EQ(0UL, route_stats_context_.stats().upstream_rq_retry_.value()); } @@ -1367,10 +1367,10 @@ TEST_F(RouterRetryStateImplTest, NoPreferredOverLimitExceeded) { EXPECT_EQ(RetryStatus::No, state_->shouldRetryHeaders(good_response_headers, request_headers, header_callback_)); - EXPECT_EQ(0UL, cluster_.trafficStats().upstream_rq_retry_limit_exceeded_.value()); + EXPECT_EQ(0UL, cluster_.trafficStats()->upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(0UL, virtual_cluster_.stats().upstream_rq_retry_limit_exceeded_.value()); EXPECT_EQ(0UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); - EXPECT_EQ(1UL, cluster_.trafficStats().upstream_rq_retry_.value()); + EXPECT_EQ(1UL, cluster_.trafficStats()->upstream_rq_retry_.value()); EXPECT_EQ(1UL, virtual_cluster_.stats().upstream_rq_retry_.value()); EXPECT_EQ(0UL, route_stats_context_.stats().upstream_rq_retry_limit_exceeded_.value()); } diff --git a/test/common/stats/BUILD b/test/common/stats/BUILD index 22e16577b110f..7ad1050423ea4 100644 --- a/test/common/stats/BUILD +++ b/test/common/stats/BUILD @@ -99,6 +99,18 @@ envoy_cc_test( ], ) +envoy_cc_test_library( + name = "real_thread_test_base", + srcs = ["real_thread_test_base.cc"], + hdrs = ["real_thread_test_base.h"], + deps = [ + "//source/common/stats:thread_local_store_lib", + "//source/common/thread_local:thread_local_lib", + "//test/mocks/thread_local:thread_local_mocks", + "//test/test_common:real_threads_test_helper_lib", + ], +) + envoy_cc_test_library( name = "stat_test_utility_lib", srcs = ["stat_test_utility.cc"], @@ -263,18 +275,15 @@ envoy_cc_test( name = "thread_local_store_test", srcs = ["thread_local_store_test.cc"], deps = [ + ":real_thread_test_base", ":stat_test_utility_lib", "//source/common/memory:stats_lib", "//source/common/stats:stats_matcher_lib", "//source/common/stats:symbol_table_lib", - "//source/common/stats:thread_local_store_lib", - "//source/common/thread_local:thread_local_lib", "//test/mocks/event:event_mocks", "//test/mocks/server:instance_mocks", "//test/mocks/stats:stats_mocks", - "//test/mocks/thread_local:thread_local_mocks", "//test/test_common:logging_lib", - "//test/test_common:real_threads_test_helper_lib", "//test/test_common:test_time_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/metrics/v3:pkg_cc_proto", diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc index 93e633fbe9995..524715f219671 100644 --- a/test/common/stats/thread_local_store_test.cc +++ b/test/common/stats/thread_local_store_test.cc @@ -15,15 +15,14 @@ #include "source/common/stats/symbol_table.h" #include "source/common/stats/tag_producer_impl.h" #include "source/common/stats/thread_local_store.h" -#include "source/common/thread_local/thread_local_impl.h" +#include "test/common/stats/real_thread_test_base.h" #include "test/common/stats/stat_test_utility.h" #include "test/mocks/event/mocks.h" #include "test/mocks/server/instance.h" #include "test/mocks/stats/mocks.h" #include "test/mocks/thread_local/mocks.h" #include "test/test_common/logging.h" -#include "test/test_common/real_threads_test_helper.h" #include "test/test_common/utility.h" #include "absl/strings/str_split.h" @@ -756,26 +755,7 @@ TEST_F(StatsThreadLocalStoreTest, SharedScopes) { tls_.shutdownThread(); } -class ThreadLocalStoreNoMocksTestBase : public testing::Test { -public: - ThreadLocalStoreNoMocksTestBase() - : alloc_(symbol_table_), store_(std::make_unique(alloc_)), - pool_(symbol_table_) {} - ~ThreadLocalStoreNoMocksTestBase() override { - if (store_ != nullptr) { - store_->shutdownThreading(); - } - } - - StatName makeStatName(absl::string_view name) { return pool_.add(name); } - - SymbolTableImpl symbol_table_; - AllocatorImpl alloc_; - ThreadLocalStoreImplPtr store_; - StatNamePool pool_; -}; - -class LookupWithStatNameTest : public ThreadLocalStoreNoMocksTestBase {}; +class LookupWithStatNameTest : public ThreadLocalStoreNoMocksTestBase, public testing::Test {}; TEST_F(LookupWithStatNameTest, All) { ScopeSharedPtr scope1 = store_->scopeFromStatName(makeStatName("scope1")); @@ -1684,39 +1664,7 @@ TEST_F(HistogramTest, ForEachHistogram) { EXPECT_EQ(deleted_histogram.unit(), Histogram::Unit::Unspecified); } -class ThreadLocalRealThreadsTestBase : public Thread::RealThreadsTestHelper, - public ThreadLocalStoreNoMocksTestBase { -protected: - static constexpr uint32_t NumScopes = 1000; - static constexpr uint32_t NumIters = 35; - -public: - ThreadLocalRealThreadsTestBase(uint32_t num_threads) - : RealThreadsTestHelper(num_threads), pool_(store_->symbolTable()) { - runOnMainBlocking([this]() { store_->initializeThreading(*main_dispatcher_, *tls_); }); - } - - ~ThreadLocalRealThreadsTestBase() override { - // TODO(chaoqin-li1123): clean this up when we figure out how to free the threading resources in - // RealThreadsTestHelper. - shutdownThreading(); - exitThreads([this]() { store_.reset(); }); - } - - void shutdownThreading() { - runOnMainBlocking([this]() { - if (!tls_->isShutdown()) { - tls_->shutdownGlobalThreading(); - } - store_->shutdownThreading(); - tls_->shutdownThread(); - }); - } - - StatNamePool pool_; -}; - -class OneWorkerThread : public ThreadLocalRealThreadsTestBase { +class OneWorkerThread : public ThreadLocalRealThreadsTestBase, public testing::Test { protected: static constexpr uint32_t NumThreads = 1; OneWorkerThread() : ThreadLocalRealThreadsTestBase(NumThreads) {} @@ -1760,7 +1708,8 @@ TEST_F(OneWorkerThread, DeleteForEachRace) { wait_for_main(); } -class ClusterShutdownCleanupStarvationTest : public ThreadLocalRealThreadsTestBase { +class ClusterShutdownCleanupStarvationTest : public ThreadLocalRealThreadsTestBase, + public testing::Test { protected: static constexpr uint32_t NumThreads = 2; @@ -1844,7 +1793,7 @@ TEST_F(ClusterShutdownCleanupStarvationTest, TwelveThreadsWithoutBlockade) { store_->sync().signal(ThreadLocalStoreImpl::MainDispatcherCleanupSync); } -class HistogramThreadTest : public ThreadLocalRealThreadsTestBase { +class HistogramThreadTest : public ThreadLocalRealThreadsTestBase, public testing::Test { protected: static constexpr uint32_t NumThreads = 10; diff --git a/test/common/tcp/conn_pool_test.cc b/test/common/tcp/conn_pool_test.cc index f6870ecabc369..78de9cb0c8bd8 100644 --- a/test/common/tcp/conn_pool_test.cc +++ b/test/common/tcp/conn_pool_test.cc @@ -569,7 +569,7 @@ TEST_F(TcpConnPoolImplTest, MaxPendingRequests) { EXPECT_EQ(ConnectionPool::PoolFailureReason::Overflow, callbacks2.reason_); - EXPECT_EQ(1U, cluster_->stats_.upstream_rq_pending_overflow_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_rq_pending_overflow_.value()); } /** @@ -597,8 +597,8 @@ TEST_F(TcpConnPoolImplTest, RemoteConnectFailure) { EXPECT_EQ(ConnectionPool::PoolFailureReason::RemoteConnectionFailure, callbacks.reason_); EXPECT_EQ("foo", callbacks.failure_reason_string_); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_connect_fail_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_rq_pending_failure_eject_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_connect_fail_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_rq_pending_failure_eject_.value()); } /** @@ -623,8 +623,8 @@ TEST_F(TcpConnPoolImplTest, LocalConnectFailure) { EXPECT_EQ(ConnectionPool::PoolFailureReason::LocalConnectionFailure, callbacks.reason_); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_connect_fail_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_rq_pending_failure_eject_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_connect_fail_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_rq_pending_failure_eject_.value()); } /** @@ -655,8 +655,8 @@ TEST_F(TcpConnPoolImplTest, ConnectTimeout) { EXPECT_EQ(ConnectionPool::PoolFailureReason::Timeout, callbacks1.reason_); EXPECT_EQ(ConnectionPool::PoolFailureReason::Timeout, callbacks2.reason_); - EXPECT_EQ(2U, cluster_->stats_.upstream_cx_connect_fail_.value()); - EXPECT_EQ(2U, cluster_->stats_.upstream_cx_connect_timeout_.value()); + EXPECT_EQ(2U, cluster_->traffic_stats_->upstream_cx_connect_fail_.value()); + EXPECT_EQ(2U, cluster_->traffic_stats_->upstream_cx_connect_timeout_.value()); } /** @@ -781,7 +781,7 @@ TEST_F(TcpConnPoolImplTest, MaxConnections) { // Request 2 should not kick off a new connection. ConnPoolCallbacks callbacks2; handle = conn_pool_->newConnection(callbacks2); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_overflow_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_overflow_.value()); EXPECT_NE(nullptr, handle); @@ -828,8 +828,8 @@ TEST_F(TcpConnPoolImplTest, MaxRequestsPerConnection) { callbacks.conn_data_.reset(); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(0U, cluster_->stats_.upstream_cx_destroy_with_active_rq_.value()); - EXPECT_EQ(1U, cluster_->stats_.upstream_cx_max_requests_.value()); + EXPECT_EQ(0U, cluster_->traffic_stats_->upstream_cx_destroy_with_active_rq_.value()); + EXPECT_EQ(1U, cluster_->traffic_stats_->upstream_cx_max_requests_.value()); } /* diff --git a/test/common/upstream/conn_pool_map_impl_test.cc b/test/common/upstream/conn_pool_map_impl_test.cc index cca21cb716185..e51a0de702f10 100644 --- a/test/common/upstream/conn_pool_map_impl_test.cc +++ b/test/common/upstream/conn_pool_map_impl_test.cc @@ -235,7 +235,7 @@ TEST_F(ConnPoolMapImplTest, GetPoolHittingLimitIncrementsFailureCounter) { ON_CALL(*mock_pools_[0], hasActiveConnections()).WillByDefault(Return(true)); test_map->getPool(2, getNeverCalledFactory()); - EXPECT_EQ(host_->cluster_.stats_.upstream_cx_pool_overflow_.value(), 1); + EXPECT_EQ(host_->cluster_.traffic_stats_->upstream_cx_pool_overflow_.value(), 1); } TEST_F(ConnPoolMapImplTest, GetPoolHittingLimitIncrementsFailureMultiple) { @@ -247,7 +247,7 @@ TEST_F(ConnPoolMapImplTest, GetPoolHittingLimitIncrementsFailureMultiple) { test_map->getPool(2, getNeverCalledFactory()); test_map->getPool(2, getNeverCalledFactory()); - EXPECT_EQ(host_->cluster_.stats_.upstream_cx_pool_overflow_.value(), 3); + EXPECT_EQ(host_->cluster_.traffic_stats_->upstream_cx_pool_overflow_.value(), 3); } TEST_F(ConnPoolMapImplTest, GetPoolHittingLimitGreaterThan1Fails) { @@ -284,7 +284,7 @@ TEST_F(ConnPoolMapImplTest, GetPoolLimitHitFollowedBySuccessDoesNotClearFailure) ON_CALL(*mock_pools_[0], hasActiveConnections()).WillByDefault(Return(false)); test_map->getPool(2, getBasicFactory()); - EXPECT_EQ(host_->cluster_.stats_.upstream_cx_pool_overflow_.value(), 1); + EXPECT_EQ(host_->cluster_.traffic_stats_->upstream_cx_pool_overflow_.value(), 1); } // Test that only the pool which are idle are actually cleared diff --git a/test/common/upstream/health_check_fuzz.cc b/test/common/upstream/health_check_fuzz.cc index e845aceb08663..dcc5ab5579f9d 100644 --- a/test/common/upstream/health_check_fuzz.cc +++ b/test/common/upstream/health_check_fuzz.cc @@ -106,7 +106,7 @@ void HttpHealthCheckFuzz::initialize(test::common::upstream::HealthCheckTestCase cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", *time_source)}; if (input.upstream_cx_success()) { - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); } expectSessionCreate(); expectStreamCreate(0); @@ -217,7 +217,7 @@ void TcpHealthCheckFuzz::initialize(test::common::upstream::HealthCheckTestCase cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", *time_source)}; if (input.upstream_cx_success()) { - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); } expectSessionCreate(); expectClientCreate(); @@ -326,7 +326,7 @@ void GrpcHealthCheckFuzz::initialize(test::common::upstream::HealthCheckTestCase cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", *time_source)}; if (input.upstream_cx_success()) { - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); } expectSessionCreate(); ON_CALL(dispatcher_, createClientConnection_(_, _, _, _)) diff --git a/test/common/upstream/health_checker_impl_test.cc b/test/common/upstream/health_checker_impl_test.cc index e621f94e01cb8..d5720de1c393c 100644 --- a/test/common/upstream/health_checker_impl_test.cc +++ b/test/common/upstream/health_checker_impl_test.cc @@ -858,7 +858,7 @@ TEST_F(HttpHealthCheckerImplTest, Success) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -881,7 +881,7 @@ TEST_F(HttpHealthCheckerImplTest, Degraded) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1011,7 +1011,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessIntervalJitterPercent) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1042,7 +1042,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessWithSpurious1xx) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1070,7 +1070,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessWithSpuriousMetadata) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1100,8 +1100,8 @@ TEST_F(HttpHealthCheckerImplTest, SuccessWithMultipleHosts) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime()), makeTestHost(cluster_->info_, "tcp://127.0.0.1:81", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1137,8 +1137,8 @@ TEST_F(HttpHealthCheckerImplTest, SuccessWithMultipleHostSets) { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; cluster_->prioritySet().getMockHostSet(1)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:81", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1171,7 +1171,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessExpectedResponseCheck) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1194,7 +1194,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessExpectedResponseStringContainsCheck) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1218,7 +1218,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessExpectedResponseHexStringContainsCheck) cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1241,7 +1241,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessExpectedResponseCheckBuffer) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1266,7 +1266,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessExpectedResponseCheckMaxBuffer) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1306,7 +1306,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessExpectedResponseCheckHttp2) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1331,7 +1331,7 @@ TEST_F(HttpHealthCheckerImplTest, FailExpectedResponseCheck) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1357,7 +1357,7 @@ TEST_F(HttpHealthCheckerImplTest, FailStatusCheckWithExpectedResponseCheck) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1382,7 +1382,7 @@ TEST_F(HttpHealthCheckerImplTest, ImmediateFailExpectedResponseCheck) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1429,7 +1429,7 @@ TEST_F(HttpHealthCheckerImplTest, ZeroRetryInterval) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1488,7 +1488,7 @@ TEST_F(HttpHealthCheckerImplTest, TlsOptions) { allocHealthChecker(yaml); cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1506,7 +1506,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessServiceCheckSetsCorrectLastHcPassTime) simTime().advanceTimeWait(std::chrono::seconds(1)); cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1589,7 +1589,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessServicePrefixPatternCheck) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1625,7 +1625,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessServiceExactPatternCheck) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1661,7 +1661,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessServiceRegexPatternCheck) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1705,7 +1705,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessServiceCheckWithCustomHostValueOnTheHos EXPECT_CALL(*this, onHostStatus(_, HealthTransition::Unchanged)); cluster_->prioritySet().getMockHostSet(0)->hosts_ = {test_host}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1752,7 +1752,7 @@ TEST_F(HttpHealthCheckerImplTest, EXPECT_CALL(*this, onHostStatus(_, HealthTransition::Unchanged)); cluster_->prioritySet().getMockHostSet(0)->hosts_ = {test_host}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1788,7 +1788,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessServiceCheckWithCustomHostValue) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1853,7 +1853,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessServiceCheckWithAdditionalHeaders) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", metadata, simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1918,7 +1918,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessServiceCheckWithoutUserAgent) { std::string current_start_time; cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", metadata, simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1956,7 +1956,7 @@ TEST_F(HttpHealthCheckerImplTest, ServiceDoesNotMatchFail) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -1988,7 +1988,7 @@ TEST_F(HttpHealthCheckerImplTest, ServicePatternDoesNotMatchFail) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -2019,7 +2019,7 @@ TEST_F(HttpHealthCheckerImplTest, ServiceNotPresentInResponseFail) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -2047,7 +2047,7 @@ TEST_F(HttpHealthCheckerImplTest, ServiceCheckRuntimeOff) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -2074,7 +2074,7 @@ TEST_F(HttpHealthCheckerImplTest, ServiceCheckRuntimeOffWithStringPattern) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -2636,7 +2636,7 @@ TEST_F(HttpHealthCheckerImplTest, HealthCheckIntervals) { EXPECT_CALL(*test_sessions_[0]->interval_timer_, enableTimer(std::chrono::milliseconds(5000), _)); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, disableTimer()); respond(0, "200", false); - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); // Needed after a response is sent. @@ -2975,7 +2975,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessServiceCheckWithAltPort) { // Prepares a host with its designated health check port. const HostWithHealthCheckMap hosts{{"127.0.0.1:80", makeHealthCheckConfig(8000)}}; appendTestHosts(cluster_, hosts); - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(hosts); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -3008,8 +3008,8 @@ TEST_F(HttpHealthCheckerImplTest, SuccessWithMultipleHostsAndAltPort) { const HostWithHealthCheckMap hosts = {{"127.0.0.1:80", makeHealthCheckConfig(8000)}, {"127.0.0.1:81", makeHealthCheckConfig(8001)}}; appendTestHosts(cluster_, hosts); - cluster_->info_->trafficStats().upstream_cx_total_.inc(); - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(hosts); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -3049,7 +3049,7 @@ TEST_F(HttpHealthCheckerImplTest, SuccessServiceCheckWithAltAddress) { const HostWithHealthCheckMap hosts{ {"127.0.0.1:80", makeHealthCheckConfigAltAddress("127.0.0.2", 8000)}}; appendTestHosts(cluster_, hosts); - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(hosts); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -3083,8 +3083,8 @@ TEST_F(HttpHealthCheckerImplTest, SuccessWithMultipleHostsAndAltAddress) { {"127.0.0.1:80", makeHealthCheckConfigAltAddress("127.0.0.2", 8000)}, {"127.0.0.2:81", makeHealthCheckConfigAltAddress("127.0.0.2", 8000)}}; appendTestHosts(cluster_, hosts); - cluster_->info_->trafficStats().upstream_cx_total_.inc(); - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(hosts); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -3180,7 +3180,7 @@ TEST_F(HttpHealthCheckerImplTest, TransportSocketMatchCriteria) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -3221,7 +3221,7 @@ TEST_F(HttpHealthCheckerImplTest, NoTransportSocketMatchCriteria) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -3238,7 +3238,7 @@ TEST_F(HttpHealthCheckerImplTest, GoAwayErrorProbeInProgress) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -3286,7 +3286,7 @@ TEST_F(HttpHealthCheckerImplTest, GoAwayProbeInProgress) { .WillRepeatedly(Return(false)); cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); @@ -3585,7 +3585,7 @@ TEST_F(HttpHealthCheckerImplTest, ServiceNameMatch) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -3621,7 +3621,7 @@ TEST_F(HttpHealthCheckerImplTest, ServiceNameMismatch) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -3647,7 +3647,7 @@ TEST_F(HttpHealthCheckerImplTest, DefaultMethodGet) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -3676,7 +3676,7 @@ TEST_F(HttpHealthCheckerImplTest, MethodHead) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectStreamCreate(0); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); @@ -4916,7 +4916,7 @@ class GrpcHealthCheckerImplTestBase : public Event::TestUsingSimulatedTime, // performed during test case (but possibly on many hosts). void expectHealthchecks(HealthTransition host_changed_state, size_t num_healthchecks) { for (size_t i = 0; i < num_healthchecks; i++) { - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectHealthcheckStart(i); } @@ -5019,7 +5019,7 @@ class GrpcHealthCheckerImplTestBase : public Event::TestUsingSimulatedTime, void runHealthCheck(std::string expected_host) { - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectHealthcheckStart(0); @@ -5169,7 +5169,7 @@ TEST_F(GrpcHealthCheckerImplTest, SuccessWithAdditionalHeaders) { cluster_->prioritySet().getMockHostSet(0)->hosts_ = { makeTestHost(cluster_->info_, "tcp://127.0.0.1:80", metadata, simTime())}; - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); expectSessionCreate(); expectHealthcheckStart(0); @@ -5641,7 +5641,7 @@ TEST_F(GrpcHealthCheckerImplTest, HealthCheckIntervals) { EXPECT_CALL(*test_sessions_[0]->interval_timer_, enableTimer(std::chrono::milliseconds(5000), _)); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, disableTimer()); respondServiceStatus(0, grpc::health::v1::HealthCheckResponse::SERVING); - cluster_->info_->trafficStats().upstream_cx_total_.inc(); + cluster_->info_->trafficStats()->upstream_cx_total_.inc(); EXPECT_CALL(*test_sessions_[0]->timeout_timer_, enableTimer(_, _)); // Needed after a response is sent. diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index f1386f2956dbe..67d4b8a02e17a 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -420,7 +420,7 @@ TEST_F(StrictDnsClusterImplTest, Basic) { EXPECT_EQ(1U, cluster.info()->resourceManager(ResourcePriority::Default).maxConnectionsPerHost()); EXPECT_EQ(990U, cluster.info()->resourceManager(ResourcePriority::High).maxConnectionsPerHost()); - cluster.info()->trafficStats().upstream_rq_total_.inc(); + cluster.info()->trafficStats()->upstream_rq_total_.inc(); EXPECT_EQ(1UL, stats_.counter("cluster.name.upstream_rq_total").value()); EXPECT_CALL(runtime_.snapshot_, featureEnabled("upstream.maintenance_mode.name", 0)); @@ -837,7 +837,7 @@ TEST_F(StrictDnsClusterImplTest, LoadAssignmentBasic) { EXPECT_EQ(3U, cluster.info()->maxRequestsPerConnection()); EXPECT_EQ(0U, cluster.info()->http2Options().hpack_table_size().value()); - cluster.info()->trafficStats().upstream_rq_total_.inc(); + cluster.info()->trafficStats()->upstream_rq_total_.inc(); EXPECT_EQ(1UL, stats_.counter("cluster.name.upstream_rq_total").value()); EXPECT_CALL(runtime_.snapshot_, featureEnabled("upstream.maintenance_mode.name", 0)); @@ -1938,7 +1938,7 @@ TEST_F(StaticClusterImplTest, AltStatName) { std::move(scope), false); cluster.initialize([] {}); // Increment a stat and verify it is emitted with alt_stat_name - cluster.info()->trafficStats().upstream_rq_total_.inc(); + cluster.info()->trafficStats()->upstream_rq_total_.inc(); EXPECT_EQ(1UL, stats_.counter("cluster.staticcluster_stats.upstream_rq_total").value()); } diff --git a/test/extensions/clusters/aggregate/cluster_test.cc b/test/extensions/clusters/aggregate/cluster_test.cc index ab83624bb6213..1e7c02c067614 100644 --- a/test/extensions/clusters/aggregate/cluster_test.cc +++ b/test/extensions/clusters/aggregate/cluster_test.cc @@ -139,7 +139,7 @@ class AggregateClusterTest : public Event::TestUsingSimulatedTime, public testin Upstream::LoadBalancerFactorySharedPtr lb_factory_; Upstream::LoadBalancerPtr lb_; Upstream::ClusterTrafficStatNames stat_names_; - Upstream::ClusterTrafficStats stats_; + std::unique_ptr stats_; std::shared_ptr primary_info_{ new NiceMock()}; std::shared_ptr secondary_info_{ diff --git a/test/extensions/clusters/original_dst/original_dst_cluster_test.cc b/test/extensions/clusters/original_dst/original_dst_cluster_test.cc index 6e465ab77d656..22ad47203bb64 100644 --- a/test/extensions/clusters/original_dst/original_dst_cluster_test.cc +++ b/test/extensions/clusters/original_dst/original_dst_cluster_test.cc @@ -72,12 +72,11 @@ class OriginalDstClusterTest : public Event::TestUsingSimulatedTime, public test void setupFromYaml(const std::string& yaml) { setup(parseClusterFromV3Yaml(yaml)); } void setup(const envoy::config::cluster::v3::Cluster& cluster_config) { - NiceMock cm; Envoy::Stats::ScopeSharedPtr scope = stats_store_.createScope(fmt::format( "cluster.{}.", cluster_config.alt_stat_name().empty() ? cluster_config.name() : cluster_config.alt_stat_name())); Envoy::Server::Configuration::TransportSocketFactoryContextImpl factory_context( - server_context_, ssl_context_manager_, *scope, cm, stats_store_, validation_visitor_); + server_context_, ssl_context_manager_, *scope, cm_, stats_store_, validation_visitor_); cluster_ = std::make_shared(server_context_, cluster_config, runtime_, factory_context, std::move(scope), false); priority_update_cb_ = cluster_->prioritySet().addPriorityUpdateCb( @@ -86,7 +85,7 @@ class OriginalDstClusterTest : public Event::TestUsingSimulatedTime, public test }); cluster_->initialize([&]() -> void { initialized_.ready(); }); } - + NiceMock cm_; NiceMock server_context_; Stats::TestUtil::TestStore stats_store_; Ssl::MockContextManager ssl_context_manager_; diff --git a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_test.cc b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_test.cc index f6ab28f56fbe2..0745401a05aeb 100644 --- a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_test.cc +++ b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_test.cc @@ -258,8 +258,8 @@ TEST_F(ProxyFilterTest, CircuitBreakerOverflowWithDnsCacheResourceManager) { filter2->decodeHeaders(request_headers_, false)); // Cluster circuit breaker overflow counter won't be incremented. - EXPECT_EQ(0, - cm_.thread_local_cluster_.cluster_.info_->stats_.upstream_rq_pending_overflow_.value()); + EXPECT_EQ(0, cm_.thread_local_cluster_.cluster_.info_->traffic_stats_ + ->upstream_rq_pending_overflow_.value()); filter2->onDestroy(); EXPECT_CALL(*handle, onDestroy()); filter_->onDestroy(); diff --git a/test/extensions/filters/http/gcp_authn/gcp_authn_filter_integration_test.cc b/test/extensions/filters/http/gcp_authn/gcp_authn_filter_integration_test.cc index 299b6b1a7ee7d..c133a78552ac3 100644 --- a/test/extensions/filters/http/gcp_authn/gcp_authn_filter_integration_test.cc +++ b/test/extensions/filters/http/gcp_authn/gcp_authn_filter_integration_test.cc @@ -270,7 +270,9 @@ TEST_P(GcpAuthnFilterIntegrationTest, BasicflowWithoutAudience) { sendRequestToDestinationAndValidateResponse(/*with_audience=*/false); // Verify request has been routed to `cluster_0` but not `gcp_authn` cluster. - EXPECT_GE(test_server_->counter("cluster.gcp_authn.upstream_cx_total")->value(), 0); + // Note that upstream_cx_total is from the lazy init ClusterInfo::trafficStats(), so there is + // no such stat yet. + EXPECT_EQ(test_server_->counter("cluster.gcp_authn.upstream_cx_total"), nullptr); EXPECT_GE(test_server_->counter("cluster.cluster_0.upstream_cx_total")->value(), 1); // Clean up the codec and connections. diff --git a/test/extensions/filters/network/common/redis/client_impl_test.cc b/test/extensions/filters/network/common/redis/client_impl_test.cc index bcd4d4e3810c1..4d616e46c690e 100644 --- a/test/extensions/filters/network/common/redis/client_impl_test.cc +++ b/test/extensions/filters/network/common/redis/client_impl_test.cc @@ -81,7 +81,7 @@ class RedisClientImplTest : public testing::Test, client_ = ClientImpl::create(host_, dispatcher_, Common::Redis::EncoderPtr{encoder_}, *this, *config_, redis_command_stats_, stats_, false); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_cx_total_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_cx_total_.value()); EXPECT_EQ(1UL, host_->stats_.cx_total_.value()); EXPECT_EQ(false, client_->active()); @@ -117,8 +117,8 @@ class RedisClientImplTest : public testing::Test, EXPECT_CALL(*flush_timer_, enabled()).WillOnce(Return(false)); client_->initialize(auth_username_, auth_password_); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_total_.value()); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_rq_active_.value()); EXPECT_EQ(1UL, host_->stats_.rq_total_.value()); EXPECT_EQ(1UL, host_->stats_.rq_active_.value()); @@ -308,8 +308,8 @@ TEST_F(RedisClientImplTest, Basic) { PoolRequest* handle2 = client_->makeRequest(request2, callbacks2); EXPECT_NE(nullptr, handle2); - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_total_.value()); - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_active_.value()); EXPECT_EQ(2UL, host_->stats_.rq_total_.value()); EXPECT_EQ(2UL, host_->stats_.rq_active_.value()); @@ -385,8 +385,8 @@ TEST_F(RedisClientImplTest, CommandStatsDisabledSingleRequest) { onConnected(); // Regular Envoy stats function as normal - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_total_.value()); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_rq_active_.value()); EXPECT_EQ(1UL, host_->stats_.rq_total_.value()); EXPECT_EQ(1UL, host_->stats_.rq_active_.value()); @@ -449,8 +449,8 @@ TEST_F(RedisClientImplTest, CommandStatsEnabledTwoRequests) { EXPECT_NE(nullptr, handle2); // Regular Envoy stats function as normal - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_total_.value()); - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_active_.value()); EXPECT_EQ(2UL, host_->stats_.rq_total_.value()); EXPECT_EQ(2UL, host_->stats_.rq_active_.value()); @@ -513,8 +513,8 @@ TEST_F(RedisClientImplTest, InitializedWithAuthPassword) { EXPECT_CALL(*flush_timer_, enabled()).WillOnce(Return(false)); client_->initialize(auth_username_, auth_password_); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_total_.value()); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_rq_active_.value()); EXPECT_EQ(1UL, host_->stats_.rq_total_.value()); EXPECT_EQ(1UL, host_->stats_.rq_active_.value()); @@ -535,8 +535,8 @@ TEST_F(RedisClientImplTest, InitializedWithAuthAcl) { EXPECT_CALL(*flush_timer_, enabled()).WillOnce(Return(false)); client_->initialize(auth_username_, auth_password_); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_total_.value()); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_rq_active_.value()); EXPECT_EQ(1UL, host_->stats_.rq_total_.value()); EXPECT_EQ(1UL, host_->stats_.rq_active_.value()); @@ -612,7 +612,7 @@ TEST_F(RedisClientImplTest, Cancel) { EXPECT_CALL(*connect_or_op_timer_, disableTimer()); client_->close(); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_cancelled_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_rq_cancelled_.value()); } TEST_F(RedisClientImplTest, FailAll) { @@ -639,8 +639,9 @@ TEST_F(RedisClientImplTest, FailAll) { EXPECT_CALL(connection_callbacks, onEvent(Network::ConnectionEvent::RemoteClose)); upstream_connection_->raiseEvent(Network::ConnectionEvent::RemoteClose); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_cx_destroy_with_active_rq_.value()); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_cx_destroy_remote_with_active_rq_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_cx_destroy_with_active_rq_.value()); + EXPECT_EQ(1UL, + host_->cluster_.traffic_stats_->upstream_cx_destroy_remote_with_active_rq_.value()); } TEST_F(RedisClientImplTest, FailAllWithCancel) { @@ -666,9 +667,9 @@ TEST_F(RedisClientImplTest, FailAllWithCancel) { EXPECT_CALL(connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)); upstream_connection_->raiseEvent(Network::ConnectionEvent::LocalClose); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_cx_destroy_with_active_rq_.value()); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_cx_destroy_local_with_active_rq_.value()); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_cancelled_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_cx_destroy_with_active_rq_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_cx_destroy_local_with_active_rq_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_rq_cancelled_.value()); } TEST_F(RedisClientImplTest, ProtocolError) { @@ -696,7 +697,7 @@ TEST_F(RedisClientImplTest, ProtocolError) { EXPECT_CALL(*connect_or_op_timer_, disableTimer()); upstream_read_filter_->onData(fake_data, false); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_cx_protocol_error_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_cx_protocol_error_.value()); EXPECT_EQ(1UL, host_->stats_.rq_error_.value()); } @@ -718,7 +719,7 @@ TEST_F(RedisClientImplTest, ConnectFail) { EXPECT_CALL(*connect_or_op_timer_, disableTimer()); upstream_connection_->raiseEvent(Network::ConnectionEvent::RemoteClose); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_cx_connect_fail_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_cx_connect_fail_.value()); EXPECT_EQ(1UL, host_->stats_.cx_connect_fail_.value()); } @@ -753,7 +754,7 @@ TEST_F(RedisClientImplTest, OutlierDisabled) { EXPECT_CALL(*connect_or_op_timer_, disableTimer()); upstream_connection_->raiseEvent(Network::ConnectionEvent::RemoteClose); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_cx_connect_fail_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_cx_connect_fail_.value()); EXPECT_EQ(1UL, host_->stats_.cx_connect_fail_.value()); } @@ -776,7 +777,7 @@ TEST_F(RedisClientImplTest, ConnectTimeout) { EXPECT_CALL(*connect_or_op_timer_, disableTimer()); connect_or_op_timer_->invokeCallback(); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_cx_connect_timeout_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_cx_connect_timeout_.value()); EXPECT_EQ(1UL, host_->stats_.cx_connect_fail_.value()); } @@ -794,8 +795,8 @@ TEST_F(RedisClientImplTest, OpTimeout) { onConnected(); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_total_.value()); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_rq_active_.value()); EXPECT_CALL(callbacks1, onResponse_(_)); EXPECT_CALL(*connect_or_op_timer_, disableTimer()); @@ -803,8 +804,8 @@ TEST_F(RedisClientImplTest, OpTimeout) { putResult(Upstream::Outlier::Result::ExtOriginRequestSuccess, _)); respond(); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_total_.value()); - EXPECT_EQ(0UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(0UL, host_->cluster_.traffic_stats_->upstream_rq_active_.value()); EXPECT_CALL(*encoder_, encode(Ref(request1), _)); EXPECT_CALL(*flush_timer_, enabled()).WillOnce(Return(false)); @@ -819,10 +820,10 @@ TEST_F(RedisClientImplTest, OpTimeout) { EXPECT_CALL(*connect_or_op_timer_, disableTimer()); connect_or_op_timer_->invokeCallback(); - EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_timeout_.value()); + EXPECT_EQ(1UL, host_->cluster_.traffic_stats_->upstream_rq_timeout_.value()); EXPECT_EQ(1UL, host_->stats_.rq_timeout_.value()); - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_total_.value()); - EXPECT_EQ(0UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(0UL, host_->cluster_.traffic_stats_->upstream_rq_active_.value()); } TEST_F(RedisClientImplTest, AskRedirection) { @@ -846,8 +847,8 @@ TEST_F(RedisClientImplTest, AskRedirection) { PoolRequest* handle2 = client_->makeRequest(request2, callbacks2); EXPECT_NE(nullptr, handle2); - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_total_.value()); - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_active_.value()); EXPECT_EQ(2UL, host_->stats_.rq_total_.value()); EXPECT_EQ(2UL, host_->stats_.rq_active_.value()); @@ -903,8 +904,8 @@ TEST_F(RedisClientImplTest, MovedRedirection) { PoolRequest* handle2 = client_->makeRequest(request2, callbacks2); EXPECT_NE(nullptr, handle2); - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_total_.value()); - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_active_.value()); EXPECT_EQ(2UL, host_->stats_.rq_total_.value()); EXPECT_EQ(2UL, host_->stats_.rq_active_.value()); @@ -960,8 +961,8 @@ TEST_F(RedisClientImplTest, RedirectionFailure) { PoolRequest* handle2 = client_->makeRequest(request2, callbacks2); EXPECT_NE(nullptr, handle2); - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_total_.value()); - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_active_.value()); EXPECT_EQ(2UL, host_->stats_.rq_total_.value()); EXPECT_EQ(2UL, host_->stats_.rq_active_.value()); @@ -981,8 +982,10 @@ TEST_F(RedisClientImplTest, RedirectionFailure) { putResult(Upstream::Outlier::Result::ExtOriginRequestSuccess, _)); callbacks_->onRespValue(std::move(response1)); - EXPECT_EQ(0UL, host_->cluster_.stats_.upstream_internal_redirect_succeeded_total_.value()); - EXPECT_EQ(0UL, host_->cluster_.stats_.upstream_internal_redirect_failed_total_.value()); + EXPECT_EQ(0UL, + host_->cluster_.traffic_stats_->upstream_internal_redirect_succeeded_total_.value()); + EXPECT_EQ(0UL, + host_->cluster_.traffic_stats_->upstream_internal_redirect_failed_total_.value()); // Test a truncated MOVED error response that cannot be parsed properly. Common::Redis::RespValuePtr response2(new Common::Redis::RespValue()); @@ -994,8 +997,10 @@ TEST_F(RedisClientImplTest, RedirectionFailure) { putResult(Upstream::Outlier::Result::ExtOriginRequestSuccess, _)); callbacks_->onRespValue(std::move(response2)); - EXPECT_EQ(0UL, host_->cluster_.stats_.upstream_internal_redirect_succeeded_total_.value()); - EXPECT_EQ(0UL, host_->cluster_.stats_.upstream_internal_redirect_failed_total_.value()); + EXPECT_EQ(0UL, + host_->cluster_.traffic_stats_->upstream_internal_redirect_succeeded_total_.value()); + EXPECT_EQ(0UL, + host_->cluster_.traffic_stats_->upstream_internal_redirect_failed_total_.value()); })); upstream_read_filter_->onData(fake_data, false); @@ -1025,8 +1030,8 @@ TEST_F(RedisClientImplTest, AskRedirectionNotEnabled) { PoolRequest* handle2 = client_->makeRequest(request2, callbacks2); EXPECT_NE(nullptr, handle2); - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_total_.value()); - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_active_.value()); EXPECT_EQ(2UL, host_->stats_.rq_total_.value()); EXPECT_EQ(2UL, host_->stats_.rq_active_.value()); @@ -1044,8 +1049,10 @@ TEST_F(RedisClientImplTest, AskRedirectionNotEnabled) { putResult(Upstream::Outlier::Result::ExtOriginRequestSuccess, _)); callbacks_->onRespValue(std::move(response1)); - EXPECT_EQ(0UL, host_->cluster_.stats_.upstream_internal_redirect_failed_total_.value()); - EXPECT_EQ(0UL, host_->cluster_.stats_.upstream_internal_redirect_succeeded_total_.value()); + EXPECT_EQ(0UL, + host_->cluster_.traffic_stats_->upstream_internal_redirect_failed_total_.value()); + EXPECT_EQ(0UL, + host_->cluster_.traffic_stats_->upstream_internal_redirect_succeeded_total_.value()); Common::Redis::RespValuePtr response2(new Common::Redis::RespValue()); response2->type(Common::Redis::RespType::Error); @@ -1057,8 +1064,10 @@ TEST_F(RedisClientImplTest, AskRedirectionNotEnabled) { putResult(Upstream::Outlier::Result::ExtOriginRequestSuccess, _)); callbacks_->onRespValue(std::move(response2)); - EXPECT_EQ(0UL, host_->cluster_.stats_.upstream_internal_redirect_failed_total_.value()); - EXPECT_EQ(0UL, host_->cluster_.stats_.upstream_internal_redirect_succeeded_total_.value()); + EXPECT_EQ(0UL, + host_->cluster_.traffic_stats_->upstream_internal_redirect_failed_total_.value()); + EXPECT_EQ(0UL, + host_->cluster_.traffic_stats_->upstream_internal_redirect_succeeded_total_.value()); })); upstream_read_filter_->onData(fake_data, false); @@ -1088,8 +1097,8 @@ TEST_F(RedisClientImplTest, MovedRedirectionNotEnabled) { PoolRequest* handle2 = client_->makeRequest(request2, callbacks2); EXPECT_NE(nullptr, handle2); - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_total_.value()); - EXPECT_EQ(2UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_total_.value()); + EXPECT_EQ(2UL, host_->cluster_.traffic_stats_->upstream_rq_active_.value()); EXPECT_EQ(2UL, host_->stats_.rq_total_.value()); EXPECT_EQ(2UL, host_->stats_.rq_active_.value()); @@ -1106,8 +1115,10 @@ TEST_F(RedisClientImplTest, MovedRedirectionNotEnabled) { putResult(Upstream::Outlier::Result::ExtOriginRequestSuccess, _)); callbacks_->onRespValue(std::move(response1)); - EXPECT_EQ(0UL, host_->cluster_.stats_.upstream_internal_redirect_succeeded_total_.value()); - EXPECT_EQ(0UL, host_->cluster_.stats_.upstream_internal_redirect_failed_total_.value()); + EXPECT_EQ(0UL, + host_->cluster_.traffic_stats_->upstream_internal_redirect_succeeded_total_.value()); + EXPECT_EQ(0UL, + host_->cluster_.traffic_stats_->upstream_internal_redirect_failed_total_.value()); Common::Redis::RespValuePtr response2(new Common::Redis::RespValue()); response2->type(Common::Redis::RespType::Error); @@ -1119,8 +1130,10 @@ TEST_F(RedisClientImplTest, MovedRedirectionNotEnabled) { putResult(Upstream::Outlier::Result::ExtOriginRequestSuccess, _)); callbacks_->onRespValue(std::move(response2)); - EXPECT_EQ(0UL, host_->cluster_.stats_.upstream_internal_redirect_succeeded_total_.value()); - EXPECT_EQ(0UL, host_->cluster_.stats_.upstream_internal_redirect_failed_total_.value()); + EXPECT_EQ(0UL, + host_->cluster_.traffic_stats_->upstream_internal_redirect_succeeded_total_.value()); + EXPECT_EQ(0UL, + host_->cluster_.traffic_stats_->upstream_internal_redirect_failed_total_.value()); })); upstream_read_filter_->onData(fake_data, false); diff --git a/test/extensions/filters/udp/udp_proxy/udp_proxy_filter_test.cc b/test/extensions/filters/udp/udp_proxy/udp_proxy_filter_test.cc index 6fae57176a9b8..9ef89d3269a80 100644 --- a/test/extensions/filters/udp/udp_proxy/udp_proxy_filter_test.cc +++ b/test/extensions/filters/udp/udp_proxy/udp_proxy_filter_test.cc @@ -578,19 +578,19 @@ stat_prefix: foo test_sessions_[0].expectWriteToUpstream("hello", 0, nullptr, true); recvDataFromDownstream("10.0.0.1:1000", "10.0.0.2:80", "hello"); checkTransferStats(5 /*rx_bytes*/, 1 /*rx_datagrams*/, 0 /*tx_bytes*/, 0 /*tx_datagrams*/); - EXPECT_EQ(5, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_ - .upstream_cx_tx_bytes_total_.value()); + EXPECT_EQ(5, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_ + ->traffic_stats_->upstream_cx_tx_bytes_total_.value()); test_sessions_[0].recvDataFromUpstream("world2", 0, SOCKET_ERROR_MSG_SIZE); checkTransferStats(5 /*rx_bytes*/, 1 /*rx_datagrams*/, 0 /*tx_bytes*/, 0 /*tx_datagrams*/); - EXPECT_EQ(6, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_ - .upstream_cx_rx_bytes_total_.value()); + EXPECT_EQ(6, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_ + ->traffic_stats_->upstream_cx_rx_bytes_total_.value()); EXPECT_EQ(1, config_->stats().downstream_sess_tx_errors_.value()); test_sessions_[0].recvDataFromUpstream("world2", SOCKET_ERROR_MSG_SIZE, 0); checkTransferStats(5 /*rx_bytes*/, 1 /*rx_datagrams*/, 0 /*tx_bytes*/, 0 /*tx_datagrams*/); - EXPECT_EQ(6, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_ - .upstream_cx_rx_bytes_total_.value()); + EXPECT_EQ(6, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_ + ->traffic_stats_->upstream_cx_rx_bytes_total_.value()); EXPECT_EQ( 1, TestUtility::findCounter( factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_store_, @@ -600,8 +600,8 @@ stat_prefix: foo test_sessions_[0].expectWriteToUpstream("hello", SOCKET_ERROR_MSG_SIZE); recvDataFromDownstream("10.0.0.1:1000", "10.0.0.2:80", "hello"); checkTransferStats(10 /*rx_bytes*/, 2 /*rx_datagrams*/, 0 /*tx_bytes*/, 0 /*tx_datagrams*/); - EXPECT_EQ(5, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_ - .upstream_cx_tx_bytes_total_.value()); + EXPECT_EQ(5, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_ + ->traffic_stats_->upstream_cx_tx_bytes_total_.value()); EXPECT_EQ( 1, TestUtility::findCounter( factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_store_, @@ -676,8 +676,8 @@ stat_prefix: foo EXPECT_CALL(factory_context_.cluster_manager_.thread_local_cluster_.lb_, chooseHost(_)) .WillOnce(Return(nullptr)); recvDataFromDownstream("10.0.0.1:1000", "10.0.0.2:80", "hello"); - EXPECT_EQ(1, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_ - .upstream_cx_none_healthy_.value()); + EXPECT_EQ(1, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_ + ->traffic_stats_->upstream_cx_none_healthy_.value()); } // No cluster at filter creation. @@ -784,8 +784,8 @@ stat_prefix: foo // This should hit the session circuit breaker. recvDataFromDownstream("10.0.0.2:1000", "10.0.0.2:80", "hello"); - EXPECT_EQ(1, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_ - .upstream_cx_overflow_.value()); + EXPECT_EQ(1, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_ + ->traffic_stats_->upstream_cx_overflow_.value()); EXPECT_EQ(1, config_->stats().downstream_sess_total_.value()); EXPECT_EQ(1, config_->stats().downstream_sess_active_.value()); @@ -980,8 +980,8 @@ use_per_packet_load_balancing: true recvDataFromDownstream("10.0.0.1:1000", "10.0.0.2:80", "hello"); EXPECT_EQ(0, config_->stats().downstream_sess_total_.value()); EXPECT_EQ(0, config_->stats().downstream_sess_active_.value()); - EXPECT_EQ(1, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_ - .upstream_cx_none_healthy_.value()); + EXPECT_EQ(1, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_ + ->traffic_stats_->upstream_cx_none_healthy_.value()); } // Verify that when on second packet no host is available, message is dropped. @@ -1005,16 +1005,16 @@ use_per_packet_load_balancing: true recvDataFromDownstream("10.0.0.1:1000", "10.0.0.2:80", "hello"); EXPECT_EQ(1, config_->stats().downstream_sess_total_.value()); EXPECT_EQ(1, config_->stats().downstream_sess_active_.value()); - EXPECT_EQ(0, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_ - .upstream_cx_none_healthy_.value()); + EXPECT_EQ(0, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_ + ->traffic_stats_->upstream_cx_none_healthy_.value()); EXPECT_CALL(factory_context_.cluster_manager_.thread_local_cluster_.lb_, chooseHost(_)) .WillOnce(Return(nullptr)); recvDataFromDownstream("10.0.0.1:1000", "10.0.0.2:80", "hello2"); EXPECT_EQ(1, config_->stats().downstream_sess_total_.value()); EXPECT_EQ(1, config_->stats().downstream_sess_active_.value()); - EXPECT_EQ(1, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_ - .upstream_cx_none_healthy_.value()); + EXPECT_EQ(1, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_ + ->traffic_stats_->upstream_cx_none_healthy_.value()); } // Verify that all sessions for a host are removed when a host is removed. @@ -1117,8 +1117,8 @@ use_per_packet_load_balancing: true 0, 0, 0, 0, 0); recvDataFromDownstream("10.0.0.1:1000", "10.0.0.2:80", "hello"); - EXPECT_EQ(1, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_->stats_ - .upstream_cx_overflow_.value()); + EXPECT_EQ(1, factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_ + ->traffic_stats_->upstream_cx_overflow_.value()); } // Make sure socket option is set correctly if use_original_src_ip is set in case of ipv6. diff --git a/test/extensions/stats_sinks/common/statsd/statsd_test.cc b/test/extensions/stats_sinks/common/statsd/statsd_test.cc index 8e036f008e076..fcadb4d6a7102 100644 --- a/test/extensions/stats_sinks/common/statsd/statsd_test.cc +++ b/test/extensions/stats_sinks/common/statsd/statsd_test.cc @@ -226,13 +226,13 @@ TEST_F(TcpStatsdSinkTest, Overflow) { // Synthetically set buffer above high watermark. Make sure we don't write anything. cluster_manager_.active_clusters_["fake_cluster"] ->info_->trafficStats() - .upstream_cx_tx_bytes_buffered_.set(1024 * 1024 * 17); + ->upstream_cx_tx_bytes_buffered_.set(1024 * 1024 * 17); sink_->flush(snapshot_); // Lower and make sure we write. cluster_manager_.active_clusters_["fake_cluster"] ->info_->trafficStats() - .upstream_cx_tx_bytes_buffered_.set(1024 * 1024 * 15); + ->upstream_cx_tx_bytes_buffered_.set(1024 * 1024 * 15); expectCreateConnection(); EXPECT_CALL(*connection_, write(BufferStringEqual("envoy.test_counter:1|c\n"), _)); sink_->flush(snapshot_); @@ -240,7 +240,7 @@ TEST_F(TcpStatsdSinkTest, Overflow) { // Raise and make sure we don't write and kill connection. cluster_manager_.active_clusters_["fake_cluster"] ->info_->trafficStats() - .upstream_cx_tx_bytes_buffered_.set(1024 * 1024 * 17); + ->upstream_cx_tx_bytes_buffered_.set(1024 * 1024 * 17); EXPECT_CALL(*connection_, close(Network::ConnectionCloseType::NoFlush)); sink_->flush(snapshot_); diff --git a/test/extensions/stats_sinks/hystrix/hystrix_test.cc b/test/extensions/stats_sinks/hystrix/hystrix_test.cc index 53dd5278abd76..0a71f9df68360 100644 --- a/test/extensions/stats_sinks/hystrix/hystrix_test.cc +++ b/test/extensions/stats_sinks/hystrix/hystrix_test.cc @@ -82,9 +82,9 @@ class ClusterTestInfo { ON_CALL(error_4xx_counter_, value()).WillByDefault(Return((i + 1) * error_4xx_step)); ON_CALL(retry_4xx_counter_, value()).WillByDefault(Return((i + 1) * error_4xx_retry_step)); ON_CALL(success_counter_, value()).WillByDefault(Return((i + 1) * success_step)); - cluster_info_->trafficStats().upstream_rq_timeout_.add(timeout_step); - cluster_info_->trafficStats().upstream_rq_per_try_timeout_.add(timeout_retry_step); - cluster_info_->trafficStats().upstream_rq_pending_overflow_.add(rejected_step); + cluster_info_->trafficStats()->upstream_rq_timeout_.add(timeout_step); + cluster_info_->trafficStats()->upstream_rq_per_try_timeout_.add(timeout_retry_step); + cluster_info_->trafficStats()->upstream_rq_pending_overflow_.add(rejected_step); } NiceMock cluster_; diff --git a/test/integration/base_integration_test.h b/test/integration/base_integration_test.h index 25fb42a406094..4ed6e08f19ca3 100644 --- a/test/integration/base_integration_test.h +++ b/test/integration/base_integration_test.h @@ -118,6 +118,32 @@ class BaseIntegrationTest : protected Logger::Loggable { void registerPort(const std::string& key, uint32_t port); uint32_t lookupPort(const std::string& key); + /** + * @brief Schedule a callback on main thread to force create the traffic stats for the given + * cluster. + * + * @param cluster_name name of the cluster. + * @return whether cluster is found. + */ + bool forceCreationOfClusterTrafficStats(absl::string_view cluster_name) { + // With https://github.com/envoyproxy/envoy/pull/23921 ClusterInfo::trafficStats is lazy init. + // We need to trigger creation of ClusterInfo::trafficStats() by calling the * operator. + absl::Notification notifier; + + bool cluster_found = false; + test_server_->server().dispatcher().post([&]() { + const Upstream::ClusterConstOptRef& cluster_ref = + test_server_->server().clusterManager().clusters().getCluster(cluster_name); + if (cluster_ref.has_value()) { + *cluster_ref->get().info()->trafficStats(); + cluster_found = true; + } + notifier.Notify(); + }); + notifier.WaitForNotification(); + return cluster_found; + } + // Set the endpoint's socket address to point at upstream at given index. void setUpstreamAddress(uint32_t upstream_index, envoy::config::endpoint::v3::LbEndpoint& endpoint) const; diff --git a/test/integration/cds_integration_test.cc b/test/integration/cds_integration_test.cc index 8e03b0421201e..c7633fa4902b8 100644 --- a/test/integration/cds_integration_test.cc +++ b/test/integration/cds_integration_test.cc @@ -195,6 +195,8 @@ TEST_P(CdsIntegrationTest, CdsClusterUpDownUp) { TEST_P(CdsIntegrationTest, CdsClusterTeardownWhileConnecting) { initialize(); test_server_->waitForCounterGe("cluster_manager.cluster_added", 1); + ASSERT_TRUE(forceCreationOfClusterTrafficStats("cluster_1")); + test_server_->waitForCounterExists("cluster.cluster_1.upstream_cx_total"); Stats::CounterSharedPtr cx_counter = test_server_->counter("cluster.cluster_1.upstream_cx_total"); // Confirm no upstream connection is attempted so far. diff --git a/test/integration/integration_admin_test.cc b/test/integration/integration_admin_test.cc index f9e5b7027072c..8fcb2cfc2f797 100644 --- a/test/integration/integration_admin_test.cc +++ b/test/integration/integration_admin_test.cc @@ -129,6 +129,7 @@ TEST_P(IntegrationAdminTest, Admin) { initialize(); BufferingStreamDecoderPtr response; + ASSERT_TRUE(forceCreationOfClusterTrafficStats("cluster_0")); EXPECT_EQ("404", request("admin", "GET", "/notfound", response)); EXPECT_EQ("text/plain; charset=UTF-8", contentType(response)); EXPECT_THAT(response->body(), HasSubstr("invalid path. admin commands are:")); diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index 8f0392873fe01..22c7af3c3b058 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -2460,9 +2460,11 @@ TEST_P(IntegrationTest, SetRouteToDelegatingRouteWithClusterOverride) { EXPECT_EQ("200", response->headers().getStatusValue()); // Even though headers specify cluster_0, set_route_filter modifies cached route cluster of - // current request to cluster_override - EXPECT_EQ(0, test_server_->counter("cluster.cluster_0.upstream_cx_total")->value()); - EXPECT_EQ(0, test_server_->counter("cluster.cluster_0.upstream_rq_total")->value()); + // current request to cluster_override. + // cluster_0 trafficStats() not created yet. + EXPECT_EQ(test_server_->counter("cluster.cluster_0.upstream_cx_total"), nullptr); + EXPECT_EQ(test_server_->counter("cluster.cluster_0.upstream_rq_total"), nullptr); + EXPECT_EQ(1, test_server_->counter("cluster.cluster_override.upstream_cx_total")->value()); EXPECT_EQ(1, test_server_->counter("cluster.cluster_override.upstream_rq_200")->value()); EXPECT_THAT(waitForAccessLog(access_log_name_), HasSubstr("cluster_override")); diff --git a/test/integration/xfcc_integration_test.cc b/test/integration/xfcc_integration_test.cc index 544991fcfd848..232417a1cfb72 100644 --- a/test/integration/xfcc_integration_test.cc +++ b/test/integration/xfcc_integration_test.cc @@ -917,7 +917,7 @@ TEST_P(XfccIntegrationTest, TagExtractedNameGenerationTest) { extracted_name_map.erase(it); } }; - + ASSERT_TRUE(forceCreationOfClusterTrafficStats("cluster_0")); for (const Stats::CounterSharedPtr& counter : test_server_->counters()) { test_name_against_mapping(tag_extracted_counter_map, *counter); } diff --git a/test/mocks/upstream/cluster_info.cc b/test/mocks/upstream/cluster_info.cc index 4ac62f1fcff64..8df68e36ec973 100644 --- a/test/mocks/upstream/cluster_info.cc +++ b/test/mocks/upstream/cluster_info.cc @@ -54,14 +54,14 @@ MockUpstreamLocalAddressSelector::MockUpstreamLocalAddressSelector( MockClusterInfo::MockClusterInfo() : http2_options_(::Envoy::Http2::Utility::initializeAndValidateOptions( envoy::config::core::v3::Http2ProtocolOptions())), - stat_names_(stats_store_.symbolTable()), + traffic_stat_names_(stats_store_.symbolTable()), config_update_stats_names_(stats_store_.symbolTable()), lb_stat_names_(stats_store_.symbolTable()), endpoint_stat_names_(stats_store_.symbolTable()), cluster_load_report_stat_names_(stats_store_.symbolTable()), cluster_circuit_breakers_stat_names_(stats_store_.symbolTable()), cluster_request_response_size_stat_names_(stats_store_.symbolTable()), cluster_timeout_budget_stat_names_(stats_store_.symbolTable()), - stats_(ClusterInfoImpl::generateStats(stats_store_, stat_names_)), + traffic_stats_(std::make_unique(traffic_stat_names_, stats_store_)), config_update_stats_(config_update_stats_names_, stats_store_), lb_stats_(lb_stat_names_, stats_store_), endpoint_stats_(endpoint_stat_names_, stats_store_), transport_socket_matcher_(new NiceMock()), @@ -99,7 +99,7 @@ MockClusterInfo::MockClusterInfo() .WillByDefault(ReturnPointee(&max_response_headers_count_)); ON_CALL(*this, maxRequestsPerConnection()) .WillByDefault(ReturnPointee(&max_requests_per_connection_)); - ON_CALL(*this, trafficStats()).WillByDefault(ReturnRef(stats_)); + ON_CALL(*this, trafficStats()).WillByDefault(ReturnRef(traffic_stats_)); ON_CALL(*this, lbStats()).WillByDefault(ReturnRef(lb_stats_)); ON_CALL(*this, configUpdateStats()).WillByDefault(ReturnRef(config_update_stats_)); ON_CALL(*this, endpointStats()).WillByDefault(ReturnRef(endpoint_stats_)); diff --git a/test/mocks/upstream/cluster_info.h b/test/mocks/upstream/cluster_info.h index b90b77d9e5255..208b32e53c0c0 100644 --- a/test/mocks/upstream/cluster_info.h +++ b/test/mocks/upstream/cluster_info.h @@ -151,7 +151,7 @@ class MockClusterInfo : public ClusterInfo { MOCK_METHOD(const std::string&, observabilityName, (), (const)); MOCK_METHOD(ResourceManager&, resourceManager, (ResourcePriority priority), (const)); MOCK_METHOD(TransportSocketMatcher&, transportSocketMatcher, (), (const)); - MOCK_METHOD(ClusterTrafficStats&, trafficStats, (), (const)); + MOCK_METHOD(std::unique_ptr&, trafficStats, (), (const)); MOCK_METHOD(ClusterLbStats&, lbStats, (), (const)); MOCK_METHOD(ClusterEndpointStats&, endpointStats, (), (const)); MOCK_METHOD(ClusterConfigUpdateStats&, configUpdateStats, (), (const)); @@ -201,7 +201,7 @@ class MockClusterInfo : public ClusterInfo { uint64_t max_requests_per_connection_{}; uint32_t max_response_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT}; NiceMock stats_store_; - ClusterTrafficStatNames stat_names_; + ClusterTrafficStatNames traffic_stat_names_; ClusterConfigUpdateStatNames config_update_stats_names_; ClusterLbStatNames lb_stat_names_; ClusterEndpointStatNames endpoint_stat_names_; @@ -209,7 +209,7 @@ class MockClusterInfo : public ClusterInfo { ClusterCircuitBreakersStatNames cluster_circuit_breakers_stat_names_; ClusterRequestResponseSizeStatNames cluster_request_response_size_stat_names_; ClusterTimeoutBudgetStatNames cluster_timeout_budget_stat_names_; - ClusterTrafficStats stats_; + std::unique_ptr traffic_stats_; ClusterConfigUpdateStats config_update_stats_; ClusterLbStats lb_stats_; ClusterEndpointStats endpoint_stats_; diff --git a/test/test_common/real_threads_test_helper.h b/test/test_common/real_threads_test_helper.h index 4d591b1dc639b..53978f2ed7e5f 100644 --- a/test/test_common/real_threads_test_helper.h +++ b/test/test_common/real_threads_test_helper.h @@ -1,3 +1,5 @@ +#pragma once + #include "source/common/event/dispatcher_impl.h" #include "source/common/thread_local/thread_local_impl.h" diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index a674aa44a8d8a..1154396185106 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -895,6 +895,7 @@ metatable microbenchmarks midp milli +mimics misconfiguration misconfigured mixin From 803004f5663a5c37295af7efc7b3dea1eaa0f349 Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Tue, 6 Dec 2022 20:36:52 -0500 Subject: [PATCH 02/12] revert some unwanted updates Signed-off-by: Xin Zhuang --- api/envoy/config/metrics/v3/stats.proto | 2 -- envoy/stats/stats_macros.h | 4 ++-- .../filters/http/grpc_stats/grpc_stats_filter.cc | 8 ++++---- tools/spelling/spelling_dictionary.txt | 1 - 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/api/envoy/config/metrics/v3/stats.proto b/api/envoy/config/metrics/v3/stats.proto index 388ad1275fb75..fb73e91f8f998 100644 --- a/api/envoy/config/metrics/v3/stats.proto +++ b/api/envoy/config/metrics/v3/stats.proto @@ -109,8 +109,6 @@ message StatsConfig { // 3600000 // ] repeated HistogramBucketSettings histogram_bucket_settings = 4; - // When true, enable lazy init feature for stats that's of Stats::LazyInit type. - bool enable_lazy_init = 5; } // Configuration for disabling stat instantiation. diff --git a/envoy/stats/stats_macros.h b/envoy/stats/stats_macros.h index dd36d903ff5db..6ffca3ea0d118 100644 --- a/envoy/stats/stats_macros.h +++ b/envoy/stats/stats_macros.h @@ -152,7 +152,6 @@ static inline std::string statPrefixJoin(absl::string_view prefix, absl::string_ */ #define MAKE_STATS_STRUCT(StatsStruct, StatNamesStruct, ALL_STATS) \ struct StatsStruct { \ - using StatNameType = StatNamesStruct; \ StatsStruct(const StatNamesStruct& stat_names, Envoy::Stats::Scope& scope, \ Envoy::Stats::StatName prefix = Envoy::Stats::StatName()) \ : stat_names_(stat_names) \ @@ -160,8 +159,9 @@ static inline std::string statPrefixJoin(absl::string_view prefix, absl::string_ MAKE_STATS_STRUCT_HISTOGRAM_HELPER_, \ MAKE_STATS_STRUCT_TEXT_READOUT_HELPER_, \ MAKE_STATS_STRUCT_STATNAME_HELPER_) {} \ - const StatNameType& stat_names_; \ + const StatNamesStruct& stat_names_; \ ALL_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT, GENERATE_HISTOGRAM_STRUCT, \ GENERATE_TEXT_READOUT_STRUCT, GENERATE_STATNAME_STRUCT) \ } + } // namespace Envoy diff --git a/source/extensions/filters/http/grpc_stats/grpc_stats_filter.cc b/source/extensions/filters/http/grpc_stats/grpc_stats_filter.cc index b35f382d0b0af..5f0d2200fa2dc 100644 --- a/source/extensions/filters/http/grpc_stats/grpc_stats_filter.cc +++ b/source/extensions/filters/http/grpc_stats/grpc_stats_filter.cc @@ -93,7 +93,7 @@ struct Config { Config(const envoy::extensions::filters::http::grpc_stats::v3::FilterConfig& proto_config, Server::Configuration::FactoryContext& context) : context_(context.grpcContext()), emit_filter_state_(proto_config.emit_filter_state()), - enable_traffic_stats_(proto_config.enable_upstream_stats()), + enable_upstream_stats_(proto_config.enable_upstream_stats()), replace_dots_in_grpc_service_name_(proto_config.replace_dots_in_grpc_service_name()) { switch (proto_config.per_method_stat_specifier_case()) { @@ -137,7 +137,7 @@ struct Config { } Grpc::Context& context_; const bool emit_filter_state_; - const bool enable_traffic_stats_; + const bool enable_upstream_stats_; const bool replace_dots_in_grpc_service_name_; bool stats_for_all_methods_{false}; absl::optional allowlist_; @@ -262,11 +262,11 @@ class GrpcStatsFilter : public Http::PassThroughFilter { } void maybeChargeUpstreamStat() { - if (!config_->enable_traffic_stats_) { + if (!config_->enable_upstream_stats_) { return; } StreamInfo::TimingUtility timing(decoder_callbacks_->streamInfo()); - if (config_->enable_traffic_stats_ && timing.lastUpstreamTxByteSent().has_value() && + if (config_->enable_upstream_stats_ && timing.lastUpstreamTxByteSent().has_value() && timing.lastUpstreamRxByteReceived().has_value()) { std::chrono::milliseconds chrono_duration = std::chrono::duration_cast( diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index 1154396185106..a674aa44a8d8a 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -895,7 +895,6 @@ metatable microbenchmarks midp milli -mimics misconfiguration misconfigured mixin From 74059e40d90d94349f9770abd6c8457e533d2817 Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Wed, 7 Dec 2022 11:34:33 -0500 Subject: [PATCH 03/12] revert more unwanted updates Signed-off-by: Xin Zhuang --- .../gcp_authn_filter_integration_test.cc | 4 +-- test/integration/base_integration_test.h | 26 ------------------- test/integration/cds_integration_test.cc | 2 -- test/integration/integration_admin_test.cc | 1 - 4 files changed, 1 insertion(+), 32 deletions(-) diff --git a/test/extensions/filters/http/gcp_authn/gcp_authn_filter_integration_test.cc b/test/extensions/filters/http/gcp_authn/gcp_authn_filter_integration_test.cc index c133a78552ac3..1134c0434d9dc 100644 --- a/test/extensions/filters/http/gcp_authn/gcp_authn_filter_integration_test.cc +++ b/test/extensions/filters/http/gcp_authn/gcp_authn_filter_integration_test.cc @@ -270,9 +270,7 @@ TEST_P(GcpAuthnFilterIntegrationTest, BasicflowWithoutAudience) { sendRequestToDestinationAndValidateResponse(/*with_audience=*/false); // Verify request has been routed to `cluster_0` but not `gcp_authn` cluster. - // Note that upstream_cx_total is from the lazy init ClusterInfo::trafficStats(), so there is - // no such stat yet. - EXPECT_EQ(test_server_->counter("cluster.gcp_authn.upstream_cx_total"), nullptr); + EXPECT_EQ(test_server_->counter("cluster.gcp_authn.upstream_cx_total")->value(), 0); EXPECT_GE(test_server_->counter("cluster.cluster_0.upstream_cx_total")->value(), 1); // Clean up the codec and connections. diff --git a/test/integration/base_integration_test.h b/test/integration/base_integration_test.h index 4ed6e08f19ca3..25fb42a406094 100644 --- a/test/integration/base_integration_test.h +++ b/test/integration/base_integration_test.h @@ -118,32 +118,6 @@ class BaseIntegrationTest : protected Logger::Loggable { void registerPort(const std::string& key, uint32_t port); uint32_t lookupPort(const std::string& key); - /** - * @brief Schedule a callback on main thread to force create the traffic stats for the given - * cluster. - * - * @param cluster_name name of the cluster. - * @return whether cluster is found. - */ - bool forceCreationOfClusterTrafficStats(absl::string_view cluster_name) { - // With https://github.com/envoyproxy/envoy/pull/23921 ClusterInfo::trafficStats is lazy init. - // We need to trigger creation of ClusterInfo::trafficStats() by calling the * operator. - absl::Notification notifier; - - bool cluster_found = false; - test_server_->server().dispatcher().post([&]() { - const Upstream::ClusterConstOptRef& cluster_ref = - test_server_->server().clusterManager().clusters().getCluster(cluster_name); - if (cluster_ref.has_value()) { - *cluster_ref->get().info()->trafficStats(); - cluster_found = true; - } - notifier.Notify(); - }); - notifier.WaitForNotification(); - return cluster_found; - } - // Set the endpoint's socket address to point at upstream at given index. void setUpstreamAddress(uint32_t upstream_index, envoy::config::endpoint::v3::LbEndpoint& endpoint) const; diff --git a/test/integration/cds_integration_test.cc b/test/integration/cds_integration_test.cc index c7633fa4902b8..8e03b0421201e 100644 --- a/test/integration/cds_integration_test.cc +++ b/test/integration/cds_integration_test.cc @@ -195,8 +195,6 @@ TEST_P(CdsIntegrationTest, CdsClusterUpDownUp) { TEST_P(CdsIntegrationTest, CdsClusterTeardownWhileConnecting) { initialize(); test_server_->waitForCounterGe("cluster_manager.cluster_added", 1); - ASSERT_TRUE(forceCreationOfClusterTrafficStats("cluster_1")); - test_server_->waitForCounterExists("cluster.cluster_1.upstream_cx_total"); Stats::CounterSharedPtr cx_counter = test_server_->counter("cluster.cluster_1.upstream_cx_total"); // Confirm no upstream connection is attempted so far. diff --git a/test/integration/integration_admin_test.cc b/test/integration/integration_admin_test.cc index 8fcb2cfc2f797..f9e5b7027072c 100644 --- a/test/integration/integration_admin_test.cc +++ b/test/integration/integration_admin_test.cc @@ -129,7 +129,6 @@ TEST_P(IntegrationAdminTest, Admin) { initialize(); BufferingStreamDecoderPtr response; - ASSERT_TRUE(forceCreationOfClusterTrafficStats("cluster_0")); EXPECT_EQ("404", request("admin", "GET", "/notfound", response)); EXPECT_EQ("text/plain; charset=UTF-8", contentType(response)); EXPECT_THAT(response->body(), HasSubstr("invalid path. admin commands are:")); From fea4a482ebe66e65062bee332dbf52b192346433 Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Wed, 7 Dec 2022 11:38:44 -0500 Subject: [PATCH 04/12] heh, vscode is slower than git Signed-off-by: Xin Zhuang --- test/integration/xfcc_integration_test.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/xfcc_integration_test.cc b/test/integration/xfcc_integration_test.cc index 232417a1cfb72..ee8b1422cc5ff 100644 --- a/test/integration/xfcc_integration_test.cc +++ b/test/integration/xfcc_integration_test.cc @@ -917,7 +917,6 @@ TEST_P(XfccIntegrationTest, TagExtractedNameGenerationTest) { extracted_name_map.erase(it); } }; - ASSERT_TRUE(forceCreationOfClusterTrafficStats("cluster_0")); for (const Stats::CounterSharedPtr& counter : test_server_->counters()) { test_name_against_mapping(tag_extracted_counter_map, *counter); } From b69c02c8317ffd5055e858e395a6d650f20c8f88 Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Wed, 7 Dec 2022 11:40:27 -0500 Subject: [PATCH 05/12] revert more unwanted updates Signed-off-by: Xin Zhuang --- test/integration/integration_test.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index 22c7af3c3b058..997a870d88c64 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -2461,9 +2461,8 @@ TEST_P(IntegrationTest, SetRouteToDelegatingRouteWithClusterOverride) { // Even though headers specify cluster_0, set_route_filter modifies cached route cluster of // current request to cluster_override. - // cluster_0 trafficStats() not created yet. - EXPECT_EQ(test_server_->counter("cluster.cluster_0.upstream_cx_total"), nullptr); - EXPECT_EQ(test_server_->counter("cluster.cluster_0.upstream_rq_total"), nullptr); + EXPECT_EQ(test_server_->counter("cluster.cluster_0.upstream_cx_total")->value(), 0); + EXPECT_EQ(test_server_->counter("cluster.cluster_0.upstream_rq_total")->value(), 0); EXPECT_EQ(1, test_server_->counter("cluster.cluster_override.upstream_cx_total")->value()); EXPECT_EQ(1, test_server_->counter("cluster.cluster_override.upstream_rq_200")->value()); From 9de8e10cdc85ea106b3a7a6f50f2864daa334269 Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Wed, 7 Dec 2022 12:57:07 -0500 Subject: [PATCH 06/12] fixes for Josh comments Signed-off-by: Xin Zhuang --- envoy/upstream/upstream.h | 1 + source/common/conn_pool/conn_pool_base.cc | 16 +++++++++------- source/common/upstream/upstream_impl.cc | 2 +- source/common/upstream/upstream_impl.h | 6 +++--- .../filters/network/common/redis/client_impl.cc | 5 +++-- test/mocks/upstream/cluster_info.h | 4 ++-- 6 files changed, 19 insertions(+), 15 deletions(-) diff --git a/envoy/upstream/upstream.h b/envoy/upstream/upstream.h index 2e81a6191b17a..fcbc2188a06bb 100644 --- a/envoy/upstream/upstream.h +++ b/envoy/upstream/upstream.h @@ -765,6 +765,7 @@ 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); +using ClusterTrafficStatsPtr = std::unique_ptr; MAKE_STAT_NAMES_STRUCT(ClusterLoadReportStatNames, ALL_CLUSTER_LOAD_REPORT_STATS); MAKE_STATS_STRUCT(ClusterLoadReportStats, ClusterLoadReportStatNames, diff --git a/source/common/conn_pool/conn_pool_base.cc b/source/common/conn_pool/conn_pool_base.cc index e2d308ca4ad89..77a57b17f24cd 100644 --- a/source/common/conn_pool/conn_pool_base.cc +++ b/source/common/conn_pool/conn_pool_base.cc @@ -167,15 +167,16 @@ void ConnPoolImplBase::attachStreamToClient(Envoy::ConnectionPool::ActiveClient& AttachContext& context) { ASSERT(client.readyForStream()); + Upstream::ClusterTrafficStats& traffic_stats = *host_->cluster().trafficStats(); if (client.state() == Envoy::ConnectionPool::ActiveClient::State::ReadyForEarlyData) { - host_->cluster().trafficStats()->upstream_rq_0rtt_.inc(); + traffic_stats.upstream_rq_0rtt_.inc(); } if (enforceMaxRequests() && !host_->cluster().resourceManager(priority_).requests().canCreate()) { ENVOY_LOG(debug, "max streams overflow"); onPoolFailure(client.real_host_description_, absl::string_view(), ConnectionPool::PoolFailureReason::Overflow, context); - host_->cluster().trafficStats()->upstream_rq_pending_overflow_.inc(); + traffic_stats.upstream_rq_pending_overflow_.inc(); return; } ENVOY_CONN_LOG(debug, "creating stream", client); @@ -185,7 +186,7 @@ void ConnPoolImplBase::attachStreamToClient(Envoy::ConnectionPool::ActiveClient& client.remaining_streams_--; if (client.remaining_streams_ == 0) { ENVOY_CONN_LOG(debug, "maximum streams per connection, start draining", client); - host_->cluster().trafficStats()->upstream_cx_max_requests_.inc(); + traffic_stats.upstream_cx_max_requests_.inc(); transitionActiveClientState(client, Envoy::ConnectionPool::ActiveClient::State::Draining); } else if (capacity == 1) { // As soon as the new stream is created, the client will be maxed out. @@ -202,8 +203,8 @@ void ConnPoolImplBase::attachStreamToClient(Envoy::ConnectionPool::ActiveClient& num_active_streams_++; host_->stats().rq_total_.inc(); host_->stats().rq_active_.inc(); - host_->cluster().trafficStats()->upstream_rq_total_.inc(); - host_->cluster().trafficStats()->upstream_rq_active_.inc(); + traffic_stats.upstream_rq_total_.inc(); + traffic_stats.upstream_rq_active_.inc(); host_->cluster().resourceManager(priority_).requests().inc(); onPoolReady(client, context); @@ -606,8 +607,9 @@ void ConnPoolImplBase::onConnectionEvent(ActiveClient& client, absl::string_view PendingStream::PendingStream(ConnPoolImplBase& parent, bool can_send_early_data) : parent_(parent), can_send_early_data_(can_send_early_data) { - parent_.host()->cluster().trafficStats()->upstream_rq_pending_total_.inc(); - parent_.host()->cluster().trafficStats()->upstream_rq_pending_active_.inc(); + Upstream::ClusterTrafficStats& traffic_stats = *parent_.host()->cluster().trafficStats(); + traffic_stats.upstream_rq_pending_total_.inc(); + traffic_stats.upstream_rq_pending_active_.inc(); parent_.host()->cluster().resourceManager(parent_.priority()).pendingRequests().inc(); } diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index a1a7d8bd8b9b0..6d0593a4feb5d 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -846,7 +846,7 @@ void MainPrioritySetImpl::updateCrossPriorityHostMap(const HostVector& hosts_add } } -std::unique_ptr +ClusterTrafficStatsPtr ClusterInfoImpl::generateStats(Stats::Scope& scope, const ClusterTrafficStatNames& stat_names) { return std::make_unique(stat_names, scope); } diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 1607612e6d195..0f8efcc7b0c8c 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -702,7 +702,7 @@ class ClusterInfoImpl : public ClusterInfo, Stats::ScopeSharedPtr&& stats_scope, bool added_via_api, Server::Configuration::TransportSocketFactoryContext&); - static std::unique_ptr + static ClusterTrafficStatsPtr generateStats(Stats::Scope& scope, const ClusterTrafficStatNames& cluster_stat_names); static ClusterLoadReportStats generateLoadReportStats(Stats::Scope& scope, const ClusterLoadReportStatNames& stat_names); @@ -790,7 +790,7 @@ class ClusterInfoImpl : public ClusterInfo, const std::string& observabilityName() const override { return observability_name_; } ResourceManager& resourceManager(ResourcePriority priority) const override; TransportSocketMatcher& transportSocketMatcher() const override { return *socket_matcher_; } - std::unique_ptr& trafficStats() const override { return traffic_stats_; } + ClusterTrafficStatsPtr& 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_; } @@ -912,7 +912,7 @@ class ClusterInfoImpl : public ClusterInfo, const uint32_t per_connection_buffer_limit_bytes_; TransportSocketMatcherPtr socket_matcher_; Stats::ScopeSharedPtr stats_scope_; - mutable std::unique_ptr traffic_stats_; + mutable ClusterTrafficStatsPtr traffic_stats_; mutable ClusterConfigUpdateStats config_update_stats_; mutable ClusterLbStats lb_stats_; mutable ClusterEndpointStats endpoint_stats_; diff --git a/source/extensions/filters/network/common/redis/client_impl.cc b/source/extensions/filters/network/common/redis/client_impl.cc index 5bf5b690d082f..c34b7cfda5582 100644 --- a/source/extensions/filters/network/common/redis/client_impl.cc +++ b/source/extensions/filters/network/common/redis/client_impl.cc @@ -77,9 +77,10 @@ ClientImpl::ClientImpl(Upstream::HostConstSharedPtr host, Event::Dispatcher& dis flush_timer_(dispatcher.createTimer([this]() { flushBufferAndResetTimer(); })), time_source_(dispatcher.timeSource()), redis_command_stats_(redis_command_stats), scope_(scope), is_transaction_client_(is_transaction_client) { - host->cluster().trafficStats()->upstream_cx_total_.inc(); + Upstream::ClusterTrafficStats& traffic_stats = *host->cluster().trafficStats(); + traffic_stats.upstream_cx_total_.inc(); host->stats().cx_total_.inc(); - host->cluster().trafficStats()->upstream_cx_active_.inc(); + traffic_stats.upstream_cx_active_.inc(); host->stats().cx_active_.inc(); connect_or_op_timer_->enableTimer(host->cluster().connectTimeout()); } diff --git a/test/mocks/upstream/cluster_info.h b/test/mocks/upstream/cluster_info.h index 208b32e53c0c0..3f2800eae89ff 100644 --- a/test/mocks/upstream/cluster_info.h +++ b/test/mocks/upstream/cluster_info.h @@ -151,7 +151,7 @@ class MockClusterInfo : public ClusterInfo { MOCK_METHOD(const std::string&, observabilityName, (), (const)); MOCK_METHOD(ResourceManager&, resourceManager, (ResourcePriority priority), (const)); MOCK_METHOD(TransportSocketMatcher&, transportSocketMatcher, (), (const)); - MOCK_METHOD(std::unique_ptr&, trafficStats, (), (const)); + MOCK_METHOD(ClusterTrafficStatsPtr&, trafficStats, (), (const)); MOCK_METHOD(ClusterLbStats&, lbStats, (), (const)); MOCK_METHOD(ClusterEndpointStats&, endpointStats, (), (const)); MOCK_METHOD(ClusterConfigUpdateStats&, configUpdateStats, (), (const)); @@ -209,7 +209,7 @@ class MockClusterInfo : public ClusterInfo { ClusterCircuitBreakersStatNames cluster_circuit_breakers_stat_names_; ClusterRequestResponseSizeStatNames cluster_request_response_size_stat_names_; ClusterTimeoutBudgetStatNames cluster_timeout_budget_stat_names_; - std::unique_ptr traffic_stats_; + ClusterTrafficStatsPtr traffic_stats_; ClusterConfigUpdateStats config_update_stats_; ClusterLbStats lb_stats_; ClusterEndpointStats endpoint_stats_; From 58ba476c5d895154423b86425c59d71bf09a7c58 Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Thu, 8 Dec 2022 20:22:18 -0500 Subject: [PATCH 07/12] re add the helper files Signed-off-by: Xin Zhuang --- test/common/stats/real_thread_test_base.cc | 43 ++++++++++++++++++++++ test/common/stats/real_thread_test_base.h | 40 ++++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 test/common/stats/real_thread_test_base.cc create mode 100644 test/common/stats/real_thread_test_base.h diff --git a/test/common/stats/real_thread_test_base.cc b/test/common/stats/real_thread_test_base.cc new file mode 100644 index 0000000000000..7350765e018b6 --- /dev/null +++ b/test/common/stats/real_thread_test_base.cc @@ -0,0 +1,43 @@ +#include "test/common/stats/real_thread_test_base.h" + +namespace Envoy { +namespace Stats { + +ThreadLocalStoreNoMocksTestBase::ThreadLocalStoreNoMocksTestBase() + : alloc_(symbol_table_), store_(std::make_unique(alloc_)), + pool_(symbol_table_) {} + +ThreadLocalStoreNoMocksTestBase::~ThreadLocalStoreNoMocksTestBase() { + if (store_ != nullptr) { + store_->shutdownThreading(); + } +} + +StatName ThreadLocalStoreNoMocksTestBase::makeStatName(absl::string_view name) { + return pool_.add(name); +} + +ThreadLocalRealThreadsTestBase::ThreadLocalRealThreadsTestBase(uint32_t num_threads) + : RealThreadsTestHelper(num_threads) { + runOnMainBlocking([this]() { store_->initializeThreading(*main_dispatcher_, *tls_); }); +} + +ThreadLocalRealThreadsTestBase::~ThreadLocalRealThreadsTestBase() { + // TODO(chaoqin-li1123): clean this up when we figure out how to free the threading resources in + // RealThreadsTestHelper. + shutdownThreading(); + exitThreads([this]() { store_.reset(); }); +} + +void ThreadLocalRealThreadsTestBase::shutdownThreading() { + runOnMainBlocking([this]() { + if (!tls_->isShutdown()) { + tls_->shutdownGlobalThreading(); + } + store_->shutdownThreading(); + tls_->shutdownThread(); + }); +} + +} // namespace Stats +} // namespace Envoy diff --git a/test/common/stats/real_thread_test_base.h b/test/common/stats/real_thread_test_base.h new file mode 100644 index 0000000000000..ef326f7515788 --- /dev/null +++ b/test/common/stats/real_thread_test_base.h @@ -0,0 +1,40 @@ +#pragma once + +#include "source/common/stats/thread_local_store.h" +#include "source/common/thread_local/thread_local_impl.h" + +#include "test/mocks/thread_local/mocks.h" +#include "test/test_common/real_threads_test_helper.h" + +namespace Envoy { +namespace Stats { + +class ThreadLocalStoreNoMocksTestBase { +public: + ThreadLocalStoreNoMocksTestBase(); + ~ThreadLocalStoreNoMocksTestBase(); + + StatName makeStatName(absl::string_view name); + + SymbolTableImpl symbol_table_; + AllocatorImpl alloc_; + ThreadLocalStoreImplPtr store_; + StatNamePool pool_; +}; + +class ThreadLocalRealThreadsTestBase : public Thread::RealThreadsTestHelper, + public ThreadLocalStoreNoMocksTestBase { +protected: + static constexpr uint32_t NumScopes = 1000; + static constexpr uint32_t NumIters = 35; + +public: + ThreadLocalRealThreadsTestBase(uint32_t num_threads); + + ~ThreadLocalRealThreadsTestBase(); + + void shutdownThreading(); +}; + +} // namespace Stats +} // namespace Envoy From 150acb0c7f642287a44f14841e183744808d7b45 Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Mon, 12 Dec 2022 11:13:34 -0500 Subject: [PATCH 08/12] add comments for testing base classes. Signed-off-by: Xin Zhuang --- envoy/upstream/upstream.h | 3 +++ test/common/stats/real_thread_test_base.h | 1 + test/extensions/clusters/aggregate/cluster_test.cc | 2 +- test/integration/xfcc_integration_test.cc | 1 + test/test_common/real_threads_test_helper.h | 2 ++ 5 files changed, 8 insertions(+), 1 deletion(-) diff --git a/envoy/upstream/upstream.h b/envoy/upstream/upstream.h index fcbc2188a06bb..bf4df8715848c 100644 --- a/envoy/upstream/upstream.h +++ b/envoy/upstream/upstream.h @@ -1062,6 +1062,9 @@ class ClusterInfo : public Http::FilterChainFactory { /** * @return std::unique_ptr& all traffic related stats for this cluster. + * NOTE: Returning "std::unique_ptr&" for now to make way for future + * lazy-init on trafficStats(). See + * https://github.com/envoyproxy/envoy/pull/23921#issuecomment-1335239116 for more context. */ virtual std::unique_ptr& trafficStats() const PURE; diff --git a/test/common/stats/real_thread_test_base.h b/test/common/stats/real_thread_test_base.h index ef326f7515788..73ef1008bee7e 100644 --- a/test/common/stats/real_thread_test_base.h +++ b/test/common/stats/real_thread_test_base.h @@ -22,6 +22,7 @@ class ThreadLocalStoreNoMocksTestBase { StatNamePool pool_; }; +// Helper base class for threadlocal stats testing in multiple-workers setup. class ThreadLocalRealThreadsTestBase : public Thread::RealThreadsTestHelper, public ThreadLocalStoreNoMocksTestBase { protected: diff --git a/test/extensions/clusters/aggregate/cluster_test.cc b/test/extensions/clusters/aggregate/cluster_test.cc index 1e7c02c067614..c49f72ea8730f 100644 --- a/test/extensions/clusters/aggregate/cluster_test.cc +++ b/test/extensions/clusters/aggregate/cluster_test.cc @@ -139,7 +139,7 @@ class AggregateClusterTest : public Event::TestUsingSimulatedTime, public testin Upstream::LoadBalancerFactorySharedPtr lb_factory_; Upstream::LoadBalancerPtr lb_; Upstream::ClusterTrafficStatNames stat_names_; - std::unique_ptr stats_; + Upstream::ClusterTrafficStatsPtr stats_; std::shared_ptr primary_info_{ new NiceMock()}; std::shared_ptr secondary_info_{ diff --git a/test/integration/xfcc_integration_test.cc b/test/integration/xfcc_integration_test.cc index ee8b1422cc5ff..544991fcfd848 100644 --- a/test/integration/xfcc_integration_test.cc +++ b/test/integration/xfcc_integration_test.cc @@ -917,6 +917,7 @@ TEST_P(XfccIntegrationTest, TagExtractedNameGenerationTest) { extracted_name_map.erase(it); } }; + for (const Stats::CounterSharedPtr& counter : test_server_->counters()) { test_name_against_mapping(tag_extracted_counter_map, *counter); } diff --git a/test/test_common/real_threads_test_helper.h b/test/test_common/real_threads_test_helper.h index 53978f2ed7e5f..e1191d32bcf54 100644 --- a/test/test_common/real_threads_test_helper.h +++ b/test/test_common/real_threads_test_helper.h @@ -8,6 +8,8 @@ namespace Envoy { namespace Thread { +// Base class for both performance and unit tests, no mocks or other "slow" test structures should +// be added to it. class RealThreadsTestHelper { public: // Helper class to block on a number of multi-threaded operations occurring. From 98c167a3e0e3a260cab28ba603bee1521eb71531 Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Mon, 12 Dec 2022 11:45:04 -0500 Subject: [PATCH 09/12] update alias Signed-off-by: Xin Zhuang --- envoy/upstream/upstream.h | 10 +++++----- source/common/upstream/upstream_impl.cc | 2 +- source/common/upstream/upstream_impl.h | 6 +++--- test/extensions/clusters/aggregate/cluster_test.cc | 2 +- test/mocks/upstream/cluster_info.h | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/envoy/upstream/upstream.h b/envoy/upstream/upstream.h index bf4df8715848c..37b4e1e197ebd 100644 --- a/envoy/upstream/upstream.h +++ b/envoy/upstream/upstream.h @@ -765,7 +765,7 @@ 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); -using ClusterTrafficStatsPtr = std::unique_ptr; +using LazyClusterTrafficStats = std::unique_ptr; MAKE_STAT_NAMES_STRUCT(ClusterLoadReportStatNames, ALL_CLUSTER_LOAD_REPORT_STATS); MAKE_STATS_STRUCT(ClusterLoadReportStats, ClusterLoadReportStatNames, @@ -1061,12 +1061,12 @@ class ClusterInfo : public Http::FilterChainFactory { virtual ClusterEndpointStats& endpointStats() const PURE; /** - * @return std::unique_ptr& all traffic related stats for this cluster. - * NOTE: Returning "std::unique_ptr&" for now to make way for future - * lazy-init on trafficStats(). See + * @return LazyClusterTrafficStats& all traffic related stats for this cluster. + * NOTE: LazyClusterTrafficStats for now is an alias of "std::unique_ptr", + * this is to make way for future lazy-init on trafficStats(). See * https://github.com/envoyproxy/envoy/pull/23921#issuecomment-1335239116 for more context. */ - virtual std::unique_ptr& trafficStats() const PURE; + virtual LazyClusterTrafficStats& trafficStats() const PURE; /** * @return the stats scope that contains all cluster stats. This can be used to produce dynamic diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 6d0593a4feb5d..036e955e89c05 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -846,7 +846,7 @@ void MainPrioritySetImpl::updateCrossPriorityHostMap(const HostVector& hosts_add } } -ClusterTrafficStatsPtr +LazyClusterTrafficStats ClusterInfoImpl::generateStats(Stats::Scope& scope, const ClusterTrafficStatNames& stat_names) { return std::make_unique(stat_names, scope); } diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 0f8efcc7b0c8c..4bf10d54577b4 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -702,7 +702,7 @@ class ClusterInfoImpl : public ClusterInfo, Stats::ScopeSharedPtr&& stats_scope, bool added_via_api, Server::Configuration::TransportSocketFactoryContext&); - static ClusterTrafficStatsPtr + static LazyClusterTrafficStats generateStats(Stats::Scope& scope, const ClusterTrafficStatNames& cluster_stat_names); static ClusterLoadReportStats generateLoadReportStats(Stats::Scope& scope, const ClusterLoadReportStatNames& stat_names); @@ -790,7 +790,7 @@ class ClusterInfoImpl : public ClusterInfo, const std::string& observabilityName() const override { return observability_name_; } ResourceManager& resourceManager(ResourcePriority priority) const override; TransportSocketMatcher& transportSocketMatcher() const override { return *socket_matcher_; } - ClusterTrafficStatsPtr& trafficStats() const override { return traffic_stats_; } + LazyClusterTrafficStats& 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_; } @@ -912,7 +912,7 @@ class ClusterInfoImpl : public ClusterInfo, const uint32_t per_connection_buffer_limit_bytes_; TransportSocketMatcherPtr socket_matcher_; Stats::ScopeSharedPtr stats_scope_; - mutable ClusterTrafficStatsPtr traffic_stats_; + mutable LazyClusterTrafficStats traffic_stats_; mutable ClusterConfigUpdateStats config_update_stats_; mutable ClusterLbStats lb_stats_; mutable ClusterEndpointStats endpoint_stats_; diff --git a/test/extensions/clusters/aggregate/cluster_test.cc b/test/extensions/clusters/aggregate/cluster_test.cc index c49f72ea8730f..788b85f5dcfe2 100644 --- a/test/extensions/clusters/aggregate/cluster_test.cc +++ b/test/extensions/clusters/aggregate/cluster_test.cc @@ -139,7 +139,7 @@ class AggregateClusterTest : public Event::TestUsingSimulatedTime, public testin Upstream::LoadBalancerFactorySharedPtr lb_factory_; Upstream::LoadBalancerPtr lb_; Upstream::ClusterTrafficStatNames stat_names_; - Upstream::ClusterTrafficStatsPtr stats_; + Upstream::LazyClusterTrafficStats stats_; std::shared_ptr primary_info_{ new NiceMock()}; std::shared_ptr secondary_info_{ diff --git a/test/mocks/upstream/cluster_info.h b/test/mocks/upstream/cluster_info.h index 3f2800eae89ff..17c9cf2b10afd 100644 --- a/test/mocks/upstream/cluster_info.h +++ b/test/mocks/upstream/cluster_info.h @@ -151,7 +151,7 @@ class MockClusterInfo : public ClusterInfo { MOCK_METHOD(const std::string&, observabilityName, (), (const)); MOCK_METHOD(ResourceManager&, resourceManager, (ResourcePriority priority), (const)); MOCK_METHOD(TransportSocketMatcher&, transportSocketMatcher, (), (const)); - MOCK_METHOD(ClusterTrafficStatsPtr&, trafficStats, (), (const)); + MOCK_METHOD(LazyClusterTrafficStats&, trafficStats, (), (const)); MOCK_METHOD(ClusterLbStats&, lbStats, (), (const)); MOCK_METHOD(ClusterEndpointStats&, endpointStats, (), (const)); MOCK_METHOD(ClusterConfigUpdateStats&, configUpdateStats, (), (const)); @@ -209,7 +209,7 @@ class MockClusterInfo : public ClusterInfo { ClusterCircuitBreakersStatNames cluster_circuit_breakers_stat_names_; ClusterRequestResponseSizeStatNames cluster_request_response_size_stat_names_; ClusterTimeoutBudgetStatNames cluster_timeout_budget_stat_names_; - ClusterTrafficStatsPtr traffic_stats_; + LazyClusterTrafficStats traffic_stats_; ClusterConfigUpdateStats config_update_stats_; ClusterLbStats lb_stats_; ClusterEndpointStats endpoint_stats_; From d7bb525e34e0854abec1c1fa16625de548777e13 Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Tue, 13 Dec 2022 09:57:52 -0500 Subject: [PATCH 10/12] move comment Signed-off-by: Xin Zhuang --- envoy/upstream/upstream.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/envoy/upstream/upstream.h b/envoy/upstream/upstream.h index 37b4e1e197ebd..80221d2b8477f 100644 --- a/envoy/upstream/upstream.h +++ b/envoy/upstream/upstream.h @@ -765,6 +765,11 @@ 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", + * 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; MAKE_STAT_NAMES_STRUCT(ClusterLoadReportStatNames, ALL_CLUSTER_LOAD_REPORT_STATS); @@ -1062,9 +1067,6 @@ class ClusterInfo : public Http::FilterChainFactory { /** * @return LazyClusterTrafficStats& all traffic related stats for this cluster. - * NOTE: LazyClusterTrafficStats for now is an alias of "std::unique_ptr", - * this is to make way for future lazy-init on trafficStats(). See - * https://github.com/envoyproxy/envoy/pull/23921#issuecomment-1335239116 for more context. */ virtual LazyClusterTrafficStats& trafficStats() const PURE; From a1dfeb3bb2290e9f0dd58eb7ac3990229d53b7ce Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Tue, 13 Dec 2022 15:19:48 -0500 Subject: [PATCH 11/12] fix comment Signed-off-by: Xin Zhuang --- envoy/upstream/upstream.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/envoy/upstream/upstream.h b/envoy/upstream/upstream.h index 80221d2b8477f..399fe72cb8381 100644 --- a/envoy/upstream/upstream.h +++ b/envoy/upstream/upstream.h @@ -1066,7 +1066,7 @@ class ClusterInfo : public Http::FilterChainFactory { virtual ClusterEndpointStats& endpointStats() const PURE; /** - * @return LazyClusterTrafficStats& all traffic related stats for this cluster. + * @return all traffic related stats for this cluster. */ virtual LazyClusterTrafficStats& trafficStats() const PURE; From 79ce9bfb3ffe9be8a910d12ce95fcb4e22404da3 Mon Sep 17 00:00:00 2001 From: Xin Zhuang Date: Tue, 13 Dec 2022 16:30:05 -0500 Subject: [PATCH 12/12] revert some unrelated changes that could be included in the next PR Signed-off-by: Xin Zhuang --- source/common/upstream/upstream_impl.cc | 4 +- source/common/upstream/upstream_impl.h | 4 +- test/common/stats/BUILD | 18 ++---- test/common/stats/real_thread_test_base.cc | 43 ------------- test/common/stats/real_thread_test_base.h | 41 ------------- test/common/stats/thread_local_store_test.cc | 63 ++++++++++++++++++-- 6 files changed, 65 insertions(+), 108 deletions(-) delete mode 100644 test/common/stats/real_thread_test_base.cc delete mode 100644 test/common/stats/real_thread_test_base.h diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 036e955e89c05..4241fbc7643f6 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -846,8 +846,8 @@ void MainPrioritySetImpl::updateCrossPriorityHostMap(const HostVector& hosts_add } } -LazyClusterTrafficStats -ClusterInfoImpl::generateStats(Stats::Scope& scope, const ClusterTrafficStatNames& stat_names) { +LazyClusterTrafficStats ClusterInfoImpl::generateStats(Stats::Scope& scope, + const ClusterTrafficStatNames& stat_names) { return std::make_unique(stat_names, scope); } diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 4bf10d54577b4..a8c1edbb98871 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -702,8 +702,8 @@ 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 LazyClusterTrafficStats generateStats(Stats::Scope& scope, + const ClusterTrafficStatNames& cluster_stat_names); static ClusterLoadReportStats generateLoadReportStats(Stats::Scope& scope, const ClusterLoadReportStatNames& stat_names); static ClusterCircuitBreakersStats diff --git a/test/common/stats/BUILD b/test/common/stats/BUILD index 7ad1050423ea4..af53d9d98bce2 100644 --- a/test/common/stats/BUILD +++ b/test/common/stats/BUILD @@ -99,18 +99,6 @@ envoy_cc_test( ], ) -envoy_cc_test_library( - name = "real_thread_test_base", - srcs = ["real_thread_test_base.cc"], - hdrs = ["real_thread_test_base.h"], - deps = [ - "//source/common/stats:thread_local_store_lib", - "//source/common/thread_local:thread_local_lib", - "//test/mocks/thread_local:thread_local_mocks", - "//test/test_common:real_threads_test_helper_lib", - ], -) - envoy_cc_test_library( name = "stat_test_utility_lib", srcs = ["stat_test_utility.cc"], @@ -275,15 +263,17 @@ envoy_cc_test( name = "thread_local_store_test", srcs = ["thread_local_store_test.cc"], deps = [ - ":real_thread_test_base", ":stat_test_utility_lib", "//source/common/memory:stats_lib", "//source/common/stats:stats_matcher_lib", - "//source/common/stats:symbol_table_lib", + "//source/common/stats:thread_local_store_lib", + "//source/common/thread_local:thread_local_lib", "//test/mocks/event:event_mocks", "//test/mocks/server:instance_mocks", "//test/mocks/stats:stats_mocks", + "//test/mocks/thread_local:thread_local_mocks", "//test/test_common:logging_lib", + "//test/test_common:real_threads_test_helper_lib", "//test/test_common:test_time_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/metrics/v3:pkg_cc_proto", diff --git a/test/common/stats/real_thread_test_base.cc b/test/common/stats/real_thread_test_base.cc deleted file mode 100644 index 7350765e018b6..0000000000000 --- a/test/common/stats/real_thread_test_base.cc +++ /dev/null @@ -1,43 +0,0 @@ -#include "test/common/stats/real_thread_test_base.h" - -namespace Envoy { -namespace Stats { - -ThreadLocalStoreNoMocksTestBase::ThreadLocalStoreNoMocksTestBase() - : alloc_(symbol_table_), store_(std::make_unique(alloc_)), - pool_(symbol_table_) {} - -ThreadLocalStoreNoMocksTestBase::~ThreadLocalStoreNoMocksTestBase() { - if (store_ != nullptr) { - store_->shutdownThreading(); - } -} - -StatName ThreadLocalStoreNoMocksTestBase::makeStatName(absl::string_view name) { - return pool_.add(name); -} - -ThreadLocalRealThreadsTestBase::ThreadLocalRealThreadsTestBase(uint32_t num_threads) - : RealThreadsTestHelper(num_threads) { - runOnMainBlocking([this]() { store_->initializeThreading(*main_dispatcher_, *tls_); }); -} - -ThreadLocalRealThreadsTestBase::~ThreadLocalRealThreadsTestBase() { - // TODO(chaoqin-li1123): clean this up when we figure out how to free the threading resources in - // RealThreadsTestHelper. - shutdownThreading(); - exitThreads([this]() { store_.reset(); }); -} - -void ThreadLocalRealThreadsTestBase::shutdownThreading() { - runOnMainBlocking([this]() { - if (!tls_->isShutdown()) { - tls_->shutdownGlobalThreading(); - } - store_->shutdownThreading(); - tls_->shutdownThread(); - }); -} - -} // namespace Stats -} // namespace Envoy diff --git a/test/common/stats/real_thread_test_base.h b/test/common/stats/real_thread_test_base.h deleted file mode 100644 index 73ef1008bee7e..0000000000000 --- a/test/common/stats/real_thread_test_base.h +++ /dev/null @@ -1,41 +0,0 @@ -#pragma once - -#include "source/common/stats/thread_local_store.h" -#include "source/common/thread_local/thread_local_impl.h" - -#include "test/mocks/thread_local/mocks.h" -#include "test/test_common/real_threads_test_helper.h" - -namespace Envoy { -namespace Stats { - -class ThreadLocalStoreNoMocksTestBase { -public: - ThreadLocalStoreNoMocksTestBase(); - ~ThreadLocalStoreNoMocksTestBase(); - - StatName makeStatName(absl::string_view name); - - SymbolTableImpl symbol_table_; - AllocatorImpl alloc_; - ThreadLocalStoreImplPtr store_; - StatNamePool pool_; -}; - -// Helper base class for threadlocal stats testing in multiple-workers setup. -class ThreadLocalRealThreadsTestBase : public Thread::RealThreadsTestHelper, - public ThreadLocalStoreNoMocksTestBase { -protected: - static constexpr uint32_t NumScopes = 1000; - static constexpr uint32_t NumIters = 35; - -public: - ThreadLocalRealThreadsTestBase(uint32_t num_threads); - - ~ThreadLocalRealThreadsTestBase(); - - void shutdownThreading(); -}; - -} // namespace Stats -} // namespace Envoy diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc index 524715f219671..93e633fbe9995 100644 --- a/test/common/stats/thread_local_store_test.cc +++ b/test/common/stats/thread_local_store_test.cc @@ -15,14 +15,15 @@ #include "source/common/stats/symbol_table.h" #include "source/common/stats/tag_producer_impl.h" #include "source/common/stats/thread_local_store.h" +#include "source/common/thread_local/thread_local_impl.h" -#include "test/common/stats/real_thread_test_base.h" #include "test/common/stats/stat_test_utility.h" #include "test/mocks/event/mocks.h" #include "test/mocks/server/instance.h" #include "test/mocks/stats/mocks.h" #include "test/mocks/thread_local/mocks.h" #include "test/test_common/logging.h" +#include "test/test_common/real_threads_test_helper.h" #include "test/test_common/utility.h" #include "absl/strings/str_split.h" @@ -755,7 +756,26 @@ TEST_F(StatsThreadLocalStoreTest, SharedScopes) { tls_.shutdownThread(); } -class LookupWithStatNameTest : public ThreadLocalStoreNoMocksTestBase, public testing::Test {}; +class ThreadLocalStoreNoMocksTestBase : public testing::Test { +public: + ThreadLocalStoreNoMocksTestBase() + : alloc_(symbol_table_), store_(std::make_unique(alloc_)), + pool_(symbol_table_) {} + ~ThreadLocalStoreNoMocksTestBase() override { + if (store_ != nullptr) { + store_->shutdownThreading(); + } + } + + StatName makeStatName(absl::string_view name) { return pool_.add(name); } + + SymbolTableImpl symbol_table_; + AllocatorImpl alloc_; + ThreadLocalStoreImplPtr store_; + StatNamePool pool_; +}; + +class LookupWithStatNameTest : public ThreadLocalStoreNoMocksTestBase {}; TEST_F(LookupWithStatNameTest, All) { ScopeSharedPtr scope1 = store_->scopeFromStatName(makeStatName("scope1")); @@ -1664,7 +1684,39 @@ TEST_F(HistogramTest, ForEachHistogram) { EXPECT_EQ(deleted_histogram.unit(), Histogram::Unit::Unspecified); } -class OneWorkerThread : public ThreadLocalRealThreadsTestBase, public testing::Test { +class ThreadLocalRealThreadsTestBase : public Thread::RealThreadsTestHelper, + public ThreadLocalStoreNoMocksTestBase { +protected: + static constexpr uint32_t NumScopes = 1000; + static constexpr uint32_t NumIters = 35; + +public: + ThreadLocalRealThreadsTestBase(uint32_t num_threads) + : RealThreadsTestHelper(num_threads), pool_(store_->symbolTable()) { + runOnMainBlocking([this]() { store_->initializeThreading(*main_dispatcher_, *tls_); }); + } + + ~ThreadLocalRealThreadsTestBase() override { + // TODO(chaoqin-li1123): clean this up when we figure out how to free the threading resources in + // RealThreadsTestHelper. + shutdownThreading(); + exitThreads([this]() { store_.reset(); }); + } + + void shutdownThreading() { + runOnMainBlocking([this]() { + if (!tls_->isShutdown()) { + tls_->shutdownGlobalThreading(); + } + store_->shutdownThreading(); + tls_->shutdownThread(); + }); + } + + StatNamePool pool_; +}; + +class OneWorkerThread : public ThreadLocalRealThreadsTestBase { protected: static constexpr uint32_t NumThreads = 1; OneWorkerThread() : ThreadLocalRealThreadsTestBase(NumThreads) {} @@ -1708,8 +1760,7 @@ TEST_F(OneWorkerThread, DeleteForEachRace) { wait_for_main(); } -class ClusterShutdownCleanupStarvationTest : public ThreadLocalRealThreadsTestBase, - public testing::Test { +class ClusterShutdownCleanupStarvationTest : public ThreadLocalRealThreadsTestBase { protected: static constexpr uint32_t NumThreads = 2; @@ -1793,7 +1844,7 @@ TEST_F(ClusterShutdownCleanupStarvationTest, TwelveThreadsWithoutBlockade) { store_->sync().signal(ThreadLocalStoreImpl::MainDispatcherCleanupSync); } -class HistogramThreadTest : public ThreadLocalRealThreadsTestBase, public testing::Test { +class HistogramThreadTest : public ThreadLocalRealThreadsTestBase { protected: static constexpr uint32_t NumThreads = 10;