From 5b0d56db1d72f8a6ff7d14d6c3cc381dd1f40f81 Mon Sep 17 00:00:00 2001 From: botengyao Date: Tue, 15 Oct 2024 10:08:58 -0400 Subject: [PATCH] tcp_proxy_protocol: reduce stats contention by moving the stats to factory (#36534) Commit Message: reduce data plane stats contention by moving the stats to the factory Additional Description: Risk Level: low Testing: Docs Changes: Release Notes: Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Boteng Yao --- .../proxy_protocol/proxy_protocol.cc | 14 +++++--------- .../proxy_protocol/proxy_protocol.h | 11 ++++++++--- .../proxy_protocol/proxy_protocol_test.cc | 6 +++++- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.cc b/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.cc index 9ce25ebaea7c..672d856d33d7 100644 --- a/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.cc @@ -21,17 +21,12 @@ namespace Extensions { namespace TransportSockets { namespace ProxyProtocol { -UpstreamProxyProtocolStats generateUpstreamProxyProtocolStats(Stats::Scope& stats_scope) { - const char prefix[]{"upstream.proxyprotocol."}; - return {ALL_PROXY_PROTOCOL_TRANSPORT_SOCKET_STATS(POOL_COUNTER_PREFIX(stats_scope, prefix))}; -} - UpstreamProxyProtocolSocket::UpstreamProxyProtocolSocket( Network::TransportSocketPtr&& transport_socket, Network::TransportSocketOptionsConstSharedPtr options, ProxyProtocolConfig config, - Stats::Scope& scope) + const UpstreamProxyProtocolStats& stats) : PassthroughSocket(std::move(transport_socket)), options_(options), version_(config.version()), - stats_(generateUpstreamProxyProtocolStats(scope)), + stats_(stats), pass_all_tlvs_(config.has_pass_through_tlvs() ? config.pass_through_tlvs().match_type() == ProxyProtocolPassThroughTLVs::INCLUDE_ALL : false) { @@ -142,7 +137,8 @@ void UpstreamProxyProtocolSocket::onConnected() { UpstreamProxyProtocolSocketFactory::UpstreamProxyProtocolSocketFactory( Network::UpstreamTransportSocketFactoryPtr transport_socket_factory, ProxyProtocolConfig config, Stats::Scope& scope) - : PassthroughFactory(std::move(transport_socket_factory)), config_(config), scope_(scope) {} + : PassthroughFactory(std::move(transport_socket_factory)), config_(config), + stats_(generateUpstreamProxyProtocolStats(scope)) {} Network::TransportSocketPtr UpstreamProxyProtocolSocketFactory::createTransportSocket( Network::TransportSocketOptionsConstSharedPtr options, @@ -152,7 +148,7 @@ Network::TransportSocketPtr UpstreamProxyProtocolSocketFactory::createTransportS return nullptr; } return std::make_unique(std::move(inner_socket), options, config_, - scope_); + stats_); } void UpstreamProxyProtocolSocketFactory::hashKey( diff --git a/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.h b/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.h index 1a92423c9a1c..f86c9e9cce29 100644 --- a/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.h +++ b/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.h @@ -33,7 +33,7 @@ class UpstreamProxyProtocolSocket : public TransportSockets::PassthroughSocket, public: UpstreamProxyProtocolSocket(Network::TransportSocketPtr&& transport_socket, Network::TransportSocketOptionsConstSharedPtr options, - ProxyProtocolConfig config, Stats::Scope& scope); + ProxyProtocolConfig config, const UpstreamProxyProtocolStats& stats); void setTransportSocketCallbacks(Network::TransportSocketCallbacks& callbacks) override; Network::IoResult doWrite(Buffer::Instance& buffer, bool end_stream) override; @@ -49,7 +49,7 @@ class UpstreamProxyProtocolSocket : public TransportSockets::PassthroughSocket, Network::TransportSocketCallbacks* callbacks_{}; Buffer::OwnedImpl header_buffer_{}; ProxyProtocolConfig_Version version_{ProxyProtocolConfig_Version::ProxyProtocolConfig_Version_V1}; - UpstreamProxyProtocolStats stats_; + const UpstreamProxyProtocolStats& stats_; const bool pass_all_tlvs_; absl::flat_hash_set pass_through_tlvs_{}; }; @@ -67,9 +67,14 @@ class UpstreamProxyProtocolSocketFactory : public PassthroughFactory { void hashKey(std::vector& key, Network::TransportSocketOptionsConstSharedPtr options) const override; + static UpstreamProxyProtocolStats generateUpstreamProxyProtocolStats(Stats::Scope& stats_scope) { + const char prefix[]{"upstream.proxyprotocol."}; + return {ALL_PROXY_PROTOCOL_TRANSPORT_SOCKET_STATS(POOL_COUNTER_PREFIX(stats_scope, prefix))}; + } + private: ProxyProtocolConfig config_; - Stats::Scope& scope_; + UpstreamProxyProtocolStats stats_; }; } // namespace ProxyProtocol diff --git a/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc b/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc index 91a223e22106..748a4113373d 100644 --- a/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc @@ -35,13 +35,16 @@ namespace { class ProxyProtocolTest : public testing::Test { public: + ProxyProtocolTest() + : stats_(UpstreamProxyProtocolSocketFactory::generateUpstreamProxyProtocolStats( + *stats_store_.rootScope())) {} void initialize(ProxyProtocolConfig& config, Network::TransportSocketOptionsConstSharedPtr socket_options) { auto inner_socket = std::make_unique>(); inner_socket_ = inner_socket.get(); ON_CALL(transport_callbacks_, ioHandle()).WillByDefault(ReturnRef(io_handle_)); proxy_protocol_socket_ = std::make_unique( - std::move(inner_socket), socket_options, config, *stats_store_.rootScope()); + std::move(inner_socket), socket_options, config, stats_); proxy_protocol_socket_->setTransportSocketCallbacks(transport_callbacks_); proxy_protocol_socket_->onConnected(); } @@ -51,6 +54,7 @@ class ProxyProtocolTest : public testing::Test { std::unique_ptr proxy_protocol_socket_; NiceMock transport_callbacks_; Stats::TestUtil::TestStore stats_store_; + UpstreamProxyProtocolStats stats_; }; // Test injects PROXY protocol header only once