From 2d8efee73e2281be570e9c3d8ba3323e573eda3d Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Fri, 1 Jul 2022 12:49:27 +0000 Subject: [PATCH] refactor(userspace/falco): improve design and docs of stats writer Signed-off-by: Jason Dellaluce --- .../falco/app_actions/process_events.cpp | 6 +- userspace/falco/stats_writer.cpp | 87 +++++++++++-------- userspace/falco/stats_writer.h | 87 ++++++++++++++++--- 3 files changed, 128 insertions(+), 52 deletions(-) diff --git a/userspace/falco/app_actions/process_events.cpp b/userspace/falco/app_actions/process_events.cpp index 17951152..03aa1401 100644 --- a/userspace/falco/app_actions/process_events.cpp +++ b/userspace/falco/app_actions/process_events.cpp @@ -45,7 +45,7 @@ application::run_result application::do_inspect(syscall_evt_drop_mgr &sdropmgr, { int32_t rc; sinsp_evt* ev; - stats_writer::state stats_state; + stats_writer::collector stats_collector(statsw); uint64_t duration_start = 0; uint32_t timeouts_since_last_success_or_msg = 0; std::size_t source_idx; @@ -79,7 +79,7 @@ application::run_result application::do_inspect(syscall_evt_drop_mgr &sdropmgr, rc = m_state->inspector->next(&ev); - statsw->handle(m_state->inspector, stats_state); + stats_collector.collect(m_state->inspector); if(m_state->terminate.load(std::memory_order_acquire) || m_state->restart.load(std::memory_order_acquire)) @@ -204,7 +204,7 @@ application::run_result application::process_events() if (!m_options.stats_filename.empty()) { std::string err; - if (!stats_writer::set_timer(m_options.stats_interval, err)) + if (!stats_writer::init_ticker(m_options.stats_interval, err)) { return run_result::fatal(err); } diff --git a/userspace/falco/stats_writer.cpp b/userspace/falco/stats_writer.cpp index bfaf24b9..8aa1bc6a 100644 --- a/userspace/falco/stats_writer.cpp +++ b/userspace/falco/stats_writer.cpp @@ -24,17 +24,17 @@ limitations under the License. #include "banned.h" // This raises a compilation error when certain functions are used #include "logger.h" -// note: uint16_t is enough because we don't care about +// note: ticker_t is an uint16_t, which is enough because we don't care about // overflows here. Threads calling stats_writer::handle() will just // check that this value changed since their last observation. -static std::atomic s_last_tick((uint16_t) 0); +static std::atomic s_timer((stats_writer::ticker_t) 0); static void timer_handler(int signum) { - s_last_tick.fetch_add(1, std::memory_order_relaxed); + s_timer.fetch_add(1, std::memory_order_relaxed); } -bool stats_writer::set_timer(uint32_t interval_msec, string &err) +bool stats_writer::init_ticker(uint32_t interval_msec, string &err) { struct itimerval timer; struct sigaction handler; @@ -59,10 +59,15 @@ bool stats_writer::set_timer(uint32_t interval_msec, string &err) return true; } +stats_writer::ticker_t stats_writer::get_ticker() +{ + return s_timer.load(std::memory_order_relaxed); +} + stats_writer::stats_writer() : m_initialized(false), m_total_samples(0) { - // the stats writter do nothing + } stats_writer::stats_writer(const std::string &filename) @@ -82,37 +87,9 @@ stats_writer::~stats_writer() } } -void stats_writer::handle(const std::shared_ptr& inspector, stats_writer::state& s) +bool stats_writer::has_output() const { - if (m_initialized) - { - auto tick = s_last_tick.load(std::memory_order_relaxed); - if (tick != s.last_tick) - { - // gather stats sample and fill-up message - stats_writer::msg msg; - msg.stop = false; - inspector->get_capture_stats(&msg.stats); - if(s.samples == 1) - { - msg.delta = msg.stats; - } - else - { - msg.delta.n_evts = msg.stats.n_evts - s.last_stats.n_evts; - msg.delta.n_drops = msg.stats.n_drops - s.last_stats.n_drops; - msg.delta.n_preemptions = msg.stats.n_preemptions - s.last_stats.n_preemptions; - } - - // update state - s.samples++; - s.last_tick = tick; - s.last_stats = msg.stats; - - // push message into the queue - push(msg); - } - } + return m_initialized; } void stats_writer::stop_worker() @@ -140,13 +117,14 @@ void stats_writer::worker() noexcept stats_writer::msg m; while(true) { - // Block until a message becomes available. + // blocks until a message becomes availables m_queue.pop(m); if (m.stop) { return; } + m_total_samples++; try { jmsg["sample"] = m_num_stats; @@ -166,3 +144,40 @@ void stats_writer::worker() noexcept } } } + +stats_writer::collector::collector(std::shared_ptr writer) + : m_writer(writer), m_last_tick(0), m_samples(0) +{ + +} + +void stats_writer::collector::collect(std::shared_ptr inspector) +{ + // just skip if no output is configured + if (m_writer->has_output()) + { + // collect stats once per each ticker period + auto tick = stats_writer::get_ticker(); + if (tick != m_last_tick) + { + stats_writer::msg msg; + msg.stop = false; + inspector->get_capture_stats(&msg.stats); + m_samples++; + if(m_samples == 1) + { + msg.delta = msg.stats; + } + else + { + msg.delta.n_evts = msg.stats.n_evts - m_last_stats.n_evts; + msg.delta.n_drops = msg.stats.n_drops - m_last_stats.n_drops; + msg.delta.n_preemptions = msg.stats.n_preemptions - m_last_stats.n_preemptions; + } + + m_last_tick = tick; + m_last_stats = msg.stats; + m_writer->push(msg); + } + } +} diff --git a/userspace/falco/stats_writer.h b/userspace/falco/stats_writer.h index 7eee6458..b2764fe9 100644 --- a/userspace/falco/stats_writer.h +++ b/userspace/falco/stats_writer.h @@ -24,28 +24,86 @@ limitations under the License. #include "tbb/concurrent_queue.h" -// Periodically collects scap stats files and writes them to a file as -// json. - +/*! + \brief Writes stats samples collected from inspectors into a given output. + Users must use a stats_writer::collector in order to collect and write stats + into a given stats_writer. This class is thread-safe, and can be shared + across multiple stats_writer::collector instances from different threads. +*/ class stats_writer { public: - struct state - { - inline state(): samples(0) { } + /*! + \brief Value of a ticker that dictates when stats are collected + */ + typedef uint16_t ticker_t; - uint64_t samples; - uint16_t last_tick; - scap_stats last_stats; + /*! + \brief Collects stats samples from an inspector and uses a writer + to print them in a given output. Stats are collected periodically every + time the value of stats_writer::get_ticker() changes. + This class is not thread-safe. + */ + class collector + { + public: + /*! + \brief Initializes the collector with the given writer + */ + explicit collector(std::shared_ptr writer); + + /*! + \brief Collects one stats sample from an inspector + */ + void collect(std::shared_ptr inspector); + + private: + std::shared_ptr m_writer; + stats_writer::ticker_t m_last_tick; + uint64_t m_samples; + scap_stats m_last_stats; }; - stats_writer(); - explicit stats_writer(const std::string &filename); + stats_writer(const stats_writer&) = delete; + + stats_writer(stats_writer&&) = delete; + + stats_writer& operator=(const stats_writer&) = delete; + + stats_writer& operator=(stats_writer&&) = delete; + ~stats_writer(); - void handle(const std::shared_ptr& inspector, stats_writer::state& s); + /*! + \brief Initializes a writer without any output. + With this contructor, has_output() always returns false + */ + stats_writer(); - static bool set_timer(uint32_t interval_msec, std::string &err); + /*! + \brief Initializes a writer that prints to a file at the given filename. + With this contructor, has_output() always returns true + */ + explicit stats_writer(const std::string &filename); + + /*! + \brief Returns true if the writer is configured with a valid output + */ + inline bool has_output() const; + + /*! + \brief Initializes the ticker with a given interval period defined + in milliseconds. Subsequent calls to init_ticker will dismiss the + previously-initialized ticker. Internally, this uses a timer + signal handler. + */ + static bool init_ticker(uint32_t interval_msec, std::string &err); + + /*! + \brief Returns the current value of the ticker. + This function is thread-safe. + */ + inline static ticker_t get_ticker(); private: struct msg @@ -64,4 +122,7 @@ private: std::thread m_worker; std::ofstream m_output; tbb::concurrent_bounded_queue m_queue; + + // note: in this way, only collectors can push into the queue + friend class stats_writer::collector; }; \ No newline at end of file