From fcafc2d87760a6aa01b1d898e568a80cfb568a44 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Mon, 17 Sep 2018 14:08:56 -0400 Subject: [PATCH] time: Remove all usages of chrono::*::now() and instead plumb in TimeSystem or TimeSource (#4434) In order to fully control time during tests, direct calls to system-provided time functions must not be used. These had previously not been fixed. In at least one case I think there was real-time deltas being compared in a test, which really needed to be mocked, so that change had to occur in this PR. Another step toward resolution of #4160 Risk Level: medium, due to size of PR. Testing: //test/... Docs Changes: n/a Release Notes: n/a Signed-off-by: Joshua Marantz --- include/envoy/stats/timespan.h | 7 ++-- source/common/http/async_client_impl.cc | 2 +- source/common/http/conn_manager_impl.cc | 13 ++++--- source/common/http/conn_manager_impl.h | 5 ++- source/common/http/http1/conn_pool.cc | 7 ++-- source/common/http/http2/conn_pool.cc | 7 ++-- .../common/http/websocket/ws_handler_impl.cc | 4 +-- .../common/http/websocket/ws_handler_impl.h | 3 +- .../common/request_info/request_info_impl.h | 28 ++++++++------- source/common/router/config_impl.cc | 9 ++--- source/common/router/config_impl.h | 1 + source/common/router/router.cc | 17 +++++---- source/common/tcp/conn_pool.cc | 7 ++-- source/common/tcp_proxy/tcp_proxy.cc | 5 +-- source/common/tcp_proxy/tcp_proxy.h | 3 +- .../common/upstream/cluster_manager_impl.cc | 6 ++-- source/common/upstream/health_checker_impl.cc | 2 +- .../extensions/filters/http/dynamo/config.cc | 4 +-- .../filters/http/dynamo/dynamo_filter.cc | 4 +-- .../filters/http/dynamo/dynamo_filter.h | 7 ++-- .../filters/http/jwt_authn/authenticator.cc | 8 +++-- .../filters/http/jwt_authn/filter_config.h | 12 ++++--- .../filters/http/jwt_authn/jwks_cache.cc | 23 +++++++----- .../filters/http/jwt_authn/jwks_cache.h | 4 ++- .../network/http_connection_manager/config.cc | 2 +- .../filters/network/mongo_proxy/config.cc | 5 +-- .../filters/network/mongo_proxy/proxy.cc | 13 +++---- .../filters/network/mongo_proxy/proxy.h | 10 ++++-- .../filters/network/tcp_proxy/config.cc | 4 +-- .../filters/network/thrift_proxy/config.cc | 4 +-- .../network/thrift_proxy/conn_manager.cc | 5 +-- .../network/thrift_proxy/conn_manager.h | 7 ++-- .../extensions/stat_sinks/hystrix/hystrix.cc | 2 +- .../transport_sockets/capture/BUILD | 1 + .../transport_sockets/capture/capture.cc | 17 ++++----- .../transport_sockets/capture/capture.h | 8 +++-- .../transport_sockets/capture/config.cc | 12 +++---- source/server/connection_handler_impl.cc | 8 +++-- source/server/connection_handler_impl.h | 3 +- source/server/http/admin.cc | 2 +- .../http/conn_manager_impl_fuzz_test.cc | 4 ++- test/common/http/conn_manager_impl_test.cc | 6 ++-- test/common/http/user_agent_test.cc | 4 ++- .../network/filter_manager_impl_test.cc | 3 +- .../request_info/request_info_impl_test.cc | 35 ++++++++++++------- test/common/tcp_proxy/tcp_proxy_test.cc | 14 +++++--- .../upstream/cluster_manager_impl_test.cc | 34 +++++++++++++++++- .../filters/http/dynamo/dynamo_filter_test.cc | 3 +- test/extensions/filters/http/jwt_authn/BUILD | 1 + .../filters/http/jwt_authn/jwks_cache_test.cc | 12 ++++--- .../filters/http/lua/lua_filter_test.cc | 3 +- .../filters/http/lua/wrappers_test.cc | 10 +++--- .../filters/network/mongo_proxy/proxy_test.cc | 4 +-- .../network/thrift_proxy/conn_manager_test.cc | 5 +-- test/tools/router_check/router.cc | 9 +++-- tools/check_format.py | 3 +- tools/check_format_test_helper.py | 6 +++- tools/testdata/check_format/steady_clock.cc | 5 +++ tools/testdata/check_format/system_clock.cc | 5 +++ 59 files changed, 295 insertions(+), 162 deletions(-) create mode 100644 tools/testdata/check_format/steady_clock.cc create mode 100644 tools/testdata/check_format/system_clock.cc diff --git a/include/envoy/stats/timespan.h b/include/envoy/stats/timespan.h index 1d3877bf579c..90bd0d855e57 100644 --- a/include/envoy/stats/timespan.h +++ b/include/envoy/stats/timespan.h @@ -15,8 +15,8 @@ namespace Stats { */ class Timespan { public: - Timespan(Histogram& histogram) - : histogram_(histogram), start_(std::chrono::steady_clock::now()) {} + Timespan(Histogram& histogram, TimeSource& time_source) + : time_source_(time_source), histogram_(histogram), start_(time_source.monotonicTime()) {} /** * Complete the timespan and send the time to the histogram. @@ -27,11 +27,12 @@ class Timespan { * Get duration since the creation of the span. */ std::chrono::milliseconds getRawDuration() { - return std::chrono::duration_cast(std::chrono::steady_clock::now() - + return std::chrono::duration_cast(time_source_.monotonicTime() - start_); } private: + TimeSource& time_source_; Histogram& histogram_; const MonotonicTime start_; }; diff --git a/source/common/http/async_client_impl.cc b/source/common/http/async_client_impl.cc index 53df6b1c52dc..215559e70185 100644 --- a/source/common/http/async_client_impl.cc +++ b/source/common/http/async_client_impl.cc @@ -78,7 +78,7 @@ AsyncStreamImpl::AsyncStreamImpl(AsyncClientImpl& parent, AsyncClient::StreamCal const absl::optional& timeout, bool buffer_body_for_retry) : parent_(parent), stream_callbacks_(callbacks), stream_id_(parent.config_.random_.random()), - router_(parent.config_), request_info_(Protocol::Http11), + router_(parent.config_), request_info_(Protocol::Http11, parent.dispatcher().timeSystem()), tracing_config_(Tracing::EgressConfig::get()), route_(std::make_shared(parent_.cluster_.name(), timeout)) { if (buffer_body_for_retry) { diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 945b341cc278..2be8ee025522 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -59,16 +59,18 @@ ConnectionManagerImpl::ConnectionManagerImpl(ConnectionManagerConfig& config, Tracing::HttpTracer& tracer, Runtime::Loader& runtime, const LocalInfo::LocalInfo& local_info, Upstream::ClusterManager& cluster_manager, - Server::OverloadManager* overload_manager) + Server::OverloadManager* overload_manager, + Event::TimeSystem& time_system) : config_(config), stats_(config_.stats()), - conn_length_(new Stats::Timespan(stats_.named_.downstream_cx_length_ms_)), + conn_length_(new Stats::Timespan(stats_.named_.downstream_cx_length_ms_, time_system)), drain_close_(drain_close), random_generator_(random_generator), tracer_(tracer), runtime_(runtime), local_info_(local_info), cluster_manager_(cluster_manager), listener_stats_(config_.listenerStats()), overload_stop_accepting_requests_( overload_manager ? overload_manager->getThreadLocalOverloadState().getState( Server::OverloadActionNames::get().StopAcceptingRequests) - : Server::OverloadManager::getInactiveState()) {} + : Server::OverloadManager::getInactiveState()), + time_system_(time_system) {} const HeaderMapImpl& ConnectionManagerImpl::continueHeader() { CONSTRUCT_ON_FIRST_USE(HeaderMapImpl, @@ -360,8 +362,9 @@ ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connect : connection_manager_(connection_manager), snapped_route_config_(connection_manager.config_.routeConfigProvider().config()), stream_id_(connection_manager.random_generator_.random()), - request_timer_(new Stats::Timespan(connection_manager_.stats_.named_.downstream_rq_time_)), - request_info_(connection_manager_.codec_->protocol()) { + request_timer_(new Stats::Timespan(connection_manager_.stats_.named_.downstream_rq_time_, + connection_manager_.timeSystem())), + request_info_(connection_manager_.codec_->protocol(), connection_manager_.timeSystem()) { connection_manager_.stats_.named_.downstream_rq_total_.inc(); connection_manager_.stats_.named_.downstream_rq_active_.inc(); if (connection_manager_.codec_->protocol() == Protocol::Http2) { diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index c53d5c79f12b..3751cbd8630b 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -51,7 +51,7 @@ class ConnectionManagerImpl : Logger::Loggable, Runtime::RandomGenerator& random_generator, Tracing::HttpTracer& tracer, Runtime::Loader& runtime, const LocalInfo::LocalInfo& local_info, Upstream::ClusterManager& cluster_manager, - Server::OverloadManager* overload_manager); + Server::OverloadManager* overload_manager, Event::TimeSystem& time_system); ~ConnectionManagerImpl(); static ConnectionManagerStats generateStats(const std::string& prefix, Stats::Scope& scope); @@ -84,6 +84,8 @@ class ConnectionManagerImpl : Logger::Loggable, codec_->onUnderlyingConnectionBelowWriteBufferLowWatermark(); } + Event::TimeSystem& timeSystem() { return time_system_; } + private: struct ActiveStream; @@ -459,6 +461,7 @@ class ConnectionManagerImpl : Logger::Loggable, Network::ReadFilterCallbacks* read_callbacks_{}; ConnectionManagerListenerStats& listener_stats_; const Server::OverloadActionState& overload_stop_accepting_requests_; + Event::TimeSystem& time_system_; }; } // namespace Http diff --git a/source/common/http/http1/conn_pool.cc b/source/common/http/http1/conn_pool.cc index 9bb8545c5ce4..607063e0e24b 100644 --- a/source/common/http/http1/conn_pool.cc +++ b/source/common/http/http1/conn_pool.cc @@ -312,8 +312,8 @@ ConnPoolImpl::ActiveClient::ActiveClient(ConnPoolImpl& parent) connect_timer_(parent_.dispatcher_.createTimer([this]() -> void { onConnectTimeout(); })), remaining_requests_(parent_.host_->cluster().maxRequestsPerConnection()) { - parent_.conn_connect_ms_.reset( - new Stats::Timespan(parent_.host_->cluster().stats().upstream_cx_connect_ms_)); + parent_.conn_connect_ms_.reset(new Stats::Timespan( + parent_.host_->cluster().stats().upstream_cx_connect_ms_, parent_.dispatcher_.timeSystem())); Upstream::Host::CreateConnectionData data = parent_.host_->createConnection(parent_.dispatcher_, parent_.socket_options_); real_host_description_ = data.host_description_; @@ -325,7 +325,8 @@ ConnPoolImpl::ActiveClient::ActiveClient(ConnPoolImpl& parent) parent_.host_->cluster().stats().upstream_cx_http1_total_.inc(); parent_.host_->stats().cx_total_.inc(); parent_.host_->stats().cx_active_.inc(); - conn_length_.reset(new Stats::Timespan(parent_.host_->cluster().stats().upstream_cx_length_ms_)); + conn_length_.reset(new Stats::Timespan(parent_.host_->cluster().stats().upstream_cx_length_ms_, + parent_.dispatcher_.timeSystem())); connect_timer_->enableTimer(parent_.host_->cluster().connectTimeout()); parent_.host_->cluster().resourceManager(parent_.priority_).connections().inc(); diff --git a/source/common/http/http2/conn_pool.cc b/source/common/http/http2/conn_pool.cc index cd8f6c3f99ab..033cb1e2bb6b 100644 --- a/source/common/http/http2/conn_pool.cc +++ b/source/common/http/http2/conn_pool.cc @@ -219,8 +219,8 @@ ConnPoolImpl::ActiveClient::ActiveClient(ConnPoolImpl& parent) : parent_(parent), connect_timer_(parent_.dispatcher_.createTimer([this]() -> void { onConnectTimeout(); })) { - parent_.conn_connect_ms_.reset( - new Stats::Timespan(parent_.host_->cluster().stats().upstream_cx_connect_ms_)); + parent_.conn_connect_ms_.reset(new Stats::Timespan( + parent_.host_->cluster().stats().upstream_cx_connect_ms_, parent_.dispatcher_.timeSystem())); Upstream::Host::CreateConnectionData data = parent_.host_->createConnection(parent_.dispatcher_, parent_.socket_options_); real_host_description_ = data.host_description_; @@ -235,7 +235,8 @@ ConnPoolImpl::ActiveClient::ActiveClient(ConnPoolImpl& parent) parent_.host_->cluster().stats().upstream_cx_total_.inc(); parent_.host_->cluster().stats().upstream_cx_active_.inc(); parent_.host_->cluster().stats().upstream_cx_http2_total_.inc(); - conn_length_.reset(new Stats::Timespan(parent_.host_->cluster().stats().upstream_cx_length_ms_)); + conn_length_.reset(new Stats::Timespan(parent_.host_->cluster().stats().upstream_cx_length_ms_, + parent_.dispatcher_.timeSystem())); client_->setConnectionStats({parent_.host_->cluster().stats().upstream_cx_rx_bytes_total_, parent_.host_->cluster().stats().upstream_cx_rx_bytes_buffered_, diff --git a/source/common/http/websocket/ws_handler_impl.cc b/source/common/http/websocket/ws_handler_impl.cc index 5906c54b5d92..025c7c28d417 100644 --- a/source/common/http/websocket/ws_handler_impl.cc +++ b/source/common/http/websocket/ws_handler_impl.cc @@ -47,8 +47,8 @@ WsHandlerImpl::WsHandlerImpl(HeaderMap& request_headers, RequestInfo::RequestInf WebSocketProxyCallbacks& callbacks, Upstream::ClusterManager& cluster_manager, Network::ReadFilterCallbacks* read_callbacks, - TcpProxy::ConfigSharedPtr config) - : TcpProxy::Filter(config, cluster_manager), request_headers_(request_headers), + TcpProxy::ConfigSharedPtr config, Event::TimeSystem& time_system) + : TcpProxy::Filter(config, cluster_manager, time_system), request_headers_(request_headers), request_info_(request_info), route_entry_(route_entry), ws_callbacks_(callbacks) { // set_connection_stats == false because the http connection manager has already set them diff --git a/source/common/http/websocket/ws_handler_impl.h b/source/common/http/websocket/ws_handler_impl.h index 0cd90a87677f..51feae71ca2d 100644 --- a/source/common/http/websocket/ws_handler_impl.h +++ b/source/common/http/websocket/ws_handler_impl.h @@ -35,7 +35,8 @@ class WsHandlerImpl : public TcpProxy::Filter, public Http::WebSocketProxy { WsHandlerImpl(HeaderMap& request_headers, RequestInfo::RequestInfo& request_info, const Router::RouteEntry& route_entry, WebSocketProxyCallbacks& callbacks, Upstream::ClusterManager& cluster_manager, - Network::ReadFilterCallbacks* read_callbacks, TcpProxy::ConfigSharedPtr config); + Network::ReadFilterCallbacks* read_callbacks, TcpProxy::ConfigSharedPtr config, + Event::TimeSystem& time_system); // Upstream::LoadBalancerContext const Router::MetadataMatchCriteria* metadataMatchCriteria() override { diff --git a/source/common/request_info/request_info_impl.h b/source/common/request_info/request_info_impl.h index d2a2c2879516..0d6a9ef52a73 100644 --- a/source/common/request_info/request_info_impl.h +++ b/source/common/request_info/request_info_impl.h @@ -3,6 +3,7 @@ #include #include +#include "envoy/common/time.h" #include "envoy/request_info/request_info.h" #include "common/common/assert.h" @@ -12,11 +13,13 @@ namespace Envoy { namespace RequestInfo { struct RequestInfoImpl : public RequestInfo { - RequestInfoImpl() - : start_time_(std::chrono::system_clock::now()), - start_time_monotonic_(std::chrono::steady_clock::now()) {} + explicit RequestInfoImpl(TimeSource& time_source) + : time_source_(time_source), start_time_(time_source.systemTime()), + start_time_monotonic_(time_source.monotonicTime()) {} - RequestInfoImpl(Http::Protocol protocol) : RequestInfoImpl() { protocol_ = protocol; } + RequestInfoImpl(Http::Protocol protocol, TimeSource& time_source) : RequestInfoImpl(time_source) { + protocol_ = protocol; + } SystemTime startTime() const override { return start_time_; } @@ -37,7 +40,7 @@ struct RequestInfoImpl : public RequestInfo { void onLastDownstreamRxByteReceived() override { ASSERT(!last_downstream_rx_byte_received); - last_downstream_rx_byte_received = std::chrono::steady_clock::now(); + last_downstream_rx_byte_received = time_source_.monotonicTime(); } absl::optional firstUpstreamTxByteSent() const override { @@ -46,7 +49,7 @@ struct RequestInfoImpl : public RequestInfo { void onFirstUpstreamTxByteSent() override { ASSERT(!first_upstream_tx_byte_sent_); - first_upstream_tx_byte_sent_ = std::chrono::steady_clock::now(); + first_upstream_tx_byte_sent_ = time_source_.monotonicTime(); } absl::optional lastUpstreamTxByteSent() const override { @@ -55,7 +58,7 @@ struct RequestInfoImpl : public RequestInfo { void onLastUpstreamTxByteSent() override { ASSERT(!last_upstream_tx_byte_sent_); - last_upstream_tx_byte_sent_ = std::chrono::steady_clock::now(); + last_upstream_tx_byte_sent_ = time_source_.monotonicTime(); } absl::optional firstUpstreamRxByteReceived() const override { @@ -64,7 +67,7 @@ struct RequestInfoImpl : public RequestInfo { void onFirstUpstreamRxByteReceived() override { ASSERT(!first_upstream_rx_byte_received_); - first_upstream_rx_byte_received_ = std::chrono::steady_clock::now(); + first_upstream_rx_byte_received_ = time_source_.monotonicTime(); } absl::optional lastUpstreamRxByteReceived() const override { @@ -73,7 +76,7 @@ struct RequestInfoImpl : public RequestInfo { void onLastUpstreamRxByteReceived() override { ASSERT(!last_upstream_rx_byte_received_); - last_upstream_rx_byte_received_ = std::chrono::steady_clock::now(); + last_upstream_rx_byte_received_ = time_source_.monotonicTime(); } absl::optional firstDownstreamTxByteSent() const override { @@ -82,7 +85,7 @@ struct RequestInfoImpl : public RequestInfo { void onFirstDownstreamTxByteSent() override { ASSERT(!first_downstream_tx_byte_sent_); - first_downstream_tx_byte_sent_ = std::chrono::steady_clock::now(); + first_downstream_tx_byte_sent_ = time_source_.monotonicTime(); } absl::optional lastDownstreamTxByteSent() const override { @@ -91,7 +94,7 @@ struct RequestInfoImpl : public RequestInfo { void onLastDownstreamTxByteSent() override { ASSERT(!last_downstream_tx_byte_sent_); - last_downstream_tx_byte_sent_ = std::chrono::steady_clock::now(); + last_downstream_tx_byte_sent_ = time_source_.monotonicTime(); } absl::optional requestComplete() const override { @@ -100,7 +103,7 @@ struct RequestInfoImpl : public RequestInfo { void onRequestComplete() override { ASSERT(!final_time_); - final_time_ = std::chrono::steady_clock::now(); + final_time_ = time_source_.monotonicTime(); } void resetUpstreamTimings() override { @@ -188,6 +191,7 @@ struct RequestInfoImpl : public RequestInfo { const std::string& requestedServerName() const override { return requested_server_name_; } + TimeSource& time_source_; const SystemTime start_time_; const MonotonicTime start_time_monotonic_; diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 7931a54afd82..9ff3fef9a338 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -267,7 +267,8 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, opaque_config_(parseOpaqueConfig(route)), decorator_(parseDecorator(route)), direct_response_code_(ConfigUtility::parseDirectResponseCode(route)), direct_response_body_(ConfigUtility::parseDirectResponseBody(route)), - per_filter_configs_(route.per_filter_config(), factory_context) { + per_filter_configs_(route.per_filter_config(), factory_context), + time_system_(factory_context.dispatcher().timeSystem()) { if (route.route().has_metadata_match()) { const auto filter_it = route.route().metadata_match().filter_metadata().find( Envoy::Config::MetadataFilters::get().ENVOY_LB); @@ -352,9 +353,9 @@ Http::WebSocketProxyPtr RouteEntryImplBase::createWebSocketProxy( Http::HeaderMap& request_headers, RequestInfo::RequestInfo& request_info, Http::WebSocketProxyCallbacks& callbacks, Upstream::ClusterManager& cluster_manager, Network::ReadFilterCallbacks* read_callbacks) const { - return std::make_unique(request_headers, request_info, *this, - callbacks, cluster_manager, - read_callbacks, websocket_config_); + return std::make_unique( + request_headers, request_info, *this, callbacks, cluster_manager, read_callbacks, + websocket_config_, time_system_); } void RouteEntryImplBase::finalizeRequestHeaders(Http::HeaderMap& headers, diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index 1ed787e31642..0ce488a2d3d4 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -558,6 +558,7 @@ class RouteEntryImplBase : public RouteEntry, const absl::optional direct_response_code_; std::string direct_response_body_; PerFilterConfigs per_filter_configs_; + Event::TimeSystem& time_system_; }; /** diff --git a/source/common/router/router.cc b/source/common/router/router.cc index d13650997c74..cd7f68fa2b01 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -409,7 +409,8 @@ void Filter::maybeDoShadowing() { void Filter::onRequestComplete() { downstream_end_stream_ = true; - downstream_request_complete_time_ = std::chrono::steady_clock::now(); + Event::Dispatcher& dispatcher = callbacks_->dispatcher(); + downstream_request_complete_time_ = dispatcher.timeSystem().monotonicTime(); // Possible that we got an immediate reset. if (upstream_request_) { @@ -419,8 +420,7 @@ void Filter::onRequestComplete() { upstream_request_->setupPerTryTimeout(); if (timeout_.global_timeout_.count() > 0) { - response_timeout_ = - callbacks_->dispatcher().createTimer([this]() -> void { onResponseTimeout(); }); + response_timeout_ = dispatcher.createTimer([this]() -> void { onResponseTimeout(); }); response_timeout_->enableTimer(timeout_.global_timeout_); } } @@ -617,7 +617,8 @@ void Filter::onUpstreamHeaders(const uint64_t response_code, Http::HeaderMapPtr& // Only send upstream service time if we received the complete request and this is not a // premature response. if (DateUtil::timePointValid(downstream_request_complete_time_)) { - MonotonicTime response_received_time = std::chrono::steady_clock::now(); + Event::Dispatcher& dispatcher = callbacks_->dispatcher(); + MonotonicTime response_received_time = dispatcher.timeSystem().monotonicTime(); std::chrono::milliseconds ms = std::chrono::duration_cast( response_received_time - downstream_request_complete_time_); if (!config_.suppress_envoy_headers_) { @@ -684,8 +685,9 @@ void Filter::onUpstreamComplete() { if (config_.emit_dynamic_stats_ && !callbacks_->requestInfo().healthCheck() && DateUtil::timePointValid(downstream_request_complete_time_)) { + Event::Dispatcher& dispatcher = callbacks_->dispatcher(); std::chrono::milliseconds response_time = std::chrono::duration_cast( - std::chrono::steady_clock::now() - downstream_request_complete_time_); + dispatcher.timeSystem().monotonicTime() - downstream_request_complete_time_); upstream_request_->upstream_host_->outlierDetector().putResponseTime(response_time); @@ -780,8 +782,9 @@ void Filter::doRetry() { Filter::UpstreamRequest::UpstreamRequest(Filter& parent, Http::ConnectionPool::Instance& pool) : parent_(parent), conn_pool_(pool), grpc_rq_success_deferred_(false), - request_info_(pool.protocol()), calling_encode_headers_(false), upstream_canary_(false), - encode_complete_(false), encode_trailers_(false) { + request_info_(pool.protocol(), parent_.callbacks_->dispatcher().timeSystem()), + calling_encode_headers_(false), upstream_canary_(false), encode_complete_(false), + encode_trailers_(false) { if (parent_.config_.start_child_span_) { span_ = parent_.callbacks_->activeSpan().spawnChild( diff --git a/source/common/tcp/conn_pool.cc b/source/common/tcp/conn_pool.cc index 3204186b724b..21a7f6581ece 100644 --- a/source/common/tcp/conn_pool.cc +++ b/source/common/tcp/conn_pool.cc @@ -309,8 +309,8 @@ ConnPoolImpl::ActiveConn::ActiveConn(ConnPoolImpl& parent) connect_timer_(parent_.dispatcher_.createTimer([this]() -> void { onConnectTimeout(); })), remaining_requests_(parent_.host_->cluster().maxRequestsPerConnection()), timed_out_(false) { - parent_.conn_connect_ms_.reset( - new Stats::Timespan(parent_.host_->cluster().stats().upstream_cx_connect_ms_)); + parent_.conn_connect_ms_.reset(new Stats::Timespan( + parent_.host_->cluster().stats().upstream_cx_connect_ms_, parent_.dispatcher_.timeSystem())); Upstream::Host::CreateConnectionData data = parent_.host_->createConnection(parent_.dispatcher_, parent_.socket_options_); @@ -329,7 +329,8 @@ ConnPoolImpl::ActiveConn::ActiveConn(ConnPoolImpl& parent) parent_.host_->cluster().stats().upstream_cx_active_.inc(); parent_.host_->stats().cx_total_.inc(); parent_.host_->stats().cx_active_.inc(); - conn_length_.reset(new Stats::Timespan(parent_.host_->cluster().stats().upstream_cx_length_ms_)); + conn_length_.reset(new Stats::Timespan(parent_.host_->cluster().stats().upstream_cx_length_ms_, + parent_.dispatcher_.timeSystem())); connect_timer_->enableTimer(parent_.host_->cluster().connectTimeout()); parent_.host_->cluster().resourceManager(parent_.priority_).connections().inc(); diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index e4517f158111..4e148befd3ea 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -122,9 +122,10 @@ UpstreamDrainManager& Config::drainManager() { return upstream_drain_manager_slot_->getTyped(); } -Filter::Filter(ConfigSharedPtr config, Upstream::ClusterManager& cluster_manager) +Filter::Filter(ConfigSharedPtr config, Upstream::ClusterManager& cluster_manager, + TimeSource& time_source) : config_(config), cluster_manager_(cluster_manager), downstream_callbacks_(*this), - upstream_callbacks_(new UpstreamCallbacks(this)) { + upstream_callbacks_(new UpstreamCallbacks(this)), request_info_(time_source) { ASSERT(config != nullptr); } diff --git a/source/common/tcp_proxy/tcp_proxy.h b/source/common/tcp_proxy/tcp_proxy.h index 3fa588f3702d..09bf0654123b 100644 --- a/source/common/tcp_proxy/tcp_proxy.h +++ b/source/common/tcp_proxy/tcp_proxy.h @@ -145,7 +145,8 @@ class Filter : public Network::ReadFilter, Tcp::ConnectionPool::Callbacks, protected Logger::Loggable { public: - Filter(ConfigSharedPtr config, Upstream::ClusterManager& cluster_manager); + Filter(ConfigSharedPtr config, Upstream::ClusterManager& cluster_manager, + TimeSource& time_source); ~Filter(); // Network::ReadFilter diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 8eb07ee7b55b..17ef30752fc3 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -389,7 +389,7 @@ bool ClusterManagerImpl::scheduleUpdate(const Cluster& cluster, uint32_t priorit // Has an update_merge_window gone by since the last update? If so, don't schedule // the update so it can be applied immediately. Ditto if this is not a mergeable update. - const auto delta = std::chrono::steady_clock::now() - updates->last_updated_; + const auto delta = time_source_.monotonicTime() - updates->last_updated_; const uint64_t delta_ms = std::chrono::duration_cast(delta).count(); const bool out_of_merge_window = delta_ms > timeout; if (out_of_merge_window || !mergeable) { @@ -412,7 +412,7 @@ bool ClusterManagerImpl::scheduleUpdate(const Cluster& cluster, uint32_t priorit cm_stats_.update_merge_cancelled_.inc(); } - updates->last_updated_ = std::chrono::steady_clock::now(); + updates->last_updated_ = time_source_.monotonicTime(); return false; } @@ -445,7 +445,7 @@ void ClusterManagerImpl::applyUpdates(const Cluster& cluster, uint32_t priority, cm_stats_.cluster_updated_via_merge_.inc(); updates.timer_enabled_ = false; - updates.last_updated_ = std::chrono::steady_clock::now(); + updates.last_updated_ = time_source_.monotonicTime(); } bool ClusterManagerImpl::addOrUpdateCluster(const envoy::api::v2::Cluster& cluster, diff --git a/source/common/upstream/health_checker_impl.cc b/source/common/upstream/health_checker_impl.cc index a4f0f8714d74..c91490e939f0 100644 --- a/source/common/upstream/health_checker_impl.cc +++ b/source/common/upstream/health_checker_impl.cc @@ -153,7 +153,7 @@ void HttpHealthCheckerImpl::HttpActiveHealthCheckSession::onInterval() { {Http::Headers::get().Path, parent_.path_}, {Http::Headers::get().UserAgent, Http::Headers::get().UserAgentValues.EnvoyHealthChecker}}; Router::FilterUtility::setUpstreamScheme(request_headers, *parent_.cluster_.info()); - RequestInfo::RequestInfoImpl request_info(protocol_); + RequestInfo::RequestInfoImpl request_info(protocol_, parent_.dispatcher_.timeSystem()); request_info.setDownstreamLocalAddress(local_address_); request_info.setDownstreamRemoteAddress(local_address_); request_info.onUpstreamHostSelected(host_); diff --git a/source/extensions/filters/http/dynamo/config.cc b/source/extensions/filters/http/dynamo/config.cc index b1285bbdf5ca..66d17b210a0f 100644 --- a/source/extensions/filters/http/dynamo/config.cc +++ b/source/extensions/filters/http/dynamo/config.cc @@ -15,8 +15,8 @@ Http::FilterFactoryCb DynamoFilterConfig::createFilter(const std::string& stat_prefix, Server::Configuration::FactoryContext& context) { return [&context, stat_prefix](Http::FilterChainFactoryCallbacks& callbacks) -> void { - callbacks.addStreamFilter(Http::StreamFilterSharedPtr{ - new Dynamo::DynamoFilter(context.runtime(), stat_prefix, context.scope())}); + callbacks.addStreamFilter(Http::StreamFilterSharedPtr{new Dynamo::DynamoFilter( + context.runtime(), stat_prefix, context.scope(), context.dispatcher().timeSystem())}); }; } diff --git a/source/extensions/filters/http/dynamo/dynamo_filter.cc b/source/extensions/filters/http/dynamo/dynamo_filter.cc index 504fba6dce21..974e5bf3ab59 100644 --- a/source/extensions/filters/http/dynamo/dynamo_filter.cc +++ b/source/extensions/filters/http/dynamo/dynamo_filter.cc @@ -23,7 +23,7 @@ namespace Dynamo { Http::FilterHeadersStatus DynamoFilter::decodeHeaders(Http::HeaderMap& headers, bool) { if (enabled_) { - start_decode_ = std::chrono::steady_clock::now(); + start_decode_ = time_system_.monotonicTime(); operation_ = RequestParser::parseOperation(headers); return Http::FilterHeadersStatus::StopIteration; } else { @@ -172,7 +172,7 @@ void DynamoFilter::chargeBasicStats(uint64_t status) { void DynamoFilter::chargeStatsPerEntity(const std::string& entity, const std::string& entity_type, uint64_t status) { std::chrono::milliseconds latency = std::chrono::duration_cast( - std::chrono::steady_clock::now() - start_decode_); + time_system_.monotonicTime() - start_decode_); std::string group_string = Http::CodeUtility::groupStringForResponseCode(static_cast(status)); diff --git a/source/extensions/filters/http/dynamo/dynamo_filter.h b/source/extensions/filters/http/dynamo/dynamo_filter.h index f0a0109bcbbf..fd98ecda1f17 100644 --- a/source/extensions/filters/http/dynamo/dynamo_filter.h +++ b/source/extensions/filters/http/dynamo/dynamo_filter.h @@ -24,8 +24,10 @@ namespace Dynamo { */ class DynamoFilter : public Http::StreamFilter { public: - DynamoFilter(Runtime::Loader& runtime, const std::string& stat_prefix, Stats::Scope& scope) - : runtime_(runtime), stat_prefix_(stat_prefix + "dynamodb."), scope_(scope) { + DynamoFilter(Runtime::Loader& runtime, const std::string& stat_prefix, Stats::Scope& scope, + Event::TimeSystem& time_system) + : runtime_(runtime), stat_prefix_(stat_prefix + "dynamodb."), scope_(scope), + time_system_(time_system) { enabled_ = runtime_.snapshot().featureEnabled("dynamodb.filter_enabled", 100); } @@ -74,6 +76,7 @@ class DynamoFilter : public Http::StreamFilter { Http::HeaderMap* response_headers_; Http::StreamDecoderFilterCallbacks* decoder_callbacks_{}; Http::StreamEncoderFilterCallbacks* encoder_callbacks_{}; + Event::TimeSystem& time_system_; }; } // namespace Dynamo diff --git a/source/extensions/filters/http/jwt_authn/authenticator.cc b/source/extensions/filters/http/jwt_authn/authenticator.cc index 9b558de3b422..da5b05300e60 100644 --- a/source/extensions/filters/http/jwt_authn/authenticator.cc +++ b/source/extensions/filters/http/jwt_authn/authenticator.cc @@ -37,6 +37,8 @@ class AuthenticatorImpl : public Logger::Loggable, void onDestroy() override; void sanitizePayloadHeaders(Http::HeaderMap& headers) const override; + TimeSource& timeSource() { return config_->timeSource(); } + private: // Verify with a specific public key. void verifyKey(); @@ -116,9 +118,9 @@ void AuthenticatorImpl::verify(Http::HeaderMap& headers, Authenticator::Callback // the abseil time functionality instead or use the jwt_verify_lib to check // the validity of a JWT. // Check "exp" claim. - const auto unix_timestamp = std::chrono::duration_cast( - std::chrono::system_clock::now().time_since_epoch()) - .count(); + const auto unix_timestamp = + std::chrono::duration_cast(timeSource().systemTime().time_since_epoch()) + .count(); // If the nbf claim does *not* appear in the JWT, then the nbf field is defaulted // to 0. if (jwt_.nbf_ > unix_timestamp) { diff --git a/source/extensions/filters/http/jwt_authn/filter_config.h b/source/extensions/filters/http/jwt_authn/filter_config.h index 64743d25e4a1..cb400e6dffcb 100644 --- a/source/extensions/filters/http/jwt_authn/filter_config.h +++ b/source/extensions/filters/http/jwt_authn/filter_config.h @@ -24,8 +24,9 @@ class ThreadLocalCache : public ThreadLocal::ThreadLocalObject { public: // Load the config from envoy config. ThreadLocalCache( - const ::envoy::config::filter::http::jwt_authn::v2alpha::JwtAuthentication& config) { - jwks_cache_ = JwksCache::create(config); + const ::envoy::config::filter::http::jwt_authn::v2alpha::JwtAuthentication& config, + TimeSource& time_source) { + jwks_cache_ = JwksCache::create(config, time_source); } // Get the JwksCache object. @@ -62,10 +63,11 @@ class FilterConfig : public Logger::Loggable { const ::envoy::config::filter::http::jwt_authn::v2alpha::JwtAuthentication& proto_config, const std::string& stats_prefix, Server::Configuration::FactoryContext& context) : proto_config_(proto_config), stats_(generateStats(stats_prefix, context.scope())), - tls_(context.threadLocal().allocateSlot()), cm_(context.clusterManager()) { + tls_(context.threadLocal().allocateSlot()), cm_(context.clusterManager()), + time_source_(context.dispatcher().timeSystem()) { ENVOY_LOG(info, "Loaded JwtAuthConfig: {}", proto_config_.DebugString()); tls_->set([this](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr { - return std::make_shared(proto_config_); + return std::make_shared(proto_config_, time_source_); }); extractor_ = Extractor::create(proto_config_); } @@ -82,6 +84,7 @@ class FilterConfig : public Logger::Loggable { ThreadLocalCache& getCache() { return tls_->getTyped(); } Upstream::ClusterManager& cm() { return cm_; } + TimeSource& timeSource() { return time_source_; } // Get the token extractor. const Extractor& getExtractor() const { return *extractor_; } @@ -102,6 +105,7 @@ class FilterConfig : public Logger::Loggable { Upstream::ClusterManager& cm_; // The object to extract tokens. ExtractorConstPtr extractor_; + TimeSource& time_source_; }; typedef std::shared_ptr FilterConfigSharedPtr; diff --git a/source/extensions/filters/http/jwt_authn/jwks_cache.cc b/source/extensions/filters/http/jwt_authn/jwks_cache.cc index 3648113dbc30..95a89751b7ef 100644 --- a/source/extensions/filters/http/jwt_authn/jwks_cache.cc +++ b/source/extensions/filters/http/jwt_authn/jwks_cache.cc @@ -3,6 +3,8 @@ #include #include +#include "envoy/common/time.h" + #include "common/common/logger.h" #include "common/config/datasource.h" #include "common/protobuf/utility.h" @@ -25,7 +27,8 @@ constexpr int PubkeyCacheExpirationSec = 600; class JwksDataImpl : public JwksCache::JwksData, public Logger::Loggable { public: - JwksDataImpl(const JwtProvider& jwt_provider) : jwt_provider_(jwt_provider) { + JwksDataImpl(const JwtProvider& jwt_provider, TimeSource& time_source) + : jwt_provider_(jwt_provider), time_source_(time_source) { std::vector audiences; for (const auto& aud : jwt_provider_.audiences()) { audiences.push_back(aud); @@ -53,7 +56,7 @@ class JwksDataImpl : public JwksCache::JwksData, public Logger::Loggable= expiration_time_; } + bool isExpired() const override { return time_source_.monotonicTime() >= expiration_time_; } const ::google::jwt_verify::Jwks* setRemoteJwks(::google::jwt_verify::JwksPtr&& jwks) override { return setKey(std::move(jwks), getRemoteJwksExpirationTime()); @@ -62,7 +65,7 @@ class JwksDataImpl : public JwksCache::JwksData, public Logger::Loggable void { filter_manager.addFilter(std::make_shared( stat_prefix, context.scope(), context.runtime(), access_log, fault_config, - context.drainDecision(), context.random())); + context.drainDecision(), context.random(), context.dispatcher().timeSystem())); }; } diff --git a/source/extensions/filters/network/mongo_proxy/proxy.cc b/source/extensions/filters/network/mongo_proxy/proxy.cc index 5366ba33c0e2..4b72b96a0e62 100644 --- a/source/extensions/filters/network/mongo_proxy/proxy.cc +++ b/source/extensions/filters/network/mongo_proxy/proxy.cc @@ -21,8 +21,9 @@ namespace Extensions { namespace NetworkFilters { namespace MongoProxy { -AccessLog::AccessLog(const std::string& file_name, - Envoy::AccessLog::AccessLogManager& log_manager) { +AccessLog::AccessLog(const std::string& file_name, Envoy::AccessLog::AccessLogManager& log_manager, + TimeSource& time_source) + : time_source_(time_source) { file_ = log_manager.createAccessLog(file_name); } @@ -31,7 +32,7 @@ void AccessLog::logMessage(const Message& message, bool full, static const std::string log_format = "{{\"time\": \"{}\", \"message\": {}, \"upstream_host\": \"{}\"}}\n"; - SystemTime now = std::chrono::system_clock::now(); + SystemTime now = time_source_.systemTime(); std::string log_line = fmt::format(log_format, AccessLogDateTimeFormatter::fromTime(now), message.toString(full), upstream_host ? upstream_host->address()->asString() : "-"); @@ -43,10 +44,10 @@ ProxyFilter::ProxyFilter(const std::string& stat_prefix, Stats::Scope& scope, Runtime::Loader& runtime, AccessLogSharedPtr access_log, const FaultConfigSharedPtr& fault_config, const Network::DrainDecision& drain_decision, - Runtime::RandomGenerator& generator) + Runtime::RandomGenerator& generator, Event::TimeSystem& time_system) : stat_prefix_(stat_prefix), scope_(scope), stats_(generateStats(stat_prefix, scope)), runtime_(runtime), drain_decision_(drain_decision), generator_(generator), - access_log_(access_log), fault_config_(fault_config) { + access_log_(access_log), fault_config_(fault_config), time_system_(time_system) { if (!runtime_.snapshot().featureEnabled(MongoRuntimeConfig::get().ConnectionLoggingEnabled, 100)) { // If we are not logging at the connection level, just release the shared pointer so that we @@ -241,7 +242,7 @@ void ProxyFilter::chargeReplyStats(ActiveQuery& active_query, const std::string& scope_.histogram(fmt::format("{}.reply_size", prefix)).recordValue(reply_documents_byte_size); scope_.histogram(fmt::format("{}.reply_time_ms", prefix)) .recordValue(std::chrono::duration_cast( - std::chrono::steady_clock::now() - active_query.start_time_) + time_system_.monotonicTime() - active_query.start_time_) .count()); } diff --git a/source/extensions/filters/network/mongo_proxy/proxy.h b/source/extensions/filters/network/mongo_proxy/proxy.h index ea825ca1a9af..f413889b3cfb 100644 --- a/source/extensions/filters/network/mongo_proxy/proxy.h +++ b/source/extensions/filters/network/mongo_proxy/proxy.h @@ -85,12 +85,14 @@ struct MongoProxyStats { */ class AccessLog { public: - AccessLog(const std::string& file_name, Envoy::AccessLog::AccessLogManager& log_manager); + AccessLog(const std::string& file_name, Envoy::AccessLog::AccessLogManager& log_manager, + TimeSource& time_source); void logMessage(const Message& message, bool full, const Upstream::HostDescription* upstream_host); private: + TimeSource& time_source_; Filesystem::FileSharedPtr file_; }; @@ -127,7 +129,8 @@ class ProxyFilter : public Network::Filter, public: ProxyFilter(const std::string& stat_prefix, Stats::Scope& scope, Runtime::Loader& runtime, AccessLogSharedPtr access_log, const FaultConfigSharedPtr& fault_config, - const Network::DrainDecision& drain_decision, Runtime::RandomGenerator& generator); + const Network::DrainDecision& drain_decision, Runtime::RandomGenerator& generator, + Event::TimeSystem& time_system); ~ProxyFilter(); virtual DecoderPtr createDecoder(DecoderCallbacks& callbacks) PURE; @@ -160,7 +163,7 @@ class ProxyFilter : public Network::Filter, private: struct ActiveQuery { ActiveQuery(ProxyFilter& parent, const QueryMessage& query) - : parent_(parent), query_info_(query), start_time_(std::chrono::steady_clock::now()) { + : parent_(parent), query_info_(query), start_time_(parent_.time_system_.monotonicTime()) { parent_.stats_.op_query_active_.inc(); } @@ -205,6 +208,7 @@ class ProxyFilter : public Network::Filter, const FaultConfigSharedPtr fault_config_; Event::TimerPtr delay_timer_; Event::TimerPtr drain_close_timer_; + Event::TimeSystem& time_system_; }; class ProdProxyFilter : public ProxyFilter { diff --git a/source/extensions/filters/network/tcp_proxy/config.cc b/source/extensions/filters/network/tcp_proxy/config.cc index 160a78389da1..1f56af99f4d5 100644 --- a/source/extensions/filters/network/tcp_proxy/config.cc +++ b/source/extensions/filters/network/tcp_proxy/config.cc @@ -29,8 +29,8 @@ Network::FilterFactoryCb ConfigFactory::createFilterFactoryFromProtoTyped( Envoy::TcpProxy::ConfigSharedPtr filter_config( std::make_shared(proto_config, context)); return [filter_config, &context](Network::FilterManager& filter_manager) -> void { - filter_manager.addReadFilter( - std::make_shared(filter_config, context.clusterManager())); + filter_manager.addReadFilter(std::make_shared( + filter_config, context.clusterManager(), context.dispatcher().timeSystem())); }; } diff --git a/source/extensions/filters/network/thrift_proxy/config.cc b/source/extensions/filters/network/thrift_proxy/config.cc index 113eccbe8577..69ba81dfa206 100644 --- a/source/extensions/filters/network/thrift_proxy/config.cc +++ b/source/extensions/filters/network/thrift_proxy/config.cc @@ -106,8 +106,8 @@ Network::FilterFactoryCb ThriftProxyFilterConfigFactory::createFilterFactoryFrom std::shared_ptr filter_config(new ConfigImpl(proto_config, context)); return [filter_config, &context](Network::FilterManager& filter_manager) -> void { - filter_manager.addReadFilter( - std::make_shared(*filter_config, context.random())); + filter_manager.addReadFilter(std::make_shared( + *filter_config, context.random(), context.dispatcher().timeSystem())); }; } diff --git a/source/extensions/filters/network/thrift_proxy/conn_manager.cc b/source/extensions/filters/network/thrift_proxy/conn_manager.cc index 0a39871f5ff7..f424d9f561be 100644 --- a/source/extensions/filters/network/thrift_proxy/conn_manager.cc +++ b/source/extensions/filters/network/thrift_proxy/conn_manager.cc @@ -15,11 +15,12 @@ namespace Extensions { namespace NetworkFilters { namespace ThriftProxy { -ConnectionManager::ConnectionManager(Config& config, Runtime::RandomGenerator& random_generator) +ConnectionManager::ConnectionManager(Config& config, Runtime::RandomGenerator& random_generator, + Event::TimeSystem& time_system) : config_(config), stats_(config_.stats()), transport_(config.createTransport()), protocol_(config.createProtocol()), decoder_(std::make_unique(*transport_, *protocol_, *this)), - random_generator_(random_generator) {} + random_generator_(random_generator), time_system_(time_system) {} ConnectionManager::~ConnectionManager() {} diff --git a/source/extensions/filters/network/thrift_proxy/conn_manager.h b/source/extensions/filters/network/thrift_proxy/conn_manager.h index e514e5d86950..1633494098d6 100644 --- a/source/extensions/filters/network/thrift_proxy/conn_manager.h +++ b/source/extensions/filters/network/thrift_proxy/conn_manager.h @@ -56,7 +56,8 @@ class ConnectionManager : public Network::ReadFilter, public DecoderCallbacks, Logger::Loggable { public: - ConnectionManager(Config& config, Runtime::RandomGenerator& random_generator); + ConnectionManager(Config& config, Runtime::RandomGenerator& random_generator, + Event::TimeSystem& time_system); ~ConnectionManager(); // Network::ReadFilter @@ -114,7 +115,8 @@ class ConnectionManager : public Network::ReadFilter, public ThriftFilters::DecoderFilterCallbacks, public ThriftFilters::FilterChainFactoryCallbacks { ActiveRpc(ConnectionManager& parent) - : parent_(parent), request_timer_(new Stats::Timespan(parent_.stats_.request_time_ms_)), + : parent_(parent), request_timer_(new Stats::Timespan(parent_.stats_.request_time_ms_, + parent_.time_system_)), stream_id_(parent_.random_generator_.random()) { parent_.stats_.request_active_.inc(); } @@ -192,6 +194,7 @@ class ConnectionManager : public Network::ReadFilter, Runtime::RandomGenerator& random_generator_; bool stopped_{false}; bool half_closed_{false}; + Event::TimeSystem& time_system_; }; } // namespace ThriftProxy diff --git a/source/extensions/stat_sinks/hystrix/hystrix.cc b/source/extensions/stat_sinks/hystrix/hystrix.cc index ed339a4a4089..6c05d4a66c27 100644 --- a/source/extensions/stat_sinks/hystrix/hystrix.cc +++ b/source/extensions/stat_sinks/hystrix/hystrix.cc @@ -153,7 +153,7 @@ void HystrixSink::addHystrixCommand(ClusterStatsCache& cluster_stats_cache, std::chrono::milliseconds rolling_window_ms, const QuantileLatencyMap& histogram, std::stringstream& ss) { - std::time_t currentTime = std::chrono::system_clock::to_time_t(std::chrono::system_clock::now()); + std::time_t currentTime = std::chrono::system_clock::to_time_t(server_.timeSystem().systemTime()); ss << "data: {"; addStringToStream("type", "HystrixCommand", ss, true); diff --git a/source/extensions/transport_sockets/capture/BUILD b/source/extensions/transport_sockets/capture/BUILD index 88490ea7077f..12aca228c9a0 100644 --- a/source/extensions/transport_sockets/capture/BUILD +++ b/source/extensions/transport_sockets/capture/BUILD @@ -15,6 +15,7 @@ envoy_cc_library( srcs = ["capture.cc"], hdrs = ["capture.h"], deps = [ + "//include/envoy/event:timer_interface", "//include/envoy/network:transport_socket_interface", "//source/common/buffer:buffer_lib", "//source/common/common:assert_lib", diff --git a/source/extensions/transport_sockets/capture/capture.cc b/source/extensions/transport_sockets/capture/capture.cc index 25188ea6474a..a1a5df79b39b 100644 --- a/source/extensions/transport_sockets/capture/capture.cc +++ b/source/extensions/transport_sockets/capture/capture.cc @@ -14,8 +14,9 @@ namespace Capture { CaptureSocket::CaptureSocket( const std::string& path_prefix, envoy::config::transport_socket::capture::v2alpha::FileSink::Format format, - Network::TransportSocketPtr&& transport_socket) - : path_prefix_(path_prefix), format_(format), transport_socket_(std::move(transport_socket)) {} + Network::TransportSocketPtr&& transport_socket, Event::TimeSystem& time_system) + : path_prefix_(path_prefix), format_(format), transport_socket_(std::move(transport_socket)), + time_system_(time_system) {} void CaptureSocket::setTransportSocketCallbacks(Network::TransportSocketCallbacks& callbacks) { callbacks_ = &callbacks; @@ -61,7 +62,7 @@ Network::IoResult CaptureSocket::doRead(Buffer::Instance& buffer) { auto* event = trace_.add_events(); event->mutable_timestamp()->MergeFrom(Protobuf::util::TimeUtil::NanosecondsToTimestamp( std::chrono::duration_cast( - std::chrono::system_clock::now().time_since_epoch()) + time_system_.systemTime().time_since_epoch()) .count())); event->mutable_read()->set_data(data, result.bytes_processed_); } @@ -79,7 +80,7 @@ Network::IoResult CaptureSocket::doWrite(Buffer::Instance& buffer, bool end_stre auto* event = trace_.add_events(); event->mutable_timestamp()->MergeFrom(Protobuf::util::TimeUtil::NanosecondsToTimestamp( std::chrono::duration_cast( - std::chrono::system_clock::now().time_since_epoch()) + time_system_.systemTime().time_since_epoch()) .count())); event->mutable_write()->set_data(data, result.bytes_processed_); event->mutable_write()->set_end_stream(end_stream); @@ -94,13 +95,13 @@ const Ssl::Connection* CaptureSocket::ssl() const { return transport_socket_->ss CaptureSocketFactory::CaptureSocketFactory( const std::string& path_prefix, envoy::config::transport_socket::capture::v2alpha::FileSink::Format format, - Network::TransportSocketFactoryPtr&& transport_socket_factory) + Network::TransportSocketFactoryPtr&& transport_socket_factory, Event::TimeSystem& time_system) : path_prefix_(path_prefix), format_(format), - transport_socket_factory_(std::move(transport_socket_factory)) {} + transport_socket_factory_(std::move(transport_socket_factory)), time_system_(time_system) {} Network::TransportSocketPtr CaptureSocketFactory::createTransportSocket() const { - return std::make_unique(path_prefix_, format_, - transport_socket_factory_->createTransportSocket()); + return std::make_unique( + path_prefix_, format_, transport_socket_factory_->createTransportSocket(), time_system_); } bool CaptureSocketFactory::implementsSecureTransport() const { diff --git a/source/extensions/transport_sockets/capture/capture.h b/source/extensions/transport_sockets/capture/capture.h index 63b94042460a..f7031146718c 100644 --- a/source/extensions/transport_sockets/capture/capture.h +++ b/source/extensions/transport_sockets/capture/capture.h @@ -4,6 +4,7 @@ #include "envoy/config/transport_socket/capture/v2alpha/capture.pb.h" #include "envoy/data/tap/v2alpha/capture.pb.h" +#include "envoy/event/timer.h" #include "envoy/network/transport_socket.h" namespace Envoy { @@ -15,7 +16,7 @@ class CaptureSocket : public Network::TransportSocket { public: CaptureSocket(const std::string& path_prefix, envoy::config::transport_socket::capture::v2alpha::FileSink::Format format, - Network::TransportSocketPtr&& transport_socket); + Network::TransportSocketPtr&& transport_socket, Event::TimeSystem& time_system); // Network::TransportSocket void setTransportSocketCallbacks(Network::TransportSocketCallbacks& callbacks) override; @@ -37,13 +38,15 @@ class CaptureSocket : public Network::TransportSocket { envoy::data::tap::v2alpha::Trace trace_; Network::TransportSocketPtr transport_socket_; Network::TransportSocketCallbacks* callbacks_{}; + Event::TimeSystem& time_system_; }; class CaptureSocketFactory : public Network::TransportSocketFactory { public: CaptureSocketFactory(const std::string& path_prefix, envoy::config::transport_socket::capture::v2alpha::FileSink::Format format, - Network::TransportSocketFactoryPtr&& transport_socket_factory); + Network::TransportSocketFactoryPtr&& transport_socket_factory, + Event::TimeSystem& time_system); // Network::TransportSocketFactory Network::TransportSocketPtr createTransportSocket() const override; @@ -53,6 +56,7 @@ class CaptureSocketFactory : public Network::TransportSocketFactory { const std::string path_prefix_; const envoy::config::transport_socket::capture::v2alpha::FileSink::Format format_; Network::TransportSocketFactoryPtr transport_socket_factory_; + Event::TimeSystem& time_system_; }; } // namespace Capture diff --git a/source/extensions/transport_sockets/capture/config.cc b/source/extensions/transport_sockets/capture/config.cc index d98fbe669d9f..1e251ba6f069 100644 --- a/source/extensions/transport_sockets/capture/config.cc +++ b/source/extensions/transport_sockets/capture/config.cc @@ -26,9 +26,9 @@ Network::TransportSocketFactoryPtr UpstreamCaptureSocketConfigFactory::createTra outer_config.transport_socket(), inner_config_factory); auto inner_transport_factory = inner_config_factory.createTransportSocketFactory(*inner_factory_config, context); - return std::make_unique(outer_config.file_sink().path_prefix(), - outer_config.file_sink().format(), - std::move(inner_transport_factory)); + return std::make_unique( + outer_config.file_sink().path_prefix(), outer_config.file_sink().format(), + std::move(inner_transport_factory), context.dispatcher().timeSystem()); } Network::TransportSocketFactoryPtr @@ -44,9 +44,9 @@ DownstreamCaptureSocketConfigFactory::createTransportSocketFactory( outer_config.transport_socket(), inner_config_factory); auto inner_transport_factory = inner_config_factory.createTransportSocketFactory( *inner_factory_config, context, server_names); - return std::make_unique(outer_config.file_sink().path_prefix(), - outer_config.file_sink().format(), - std::move(inner_transport_factory)); + return std::make_unique( + outer_config.file_sink().path_prefix(), outer_config.file_sink().format(), + std::move(inner_transport_factory), context.dispatcher().timeSystem()); } ProtobufTypes::MessagePtr CaptureSocketConfigFactory::createEmptyConfigProto() { diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index 4f5c656ccf2c..02aa8d1124d5 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -219,16 +219,18 @@ void ConnectionHandlerImpl::ActiveListener::onNewConnection( // If the connection is already closed, we can just let this connection immediately die. if (new_connection->state() != Network::Connection::State::Closed) { - ActiveConnectionPtr active_connection(new ActiveConnection(*this, std::move(new_connection))); + ActiveConnectionPtr active_connection( + new ActiveConnection(*this, std::move(new_connection), parent_.dispatcher_.timeSystem())); active_connection->moveIntoList(std::move(active_connection), connections_); parent_.num_connections_++; } } ConnectionHandlerImpl::ActiveConnection::ActiveConnection(ActiveListener& listener, - Network::ConnectionPtr&& new_connection) + Network::ConnectionPtr&& new_connection, + Event::TimeSystem& time_system) : listener_(listener), connection_(std::move(new_connection)), - conn_length_(new Stats::Timespan(listener_.stats_.downstream_cx_length_ms_)) { + conn_length_(new Stats::Timespan(listener_.stats_.downstream_cx_length_ms_, time_system)) { // We just universally set no delay on connections. Theoretically we might at some point want // to make this configurable. connection_->noDelay(true); diff --git a/source/server/connection_handler_impl.h b/source/server/connection_handler_impl.h index c841ee0bc555..cf55d180c26b 100644 --- a/source/server/connection_handler_impl.h +++ b/source/server/connection_handler_impl.h @@ -110,7 +110,8 @@ class ConnectionHandlerImpl : public Network::ConnectionHandler, NonCopyable { struct ActiveConnection : LinkedObject, public Event::DeferredDeletable, public Network::ConnectionCallbacks { - ActiveConnection(ActiveListener& listener, Network::ConnectionPtr&& new_connection); + ActiveConnection(ActiveListener& listener, Network::ConnectionPtr&& new_connection, + Event::TimeSystem& time_system); ~ActiveConnection(); // Network::ConnectionCallbacks diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index cc3596756f9d..35966ce26041 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -976,7 +976,7 @@ bool AdminImpl::createNetworkFilterChain(Network::Connection& connection, // the envoy is overloaded. connection.addReadFilter(Network::ReadFilterSharedPtr{new Http::ConnectionManagerImpl( *this, server_.drainManager(), server_.random(), server_.httpTracer(), server_.runtime(), - server_.localInfo(), server_.clusterManager(), nullptr)}); + server_.localInfo(), server_.clusterManager(), nullptr, server_.timeSystem())}); return true; } diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index 979a620c60ca..f52415378e0d 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -23,6 +23,7 @@ #include "test/fuzz/fuzz_runner.h" #include "test/fuzz/utility.h" #include "test/mocks/access_log/mocks.h" +#include "test/mocks/common.h" #include "test/mocks/http/mocks.h" #include "test/mocks/local_info/mocks.h" #include "test/mocks/network/mocks.h" @@ -377,6 +378,7 @@ DEFINE_PROTO_FUZZER(const test::common::http::ConnManagerImplTestCase& input) { NiceMock local_info; NiceMock cluster_manager; NiceMock filter_callbacks; + NiceMock time_system; std::unique_ptr ssl_connection; bool connection_alive = true; @@ -390,7 +392,7 @@ DEFINE_PROTO_FUZZER(const test::common::http::ConnManagerImplTestCase& input) { std::make_shared("0.0.0.0"); ConnectionManagerImpl conn_manager(config, drain_close, random, tracer, runtime, local_info, - cluster_manager, nullptr); + cluster_manager, nullptr, time_system); conn_manager.initializeReadFilterCallbacks(filter_callbacks); std::vector streams; diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index b5acd245e00f..8362b59dff87 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -111,8 +111,8 @@ class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfi filter_callbacks_.connection_.remote_address_ = std::make_shared("0.0.0.0"); conn_manager_.reset(new ConnectionManagerImpl(*this, drain_close_, random_, tracer_, runtime_, - local_info_, cluster_manager_, - &overload_manager_)); + local_info_, cluster_manager_, &overload_manager_, + test_time_.timeSystem())); conn_manager_->initializeReadFilterCallbacks(filter_callbacks_); if (tracing) { @@ -217,7 +217,7 @@ class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfi envoy::config::filter::network::tcp_proxy::v2::TcpProxy(), factory_context_)); auto ret = std::make_unique( request_headers, request_info, route_entry, callbacks, cluster_manager, - read_callbacks, config); + read_callbacks, config, test_time_.timeSystem()); return ret; })); } diff --git a/test/common/http/user_agent_test.cc b/test/common/http/user_agent_test.cc index db581d3fb9e3..72c04be0c366 100644 --- a/test/common/http/user_agent_test.cc +++ b/test/common/http/user_agent_test.cc @@ -1,6 +1,7 @@ #include "common/http/header_map_impl.h" #include "common/http/user_agent.h" +#include "test/mocks/common.h" #include "test/mocks/stats/mocks.h" #include "test/test_common/printers.h" #include "test/test_common/utility.h" @@ -17,7 +18,8 @@ namespace Http { TEST(UserAgentTest, All) { Stats::MockStore stat_store; NiceMock original_histogram; - Stats::Timespan span(original_histogram); + NiceMock time_source; + Stats::Timespan span(original_histogram, time_source); EXPECT_CALL(stat_store.counter_, inc()).Times(5); EXPECT_CALL(stat_store, counter("test.user_agent.ios.downstream_cx_total")); diff --git a/test/common/network/filter_manager_impl_test.cc b/test/common/network/filter_manager_impl_test.cc index 8aeaf11a978b..b3a26858caa7 100644 --- a/test/common/network/filter_manager_impl_test.cc +++ b/test/common/network/filter_manager_impl_test.cc @@ -190,7 +190,8 @@ TEST_F(NetworkFilterManagerTest, RateLimitAndTcpProxy) { tcp_proxy.set_cluster("fake_cluster"); TcpProxy::ConfigSharedPtr tcp_proxy_config(new TcpProxy::Config(tcp_proxy, factory_context)); manager.addReadFilter( - std::make_shared(tcp_proxy_config, factory_context.cluster_manager_)); + std::make_shared(tcp_proxy_config, factory_context.cluster_manager_, + factory_context.dispatcher().timeSystem())); RateLimit::RequestCallbacks* request_callbacks{}; EXPECT_CALL(*rl_client, limit(_, "foo", diff --git a/test/common/request_info/request_info_impl_test.cc b/test/common/request_info/request_info_impl_test.cc index 2331e90057f6..28476cb49d82 100644 --- a/test/common/request_info/request_info_impl_test.cc +++ b/test/common/request_info/request_info_impl_test.cc @@ -27,10 +27,15 @@ std::chrono::nanoseconds checkDuration(std::chrono::nanoseconds last, return timing.value(); } -TEST(RequestInfoImplTest, TimingTest) { - MonotonicTime pre_start = std::chrono::steady_clock::now(); - RequestInfoImpl info(Http::Protocol::Http2); - MonotonicTime post_start = std::chrono::steady_clock::now(); +class RequestInfoImplTest : public testing::Test { +protected: + DangerousDeprecatedTestTime test_time_; +}; + +TEST_F(RequestInfoImplTest, TimingTest) { + MonotonicTime pre_start = test_time_.timeSystem().monotonicTime(); + RequestInfoImpl info(Http::Protocol::Http2, test_time_.timeSystem()); + MonotonicTime post_start = test_time_.timeSystem().monotonicTime(); const MonotonicTime& start = info.startTimeMonotonic(); @@ -71,8 +76,9 @@ TEST(RequestInfoImplTest, TimingTest) { dur = checkDuration(dur, info.requestComplete()); } -TEST(RequestInfoImplTest, BytesTest) { - RequestInfoImpl request_info(Http::Protocol::Http2); +TEST_F(RequestInfoImplTest, BytesTest) { + RequestInfoImpl request_info(Http::Protocol::Http2, test_time_.timeSystem()); + const uint64_t bytes_sent = 7; const uint64_t bytes_received = 12; @@ -83,7 +89,7 @@ TEST(RequestInfoImplTest, BytesTest) { EXPECT_EQ(bytes_received, request_info.bytesReceived()); } -TEST(RequestInfoImplTest, ResponseFlagTest) { +TEST_F(RequestInfoImplTest, ResponseFlagTest) { const std::vector responseFlags = {FailedLocalHealthCheck, NoHealthyUpstream, UpstreamRequestTimeout, @@ -97,7 +103,8 @@ TEST(RequestInfoImplTest, ResponseFlagTest) { FaultInjected, RateLimited}; - RequestInfoImpl request_info(Http::Protocol::Http2); + RequestInfoImpl request_info(Http::Protocol::Http2, test_time_.timeSystem()); + EXPECT_FALSE(request_info.hasAnyResponseFlag()); EXPECT_FALSE(request_info.intersectResponseFlags(0)); for (ResponseFlag flag : responseFlags) { @@ -110,15 +117,16 @@ TEST(RequestInfoImplTest, ResponseFlagTest) { } EXPECT_TRUE(request_info.hasAnyResponseFlag()); - RequestInfoImpl request_info2(Http::Protocol::Http2); + RequestInfoImpl request_info2(Http::Protocol::Http2, test_time_.timeSystem()); request_info2.setResponseFlag(FailedLocalHealthCheck); EXPECT_TRUE(request_info2.intersectResponseFlags(FailedLocalHealthCheck)); } -TEST(RequestInfoImplTest, MiscSettersAndGetters) { +TEST_F(RequestInfoImplTest, MiscSettersAndGetters) { { - RequestInfoImpl request_info(Http::Protocol::Http2); + RequestInfoImpl request_info(Http::Protocol::Http2, test_time_.timeSystem()); + EXPECT_EQ(Http::Protocol::Http2, request_info.protocol().value()); request_info.protocol(Http::Protocol::Http10); @@ -153,8 +161,9 @@ TEST(RequestInfoImplTest, MiscSettersAndGetters) { } } -TEST(RequestInfoImplTest, DynamicMetadataTest) { - RequestInfoImpl request_info(Http::Protocol::Http2); +TEST_F(RequestInfoImplTest, DynamicMetadataTest) { + RequestInfoImpl request_info(Http::Protocol::Http2, test_time_.timeSystem()); + EXPECT_EQ(0, request_info.dynamicMetadata().filter_metadata_size()); request_info.setDynamicMetadata("com.test", MessageUtil::keyValueStruct("test_key", "test_value")); diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index 1df01629ffa8..6b35afc0376f 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -429,7 +429,7 @@ class TcpProxyTest : public testing::Test { { testing::InSequence sequence; - filter_.reset(new Filter(config_, factory_context_.cluster_manager_)); + filter_.reset(new Filter(config_, factory_context_.cluster_manager_, timeSystem())); EXPECT_CALL(filter_callbacks_.connection_, enableHalfClose(true)); EXPECT_CALL(filter_callbacks_.connection_, readDisable(true)); filter_->initializeReadFilterCallbacks(filter_callbacks_); @@ -464,6 +464,8 @@ class TcpProxyTest : public testing::Test { conn_pool_callbacks_.at(conn_index)->onPoolFailure(reason, upstream_hosts_.at(conn_index)); } + Event::TimeSystem& timeSystem() { return factory_context_.dispatcher().timeSystem(); } + ConfigSharedPtr config_; NiceMock filter_callbacks_; NiceMock factory_context_; @@ -721,7 +723,7 @@ TEST_F(TcpProxyTest, WithMetadataMatch) { {Envoy::Config::MetadataFilters::get().ENVOY_LB, metadata_struct}); configure(config); - filter_.reset(new Filter(config_, factory_context_.cluster_manager_)); + filter_.reset(new Filter(config_, factory_context_.cluster_manager_, timeSystem())); const auto& metadata_criteria = filter_->metadataMatchCriteria()->metadataMatchCriteria(); @@ -734,7 +736,7 @@ TEST_F(TcpProxyTest, WithMetadataMatch) { TEST_F(TcpProxyTest, DisconnectBeforeData) { configure(defaultConfig()); - filter_.reset(new Filter(config_, factory_context_.cluster_manager_)); + filter_.reset(new Filter(config_, factory_context_.cluster_manager_, timeSystem())); filter_->initializeReadFilterCallbacks(filter_callbacks_); filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose); @@ -773,7 +775,7 @@ TEST_F(TcpProxyTest, UpstreamConnectionLimit) { new Upstream::ResourceManagerImpl(factory_context_.runtime_loader_, "fake_key", 0, 0, 0, 0)); // setup sets up expectation for tcpConnForCluster but this test is expected to NOT call that - filter_.reset(new Filter(config_, factory_context_.cluster_manager_)); + filter_.reset(new Filter(config_, factory_context_.cluster_manager_, timeSystem())); // The downstream connection closes if the proxy can't make an upstream connection. EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::NoFlush)); filter_->initializeReadFilterCallbacks(filter_callbacks_); @@ -1074,10 +1076,12 @@ class TcpProxyRoutingTest : public testing::Test { void setup() { EXPECT_CALL(filter_callbacks_, connection()).WillRepeatedly(ReturnRef(connection_)); - filter_.reset(new Filter(config_, factory_context_.cluster_manager_)); + filter_.reset(new Filter(config_, factory_context_.cluster_manager_, timeSystem())); filter_->initializeReadFilterCallbacks(filter_callbacks_); } + Event::TimeSystem& timeSystem() { return factory_context_.dispatcher().timeSystem(); } + ConfigSharedPtr config_; NiceMock connection_; NiceMock filter_callbacks_; diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index dc9cce203b9e..2cdabb23b427 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -1836,6 +1836,8 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) { // Tests that mergeable updates outside of a window get applied immediately. TEST_F(ClusterManagerImplTest, MergedUpdatesOutOfWindow) { + EXPECT_CALL(time_system_, monotonicTime()) + .WillRepeatedly(Return(MonotonicTime(std::chrono::seconds(0)))); createWithLocalClusterUpdate(); // Ensure we see the right set of added/removed hosts on every call. @@ -1856,7 +1858,10 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesOutOfWindow) { HostVector hosts_removed; // The first update should be applied immediately, because even though it's mergeable - // it's outside a merge window. + // it's outside the default merge window of 3 seconds (found in debugger as value of + // cluster.info()->lbConfig().update_merge_window() in ClusterManagerImpl::scheduleUpdate. + EXPECT_CALL(time_system_, monotonicTime()) + .WillRepeatedly(Return(MonotonicTime(std::chrono::seconds(60)))); cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed, absl::nullopt); @@ -1866,6 +1871,33 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesOutOfWindow) { EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_merge_cancelled").value()); } +// Tests that mergeable updates inside of a window are not applied immediately. +TEST_F(ClusterManagerImplTest, MergedUpdatesInsideWindow) { + EXPECT_CALL(time_system_, monotonicTime()) + .WillRepeatedly(Return(MonotonicTime(std::chrono::seconds(0)))); + createWithLocalClusterUpdate(); + + const Cluster& cluster = cluster_manager_->clusters().begin()->second; + HostVectorSharedPtr hosts( + new HostVector(cluster.prioritySet().hostSetsPerPriority()[0]->hosts())); + HostsPerLocalitySharedPtr hosts_per_locality = std::make_shared(); + HostVector hosts_added; + HostVector hosts_removed; + + // The first update will not be applied, as we make it inside the default mergeable window of + // 3 seconds (found in debugger as value of cluster.info()->lbConfig().update_merge_window() + // in ClusterManagerImpl::scheduleUpdate. + EXPECT_CALL(time_system_, monotonicTime()) + .WillRepeatedly(Return(MonotonicTime(std::chrono::seconds(2)))); + cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); + EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated").value()); + EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value()); + EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_out_of_merge_window").value()); + EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_merge_cancelled").value()); +} + // Tests that mergeable updates outside of a window get applied immediately when // merging is disabled, and that the counters are correct. TEST_F(ClusterManagerImplTest, MergedUpdatesOutOfWindowDisabled) { diff --git a/test/extensions/filters/http/dynamo/dynamo_filter_test.cc b/test/extensions/filters/http/dynamo/dynamo_filter_test.cc index f41e5a8eb9da..4d199006e1ed 100644 --- a/test/extensions/filters/http/dynamo/dynamo_filter_test.cc +++ b/test/extensions/filters/http/dynamo/dynamo_filter_test.cc @@ -33,7 +33,8 @@ class DynamoFilterTest : public testing::Test { .WillByDefault(Return(enabled)); EXPECT_CALL(loader_.snapshot_, featureEnabled("dynamodb.filter_enabled", 100)); - filter_.reset(new DynamoFilter(loader_, stat_prefix_, stats_)); + filter_.reset(new DynamoFilter(loader_, stat_prefix_, stats_, + decoder_callbacks_.dispatcher().timeSystem())); filter_->setDecoderFilterCallbacks(decoder_callbacks_); filter_->setEncoderFilterCallbacks(encoder_callbacks_); diff --git a/test/extensions/filters/http/jwt_authn/BUILD b/test/extensions/filters/http/jwt_authn/BUILD index 0e90d2c0ecc2..c25aca49c368 100644 --- a/test/extensions/filters/http/jwt_authn/BUILD +++ b/test/extensions/filters/http/jwt_authn/BUILD @@ -71,6 +71,7 @@ envoy_extension_cc_test( "//source/extensions/filters/http/common:jwks_fetcher_lib", "//source/extensions/filters/http/jwt_authn:jwks_cache_lib", "//test/extensions/filters/http/jwt_authn:test_common_lib", + "//test/test_common:test_time_lib", "//test/test_common:utility_lib", ], ) diff --git a/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc b/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc index 4610e5d86e8b..0c88776b6e8e 100644 --- a/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc +++ b/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc @@ -6,6 +6,7 @@ #include "extensions/filters/http/jwt_authn/jwks_cache.h" #include "test/extensions/filters/http/jwt_authn/test_common.h" +#include "test/test_common/test_time.h" #include "test/test_common/utility.h" using ::envoy::config::filter::http::jwt_authn::v2alpha::JwtAuthentication; @@ -21,10 +22,11 @@ class JwksCacheTest : public ::testing::Test { public: void SetUp() { MessageUtil::loadFromYaml(ExampleConfig, config_); - cache_ = JwksCache::create(config_); + cache_ = JwksCache::create(config_, test_time_.timeSystem()); jwks_ = google::jwt_verify::Jwks::createFrom(PublicKey, google::jwt_verify::Jwks::JWKS); } + DangerousDeprecatedTestTime test_time_; JwtAuthentication config_; JwksCachePtr cache_; google::jwt_verify::JwksPtr jwks_; @@ -41,7 +43,7 @@ TEST_F(JwksCacheTest, TestSetRemoteJwks) { auto& provider0 = (*config_.mutable_providers())[std::string(ProviderName)]; // Set cache_duration to 1 second to test expiration provider0.mutable_remote_jwks()->mutable_cache_duration()->set_seconds(1); - cache_ = JwksCache::create(config_); + cache_ = JwksCache::create(config_, test_time_.timeSystem()); auto jwks = cache_->findByIssuer("https://example.com"); EXPECT_TRUE(jwks->getJwksObj() == nullptr); @@ -60,7 +62,7 @@ TEST_F(JwksCacheTest, TestSetRemoteJwksWithDefaultCacheDuration) { auto& provider0 = (*config_.mutable_providers())[std::string(ProviderName)]; // Clear cache_duration to use default one. provider0.mutable_remote_jwks()->clear_cache_duration(); - cache_ = JwksCache::create(config_); + cache_ = JwksCache::create(config_, test_time_.timeSystem()); auto jwks = cache_->findByIssuer("https://example.com"); EXPECT_TRUE(jwks->getJwksObj() == nullptr); @@ -77,7 +79,7 @@ TEST_F(JwksCacheTest, TestGoodInlineJwks) { auto local_jwks = provider0.mutable_local_jwks(); local_jwks->set_inline_string(PublicKey); - cache_ = JwksCache::create(config_); + cache_ = JwksCache::create(config_, test_time_.timeSystem()); auto jwks = cache_->findByIssuer("https://example.com"); EXPECT_FALSE(jwks->getJwksObj() == nullptr); @@ -91,7 +93,7 @@ TEST_F(JwksCacheTest, TestBadInlineJwks) { auto local_jwks = provider0.mutable_local_jwks(); local_jwks->set_inline_string("BAD-JWKS"); - cache_ = JwksCache::create(config_); + cache_ = JwksCache::create(config_, test_time_.timeSystem()); auto jwks = cache_->findByIssuer("https://example.com"); EXPECT_TRUE(jwks->getJwksObj() == nullptr); diff --git a/test/extensions/filters/http/lua/lua_filter_test.cc b/test/extensions/filters/http/lua/lua_filter_test.cc index 5894fef3f66f..23fe8b3088f2 100644 --- a/test/extensions/filters/http/lua/lua_filter_test.cc +++ b/test/extensions/filters/http/lua/lua_filter_test.cc @@ -1530,7 +1530,8 @@ TEST_F(LuaHttpFilterTest, SetGetDynamicMetadata) { setup(SCRIPT); Http::TestHeaderMapImpl request_headers{{":path", "/"}}; - RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2); + DangerousDeprecatedTestTime test_time; + RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2, test_time.timeSystem()); EXPECT_EQ(0, request_info.dynamicMetadata().filter_metadata_size()); EXPECT_CALL(decoder_callbacks_, requestInfo()).WillOnce(ReturnRef(request_info)); EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq("bar"))); diff --git a/test/extensions/filters/http/lua/wrappers_test.cc b/test/extensions/filters/http/lua/wrappers_test.cc index 15e78db73561..b602006c755c 100644 --- a/test/extensions/filters/http/lua/wrappers_test.cc +++ b/test/extensions/filters/http/lua/wrappers_test.cc @@ -251,6 +251,8 @@ class LuaRequestInfoWrapperTest MessageUtil::loadFromYaml(yaml_string, metadata); return metadata; } + + DangerousDeprecatedTestTime test_time_; }; // Return the current request protocol. @@ -287,7 +289,7 @@ TEST_F(LuaRequestInfoWrapperTest, SetGetAndIterateDynamicMetadata) { InSequence s; setup(SCRIPT); - RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2); + RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2, test_time_.timeSystem()); EXPECT_EQ(0, request_info.dynamicMetadata().filter_metadata_size()); Filters::Common::Lua::LuaDeathRef wrapper( RequestInfoWrapper::create(coroutine_->luaState(), request_info), true); @@ -323,7 +325,7 @@ TEST_F(LuaRequestInfoWrapperTest, ModifyDuringIterationForDynamicMetadata) { InSequence s; setup(SCRIPT); - RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2); + RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2, test_time_.timeSystem()); Filters::Common::Lua::LuaDeathRef wrapper( RequestInfoWrapper::create(coroutine_->luaState(), request_info), true); EXPECT_THROW_WITH_MESSAGE( @@ -357,7 +359,7 @@ TEST_F(LuaRequestInfoWrapperTest, ModifyAfterIterationForDynamicMetadata) { InSequence s; setup(SCRIPT); - RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2); + RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2, test_time_.timeSystem()); EXPECT_EQ(0, request_info.dynamicMetadata().filter_metadata_size()); Filters::Common::Lua::LuaDeathRef wrapper( RequestInfoWrapper::create(coroutine_->luaState(), request_info), true); @@ -384,7 +386,7 @@ TEST_F(LuaRequestInfoWrapperTest, DontFinishIterationForDynamicMetadata) { InSequence s; setup(SCRIPT); - RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2); + RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2, test_time_.timeSystem()); Filters::Common::Lua::LuaDeathRef wrapper( RequestInfoWrapper::create(coroutine_->luaState(), request_info), true); EXPECT_THROW_WITH_MESSAGE( diff --git a/test/extensions/filters/network/mongo_proxy/proxy_test.cc b/test/extensions/filters/network/mongo_proxy/proxy_test.cc index 86bd5552c049..2cb87fc9a1e1 100644 --- a/test/extensions/filters/network/mongo_proxy/proxy_test.cc +++ b/test/extensions/filters/network/mongo_proxy/proxy_test.cc @@ -70,12 +70,12 @@ class MongoProxyFilterTest : public testing::Test { .WillByDefault(Return(true)); EXPECT_CALL(log_manager_, createAccessLog(_)).WillOnce(Return(file_)); - access_log_.reset(new AccessLog("test", log_manager_)); + access_log_.reset(new AccessLog("test", log_manager_, dispatcher_.timeSystem())); } void initializeFilter() { filter_.reset(new TestProxyFilter("test.", store_, runtime_, access_log_, fault_config_, - drain_decision_, generator_)); + drain_decision_, generator_, dispatcher_.timeSystem())); filter_->initializeReadFilterCallbacks(read_filter_callbacks_); filter_->onNewConnection(); diff --git a/test/extensions/filters/network/thrift_proxy/conn_manager_test.cc b/test/extensions/filters/network/thrift_proxy/conn_manager_test.cc index 8b96c4a83f09..86d7f394d1d6 100644 --- a/test/extensions/filters/network/thrift_proxy/conn_manager_test.cc +++ b/test/extensions/filters/network/thrift_proxy/conn_manager_test.cc @@ -100,7 +100,8 @@ class ThriftConnectionManagerTest : public testing::Test { } ON_CALL(random_, random()).WillByDefault(Return(42)); - filter_.reset(new ConnectionManager(*config_, random_)); + filter_.reset(new ConnectionManager(*config_, random_, + filter_callbacks_.connection_.dispatcher_.timeSystem())); filter_->initializeReadFilterCallbacks(filter_callbacks_); filter_->onNewConnection(); @@ -291,9 +292,9 @@ class ThriftConnectionManagerTest : public testing::Test { Buffer::OwnedImpl buffer_; Buffer::OwnedImpl write_buffer_; - std::unique_ptr filter_; NiceMock filter_callbacks_; NiceMock random_; + std::unique_ptr filter_; MockTransport* custom_transport_{}; MockProtocol* custom_protocol_{}; }; diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 3ad9bc8ec211..84503eb90e81 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -154,7 +154,8 @@ bool RouterCheckTool::compareVirtualHost(ToolConfig& tool_config, const std::str bool RouterCheckTool::compareRewritePath(ToolConfig& tool_config, const std::string& expected) { std::string actual = ""; - Envoy::RequestInfo::RequestInfoImpl request_info(Envoy::Http::Protocol::Http11); + Envoy::RequestInfo::RequestInfoImpl request_info(Envoy::Http::Protocol::Http11, + factory_context_->dispatcher().timeSystem()); if (tool_config.route_->routeEntry() != nullptr) { tool_config.route_->routeEntry()->finalizeRequestHeaders(*tool_config.headers_, request_info, true); @@ -165,7 +166,8 @@ bool RouterCheckTool::compareRewritePath(ToolConfig& tool_config, const std::str bool RouterCheckTool::compareRewriteHost(ToolConfig& tool_config, const std::string& expected) { std::string actual = ""; - Envoy::RequestInfo::RequestInfoImpl request_info(Envoy::Http::Protocol::Http11); + Envoy::RequestInfo::RequestInfoImpl request_info(Envoy::Http::Protocol::Http11, + factory_context_->dispatcher().timeSystem()); if (tool_config.route_->routeEntry() != nullptr) { tool_config.route_->routeEntry()->finalizeRequestHeaders(*tool_config.headers_, request_info, true); @@ -192,7 +194,8 @@ bool RouterCheckTool::compareHeaderField(ToolConfig& tool_config, const std::str bool RouterCheckTool::compareCustomHeaderField(ToolConfig& tool_config, const std::string& field, const std::string& expected) { std::string actual = ""; - Envoy::RequestInfo::RequestInfoImpl request_info(Envoy::Http::Protocol::Http11); + Envoy::RequestInfo::RequestInfoImpl request_info(Envoy::Http::Protocol::Http11, + factory_context_->dispatcher().timeSystem()); request_info.setDownstreamRemoteAddress(Network::Utility::getCanonicalIpv4LoopbackAddress()); if (tool_config.route_->routeEntry() != nullptr) { tool_config.route_->routeEntry()->finalizeRequestHeaders(*tool_config.headers_, request_info, diff --git a/tools/check_format.py b/tools/check_format.py index 88e1fefe1b4a..70994f72a306 100755 --- a/tools/check_format.py +++ b/tools/check_format.py @@ -167,7 +167,8 @@ def checkSourceLine(line, file_path, reportError): # legitimately show up in comments, for example this one. reportError("Don't use , use absl::Mutex for reader/writer locks.") if not whitelistedForRealTime(file_path): - if 'RealTimeSource' in line or 'RealTimeSystem' in line: + if 'RealTimeSource' in line or 'RealTimeSystem' in line or \ + 'std::chrono::system_clock::now' in line or 'std::chrono::steady_clock::now' in line: reportError("Don't reference real-world time sources from production code; use injection") if 'std::atomic_' in line: # The std::atomic_* free functions are functionally equivalent to calling diff --git a/tools/check_format_test_helper.py b/tools/check_format_test_helper.py index 300a2f39dfca..c9e7068dc09d 100755 --- a/tools/check_format_test_helper.py +++ b/tools/check_format_test_helper.py @@ -87,7 +87,7 @@ def emitStdoutAsError(stdout): def expectError(status, stdout, expected_substring): if status == 0: - logging.error("Expected failure, but succeeded") + logging.error("Expected failure `%s`, but succeeded" % expected_substring) return 1 for line in stdout: if expected_substring in line: @@ -152,6 +152,8 @@ def checkFileExpectingOK(filename): "Don't reference real-world time sources from production code; use injection") errors += fixFileExpectingFailure("real_time_source.cc", real_time_inject_error) errors += fixFileExpectingFailure("real_time_system.cc", real_time_inject_error) + errors += fixFileExpectingFailure("system_clock.cc", real_time_inject_error) + errors += fixFileExpectingFailure("steady_clock.cc", real_time_inject_error) errors += fixFileExpectingNoChange("ok_file.cc") @@ -174,6 +176,8 @@ def checkFileExpectingOK(filename): errors += checkFileExpectingError("proto_format.proto", "clang-format check failed") errors += checkFileExpectingError("real_time_source.cc", real_time_inject_error) errors += checkFileExpectingError("real_time_system.cc", real_time_inject_error) + errors += checkFileExpectingError("system_clock.cc", real_time_inject_error) + errors += checkFileExpectingError("steady_clock.cc", real_time_inject_error) errors += checkFileExpectingError("std_atomic_free_functions.cc", "std::atomic_*") errors += fixFileExpectingFailure("std_atomic_free_functions.cc", "std::atomic_*") diff --git a/tools/testdata/check_format/steady_clock.cc b/tools/testdata/check_format/steady_clock.cc new file mode 100644 index 000000000000..b636a48ffb43 --- /dev/null +++ b/tools/testdata/check_format/steady_clock.cc @@ -0,0 +1,5 @@ +namespace Envoy { + +int foo() { return std::chrono::steady_clock::now(); } + +} // namespace Envoy diff --git a/tools/testdata/check_format/system_clock.cc b/tools/testdata/check_format/system_clock.cc new file mode 100644 index 000000000000..f382a0a6aff2 --- /dev/null +++ b/tools/testdata/check_format/system_clock.cc @@ -0,0 +1,5 @@ +namespace Envoy { + +int foo() { return std::chrono::system_clock::now(); } + +} // namespace Envoy