From 409e17ed4a075f93c61c6e04a77604c05eacfc0c Mon Sep 17 00:00:00 2001 From: Victor Polevoy Date: Mon, 1 May 2023 15:54:51 +0200 Subject: [PATCH 1/2] Provide the standard logging facility implementation. As the crate is mostly used as a redis module, the logging will most likely be only useful when printed on the server. As many rustaceans are familiar and used to using the de-facto standard "log" crate, it makes sense to provide the logging implementation for this crate to enable the most-commonly used way of logging messages in Rust. --- Cargo.toml | 1 + examples/load_unload.rs | 6 +- src/context/call_reply.rs | 23 +++---- src/context/mod.rs | 23 +++---- src/lib.rs | 13 ---- src/logging.rs | 128 ++++++++++++++++++++++++++++++++------ 6 files changed, 134 insertions(+), 60 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ac8e4d0f..546c012f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -104,6 +104,7 @@ serde = { version = "1", features = ["derive"] } nix = "0.26" cfg-if = "1" redis-module-macros-internals = { path = "./redismodule-rs-macros-internals" } +log = "0.4" [dev-dependencies] anyhow = "1" diff --git a/examples/load_unload.rs b/examples/load_unload.rs index 976955f2..86f7787a 100644 --- a/examples/load_unload.rs +++ b/examples/load_unload.rs @@ -1,4 +1,4 @@ -use redis_module::{redis_module, Context, LogLevel, RedisString, Status}; +use redis_module::{logging::RedisLogLevel, redis_module, Context, RedisString, Status}; static mut GLOBAL_STATE: Option = None; @@ -10,7 +10,7 @@ fn init(ctx: &Context, args: &[RedisString]) -> Status { (before, after) }; ctx.log( - LogLevel::Warning, + RedisLogLevel::Warning, &format!("Update global state on LOAD. BEFORE: {before:?}, AFTER: {after:?}",), ); @@ -24,7 +24,7 @@ fn deinit(ctx: &Context) -> Status { (before, after) }; ctx.log( - LogLevel::Warning, + RedisLogLevel::Warning, &format!("Update global state on UNLOAD. BEFORE: {before:?}, AFTER: {after:?}"), ); diff --git a/src/context/call_reply.rs b/src/context/call_reply.rs index fa709fb6..8b00eb47 100644 --- a/src/context/call_reply.rs +++ b/src/context/call_reply.rs @@ -548,20 +548,15 @@ impl TryFrom<&str> for VerbatimStringFormat { ))); } let mut res = VerbatimStringFormat::default(); - value - .chars() - .into_iter() - .take(3) - .enumerate() - .try_for_each(|(i, c)| { - if c as u32 >= 127 { - return Err(RedisError::String( - "Verbatim format must contains only ASCI values.".to_owned(), - )); - } - res.0[i] = c as c_char; - Ok(()) - })?; + value.chars().take(3).enumerate().try_for_each(|(i, c)| { + if c as u32 >= 127 { + return Err(RedisError::String( + "Verbatim format must contains only ASCI values.".to_owned(), + )); + } + res.0[i] = c as c_char; + Ok(()) + })?; Ok(res) } } diff --git a/src/context/mod.rs b/src/context/mod.rs index 35604e0c..0d0dfa1e 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -6,11 +6,12 @@ use std::os::raw::{c_char, c_int, c_long, c_longlong}; use std::ptr::{self, NonNull}; use std::sync::atomic::{AtomicPtr, Ordering}; +use crate::add_info_section; use crate::key::{RedisKey, RedisKeyWritable}; +use crate::logging::RedisLogLevel; use crate::raw::{ModuleOptions, Version}; use crate::redisvalue::RedisValueKey; use crate::{add_info_field_long_long, add_info_field_str, raw, utils, Status}; -use crate::{add_info_section, LogLevel}; use crate::{RedisError, RedisResult, RedisString, RedisValue}; use std::ffi::CStr; @@ -135,25 +136,25 @@ impl Default for DetachedContext { } impl DetachedContext { - pub fn log(&self, level: LogLevel, message: &str) { + pub fn log(&self, level: RedisLogLevel, message: &str) { let c = self.ctx.load(Ordering::Relaxed); crate::logging::log_internal(c, level, message); } pub fn log_debug(&self, message: &str) { - self.log(LogLevel::Debug, message); + self.log(RedisLogLevel::Debug, message); } pub fn log_notice(&self, message: &str) { - self.log(LogLevel::Notice, message); + self.log(RedisLogLevel::Notice, message); } pub fn log_verbose(&self, message: &str) { - self.log(LogLevel::Verbose, message); + self.log(RedisLogLevel::Verbose, message); } pub fn log_warning(&self, message: &str) { - self.log(LogLevel::Warning, message); + self.log(RedisLogLevel::Warning, message); } pub fn set_context(&self, ctx: &Context) -> Result<(), RedisError> { @@ -266,24 +267,24 @@ impl Context { } } - pub fn log(&self, level: LogLevel, message: &str) { + pub fn log(&self, level: RedisLogLevel, message: &str) { crate::logging::log_internal(self.ctx, level, message); } pub fn log_debug(&self, message: &str) { - self.log(LogLevel::Debug, message); + self.log(RedisLogLevel::Debug, message); } pub fn log_notice(&self, message: &str) { - self.log(LogLevel::Notice, message); + self.log(RedisLogLevel::Notice, message); } pub fn log_verbose(&self, message: &str) { - self.log(LogLevel::Verbose, message); + self.log(RedisLogLevel::Verbose, message); } pub fn log_warning(&self, message: &str) { - self.log(LogLevel::Warning, message); + self.log(RedisLogLevel::Warning, message); } /// # Panics diff --git a/src/lib.rs b/src/lib.rs index b6ca34bf..acbce8aa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,7 +1,4 @@ -//#![allow(dead_code)] - pub use crate::context::InfoContext; -use strum_macros::AsRefStr; extern crate num_traits; pub mod alloc; @@ -44,16 +41,6 @@ pub use crate::raw::*; pub use crate::redismodule::*; use backtrace::Backtrace; -/// `LogLevel` is a level of logging to be specified with a Redis log directive. -#[derive(Clone, Copy, Debug, AsRefStr)] -#[strum(serialize_all = "snake_case")] -pub enum LogLevel { - Debug, - Notice, - Verbose, - Warning, -} - pub fn base_info_func( ctx: &InfoContext, for_crash_report: bool, diff --git a/src/logging.rs b/src/logging.rs index 656fc8d2..e4f2c95d 100644 --- a/src/logging.rs +++ b/src/logging.rs @@ -1,53 +1,143 @@ use crate::raw; -use crate::LogLevel; use std::ffi::CString; use std::ptr; +use strum_macros::AsRefStr; -pub(crate) fn log_internal(ctx: *mut raw::RedisModuleCtx, level: LogLevel, message: &str) { +const NOT_INITIALISED_MESSAGE: &str = "Redis module hasn't been initialised."; + +/// [RedisLogLevel] is a level of logging which can be used when +/// logging with Redis. See [raw::RedisModule_Log] and the official +/// redis [reference](https://redis.io/docs/reference/modules/modules-api-ref/). +#[derive(Clone, Copy, Debug, AsRefStr)] +#[strum(serialize_all = "snake_case")] +pub enum RedisLogLevel { + Debug, + Notice, + Verbose, + Warning, +} + +pub(crate) fn log_internal(ctx: *mut raw::RedisModuleCtx, level: RedisLogLevel, message: &str) { if cfg!(test) { return; } let level = CString::new(level.as_ref()).unwrap(); let fmt = CString::new(message).unwrap(); - unsafe { raw::RedisModule_Log.unwrap()(ctx, level.as_ptr(), fmt.as_ptr()) } + unsafe { + raw::RedisModule_Log.expect(NOT_INITIALISED_MESSAGE)(ctx, level.as_ptr(), fmt.as_ptr()) + } } /// This function should be used when a callback is returning a critical error /// to the caller since cannot load or save the data for some critical reason. #[allow(clippy::not_unsafe_ptr_arg_deref)] -pub fn log_io_error(io: *mut raw::RedisModuleIO, level: LogLevel, message: &str) { +pub fn log_io_error(io: *mut raw::RedisModuleIO, level: RedisLogLevel, message: &str) { if cfg!(test) { return; } let level = CString::new(level.as_ref()).unwrap(); let fmt = CString::new(message).unwrap(); - unsafe { raw::RedisModule_LogIOError.unwrap()(io, level.as_ptr(), fmt.as_ptr()) } + unsafe { + raw::RedisModule_LogIOError.expect(NOT_INITIALISED_MESSAGE)( + io, + level.as_ptr(), + fmt.as_ptr(), + ) + } } /// Log a message to the Redis log with the given log level, without /// requiring a context. This prevents Redis from including the module /// name in the logged message. -pub fn log(level: LogLevel, message: &str) { - log_internal(ptr::null_mut(), level, message); +pub fn log>(level: RedisLogLevel, message: T) { + log_internal(ptr::null_mut(), level, message.as_ref()); } -/// Log a message to the Redis log with DEBUG log level. -pub fn log_debug(message: &str) { - log(LogLevel::Debug, message); +/// Log a message to Redis at the [RedisLogLevel::Debug] level. +pub fn log_debug>(message: T) { + log(RedisLogLevel::Debug, message.as_ref()); } -/// Log a message to the Redis log with NOTICE log level. -pub fn log_notice(message: &str) { - log(LogLevel::Notice, message); +/// Log a message to Redis at the [RedisLogLevel::Notice] level. +pub fn log_notice>(message: T) { + log(RedisLogLevel::Notice, message.as_ref()); } -/// Log a message to the Redis log with VERBOSE log level. -pub fn log_verbose(message: &str) { - log(LogLevel::Verbose, message); +/// Log a message to Redis at the [RedisLogLevel::Verbose] level. +pub fn log_verbose>(message: T) { + log(RedisLogLevel::Verbose, message.as_ref()); } -/// Log a message to the Redis log with WARNING log level. -pub fn log_warning(message: &str) { - log(LogLevel::Warning, message); +/// Log a message to Redis at the [RedisLogLevel::Warning] level. +pub fn log_warning>(message: T) { + log(RedisLogLevel::Warning, message.as_ref()); +} + +/// The [log] crate implementation of logging. +pub mod standard_log_implementation { + use super::*; + use log::{Level, Metadata, Record, SetLoggerError}; + + static LOGGER: RedisGlobalLogger = RedisGlobalLogger; + + /// The struct which has an implementation of the [log] crate's + /// logging interface. + /// + /// # Note + /// + /// Redis does not support logging at the [log::Level::Error] level, + /// so logging at this level will be converted to logging at the + /// [log::Level::Warn] level under the hood. + struct RedisGlobalLogger; + + /// Sets this logger as a global logger. Use this method to set + /// up the logger. If this method is never called, the default + /// logger is used which redirects the logging to the standard + /// input/output streams. + /// + /// # Example + /// + /// This function may be called on a module startup, within the + /// module initialisation function (specified in the + /// [crate::redis_module] as the `init` argument, which will be used + /// for the module initialisation and will be passed to the + /// [raw::Export_RedisModule_Init] function when loading the + /// module). + #[allow(dead_code)] + pub fn setup() -> Result<(), SetLoggerError> { + log::set_logger(&LOGGER).map(|()| log::set_max_level(log::LevelFilter::Trace)) + } + + impl log::Log for RedisGlobalLogger { + fn enabled(&self, _: &Metadata) -> bool { + true + } + + fn log(&self, record: &Record) { + if !self.enabled(record.metadata()) { + return; + } + + let message = format!( + "'{}' {}:{}: {}", + record.module_path().unwrap_or_default(), + record.file().unwrap_or("Unknown"), + record.line().unwrap_or(0), + record.args() + ); + + match record.level() { + Level::Debug => log_debug(message), + Level::Error | Level::Warn => log_warning(message), + Level::Info => log_notice(message), + Level::Trace => log_verbose(message), + } + } + + fn flush(&self) { + // The flushing isn't required for the Redis logging. + } + } } +pub use standard_log_implementation::*; From de65b4ee1080f76fb86a10d74e09b6cbe75565cd Mon Sep 17 00:00:00 2001 From: Victor Polevoy Date: Thu, 4 May 2023 10:45:46 +0200 Subject: [PATCH 2/2] Use the module context for the standard logging. --- src/context/mod.rs | 12 ++++-- src/lib.rs | 6 +++ src/logging.rs | 100 +++++++++++++++++++++++++++++++++++---------- src/macros.rs | 3 ++ 4 files changed, 97 insertions(+), 24 deletions(-) diff --git a/src/context/mod.rs b/src/context/mod.rs index 0d0dfa1e..1dd06333 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -124,17 +124,23 @@ impl CallOptionsBuilder { /// It is implemented `Send` and `Sync` so it can safely be used /// from within different threads. pub struct DetachedContext { - ctx: AtomicPtr, + pub(crate) ctx: AtomicPtr, } -impl Default for DetachedContext { - fn default() -> Self { +impl DetachedContext { + pub const fn new() -> Self { DetachedContext { ctx: AtomicPtr::new(ptr::null_mut()), } } } +impl Default for DetachedContext { + fn default() -> Self { + Self::new() + } +} + impl DetachedContext { pub fn log(&self, level: RedisLogLevel, message: &str) { let c = self.ctx.load(Ordering::Relaxed); diff --git a/src/lib.rs b/src/lib.rs index acbce8aa..f83d5895 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -41,6 +41,12 @@ pub use crate::raw::*; pub use crate::redismodule::*; use backtrace::Backtrace; +/// The detached Redis module context (the context of this module). It +/// is only set to a proper value after the module is initialised via the +/// provided [redis_module] macro. +/// See [DetachedContext]. +pub static MODULE_CONTEXT: DetachedContext = DetachedContext::new(); + pub fn base_info_func( ctx: &InfoContext, for_crash_report: bool, diff --git a/src/logging.rs b/src/logging.rs index e4f2c95d..cb5a1190 100644 --- a/src/logging.rs +++ b/src/logging.rs @@ -17,12 +17,27 @@ pub enum RedisLogLevel { Warning, } -pub(crate) fn log_internal(ctx: *mut raw::RedisModuleCtx, level: RedisLogLevel, message: &str) { +impl From for RedisLogLevel { + fn from(value: log::Level) -> Self { + match value { + log::Level::Error | log::Level::Warn => Self::Warning, + log::Level::Info => Self::Notice, + log::Level::Debug => Self::Debug, + log::Level::Trace => Self::Verbose, + } + } +} + +pub(crate) fn log_internal>( + ctx: *mut raw::RedisModuleCtx, + level: L, + message: &str, +) { if cfg!(test) { return; } - let level = CString::new(level.as_ref()).unwrap(); + let level = CString::new(level.into().as_ref()).unwrap(); let fmt = CString::new(message).unwrap(); unsafe { raw::RedisModule_Log.expect(NOT_INITIALISED_MESSAGE)(ctx, level.as_ptr(), fmt.as_ptr()) @@ -76,10 +91,14 @@ pub fn log_warning>(message: T) { /// The [log] crate implementation of logging. pub mod standard_log_implementation { + use std::sync::atomic::Ordering; + + use crate::RedisError; + use super::*; - use log::{Level, Metadata, Record, SetLoggerError}; + use log::{Metadata, Record, SetLoggerError}; - static LOGGER: RedisGlobalLogger = RedisGlobalLogger; + static mut LOGGER: RedisGlobalLogger = RedisGlobalLogger(ptr::null_mut()); /// The struct which has an implementation of the [log] crate's /// logging interface. @@ -89,13 +108,32 @@ pub mod standard_log_implementation { /// Redis does not support logging at the [log::Level::Error] level, /// so logging at this level will be converted to logging at the /// [log::Level::Warn] level under the hood. - struct RedisGlobalLogger; + struct RedisGlobalLogger(*mut raw::RedisModuleCtx); + + // The pointer of the Global logger can only be changed once during + // the startup. Once one of the [std::sync::OnceLock] or + // [std::sync::OnceCell] is stabilised, we can remove these unsafe + // trait implementations in favour of using the aforementioned safe + // types. + unsafe impl Send for RedisGlobalLogger {} + unsafe impl Sync for RedisGlobalLogger {} /// Sets this logger as a global logger. Use this method to set /// up the logger. If this method is never called, the default /// logger is used which redirects the logging to the standard /// input/output streams. /// + /// # Note + /// + /// The logging context (the module context [raw::RedisModuleCtx]) + /// is set by the [crate::redis_module] macro. If another context + /// should be used, please consider using the [setup_for_context] + /// method instead. + /// + /// In case this function is invoked before the initialisation, and + /// so without the redis module context, no context will be used for + /// the logging, however, the logger will be set. + /// /// # Example /// /// This function may be called on a module startup, within the @@ -105,8 +143,22 @@ pub mod standard_log_implementation { /// [raw::Export_RedisModule_Init] function when loading the /// module). #[allow(dead_code)] - pub fn setup() -> Result<(), SetLoggerError> { - log::set_logger(&LOGGER).map(|()| log::set_max_level(log::LevelFilter::Trace)) + pub fn setup() -> Result<(), RedisError> { + let pointer = crate::MODULE_CONTEXT.ctx.load(Ordering::Relaxed); + if pointer.is_null() { + return Err(RedisError::Str(NOT_INITIALISED_MESSAGE)); + } + setup_for_context(pointer) + .map_err(|e| RedisError::String(format!("Couldn't set up the logger: {e}"))) + } + + /// The same as [setup] but sets the custom module context. + #[allow(dead_code)] + pub fn setup_for_context(context: *mut raw::RedisModuleCtx) -> Result<(), SetLoggerError> { + unsafe { + LOGGER.0 = context; + log::set_logger(&LOGGER).map(|()| log::set_max_level(log::LevelFilter::Trace)) + } } impl log::Log for RedisGlobalLogger { @@ -119,20 +171,26 @@ pub mod standard_log_implementation { return; } - let message = format!( - "'{}' {}:{}: {}", - record.module_path().unwrap_or_default(), - record.file().unwrap_or("Unknown"), - record.line().unwrap_or(0), - record.args() - ); - - match record.level() { - Level::Debug => log_debug(message), - Level::Error | Level::Warn => log_warning(message), - Level::Info => log_notice(message), - Level::Trace => log_verbose(message), - } + let message = match record.level() { + log::Level::Debug | log::Level::Trace => { + format!( + "'{}' {}:{}: {}", + record.module_path().unwrap_or_default(), + record.file().unwrap_or("Unknown"), + record.line().unwrap_or(0), + record.args() + ) + } + _ => { + format!( + "'{}' {}", + record.module_path().unwrap_or_default(), + record.args() + ) + } + }; + + log_internal(self.0, record.level(), &message); } fn flush(&self) { diff --git a/src/macros.rs b/src/macros.rs index 27f4a721..a3cda082 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -212,6 +212,9 @@ macro_rules! redis_module { ) } == raw::Status::Err as c_int { return raw::Status::Err as c_int; } let context = $crate::Context::new(ctx); + unsafe { + let _ = $crate::MODULE_CONTEXT.set_context(&context); + } let args = $crate::decode_args(ctx, argv, argc); $(