From f7570f81524b2eae2315dc0db9f1334c2b2bb7bb Mon Sep 17 00:00:00 2001 From: Dima Dorezyuk Date: Mon, 14 Oct 2024 11:07:41 +0200 Subject: [PATCH] Rust: init logging only once (#207) * Everestrs: Change logging Signed-off-by: Dima Dorezyuk * Don't call again config Signed-off-by: Dima Dorezyuk --------- Signed-off-by: Dima Dorezyuk Co-authored-by: Dima Dorezyuk --- everestrs/everestrs-build/jinja/module.jinja2 | 2 +- everestrs/everestrs/src/everestrs_sys.cpp | 34 ++++--- everestrs/everestrs/src/everestrs_sys.hpp | 5 +- everestrs/everestrs/src/lib.rs | 89 ++++++++++--------- 4 files changed, 69 insertions(+), 61 deletions(-) diff --git a/everestrs/everestrs-build/jinja/module.jinja2 b/everestrs/everestrs-build/jinja/module.jinja2 index 2ff50008..6909512b 100644 --- a/everestrs/everestrs-build/jinja/module.jinja2 +++ b/everestrs/everestrs-build/jinja/module.jinja2 @@ -105,7 +105,7 @@ impl Module { {% endfor %} ) -> ::std::sync::Arc { let runtime = ::everestrs::Runtime::new(); - let connections = ::everestrs::get_module_connections(); + let connections = runtime.get_module_connections(); let this = ::std::sync::Arc::new(Self { on_ready, {% for provide in provides %} diff --git a/everestrs/everestrs/src/everestrs_sys.cpp b/everestrs/everestrs/src/everestrs_sys.cpp index 7b32a638..08e7a65f 100644 --- a/everestrs/everestrs/src/everestrs_sys.cpp +++ b/everestrs/everestrs/src/everestrs_sys.cpp @@ -28,12 +28,6 @@ std::unique_ptr create_everest_instance(const std::string& mod rs->mqtt_external_prefix, rs->telemetry_prefix, rs->telemetry_enabled); } -std::unique_ptr create_config_instance(std::shared_ptr rs) { - // FIXME (aw): where to initialize the logger? - Everest::Logging::init(rs->logging_config_file); - return std::make_unique(rs); -} - JsonBlob json2blob(const json& j) { // I did not find a way to not copy the data at least once here. const std::string dumped = j.dump(); @@ -83,7 +77,7 @@ inline ConfigField get_config_field(const std::string& _name, int _value) { Module::Module(const std::string& module_id, const std::string& prefix, const std::string& config_file) : module_id_(module_id), rs_(std::make_shared(prefix, config_file)), - config_(create_config_instance(rs_)), + config_(std::make_unique(rs_)), handle_(create_everest_instance(module_id, rs_, *config_)) { } @@ -161,11 +155,8 @@ rust::Vec get_module_configs(rust::Str module_id, rust::Str pref return out; } -rust::Vec get_module_connections(rust::Str module_id, rust::Str prefix, rust::Str config_file) { - const auto rs = std::make_shared(std::string(prefix), std::string(config_file)); - Everest::Config config{rs}; - - const auto connections = config.get_main_config().at(std::string(module_id))["connections"]; +rust::Vec Module::get_module_connections() const { + const auto connections = config_->get_main_config().at(std::string(module_id_))["connections"]; // Iterate over the connections block. rust::Vec out; @@ -176,18 +167,25 @@ rust::Vec get_module_connections(rust::Str module_id, rust: return out; } -int Module::get_log_level() const { +int init_logging(rust::Str module_id, rust::Str prefix, rust::Str config_file) { + using namespace boost::log; + using namespace Everest::Logging; + + const std::string module_id_cpp{module_id}; + const std::string prefix_cpp{prefix}; + const std::string config_file_cpp{config_file}; + + // Init the CPP logger. + Everest::RuntimeSettings rs{prefix_cpp, config_file_cpp}; + init(rs.logging_config_file, module_id_cpp); + // Below is something really ugly. Boost's log filter rules may actually be // quite "complex" but the library does not expose any way to check the // already installed filters. We therefore reopen the config and construct // or own filter - and feed it with dummy values to determine its filtering // behaviour (the lowest severity which is accepted by the filter) - std::filesystem::path logging_path{rs_->logging_config_file}; + std::filesystem::path logging_path{rs.logging_config_file}; std::ifstream logging_config(logging_path.c_str()); - - using namespace boost::log; - using namespace Everest::Logging; - if (!logging_config.is_open()) { return info; } diff --git a/everestrs/everestrs/src/everestrs_sys.hpp b/everestrs/everestrs/src/everestrs_sys.hpp index 3c434bbc..96ddfb22 100644 --- a/everestrs/everestrs/src/everestrs_sys.hpp +++ b/everestrs/everestrs/src/everestrs_sys.hpp @@ -27,7 +27,7 @@ class Module { JsonBlob call_command(rust::Str implementation_id, size_t index, rust::Str name, JsonBlob args) const; void subscribe_variable(const Runtime& rt, rust::String implementation_id, size_t index, rust::String name) const; void publish_variable(rust::Str implementation_id, rust::Str name, JsonBlob blob) const; - int get_log_level() const; + rust::Vec get_module_connections() const; private: const std::string module_id_; @@ -39,5 +39,6 @@ class Module { std::unique_ptr create_module(rust::Str module_name, rust::Str prefix, rust::Str conf); rust::Vec get_module_configs(rust::Str module_name, rust::Str prefix, rust::Str conf); -rust::Vec get_module_connections(rust::Str module_name, rust::Str prefix, rust::Str conf); + +int init_logging(rust::Str module_name, rust::Str prefix, rust::Str conf); void log2cxx(int level, int line, rust::Str file, rust::Str message); diff --git a/everestrs/everestrs/src/lib.rs b/everestrs/everestrs/src/lib.rs index 0759a8e1..43f69be0 100644 --- a/everestrs/everestrs/src/lib.rs +++ b/everestrs/everestrs/src/lib.rs @@ -8,10 +8,14 @@ use std::convert::TryFrom; use std::path::PathBuf; use std::pin::Pin; use std::sync::Arc; +use std::sync::Once; use std::sync::RwLock; use std::sync::Weak; use thiserror::Error; +/// Prevent calling the init of loggers more than once. +static INIT_LOGGER_ONCE: Once = Once::new(); + // Reexport everything so the clients can use it. pub use serde; pub use serde_json; @@ -155,22 +159,18 @@ mod ffi { name: String, ); + /// Returns the `connections` block defined in the `config.yaml` for + /// the current module. + fn get_module_connections(self: &Module) -> Vec; + /// Publishes the given `blob` under the `implementation_id` and `name`. fn publish_variable(self: &Module, implementation_id: &str, name: &str, blob: JsonBlob); - /// Returns the severity for the cxx logger. - fn get_log_level(self: &Module) -> i32; - /// Returns the module config from cpp. fn get_module_configs(module_id: &str, prefix: &str, conf: &str) -> Vec; - /// Returns the `connections` block defined in the `config.yaml` for - /// the current module. - fn get_module_connections( - module_id: &str, - prefix: &str, - conf: &str, - ) -> Vec; + /// Call this once. + fn init_logging(module_id: &str, prefix: &str, conf: &str) -> i32; /// Logging sink for the EVerest module. fn log2cxx(level: i32, line: i32, file: &str, message: &str); @@ -195,6 +195,7 @@ impl ffi::JsonBlob { /// Very simple logger to use by the Rust modules. mod logger { use super::ffi; + use crate::INIT_LOGGER_ONCE; pub(crate) struct Logger { level: log::Level, @@ -237,19 +238,21 @@ mod logger { /// Init the logger for everest. /// /// Don't do this on your own as we must also control some cxx code. - pub(crate) fn init_logger(module: &ffi::Module) { - let level = match module.get_log_level() { - 0 => log::Level::Trace, - 1 => log::Level::Debug, - 2 => log::Level::Info, - 3 => log::Level::Warn, - 4 => log::Level::Error, - _ => log::Level::Info, - }; + pub(crate) fn init_logger(module_name: &str, prefix: &str, conf: &str) { + INIT_LOGGER_ONCE.call_once(|| { + let level = match ffi::init_logging(module_name, prefix, conf) { + 0 => log::Level::Trace, + 1 => log::Level::Debug, + 2 => log::Level::Info, + 3 => log::Level::Warn, + 4 => log::Level::Error, + _ => log::Level::Info, + }; - let logger = Self { level }; - log::set_boxed_logger(Box::new(logger)).unwrap(); - log::set_max_level(level.to_level_filter()); + let logger = Self { level }; + log::set_boxed_logger(Box::new(logger)).unwrap(); + log::set_max_level(level.to_level_filter()); + }); } } } @@ -395,13 +398,17 @@ impl Runtime { // TODO(hrapp): This function could use some error handling. pub fn new() -> Pin> { let args: Args = argh::from_env(); - let cpp_module = ffi::create_module( + logger::Logger::init_logger( &args.module, &args.prefix.to_string_lossy(), &args.conf.to_string_lossy(), ); - logger::Logger::init_logger(&cpp_module); + let cpp_module = ffi::create_module( + &args.module, + &args.prefix.to_string_lossy(), + &args.conf.to_string_lossy(), + ); Arc::pin(Self { cpp_module, @@ -428,7 +435,7 @@ impl Runtime { } } - let connections = get_module_connections(); + let connections = self.get_module_connections(); // Subscribe to all variables that might be of interest. for (implementation_id, requires) in manifest.requires { @@ -452,6 +459,15 @@ impl Runtime { // again. (self.cpp_module).as_ref().unwrap().signal_ready(self); } + + /// The interface for fetching the module connections though the C++ runtime. + pub fn get_module_connections(&self) -> HashMap { + let raw_connections = self.cpp_module.as_ref().unwrap().get_module_connections(); + raw_connections + .into_iter() + .map(|connection| (connection.implementation_id, connection.slots)) + .collect() + } } /// A store for our config values. The type is closely related to @@ -505,8 +521,16 @@ impl TryFrom<&Config> for i64 { } /// Interface for fetching the configurations through the C++ runtime. +/// +/// This is separetated from the module since the user might need the config +/// to create the [Runtime]. pub fn get_module_configs() -> HashMap> { let args: Args = argh::from_env(); + logger::Logger::init_logger( + &args.module, + &args.prefix.to_string_lossy(), + &args.conf.to_string_lossy(), + ); let raw_config = ffi::get_module_configs( &args.module, &args.prefix.to_string_lossy(), @@ -541,18 +565,3 @@ pub fn get_module_configs() -> HashMap> { out } - -/// The interface for fetching the module connections though the C++ runtime. -pub fn get_module_connections() -> HashMap { - let args: Args = argh::from_env(); - let raw_connections = ffi::get_module_connections( - &args.module, - &args.prefix.to_string_lossy(), - &args.conf.to_string_lossy(), - ); - - raw_connections - .into_iter() - .map(|connection| (connection.implementation_id, connection.slots)) - .collect() -}