From 751a955dc1878ffe560fd414d483494ada2100db Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 6 Jun 2026 15:57:38 +0800 Subject: [PATCH 1/6] refactor!: rework TaskLocalDiagnostic Signed-off-by: tison --- appenders/file/src/append.rs | 2 +- appenders/syslog/src/lib.rs | 2 +- core/src/kv.rs | 33 +++++++++++ diagnostics/task-local/src/lib.rs | 96 ++++++++++++++----------------- filters/rustlog/src/lib.rs | 2 +- 5 files changed, 79 insertions(+), 56 deletions(-) diff --git a/appenders/file/src/append.rs b/appenders/file/src/append.rs index 06e56c5..f328fa9 100644 --- a/appenders/file/src/append.rs +++ b/appenders/file/src/append.rs @@ -32,7 +32,7 @@ use crate::rotation::Rotation; /// A builder to configure and create an [`File`] appender. /// -/// See [module-level documentation](super) for usage examples. +/// See the [crate documentation](super) for usage examples. #[derive(Debug)] pub struct FileBuilder { builder: RollingFileWriterBuilder, diff --git a/appenders/syslog/src/lib.rs b/appenders/syslog/src/lib.rs index c095a36..3440935 100644 --- a/appenders/syslog/src/lib.rs +++ b/appenders/syslog/src/lib.rs @@ -273,7 +273,7 @@ mod unix_ext { /// An appender that writes log records to syslog. /// -/// See [module-level documentation](self) for usage examples. +/// See the [crate documentation](self) for usage examples. #[derive(Debug)] pub struct Syslog { sender: Mutex, diff --git a/core/src/kv.rs b/core/src/kv.rs index eb1ff5b..d5f36bb 100644 --- a/core/src/kv.rs +++ b/core/src/kv.rs @@ -80,6 +80,19 @@ impl Key<'_> { #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] pub struct KeyOwned(Cow<'static, str>); +macro_rules! impl_key_owned_from { + ($ty:ty) => { + impl From<$ty> for KeyOwned { + fn from(v: $ty) -> Self { + KeyOwned(Cow::from(v)) + } + } + }; +} + +impl_key_owned_from!(&'static str); +impl_key_owned_from!(String); + impl Borrow for KeyOwned { fn borrow(&self) -> &str { &self.0 @@ -513,6 +526,26 @@ enum ValueOwnedState { Map(Box>), } +macro_rules! impl_value_owned_from { + ($ty:ty, $new:ident) => { + impl From<$ty> for ValueOwned { + fn from(v: $ty) -> Self { + Self::$new(v) + } + } + }; +} + +impl_value_owned_from!(bool, bool); +impl_value_owned_from!(i64, i64); +impl_value_owned_from!(u64, u64); +impl_value_owned_from!(f64, f64); +impl_value_owned_from!(i128, i128); +impl_value_owned_from!(u128, u128); +impl_value_owned_from!(char, char); +impl_value_owned_from!(String, str); +impl_value_owned_from!(&'static str, str); + #[cfg(feature = "serde")] impl serde::Serialize for ValueOwned { fn serialize(&self, serializer: S) -> Result { diff --git a/diagnostics/task-local/src/lib.rs b/diagnostics/task-local/src/lib.rs index 58385a7..7574260 100644 --- a/diagnostics/task-local/src/lib.rs +++ b/diagnostics/task-local/src/lib.rs @@ -22,7 +22,7 @@ //! use logforth_diagnostic_task_local::FutureExt; //! //! let fut = async { log::info!("Hello, world!") }; -//! fut.with_task_local_context([("key".into(), "value".into())]); +//! fut.with_task_local_context([("key", "value")]); //! ``` #![cfg_attr(docsrs, feature(doc_cfg))] @@ -35,17 +35,16 @@ use std::task::Poll; use logforth_core::Diagnostic; use logforth_core::Error; -use logforth_core::kv::Key; -use logforth_core::kv::Value; use logforth_core::kv::Visitor; +use logforth_core::kv::{KeyOwned, ValueOwned}; thread_local! { - static TASK_LOCAL_MAP: RefCell> = const { RefCell::new(Vec::new()) }; + static TASK_LOCAL_MAP: RefCell> = const { RefCell::new(Vec::new()) }; } /// A diagnostic that stores key-value pairs in a task-local context. /// -/// See [module-level documentation](self) for usage examples. +/// See the [crate documentation](self) for usage examples. #[derive(Default, Debug, Clone, Copy)] #[non_exhaustive] pub struct TaskLocalDiagnostic {} @@ -55,8 +54,6 @@ impl Diagnostic for TaskLocalDiagnostic { TASK_LOCAL_MAP.with(|map| { let map = map.borrow(); for (key, value) in map.iter() { - let key = Key::borrowed(key.as_str()); - let value = Value::str(value.as_str()); visitor.visit(key.view(), value.view())?; } Ok(()) @@ -66,20 +63,19 @@ impl Diagnostic for TaskLocalDiagnostic { /// An extension trait for futures to run them with a task-local context. /// -/// See [module-level documentation](self) for usage examples. +/// See the [crate documentation](self) for usage examples. pub trait FutureExt: Future { /// Run a future with a task-local context. - fn with_task_local_context( - self, - kvs: impl IntoIterator, - ) -> impl Future + fn with_task_local_context(self, kvs: I) -> impl Future where Self: Sized, + K: Into, + V: Into, + I: IntoIterator, { - TaskLocalFuture { - future: Some(self), - context: kvs.into_iter().collect(), - } + let future = self; + let context = kvs.into_iter().map(|(k, v)| (k.into(), v.into())).collect(); + TaskLocalFuture { future, context } } } @@ -88,8 +84,8 @@ impl FutureExt for F {} #[pin_project::pin_project] struct TaskLocalFuture { #[pin] - future: Option, - context: Vec<(String, String)>, + future: F, + context: Vec<(KeyOwned, ValueOwned)>, } impl Future for TaskLocalFuture { @@ -98,45 +94,39 @@ impl Future for TaskLocalFuture { fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { let this = self.project(); - let mut fut = this.future; - if let Some(future) = fut.as_mut().as_pin_mut() { - struct Guard { - n: usize, + let future = this.future; + + struct Guard { + n: usize, + } + + impl Drop for Guard { + fn drop(&mut self) { + TASK_LOCAL_MAP.with(|map| { + let mut map = map.borrow_mut(); + for _ in 0..self.n { + map.pop(); + } + }); } + } - impl Drop for Guard { - fn drop(&mut self) { - TASK_LOCAL_MAP.with(|map| { - let mut map = map.borrow_mut(); - for _ in 0..self.n { - map.pop(); - } - }); - } + TASK_LOCAL_MAP.with(|map| { + let mut map = map.borrow_mut(); + for (key, value) in this.context.iter() { + map.push((key.clone(), value.clone())); } + }); - TASK_LOCAL_MAP.with(|map| { - let mut map = map.borrow_mut(); - for (key, value) in this.context.iter() { - map.push((key.clone(), value.clone())); - } - }); - - let n = this.context.len(); - let guard = Guard { n }; - - let result = match future.poll(cx) { - Poll::Ready(output) => { - fut.set(None); - Poll::Ready(output) - } - Poll::Pending => Poll::Pending, - }; - - drop(guard); - return result; - } + let n = this.context.len(); + let guard = Guard { n }; + + let result = match future.poll(cx) { + Poll::Ready(output) => Poll::Ready(output), + Poll::Pending => Poll::Pending, + }; - unreachable!("TaskLocalFuture polled after completion"); + drop(guard); + result } } diff --git a/filters/rustlog/src/lib.rs b/filters/rustlog/src/lib.rs index 728d209..d141e4a 100644 --- a/filters/rustlog/src/lib.rs +++ b/filters/rustlog/src/lib.rs @@ -98,7 +98,7 @@ pub const DEFAULT_FILTER_ENV: &str = "RUST_LOG"; /// Less exclusive levels (like `trace` or `info`) are considered to be more verbose than more /// exclusive levels (like `error` or `warn`). /// -/// Read more from the [crate level documentation](self) about the directive syntax and use cases. +/// Read more from the [crate documentation](self) about the directive syntax and use cases. /// /// [`Record`]: logforth_core::record::Record #[derive(Debug)] From 22f984c8f6eb1966d42b402c57c3dffcc68e8618 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 6 Jun 2026 16:01:00 +0800 Subject: [PATCH 2/6] try reduce clone Signed-off-by: tison --- core/src/kv.rs | 4 ++-- diagnostics/task-local/src/lib.rs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/kv.rs b/core/src/kv.rs index d5f36bb..8f049d0 100644 --- a/core/src/kv.rs +++ b/core/src/kv.rs @@ -130,7 +130,7 @@ impl KeyOwned { } /// A borrowed view of a key in a key-value pair. -#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] +#[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)] pub struct KeyView<'a>(RefStr<'a>); impl Borrow for KeyView<'_> { @@ -647,7 +647,7 @@ impl ValueOwned { } /// A borrowed view of a value. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy)] #[non_exhaustive] pub enum ValueView<'a> { /// The absence of a value. diff --git a/diagnostics/task-local/src/lib.rs b/diagnostics/task-local/src/lib.rs index 7574260..975984f 100644 --- a/diagnostics/task-local/src/lib.rs +++ b/diagnostics/task-local/src/lib.rs @@ -35,11 +35,11 @@ use std::task::Poll; use logforth_core::Diagnostic; use logforth_core::Error; -use logforth_core::kv::Visitor; use logforth_core::kv::{KeyOwned, ValueOwned}; +use logforth_core::kv::{KeyView, ValueView, Visitor}; thread_local! { - static TASK_LOCAL_MAP: RefCell> = const { RefCell::new(Vec::new()) }; + static TASK_LOCAL_MAP: RefCell, ValueView<'_>)>> = const { RefCell::new(Vec::new()) }; } /// A diagnostic that stores key-value pairs in a task-local context. @@ -54,7 +54,7 @@ impl Diagnostic for TaskLocalDiagnostic { TASK_LOCAL_MAP.with(|map| { let map = map.borrow(); for (key, value) in map.iter() { - visitor.visit(key.view(), value.view())?; + visitor.visit(*key, *value)?; } Ok(()) }) @@ -114,7 +114,7 @@ impl Future for TaskLocalFuture { TASK_LOCAL_MAP.with(|map| { let mut map = map.borrow_mut(); for (key, value) in this.context.iter() { - map.push((key.clone(), value.clone())); + map.push((key.view(), value.view())); } }); From 9ae80b8a7a97d8b135d6ea3a57f9214ddc6170d0 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 6 Jun 2026 16:40:46 +0800 Subject: [PATCH 3/6] fixup Signed-off-by: tison --- diagnostics/task-local/src/lib.rs | 16 +++++++--------- layouts/google-cloud-logging/src/lib.rs | 2 +- layouts/json/src/lib.rs | 2 +- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/diagnostics/task-local/src/lib.rs b/diagnostics/task-local/src/lib.rs index 975984f..0ca005b 100644 --- a/diagnostics/task-local/src/lib.rs +++ b/diagnostics/task-local/src/lib.rs @@ -35,11 +35,12 @@ use std::task::Poll; use logforth_core::Diagnostic; use logforth_core::Error; -use logforth_core::kv::{KeyOwned, ValueOwned}; -use logforth_core::kv::{KeyView, ValueView, Visitor}; +use logforth_core::kv::KeyOwned; +use logforth_core::kv::ValueOwned; +use logforth_core::kv::Visitor; thread_local! { - static TASK_LOCAL_MAP: RefCell, ValueView<'_>)>> = const { RefCell::new(Vec::new()) }; + static TASK_LOCAL_MAP: RefCell> = const { RefCell::new(Vec::new()) }; } /// A diagnostic that stores key-value pairs in a task-local context. @@ -54,7 +55,7 @@ impl Diagnostic for TaskLocalDiagnostic { TASK_LOCAL_MAP.with(|map| { let map = map.borrow(); for (key, value) in map.iter() { - visitor.visit(*key, *value)?; + visitor.visit(key.view(), value.view())?; } Ok(()) }) @@ -114,17 +115,14 @@ impl Future for TaskLocalFuture { TASK_LOCAL_MAP.with(|map| { let mut map = map.borrow_mut(); for (key, value) in this.context.iter() { - map.push((key.view(), value.view())); + map.push((key.clone(), value.clone())); } }); let n = this.context.len(); let guard = Guard { n }; - let result = match future.poll(cx) { - Poll::Ready(output) => Poll::Ready(output), - Poll::Pending => Poll::Pending, - }; + let result = future.poll(cx); drop(guard); result diff --git a/layouts/google-cloud-logging/src/lib.rs b/layouts/google-cloud-logging/src/lib.rs index 9a93a50..7353b1e 100644 --- a/layouts/google-cloud-logging/src/lib.rs +++ b/layouts/google-cloud-logging/src/lib.rs @@ -152,7 +152,7 @@ impl Visitor for KvCollector<'_> { } } - let value = match serde_json::to_value(&value) { + let value = match serde_json::to_value(value) { Ok(value) => value, Err(_) => value.to_string().into(), }; diff --git a/layouts/json/src/lib.rs b/layouts/json/src/lib.rs index 6afa8a5..8b87abe 100644 --- a/layouts/json/src/lib.rs +++ b/layouts/json/src/lib.rs @@ -113,7 +113,7 @@ struct KvCollector<'a> { impl Visitor for KvCollector<'_> { fn visit(&mut self, key: KeyView, value: ValueView) -> Result<(), Error> { let key = key.to_string(); - match serde_json::to_value(&value) { + match serde_json::to_value(value) { Ok(value) => self.kvs.insert(key, value), Err(_) => self.kvs.insert(key, value.to_string().into()), }; From 437500033b858f85f76d4ed7953d468503f552a7 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 6 Jun 2026 16:58:03 +0800 Subject: [PATCH 4/6] improve Signed-off-by: tison --- Cargo.toml | 1 + core/src/kv.rs | 9 +++++++ diagnostics/task-local/Cargo.toml | 1 + diagnostics/task-local/src/lib.rs | 39 +++++++++++++++++++++++++------ 4 files changed, 43 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 11b5133..b6052f8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -70,6 +70,7 @@ opentelemetry = { version = "0.32.0", default-features = false } opentelemetry-otlp = { version = "0.32.0", default-features = false } opentelemetry_sdk = { version = "0.32.0", default-features = false } pin-project = { version = "1.1.10" } +pollster = { version = "0.4.0" } rand = { version = "0.10.1" } serde = { version = "1.0", features = ["derive"] } serde_json = { version = "1.0" } diff --git a/core/src/kv.rs b/core/src/kv.rs index 8f049d0..f93d50b 100644 --- a/core/src/kv.rs +++ b/core/src/kv.rs @@ -30,6 +30,15 @@ pub trait Visitor { fn visit(&mut self, key: KeyView, value: ValueView) -> Result<(), Error>; } +impl Visitor for F +where + F: FnMut(KeyView, ValueView) -> Result<(), Error>, +{ + fn visit(&mut self, key: KeyView, value: ValueView) -> Result<(), Error> { + self(key, value) + } +} + /// A key in a key-value pair. #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] pub struct Key<'a>(RefStr<'a>); diff --git a/diagnostics/task-local/Cargo.toml b/diagnostics/task-local/Cargo.toml index dda98af..443874e 100644 --- a/diagnostics/task-local/Cargo.toml +++ b/diagnostics/task-local/Cargo.toml @@ -37,6 +37,7 @@ pin-project = { workspace = true } [dev-dependencies] log = { workspace = true } +pollster = { workspace = true } [lints] workspace = true diff --git a/diagnostics/task-local/src/lib.rs b/diagnostics/task-local/src/lib.rs index 0ca005b..cfc71b8 100644 --- a/diagnostics/task-local/src/lib.rs +++ b/diagnostics/task-local/src/lib.rs @@ -30,6 +30,7 @@ use std::cell::RefCell; use std::pin::Pin; +use std::sync::Arc; use std::task::Context; use std::task::Poll; @@ -39,8 +40,10 @@ use logforth_core::kv::KeyOwned; use logforth_core::kv::ValueOwned; use logforth_core::kv::Visitor; +type TaskLocalContext = Arc<[(KeyOwned, ValueOwned)]>; + thread_local! { - static TASK_LOCAL_MAP: RefCell> = const { RefCell::new(Vec::new()) }; + static TASK_LOCAL_MAP: RefCell> = const { RefCell::new(Vec::new()) }; } /// A diagnostic that stores key-value pairs in a task-local context. @@ -54,8 +57,10 @@ impl Diagnostic for TaskLocalDiagnostic { fn visit(&self, visitor: &mut dyn Visitor) -> Result<(), Error> { TASK_LOCAL_MAP.with(|map| { let map = map.borrow(); - for (key, value) in map.iter() { - visitor.visit(key.view(), value.view())?; + for context in map.iter() { + for (key, value) in context.iter() { + visitor.visit(key.view(), value.view())?; + } } Ok(()) }) @@ -86,7 +91,7 @@ impl FutureExt for F {} struct TaskLocalFuture { #[pin] future: F, - context: Vec<(KeyOwned, ValueOwned)>, + context: Arc<[(KeyOwned, ValueOwned)]>, } impl Future for TaskLocalFuture { @@ -114,9 +119,7 @@ impl Future for TaskLocalFuture { TASK_LOCAL_MAP.with(|map| { let mut map = map.borrow_mut(); - for (key, value) in this.context.iter() { - map.push((key.clone(), value.clone())); - } + map.push(this.context.clone()); }); let n = this.context.len(); @@ -128,3 +131,25 @@ impl Future for TaskLocalFuture { result } } + +#[cfg(test)] +mod tests { + use logforth_core::kv::KeyView; + use logforth_core::kv::ValueView; + + use super::*; + + #[test] + fn test_task_local_diagnostic() { + let diag = TaskLocalDiagnostic {}; + let fut = async { + diag.visit(&mut |key: KeyView<'_>, value: ValueView<'_>| { + assert_eq!(key.as_str(), "key"); + assert_eq!(value.to_str().unwrap(), "value"); + Ok(()) + }) + .unwrap(); + }; + pollster::block_on(fut.with_task_local_context([("key", "value")])); + } +} From 153116323ad918c24f989f8456ce860de493da2d Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 6 Jun 2026 17:47:08 +0800 Subject: [PATCH 5/6] view and borrowed value should be Copy Signed-off-by: tison --- core/src/kv.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/kv.rs b/core/src/kv.rs index f93d50b..4447f2b 100644 --- a/core/src/kv.rs +++ b/core/src/kv.rs @@ -40,7 +40,7 @@ where } /// A key in a key-value pair. -#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] +#[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)] pub struct Key<'a>(RefStr<'a>); impl fmt::Display for Key<'_> { @@ -363,10 +363,10 @@ impl<'a> Iterator for MapValueIter<'a> { } /// A borrowed value in a key-value pair. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy)] pub struct Value<'a>(ValueState<'a>); -#[derive(Clone)] +#[derive(Clone, Copy)] enum ValueState<'a> { None, Bool(bool), From d33dab6e9383f5b624262fa13acd4b0a6fba2410 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 6 Jun 2026 17:56:13 +0800 Subject: [PATCH 6/6] fixup Signed-off-by: tison --- diagnostics/task-local/src/lib.rs | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/diagnostics/task-local/src/lib.rs b/diagnostics/task-local/src/lib.rs index cfc71b8..e7285ef 100644 --- a/diagnostics/task-local/src/lib.rs +++ b/diagnostics/task-local/src/lib.rs @@ -102,28 +102,17 @@ impl Future for TaskLocalFuture { let future = this.future; - struct Guard { - n: usize, - } + struct Guard(()); impl Drop for Guard { fn drop(&mut self) { - TASK_LOCAL_MAP.with(|map| { - let mut map = map.borrow_mut(); - for _ in 0..self.n { - map.pop(); - } - }); + TASK_LOCAL_MAP.with(|map| map.borrow_mut().pop()); } } - TASK_LOCAL_MAP.with(|map| { - let mut map = map.borrow_mut(); - map.push(this.context.clone()); - }); + TASK_LOCAL_MAP.with(|map| map.borrow_mut().push(this.context.clone())); - let n = this.context.len(); - let guard = Guard { n }; + let guard = Guard(()); let result = future.poll(cx); @@ -143,13 +132,15 @@ mod tests { fn test_task_local_diagnostic() { let diag = TaskLocalDiagnostic {}; let fut = async { + let mut n = 0; diag.visit(&mut |key: KeyView<'_>, value: ValueView<'_>| { - assert_eq!(key.as_str(), "key"); - assert_eq!(value.to_str().unwrap(), "value"); + n += 1; + assert_eq!(key.as_str(), format!("k{n}")); + assert_eq!(value.to_str().unwrap(), format!("v{n}")); Ok(()) }) .unwrap(); }; - pollster::block_on(fut.with_task_local_context([("key", "value")])); + pollster::block_on(fut.with_task_local_context([("k1", "v1"), ("k2", "v2")])); } }