From 5c237a07dceec298214b63f0ab4e4a551390b32a Mon Sep 17 00:00:00 2001 From: Samuel Gaist Date: Tue, 16 Apr 2024 09:02:55 +0200 Subject: [PATCH] refactor(metrics): make to_text get the application state As falco may update its state at any time and thus its inspectors objects, keeping pointers to them may end up in using dangling values. Therefore, use the state of the application when requesting metrics. Optimizations such as caching of mostly static values will be done in a follow up patch. Signed-off-by: Samuel Gaist --- .../falco/app/actions/start_webserver.cpp | 11 ++--- userspace/falco/falco_metrics.cpp | 41 +++++++------------ userspace/falco/falco_metrics.h | 10 +---- userspace/falco/webserver.cpp | 16 ++++---- userspace/falco/webserver.h | 9 ++-- 5 files changed, 31 insertions(+), 56 deletions(-) diff --git a/userspace/falco/app/actions/start_webserver.cpp b/userspace/falco/app/actions/start_webserver.cpp index cbb10036..bd857ef7 100644 --- a/userspace/falco/app/actions/start_webserver.cpp +++ b/userspace/falco/app/actions/start_webserver.cpp @@ -19,7 +19,6 @@ limitations under the License. #if !defined(_WIN32) && !defined(__EMSCRIPTEN__) && !defined(MINIMAL_BUILD) #include "webserver.h" -#include "falco_metrics.h" #endif using namespace falco::app; @@ -35,7 +34,7 @@ falco::app::run_result falco::app::actions::start_webserver(falco::app::state& s falco_logger::log(falco_logger::level::DEBUG, "Skipping starting webserver in dry-run\n"); return run_result::ok(); } - + falco_configuration::webserver_config webserver_config = state.config->m_webserver_config; std::string ssl_option = (webserver_config.m_ssl_enabled ? " (SSL)" : ""); falco_logger::log(falco_logger::level::INFO, "Starting health webserver with threadiness " @@ -46,12 +45,9 @@ falco::app::run_result falco::app::actions::start_webserver(falco::app::state& s + std::to_string(webserver_config.m_listen_port) + ssl_option + "\n"); - falco_metrics metrics(state); - state.webserver.start( - state.offline_inspector, - webserver_config, - metrics); + state, + webserver_config); } #endif return run_result::ok(); @@ -73,4 +69,3 @@ falco::app::run_result falco::app::actions::stop_webserver(falco::app::state& st #endif return run_result::ok(); } - diff --git a/userspace/falco/falco_metrics.cpp b/userspace/falco/falco_metrics.cpp index 9f9acc57..3d5b4465 100644 --- a/userspace/falco/falco_metrics.cpp +++ b/userspace/falco/falco_metrics.cpp @@ -36,46 +36,33 @@ limitations under the License. */ const std::string falco_metrics::content_type = "text/plain; version=0.0.4"; -/*! - \brief Constructor that takes a \c state object to build its internal state -*/ -falco_metrics::falco_metrics(falco::app::state& state) -{ - falco_configuration::webserver_config webserver_config = state.config->m_webserver_config; - m_metrics_enabled = state.config->m_metrics_enabled && webserver_config.m_metrics_enabled; - - if (m_metrics_enabled) - { - for (const auto& source_info: state.source_infos) - { - sinsp *source_inspector = source_info.inspector.get(); - m_inspectors.push_back(source_inspector); - m_metrics_collectors.push_back(libs::metrics::libs_metrics_collector(source_inspector, state.config->m_metrics_flags)); - } - } -} /*! - \brief This method returns a textual representation of the metrics configured. + \brief this method takes an application \c state and returns a textual representation of + its configured metrics. The current implementation returns a Prometheus exposition formatted string. */ -std::string falco_metrics::to_text() const +std::string falco_metrics::to_text(const falco::app::state& state) { - if (!m_metrics_enabled) - { - return ""; - } - static const char* all_driver_engines[] = { BPF_ENGINE, KMOD_ENGINE, MODERN_BPF_ENGINE, SOURCE_PLUGIN_ENGINE, NODRIVER_ENGINE, GVISOR_ENGINE }; + std::vector inspectors; + std::vector metrics_collectors; + + for (const auto& source_info: state.source_infos) + { + sinsp *source_inspector = source_info.inspector.get(); + inspectors.push_back(source_inspector); + metrics_collectors.push_back(libs::metrics::libs_metrics_collector(source_inspector, state.config->m_metrics_flags)); + } libs::metrics::prometheus_metrics_converter prometheus_metrics_converter; std::string prometheus_text; - for (auto* inspector: m_inspectors) + for (auto* inspector: inspectors) { for (size_t i = 0; i < sizeof(all_driver_engines) / sizeof(const char*); i++) { @@ -122,7 +109,7 @@ std::string falco_metrics::to_text() const } } - for (auto metrics_collector: m_metrics_collectors) + for (auto metrics_collector: metrics_collectors) { metrics_collector.snapshot(); auto metrics_snapshot = metrics_collector.get_metrics(); diff --git a/userspace/falco/falco_metrics.h b/userspace/falco/falco_metrics.h index 67fa61d5..af4146d8 100644 --- a/userspace/falco/falco_metrics.h +++ b/userspace/falco/falco_metrics.h @@ -28,13 +28,5 @@ class falco_metrics { public: static const std::string content_type; - - falco_metrics(falco::app::state& state); - bool is_enabled() const { return m_metrics_enabled; }; - std::string to_text() const; - -private: - bool m_metrics_enabled = false; - std::vector m_inspectors; - std::vector m_metrics_collectors; + static std::string to_text(const falco::app::state& state); }; diff --git a/userspace/falco/webserver.cpp b/userspace/falco/webserver.cpp index 1df67920..7611acc1 100644 --- a/userspace/falco/webserver.cpp +++ b/userspace/falco/webserver.cpp @@ -18,6 +18,7 @@ limitations under the License. #include "webserver.h" #include "falco_utils.h" #include "falco_metrics.h" +#include "app/state.h" #include "versions_info.h" #include @@ -27,9 +28,8 @@ falco_webserver::~falco_webserver() } void falco_webserver::start( - const std::shared_ptr& inspector, - const falco_configuration::webserver_config& webserver_config, - const falco_metrics& metrics) + const falco::app::state& state, + const falco_configuration::webserver_config& webserver_config) { if (m_running) { @@ -57,19 +57,19 @@ void falco_webserver::start( [](const httplib::Request &, httplib::Response &res) { res.set_content("{\"status\": \"ok\"}", "application/json"); }); - + // setup versions endpoint - const auto versions_json_str = falco::versions_info(inspector).as_json().dump(); + const auto versions_json_str = falco::versions_info(state.offline_inspector).as_json().dump(); m_server->Get("/versions", [versions_json_str](const httplib::Request &, httplib::Response &res) { res.set_content(versions_json_str, "application/json"); }); - if (metrics.is_enabled()) + if (state.config->m_metrics_enabled && webserver_config.m_metrics_enabled) { m_server->Get("/metrics", - [metrics](const httplib::Request &, httplib::Response &res) { - res.set_content(metrics.to_text(), falco_metrics::content_type); + [&state](const httplib::Request &, httplib::Response &res) { + res.set_content(falco_metrics::to_text(state), falco_metrics::content_type); }); } // run server in a separate thread diff --git a/userspace/falco/webserver.h b/userspace/falco/webserver.h index 01f81e95..47834778 100644 --- a/userspace/falco/webserver.h +++ b/userspace/falco/webserver.h @@ -25,7 +25,9 @@ limitations under the License. #include #include -class falco_metrics; +namespace falco::app { + struct state; +} class falco_webserver { @@ -37,9 +39,8 @@ public: falco_webserver(const falco_webserver&) = delete; falco_webserver& operator = (const falco_webserver&) = delete; virtual void start( - const std::shared_ptr& inspector, - const falco_configuration::webserver_config& webserver_config, - const falco_metrics& metrics); + const falco::app::state& state, + const falco_configuration::webserver_config& webserver_config); virtual void stop(); private: