From 84a274a7813c83bb8a85b5c844b5dc955eff1d76 Mon Sep 17 00:00:00 2001 From: Matilda Smeds Date: Sun, 2 Mar 2025 11:25:10 +0100 Subject: [PATCH 1/6] Add record_all! macro for recording multiple values in one call Currently, Span.record_all() is part of the public API and accepts ValueSet as a parameter. However, constructing a ValueSet is both verbose and undocumented, making it not so practical. To make recording multiple values easier, we introduce a new macro: record_all!, which wraps the Span.record_all() function. As we don't intend anyone to call Span.record_all() directly, we hide it from the documentation. We reference the new macro from Span.record() doc comment instead. The new record_all! macro supports optional formatting sigils % and ?, ensuring a consistent DevEx with the other value-recording macros. --- tracing/src/macros.rs | 47 +++++++++++++++++++++++++++++++++++++++++++ tracing/src/span.rs | 6 ++++++ 2 files changed, 53 insertions(+) diff --git a/tracing/src/macros.rs b/tracing/src/macros.rs index b0791d63f9..b5f975b6ab 100644 --- a/tracing/src/macros.rs +++ b/tracing/src/macros.rs @@ -131,6 +131,53 @@ macro_rules! span { }; } +/// Records multiple values on a span in a single call. As with recording +/// individual values, all fields must be declared when the span is created. +/// +/// This macro supports two optional sigils: +/// - `%` uses the Display implementation. +/// - `?` uses the Debug implementation. +/// For more details, see the [top-level documentation][lib]. +/// +/// [lib]: tracing/#recording-fields +/// +/// # Example +/// +/// ``` +/// # use tracing::{field, info_span, record_all, record_value}; +/// let span = info_span!("my span", field1 = field::Empty, field2 = field::Empty).entered(); +/// record_all!(span, field1 = ?"1", field2 = %"2"); +/// # +/// ``` +#[macro_export] +macro_rules! record_all { + ($span:expr, $($field:ident = $($prefix:tt)? $value:expr),* $(,)?) => { + if let Some(meta) = $span.metadata() { + $span.record_all(&$crate::valueset!( + meta.fields(), + $($field = record_value!($($prefix)? $value)),* + )); + } + }; +} + +/// Processes a value based on an optional sigil, allowing for different +/// formatting options. This approach avoids the potentially less performant +/// tt-muncher pattern while supporting mixing sigils in the same call. +#[doc(hidden)] +#[macro_export] +macro_rules! record_value { + (% $value:expr) => { + stringify!($value) + }; + (? $value:expr) => { + &debug(&$value) as &dyn Value + }; + ($value:expr) => { + $value + }; +} + /// Constructs a span at the trace level. /// /// [Fields] and [attributes] are set using the same syntax as the [`span!`] diff --git a/tracing/src/span.rs b/tracing/src/span.rs index daa4a9ae5e..cff4d21ba5 100644 --- a/tracing/src/span.rs +++ b/tracing/src/span.rs @@ -1191,6 +1191,11 @@ impl Span { /// span.record("parting", "you will be remembered"); /// ``` /// + ///
+ /// 
+    /// **Note**: To record several values in just one call, see the [`record_all!`](crate::record_all!) macro.
+    ///
+ /// /// [`field::Empty`]: super::field::Empty /// [`Metadata`]: super::Metadata pub fn record(&self, field: &Q, value: V) -> &Self @@ -1212,6 +1217,7 @@ impl Span { } /// Records all the fields in the provided `ValueSet`. + #[doc(hidden)] pub fn record_all(&self, values: &field::ValueSet<'_>) -> &Self { let record = Record::new(values); if let Some(ref inner) = self.inner { From d9ddc6dc699499a5dc683e429f54207b93170e2b Mon Sep 17 00:00:00 2001 From: Matilda Smeds Date: Fri, 14 Mar 2025 09:55:24 +0100 Subject: [PATCH 2/6] Update tracing/src/macros.rs Co-authored-by: Hayden Stainsby --- tracing/src/macros.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracing/src/macros.rs b/tracing/src/macros.rs index b5f975b6ab..a83f451c57 100644 --- a/tracing/src/macros.rs +++ b/tracing/src/macros.rs @@ -141,7 +141,7 @@ macro_rules! span { /// /// [lib]: tracing/#recording-fields /// -/// # Example +/// # Examples /// /// ``` /// # use tracing::{field, info_span, record_all, record_value}; From a6564a057636537fc15aa8e03917a6c9f14a140a Mon Sep 17 00:00:00 2001 From: Matilda Smeds Date: Tue, 18 Mar 2025 17:46:42 +0100 Subject: [PATCH 3/6] Fix Clippy warning --- tracing/src/macros.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tracing/src/macros.rs b/tracing/src/macros.rs index a83f451c57..802d11e270 100644 --- a/tracing/src/macros.rs +++ b/tracing/src/macros.rs @@ -137,6 +137,7 @@ macro_rules! span { /// This macro supports two optional sigils: /// - `%` uses the Display implementation. /// - `?` uses the Debug implementation. +/// /// For more details, see the [top-level documentation][lib]. /// /// [lib]: tracing/#recording-fields From f80475bf27150a0ab4679655f2d1097e2ee1bc87 Mon Sep 17 00:00:00 2001 From: Matilda Smeds Date: Sat, 22 Mar 2025 06:18:31 +0100 Subject: [PATCH 4/6] As agreed, here is a simple test that verifies the value being recorded. When developing the test, I realized that the previous iteration would not support plain primitive values, such as integers or booleans. Given that span! macro supports that, I thought it to be reasonable for record_all! to do so as well. This version does. It is also simpler than the original. Looking forward to feedback! --- tracing/src/macros.rs | 27 +++++---------------------- tracing/tests/span.rs | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 24 deletions(-) diff --git a/tracing/src/macros.rs b/tracing/src/macros.rs index 802d11e270..6ec3f3c358 100644 --- a/tracing/src/macros.rs +++ b/tracing/src/macros.rs @@ -145,40 +145,23 @@ macro_rules! span { /// # Examples /// /// ``` -/// # use tracing::{field, info_span, record_all, record_value}; -/// let span = info_span!("my span", field1 = field::Empty, field2 = field::Empty).entered(); -/// record_all!(span, field1 = ?"1", field2 = %"2"); +/// # use tracing::{field, info_span, record_all}; +/// let span = info_span!("my span", field1 = field::Empty, field2 = field::Empty, field3 = field::Empty).entered(); +/// record_all!(span, field1 = ?"1", field2 = %"2", field3 = 3); /// # /// ``` #[macro_export] macro_rules! record_all { - ($span:expr, $($field:ident = $($prefix:tt)? $value:expr),* $(,)?) => { + ($span:expr, $($fields:tt)*) => { if let Some(meta) = $span.metadata() { $span.record_all(&$crate::valueset!( meta.fields(), - $($field = record_value!($($prefix)? $value)),* + $($fields)* )); } }; } -/// Processes a value based on an optional sigil, allowing for different -/// formatting options. This approach avoids the potentially less performant -/// tt-muncher pattern while supporting mixing sigils in the same call. -#[doc(hidden)] -#[macro_export] -macro_rules! record_value { - (% $value:expr) => { - stringify!($value) - }; - (? $value:expr) => { - &debug(&$value) as &dyn Value - }; - ($value:expr) => { - $value - }; -} - /// Constructs a span at the trace level. /// /// [Fields] and [attributes] are set using the same syntax as the [`span!`] diff --git a/tracing/tests/span.rs b/tracing/tests/span.rs index bc0798f162..ecc2dcd68c 100644 --- a/tracing/tests/span.rs +++ b/tracing/tests/span.rs @@ -8,8 +8,8 @@ use std::thread; use tracing::{ collect::with_default, error_span, - field::{debug, display}, - Level, Span, + field::{debug, display, Empty}, + record_all, Level, Span, }; use tracing_mock::*; @@ -612,6 +612,38 @@ fn record_new_values_for_fields() { handle.assert_finished(); } +/// Tests record_all! macro, which is a wrapper for Span.record_all(). +/// Placed here instead of tests/macros.rs, because it uses tracing_mock, which +/// requires std lib. Other macro tests exclude std lib to verify the macros do +/// not dependend on it. +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] +#[test] +fn record_all_macro_records_new_values_for_fields() { + let (collector, handle) = collector::mock() + .new_span( + expect::span() + .named("foo") + .with_fields(expect::field("bar")), + ) + .record( + expect::span().named("foo"), + expect::field("bar").with_value(&5).only(), + ) + .enter(expect::span().named("foo")) + .exit(expect::span().named("foo")) + .drop_span(expect::span().named("foo")) + .only() + .run_with_handle(); + + with_default(collector, || { + let span = tracing::span!(Level::TRACE, "foo", bar = 1); + record_all!(span, bar = 5); + span.in_scope(|| {}) + }); + + handle.assert_finished(); +} + #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] #[test] fn new_span_with_target_and_log_level() { From cab7044be405d30ee1368a64231bc66711fd5ccb Mon Sep 17 00:00:00 2001 From: Matilda Smeds Date: Wed, 26 Mar 2025 09:57:39 +0100 Subject: [PATCH 5/6] Update tracing/src/macros.rs Co-authored-by: Hayden Stainsby --- tracing/src/macros.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tracing/src/macros.rs b/tracing/src/macros.rs index 6ec3f3c358..21cb04c83e 100644 --- a/tracing/src/macros.rs +++ b/tracing/src/macros.rs @@ -148,7 +148,6 @@ macro_rules! span { /// # use tracing::{field, info_span, record_all}; /// let span = info_span!("my span", field1 = field::Empty, field2 = field::Empty, field3 = field::Empty).entered(); /// record_all!(span, field1 = ?"1", field2 = %"2", field3 = 3); -/// # /// ``` #[macro_export] macro_rules! record_all { From f93578441be9e473fb807397afed00e2d4642f66 Mon Sep 17 00:00:00 2001 From: Matilda Smeds Date: Wed, 26 Mar 2025 20:42:40 +0100 Subject: [PATCH 6/6] Make record_all test cover multiple values --- tracing/tests/span.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/tracing/tests/span.rs b/tracing/tests/span.rs index ecc2dcd68c..f2b5e5eccd 100644 --- a/tracing/tests/span.rs +++ b/tracing/tests/span.rs @@ -627,7 +627,12 @@ fn record_all_macro_records_new_values_for_fields() { ) .record( expect::span().named("foo"), - expect::field("bar").with_value(&5).only(), + expect::field("bar") + .with_value(&5) + .and(expect::field("baz").with_value(&"BAZ")) + .and(expect::field("qux").with_value(&display("qux"))) + .and(expect::field("quux").with_value(&debug("QuuX"))) + .only(), ) .enter(expect::span().named("foo")) .exit(expect::span().named("foo")) @@ -636,8 +641,15 @@ fn record_all_macro_records_new_values_for_fields() { .run_with_handle(); with_default(collector, || { - let span = tracing::span!(Level::TRACE, "foo", bar = 1); - record_all!(span, bar = 5); + let span = tracing::span!( + Level::TRACE, + "foo", + bar = 1, + baz = 2, + qux = Empty, + quux = Empty + ); + record_all!(span, bar = 5, baz = "BAZ", qux = %"qux", quux = ?"QuuX"); span.in_scope(|| {}) });