From 757ee6515a551ad0080f9c11ff5082173e08cb5d Mon Sep 17 00:00:00 2001 From: Dan Rosen Date: Fri, 19 Apr 2019 16:11:21 -0400 Subject: [PATCH] event: reintroduce dispatcher stats Signed-off-by: Dan Rosen --- api/envoy/config/bootstrap/v2/bootstrap.proto | 8 +++ bazel/repository_locations.bzl | 10 +++- docs/root/intro/version_history.rst | 1 + docs/root/operations/operations.rst | 1 + docs/root/operations/performance.rst | 51 ++++++++++++++++ include/envoy/event/dispatcher.h | 27 +++++++++ include/envoy/server/worker.h | 8 +++ source/common/event/dispatcher_impl.cc | 10 ++++ source/common/event/dispatcher_impl.h | 5 ++ source/common/event/libevent_scheduler.cc | 58 +++++++++++++++++++ source/common/event/libevent_scheduler.h | 15 +++++ source/server/config_validation/server.cc | 2 +- source/server/listener_manager_impl.cc | 17 ++++-- source/server/listener_manager_impl.h | 4 +- source/server/server.cc | 9 ++- source/server/worker_impl.cc | 4 ++ source/server/worker_impl.h | 1 + test/common/event/BUILD | 1 + test/common/event/dispatcher_impl_test.cc | 14 +++++ test/config_test/config_test.cc | 3 +- test/mocks/event/mocks.cc | 1 + test/mocks/event/mocks.h | 1 + test/mocks/server/mocks.h | 1 + test/mocks/thread_local/mocks.cc | 1 + test/server/listener_manager_impl_test.cc | 3 +- test/server/worker_impl_test.cc | 2 + tools/spelling_dictionary.txt | 1 + 27 files changed, 244 insertions(+), 15 deletions(-) create mode 100644 docs/root/operations/performance.rst diff --git a/api/envoy/config/bootstrap/v2/bootstrap.proto b/api/envoy/config/bootstrap/v2/bootstrap.proto index 7053419b7f13f..46e526224cdc2 100644 --- a/api/envoy/config/bootstrap/v2/bootstrap.proto +++ b/api/envoy/config/bootstrap/v2/bootstrap.proto @@ -118,6 +118,14 @@ message Bootstrap { // Optional overload manager configuration. envoy.config.overload.v2alpha.OverloadManager overload_manager = 15; + + // Enable :ref:`stats for event dispatcher `, defaults to false. + // Note that this records a value for each iteration of the event loop on every thread. This + // should normally be minimal overhead, but when using + // :ref:`statsd `, it will send each observed value + // over the wire individually because the statsd protocol doesn't have any way to represent a + // histogram summary. Be aware that this can be a very large volume of data. + bool enable_dispatcher_stats = 16; } // Administration interface :ref:`operations documentation diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index afac13ec52d8a..e1fdafa920036 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -129,9 +129,13 @@ REPOSITORY_LOCATIONS = dict( urls = ["https://github.com/google/benchmark/archive/505be96ab23056580a3a2315abba048f4428b04e.tar.gz"], ), com_github_libevent_libevent = dict( - sha256 = "53d4bb49b837944893b7caf9ae8eb43e94690ee5babea6469cc4a928722f99b1", - strip_prefix = "libevent-c4fbae3ae6166dddfa126734edd63213afa14dce", - urls = ["https://github.com/libevent/libevent/archive/c4fbae3ae6166dddfa126734edd63213afa14dce.tar.gz"], + sha256 = "ab3af422b7e4c6d9276b3637d87edb6cf628fd91c9206260b759778c3a28b330", + # This SHA includes the new "prepare" and "check" watchers, used for event loop performance + # stats (see https://github.com/libevent/libevent/pull/793) and the fix for a race condition + # in the watchers (see https://github.com/libevent/libevent/pull/802). + # TODO(mergeconflict): Update to v2.2 when it is released. + strip_prefix = "libevent-1cd8830de27c30c5324c75bfb6012c969c09ca2c", + urls = ["https://github.com/libevent/libevent/archive/1cd8830de27c30c5324c75bfb6012c969c09ca2c.tar.gz"], ), com_github_madler_zlib = dict( sha256 = "629380c90a77b964d896ed37163f5c3a34f6e6d897311f1df2a7016355c45eff", diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index cc7a73b1bd9be..44dc0c581a7fa 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -5,6 +5,7 @@ Version history ================ * access log: added a new field for response code details in :ref:`file access logger` and :ref:`gRPC access logger`. * dubbo_proxy: support the :ref:`Dubbo proxy filter `. +* event: added :ref:`loop duration and poll delay statistics `. * ext_authz: added option to `ext_authz` that allows the filter clearing route cache. * eds: added support to specify max time for which endpoints can be used :ref:`gRPC filter `. * http: mitigated a race condition with the :ref:`delayed_close_timeout` where it could trigger while actively flushing a pending write buffer for a downstream connection. diff --git a/docs/root/operations/operations.rst b/docs/root/operations/operations.rst index 98005b9977ba6..3f1ada49c1ae5 100644 --- a/docs/root/operations/operations.rst +++ b/docs/root/operations/operations.rst @@ -13,3 +13,4 @@ Operations and administration runtime fs_flags traffic_tapping + performance diff --git a/docs/root/operations/performance.rst b/docs/root/operations/performance.rst new file mode 100644 index 0000000000000..d7066374f3ed4 --- /dev/null +++ b/docs/root/operations/performance.rst @@ -0,0 +1,51 @@ +.. _operations_performance: + +Performance +=========== + +Envoy is architected to optimize scalability and resource utilization by running an event loop on a +:ref:`small number of threads `. The "main" thread is responsible for +control plane processing, and each "worker" thread handles a portion of the data plane processing. +Envoy exposes two statistics to monitor performance of the event loops on all these threads. + +* **Loop duration:** Some amount of processing is done on each iteration of the event loop. This + amount will naturally vary with changes in load. However, if one or more threads have an unusually + long-tailed loop duration, it may indicate a performance issue. For example, work might not be + distributed fairly across the worker threads, or there may be a long blocking operation in an + extension that's impeding progress. + +* **Poll delay:** On each iteration of the event loop, the event dispatcher polls for I/O events + and "wakes up" either when some I/O events are ready to be processed or when a timeout fires, + whichever occurs first. In the case of a timeout, we can measure the difference between the + expected wakeup time and the actual wakeup time after polling; this difference is called the "poll + delay." It's normal to see some small poll delay, usually equal to the kernel scheduler's "time + slice" or "quantum"---this depends on the specific operating system on which Envoy is + running---but if this number elevates substantially above its normal observed baseline, it likely + indicates kernel scheduler delays. + +These statistics can be enabled by setting :ref:`enable_dispatcher_stats ` +to true. + +.. warning:: + + Note that enabling dispatcher stats records a value for each iteration of the event loop on every + thread. This should normally be minimal overhead, but when using + :ref:`statsd `, it will send each observed value over + the wire individually because the statsd protocol doesn't have any way to represent a histogram + summary. Be aware that this can be a very large volume of data. + +Statistics +---------- + +The event dispatcher for the main thread has a statistics tree rooted at *server.dispatcher.*, and +the event dispatcher for each worker thread has a statistics tree rooted at +*listener_manager.worker_.dispatcher.*, each with the following statistics: + +.. csv-table:: + :header: Name, Type, Description + :widths: 1, 1, 2 + + loop_duration_us, Histogram, Event loop durations in microseconds + poll_delay_us, Histogram, Polling delays in microseconds + +Note that any auxiliary threads are not included here. diff --git a/include/envoy/event/dispatcher.h b/include/envoy/event/dispatcher.h index 1e0b52a10f270..0024b3a2e7795 100644 --- a/include/envoy/event/dispatcher.h +++ b/include/envoy/event/dispatcher.h @@ -17,11 +17,29 @@ #include "envoy/network/listen_socket.h" #include "envoy/network/listener.h" #include "envoy/network/transport_socket.h" +#include "envoy/stats/scope.h" +#include "envoy/stats/stats_macros.h" #include "envoy/thread/thread.h" namespace Envoy { namespace Event { +/** + * All dispatcher stats. @see stats_macros.h + */ +// clang-format off +#define ALL_DISPATCHER_STATS(HISTOGRAM) \ + HISTOGRAM(loop_duration_us) \ + HISTOGRAM(poll_delay_us) +// clang-format on + +/** + * Struct definition for all dispatcher stats. @see stats_macros.h + */ +struct DispatcherStats { + ALL_DISPATCHER_STATS(GENERATE_HISTOGRAM_STRUCT) +}; + /** * Callback invoked when a dispatcher post() runs. */ @@ -39,6 +57,15 @@ class Dispatcher { */ virtual TimeSource& timeSource() PURE; + /** + * Initialize stats for this dispatcher. Note that this can't generally be done at construction + * time, since the main and worker thread dispatchers are constructed before + * ThreadLocalStoreImpl::initializeThreading. + * @param scope the scope to contain the new per-dispatcher stats created here. + * @param prefix the stats prefix to identify this dispatcher. + */ + virtual void initializeStats(Stats::Scope& scope, const std::string& prefix) PURE; + /** * Clear any items in the deferred deletion queue. */ diff --git a/include/envoy/server/worker.h b/include/envoy/server/worker.h index cb845b318158c..255209531f577 100644 --- a/include/envoy/server/worker.h +++ b/include/envoy/server/worker.h @@ -43,6 +43,14 @@ class Worker { */ virtual void start(GuardDog& guard_dog) PURE; + /** + * Initialize stats for this worker's dispatcher, if available. The worker will output + * thread-specific stats under the given scope. + * @param scope the scope to contain the new per-dispatcher stats created here. + * @param prefix the stats prefix to identify this dispatcher. + */ + virtual void initializeStats(Stats::Scope& scope, const std::string& prefix) PURE; + /** * Stop the worker thread. */ diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index 8e737de4de17f..9d7badc88f4c0 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -41,6 +41,13 @@ DispatcherImpl::DispatcherImpl(Buffer::WatermarkFactoryPtr&& factory, Api::Api& DispatcherImpl::~DispatcherImpl() {} +void DispatcherImpl::initializeStats(Stats::Scope& scope, const std::string& prefix) { + stats_prefix_ = prefix + "dispatcher"; + stats_ = std::make_unique( + DispatcherStats{ALL_DISPATCHER_STATS(POOL_HISTOGRAM_PREFIX(scope, stats_prefix_ + "."))}); + base_scheduler_.initializeStats(stats_.get()); +} + void DispatcherImpl::clearDeferredDeleteList() { ASSERT(isThreadSafe()); std::vector* to_delete = current_to_delete_; @@ -158,6 +165,9 @@ void DispatcherImpl::post(std::function callback) { void DispatcherImpl::run(RunType type) { run_tid_ = api_.threadFactory().currentThreadId(); + if (!stats_prefix_.empty()) { + ENVOY_LOG(debug, "running {} on thread {}", stats_prefix_, run_tid_->debugString()); + } // Flush all post callbacks before we run the event loop. We do this because there are post // callbacks that have to get run before the initial event loop starts running. libevent does diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index a51dce3e6aaff..b712f22d879e4 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -3,6 +3,7 @@ #include #include #include +#include #include #include "envoy/api/api.h" @@ -10,6 +11,7 @@ #include "envoy/event/deferred_deletable.h" #include "envoy/event/dispatcher.h" #include "envoy/network/connection_handler.h" +#include "envoy/stats/scope.h" #include "common/common/logger.h" #include "common/common/thread.h" @@ -36,6 +38,7 @@ class DispatcherImpl : Logger::Loggable, public Dispatcher { // Event::Dispatcher TimeSource& timeSource() override { return api_.timeSource(); } + void initializeStats(Stats::Scope& scope, const std::string& prefix) override; void clearDeferredDeleteList() override; Network::ConnectionPtr createServerConnection(Network::ConnectionSocketPtr&& socket, @@ -72,6 +75,8 @@ class DispatcherImpl : Logger::Loggable, public Dispatcher { bool isThreadSafe() const { return run_tid_ == nullptr || run_tid_->isCurrentThreadId(); } Api::Api& api_; + std::string stats_prefix_; + std::unique_ptr stats_; Thread::ThreadIdPtr run_tid_; Buffer::WatermarkFactoryPtr buffer_factory_; LibeventScheduler base_scheduler_; diff --git a/source/common/event/libevent_scheduler.cc b/source/common/event/libevent_scheduler.cc index 5b35ffd18447e..df22b45ba7371 100644 --- a/source/common/event/libevent_scheduler.cc +++ b/source/common/event/libevent_scheduler.cc @@ -3,9 +3,17 @@ #include "common/common/assert.h" #include "common/event/timer_impl.h" +#include "event2/util.h" + namespace Envoy { namespace Event { +namespace { +void recordTimeval(Stats::Histogram& histogram, const timeval& tv) { + histogram.recordValue(tv.tv_sec * 1000000 + tv.tv_usec); +} +} // namespace + LibeventScheduler::LibeventScheduler() : libevent_(event_base_new()) { // The dispatcher won't work as expected if libevent hasn't been configured to use threads. RELEASE_ASSERT(Libevent::Global::initialized(), ""); @@ -41,5 +49,55 @@ void LibeventScheduler::run(Dispatcher::RunType mode) { void LibeventScheduler::loopExit() { event_base_loopexit(libevent_.get(), nullptr); } +void LibeventScheduler::initializeStats(DispatcherStats* stats) { + stats_ = stats; + // These are thread safe. + evwatch_prepare_new(libevent_.get(), &onPrepare, this); + evwatch_check_new(libevent_.get(), &onCheck, this); +} + +void LibeventScheduler::onPrepare(evwatch*, const evwatch_prepare_cb_info* info, void* arg) { + // `self` is `this`, passed in from evwatch_prepare_new. + auto self = static_cast(arg); + + // Record poll timeout and prepare time for this iteration of the event loop. The timeout is the + // expected polling duration, whereas the actual polling duration will be the difference measured + // between the prepare time and the check time immediately after polling. These are compared in + // onCheck to compute the poll_delay stat. + self->timeout_set_ = evwatch_prepare_get_timeout(info, &self->timeout_); + evutil_gettimeofday(&self->prepare_time_, nullptr); + + // If we have a check time available from a previous iteration of the event loop (that is, all but + // the first), compute the loop_duration stat. + if (self->check_time_.tv_sec != 0) { + timeval delta; + evutil_timersub(&self->prepare_time_, &self->check_time_, &delta); + recordTimeval(self->stats_->loop_duration_us_, delta); + } +} + +void LibeventScheduler::onCheck(evwatch*, const evwatch_check_cb_info*, void* arg) { + // `self` is `this`, passed in from evwatch_check_new. + auto self = static_cast(arg); + + // Record check time for this iteration of the event loop. Use this together with prepare time + // from above to compute the actual polling duration, and store it for the next iteration of the + // event loop to compute the loop duration. + evutil_gettimeofday(&self->check_time_, nullptr); + if (self->timeout_set_) { + timeval delta, delay; + evutil_timersub(&self->check_time_, &self->prepare_time_, &delta); + evutil_timersub(&delta, &self->timeout_, &delay); + + // Delay can be negative, meaning polling completed early. This happens in normal operation, + // either because I/O was ready before we hit the timeout, or just because the kernel was + // feeling saucy. Disregard negative delays in stats, since they don't indicate anything + // particularly useful. + if (delay.tv_sec >= 0) { + recordTimeval(self->stats_->poll_delay_us_, delay); + } + } +} + } // namespace Event } // namespace Envoy diff --git a/source/common/event/libevent_scheduler.h b/source/common/event/libevent_scheduler.h index 5a41e1ccf6c4f..b9157bf4059b5 100644 --- a/source/common/event/libevent_scheduler.h +++ b/source/common/event/libevent_scheduler.h @@ -6,6 +6,7 @@ #include "common/event/libevent.h" #include "event2/event.h" +#include "event2/watch.h" namespace Envoy { namespace Event { @@ -40,8 +41,22 @@ class LibeventScheduler : public Scheduler { */ event_base& base() { return *libevent_; } + /** + * Start writing stats once thread-local storage is ready to receive them (see + * ThreadLocalStoreImpl::initializeThreading). + */ + void initializeStats(DispatcherStats* stats_); + private: + static void onPrepare(evwatch*, const evwatch_prepare_cb_info* info, void* arg); + static void onCheck(evwatch*, const evwatch_check_cb_info*, void* arg); + Libevent::BasePtr libevent_; + DispatcherStats* stats_{}; // stats owned by the containing DispatcherImpl + bool timeout_set_{}; // whether there is a poll timeout in the current event loop iteration + timeval timeout_{}; // the poll timeout for the current event loop iteration, if available + timeval prepare_time_{}; // timestamp immediately before polling + timeval check_time_{}; // timestamp immediately after polling }; } // namespace Event diff --git a/source/server/config_validation/server.cc b/source/server/config_validation/server.cc index a72155a2a74a2..8c0b41e66f69d 100644 --- a/source/server/config_validation/server.cc +++ b/source/server/config_validation/server.cc @@ -86,7 +86,7 @@ void ValidationInstance::initialize(const Options& options, Configuration::InitialImpl initial_config(bootstrap); overload_manager_ = std::make_unique(dispatcher(), stats(), threadLocal(), bootstrap.overload_manager(), *api_); - listener_manager_ = std::make_unique(*this, *this, *this); + listener_manager_ = std::make_unique(*this, *this, *this, false); thread_local_.registerThread(*dispatcher_, true); runtime_loader_ = component_factory.createRuntime(*this, initial_config); secret_manager_ = std::make_unique(); diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index fc64828f82a7b..f323777a8e454 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -671,10 +671,13 @@ void ListenerImpl::setSocket(const Network::SocketSharedPtr& socket) { ListenerManagerImpl::ListenerManagerImpl(Instance& server, ListenerComponentFactory& listener_factory, - WorkerFactory& worker_factory) - : server_(server), factory_(listener_factory), stats_(generateStats(server.stats())), + WorkerFactory& worker_factory, + bool enable_dispatcher_stats) + : server_(server), factory_(listener_factory), + scope_(server.stats().createScope("listener_manager.")), stats_(generateStats(*scope_)), config_tracker_entry_(server.admin().getConfigTracker().add( - "listeners", [this] { return dumpListenerConfigs(); })) { + "listeners", [this] { return dumpListenerConfigs(); })), + enable_dispatcher_stats_(enable_dispatcher_stats) { for (uint32_t i = 0; i < server.options().concurrency(); i++) { workers_.emplace_back(worker_factory.createWorker(server.overloadManager())); } @@ -718,9 +721,7 @@ ProtobufTypes::MessagePtr ListenerManagerImpl::dumpListenerConfigs() { } ListenerManagerStats ListenerManagerImpl::generateStats(Stats::Scope& scope) { - const std::string final_prefix = "listener_manager."; - return {ALL_LISTENER_MANAGER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix), - POOL_GAUGE_PREFIX(scope, final_prefix))}; + return {ALL_LISTENER_MANAGER_STATS(POOL_COUNTER(scope), POOL_GAUGE(scope))}; } bool ListenerManagerImpl::addOrUpdateListener(const envoy::api::v2::Listener& config, @@ -1006,12 +1007,16 @@ void ListenerManagerImpl::startWorkers(GuardDog& guard_dog) { ENVOY_LOG(info, "all dependencies initialized. starting workers"); ASSERT(!workers_started_); workers_started_ = true; + uint32_t i = 0; for (const auto& worker : workers_) { ASSERT(warming_listeners_.empty()); for (const auto& listener : active_listeners_) { addListenerToWorker(*worker, *listener); } worker->start(guard_dog); + if (enable_dispatcher_stats_) { + worker->initializeStats(*scope_, fmt::format("worker_{}.", i++)); + } } } diff --git a/source/server/listener_manager_impl.h b/source/server/listener_manager_impl.h index 41da1135d4443..767369578c995 100644 --- a/source/server/listener_manager_impl.h +++ b/source/server/listener_manager_impl.h @@ -104,7 +104,7 @@ struct ListenerManagerStats { class ListenerManagerImpl : public ListenerManager, Logger::Loggable { public: ListenerManagerImpl(Instance& server, ListenerComponentFactory& listener_factory, - WorkerFactory& worker_factory); + WorkerFactory& worker_factory, bool enable_dispatcher_stats); void onListenerWarmed(ListenerImpl& listener); @@ -177,9 +177,11 @@ class ListenerManagerImpl : public ListenerManager, Logger::Loggable draining_listeners_; std::list workers_; bool workers_started_{}; + Stats::ScopePtr scope_; ListenerManagerStats stats_; ConfigTracker::EntryOwnerPtr config_tracker_entry_; LdsApiPtr lds_api_; + const bool enable_dispatcher_stats_{}; }; // TODO(mattklein123): Consider getting rid of pre-worker start and post-worker start code by diff --git a/source/server/server.cc b/source/server/server.cc index a75d0d291ba47..d360a1a98e7d2 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -301,8 +301,8 @@ void InstanceImpl::initialize(const Options& options, std::make_unique(*dispatcher_, *overload_manager_, stats_store_); // Workers get created first so they register for thread local updates. - listener_manager_ = - std::make_unique(*this, listener_component_factory_, worker_factory_); + listener_manager_ = std::make_unique( + *this, listener_component_factory_, worker_factory_, bootstrap_.enable_dispatcher_stats()); // The main thread is also registered for thread local updates so that code that does not care // whether it runs on the main thread or on workers can still use TLS. @@ -311,6 +311,11 @@ void InstanceImpl::initialize(const Options& options, // We can now initialize stats for threading. stats_store_.initializeThreading(*dispatcher_, thread_local_); + // It's now safe to start writing stats from the main thread's dispatcher. + if (bootstrap_.enable_dispatcher_stats()) { + dispatcher_->initializeStats(stats_store_, "server."); + } + // Runtime gets initialized before the main configuration since during main configuration // load things may grab a reference to the loader for later use. runtime_singleton_ = std::make_unique( diff --git a/source/server/worker_impl.cc b/source/server/worker_impl.cc index 660d1ac1cf739..a12ad75d4248e 100644 --- a/source/server/worker_impl.cc +++ b/source/server/worker_impl.cc @@ -73,6 +73,10 @@ void WorkerImpl::start(GuardDog& guard_dog) { api_.threadFactory().createThread([this, &guard_dog]() -> void { threadRoutine(guard_dog); }); } +void WorkerImpl::initializeStats(Stats::Scope& scope, const std::string& prefix) { + dispatcher_->initializeStats(scope, prefix); +} + void WorkerImpl::stop() { // It's possible for the server to cleanly shut down while cluster initialization during startup // is happening, so we might not yet have a thread. diff --git a/source/server/worker_impl.h b/source/server/worker_impl.h index b59c7356f2134..e87b1f4c228a8 100644 --- a/source/server/worker_impl.h +++ b/source/server/worker_impl.h @@ -45,6 +45,7 @@ class WorkerImpl : public Worker, Logger::Loggable { uint64_t numConnections() override; void removeListener(Network::ListenerConfig& listener, std::function completion) override; void start(GuardDog& guard_dog) override; + void initializeStats(Stats::Scope& scope, const std::string& prefix) override; void stop() override; void stopListener(Network::ListenerConfig& listener) override; void stopListeners() override; diff --git a/test/common/event/BUILD b/test/common/event/BUILD index 0a5ad582e66b2..4e432fb57625e 100644 --- a/test/common/event/BUILD +++ b/test/common/event/BUILD @@ -17,6 +17,7 @@ envoy_cc_test( "//source/common/event:dispatcher_lib", "//source/common/stats:isolated_store_lib", "//test/mocks:common_lib", + "//test/mocks/stats:stats_mocks", "//test/test_common:utility_lib", ], ) diff --git a/test/common/event/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc index af0cd7e7579c0..ba06d0a4be8dd 100644 --- a/test/common/event/dispatcher_impl_test.cc +++ b/test/common/event/dispatcher_impl_test.cc @@ -8,12 +8,17 @@ #include "common/stats/isolated_store_impl.h" #include "test/mocks/common.h" +#include "test/mocks/stats/mocks.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +using testing::_; using testing::InSequence; +using testing::NiceMock; +using testing::Return; +using testing::StartsWith; namespace Envoy { namespace Event { @@ -80,6 +85,7 @@ class DispatcherImplTest : public testing::Test { dispatcher_thread_->join(); } + NiceMock scope_; // Used in InitializeStats, must outlive dispatcher_->exit(). Api::ApiPtr api_; Thread::ThreadPtr dispatcher_thread_; DispatcherPtr dispatcher_; @@ -90,6 +96,14 @@ class DispatcherImplTest : public testing::Test { TimerPtr keepalive_timer_; }; +// TODO(mergeconflict): We also need integration testing to validate that the expected histograms +// are written when `enable_dispatcher_stats` is true. See issue #6582. +TEST_F(DispatcherImplTest, InitializeStats) { + EXPECT_CALL(scope_, histogram("test.dispatcher.loop_duration_us")); + EXPECT_CALL(scope_, histogram("test.dispatcher.poll_delay_us")); + dispatcher_->initializeStats(scope_, "test."); +} + TEST_F(DispatcherImplTest, Post) { dispatcher_->post([this]() { { diff --git a/test/config_test/config_test.cc b/test/config_test/config_test.cc index 7f1a8a7ace199..f66a74e49ded8 100644 --- a/test/config_test/config_test.cc +++ b/test/config_test/config_test.cc @@ -106,7 +106,8 @@ class ConfigTest { std::unique_ptr cluster_manager_factory_; NiceMock component_factory_; NiceMock worker_factory_; - Server::ListenerManagerImpl listener_manager_{server_, component_factory_, worker_factory_}; + Server::ListenerManagerImpl listener_manager_{server_, component_factory_, worker_factory_, + false}; Runtime::RandomGeneratorImpl random_; NiceMock os_sys_calls_; TestThreadsafeSingletonInjector os_calls{&os_sys_calls_}; diff --git a/test/mocks/event/mocks.cc b/test/mocks/event/mocks.cc index d79fe9db839cb..f1d5cdcadb19c 100644 --- a/test/mocks/event/mocks.cc +++ b/test/mocks/event/mocks.cc @@ -16,6 +16,7 @@ namespace Envoy { namespace Event { MockDispatcher::MockDispatcher() { + ON_CALL(*this, initializeStats(_, _)).WillByDefault(Return()); ON_CALL(*this, clearDeferredDeleteList()).WillByDefault(Invoke([this]() -> void { to_delete_.clear(); })); diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index a71b0e60998c9..4237a3b57ca0b 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -84,6 +84,7 @@ class MockDispatcher : public Dispatcher { } // Event::Dispatcher + MOCK_METHOD2(initializeStats, void(Stats::Scope&, const std::string&)); MOCK_METHOD0(clearDeferredDeleteList, void()); MOCK_METHOD2(createServerConnection_, Network::Connection*(Network::ConnectionSocket* socket, diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index 3374d8b5f8b85..3046f17672bab 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -308,6 +308,7 @@ class MockWorker : public Worker { MOCK_METHOD2(removeListener, void(Network::ListenerConfig& listener, std::function completion)); MOCK_METHOD1(start, void(GuardDog& guard_dog)); + MOCK_METHOD2(initializeStats, void(Stats::Scope& scope, const std::string& prefix)); MOCK_METHOD0(stop, void()); MOCK_METHOD1(stopListener, void(Network::ListenerConfig& listener)); MOCK_METHOD0(stopListeners, void()); diff --git a/test/mocks/thread_local/mocks.cc b/test/mocks/thread_local/mocks.cc index cfabd7a7f52f0..5ccc69fcf21d9 100644 --- a/test/mocks/thread_local/mocks.cc +++ b/test/mocks/thread_local/mocks.cc @@ -13,6 +13,7 @@ MockInstance::MockInstance() { ON_CALL(*this, allocateSlot()).WillByDefault(Invoke(this, &MockInstance::allocateSlot_)); ON_CALL(*this, runOnAllThreads(_)).WillByDefault(Invoke(this, &MockInstance::runOnAllThreads_)); ON_CALL(*this, shutdownThread()).WillByDefault(Invoke(this, &MockInstance::shutdownThread_)); + ON_CALL(*this, dispatcher()).WillByDefault(ReturnRef(dispatcher_)); } MockInstance::~MockInstance() { shutdownThread_(); } diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index 53f9e086ba7ee..4e112120024d0 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -61,7 +61,8 @@ class ListenerManagerImplTest : public testing::Test { ListenerManagerImplTest() : api_(Api::createApiForTest()) { ON_CALL(server_, api()).WillByDefault(ReturnRef(*api_)); EXPECT_CALL(worker_factory_, createWorker_()).WillOnce(Return(worker_)); - manager_ = std::make_unique(server_, listener_factory_, worker_factory_); + manager_ = + std::make_unique(server_, listener_factory_, worker_factory_, false); } /** diff --git a/test/server/worker_impl_test.cc b/test/server/worker_impl_test.cc index c59063e3e3315..57e85f8c77868 100644 --- a/test/server/worker_impl_test.cc +++ b/test/server/worker_impl_test.cc @@ -71,7 +71,9 @@ TEST_F(WorkerImplTest, BasicFlow) { ci.setReady(); }); + NiceMock store; worker_.start(guard_dog_); + worker_.initializeStats(store, "test"); ci.waitReady(); // After a worker is started adding/stopping/removing a listener happens on the worker thread. diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt index d16d5d0047a17..25c768b3558d7 100644 --- a/tools/spelling_dictionary.txt +++ b/tools/spelling_dictionary.txt @@ -427,6 +427,7 @@ evbuffer evbuffers evconnlistener evthread +evwatch exe execlp facto