Skip to content

tracing: fix record_all panic #3432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
conradludgate wants to merge 7 commits into
base: main
Choose a base branch
from
+421 −109
Open

tracing: fix record_all panic #3432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
442a574
replace valueset with valueset_all
conradludgate Nov 28, 2025
2b26d41
restore original valueset from before #3398
conradludgate Nov 28, 2025
5e95522
add regression test
conradludgate Nov 28, 2025
fe8b887
fix record_all
conradludgate Nov 28, 2025
38c912c
add refined tests
conradludgate Dec 2, 2025
82d260a
allow explicit valuesets to have optional fields. this breaks the pri…
conradludgate Dec 9, 2025
609f97c
fix temporary lifetimes
conradludgate Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 66 additions & 35 deletions tracing-core/src/field.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ pub struct ValueSet<'a> {
enum Values<'a> {
/// A set of field-value pairs. Fields may be for the wrong field set, some
/// fields may be missing, and fields may be in any order.
Explicit(&'a [(&'a Field, Option<&'a (dyn Value + 'a)>)]),
Explicit(&'a [(Option<&'a Field>, Option<&'a (dyn Value + 'a)>)]),
/// A list of values corresponding exactly to the fields in a `FieldSet`.
All(&'a [Option<&'a (dyn Value + 'a)>]),
}
Expand Down Expand Up @@ -1060,11 +1060,13 @@ impl ValueSet<'_> {
Values::Explicit(values) => {
let my_callsite = self.callsite();
for (field, value) in values {
if field.callsite() != my_callsite {
continue;
}
if let Some(value) = *value {
value.record(field, visitor);
if let Some(field) = field {
if field.callsite() != my_callsite {
continue;
}
if let Some(value) = *value {
value.record(field, visitor);
}
}
}
}
Expand All @@ -1089,7 +1091,8 @@ impl ValueSet<'_> {
let my_callsite = self.callsite();
values
.iter()
.filter(|(field, _)| field.callsite() == my_callsite)
.filter_map(|(field, _)| *field)
.filter(|field| field.callsite() == my_callsite)
.count()
}
Values::All(values) => values.len(),
Expand All @@ -1104,7 +1107,9 @@ impl ValueSet<'_> {
match self.values {
Values::Explicit(values) => values
.iter()
.any(|(key, val)| *key == field && val.is_some()),
.filter(|(_, val)| val.is_some())
.filter_map(|(key, _)| *key)
.any(|key| key == field),
Values::All(values) => values[field.i].is_some(),
}
}
Expand All @@ -1117,7 +1122,9 @@ impl ValueSet<'_> {
let my_callsite = self.callsite();
values
.iter()
.all(|(key, val)| val.is_none() || key.callsite() != my_callsite)
.filter(|(_, val)| val.is_some())
.filter_map(|(key, _)| *key)
.all(|key| key.callsite() != my_callsite)
}
}
}
Expand Down Expand Up @@ -1149,9 +1156,9 @@ mod private {
use super::*;

/// Restrictions on `ValueSet` lengths were removed in #2508 but this type remains for backwards compatibility.
pub trait ValidLen<'a>: Borrow<[(&'a Field, Option<&'a (dyn Value + 'a)>)]> {}
pub trait ValidLen<'a>: Borrow<[(Option<&'a Field>, Option<&'a (dyn Value + 'a)>)]> {}

impl<'a, const N: usize> ValidLen<'a> for [(&'a Field, Option<&'a (dyn Value + 'a)>); N] {}
impl<'a, const N: usize> ValidLen<'a> for [(Option<&'a Field>, Option<&'a (dyn Value + 'a)>); N] {}
}

#[cfg(test)]
Expand Down Expand Up @@ -1213,9 +1220,9 @@ mod test {
fn value_set_with_no_values_is_empty() {
let fields = TEST_META_1.fields();
let values = &[
(&fields.field("foo").unwrap(), None),
(&fields.field("bar").unwrap(), None),
(&fields.field("baz").unwrap(), None),
(Some(&fields.field("foo").unwrap()), None),
(Some(&fields.field("bar").unwrap()), None),
(Some(&fields.field("baz").unwrap()), None),
];
let valueset = fields.value_set(values);
assert!(valueset.is_empty());
Expand Down Expand Up @@ -1243,9 +1250,9 @@ mod test {
fn value_sets_with_fields_from_other_callsites_are_empty() {
let fields = TEST_META_1.fields();
let values = &[
(&fields.field("foo").unwrap(), Some(&1 as &dyn Value)),
(&fields.field("bar").unwrap(), Some(&2 as &dyn Value)),
(&fields.field("baz").unwrap(), Some(&3 as &dyn Value)),
(Some(&fields.field("foo").unwrap()), Some(&1 as &dyn Value)),
(Some(&fields.field("bar").unwrap()), Some(&2 as &dyn Value)),
(Some(&fields.field("baz").unwrap()), Some(&3 as &dyn Value)),
];
let valueset = TEST_META_2.fields().value_set(values);
assert!(valueset.is_empty())
Expand All @@ -1255,9 +1262,9 @@ mod test {
fn sparse_value_sets_are_not_empty() {
let fields = TEST_META_1.fields();
let values = &[
(&fields.field("foo").unwrap(), None),
(&fields.field("bar").unwrap(), Some(&57 as &dyn Value)),
(&fields.field("baz").unwrap(), None),
(Some(&fields.field("foo").unwrap()), None),
(Some(&fields.field("bar").unwrap()), Some(&57 as &dyn Value)),
(Some(&fields.field("baz").unwrap()), None),
];
let valueset = fields.value_set(values);
assert!(!valueset.is_empty());
Expand All @@ -1267,12 +1274,12 @@ mod test {
fn fields_from_other_callsets_are_skipped() {
let fields = TEST_META_1.fields();
let values = &[
(&fields.field("foo").unwrap(), None),
(Some(&fields.field("foo").unwrap()), None),
(
&TEST_META_2.fields().field("bar").unwrap(),
Some(&TEST_META_2.fields().field("bar").unwrap()),
Some(&57 as &dyn Value),
),
(&fields.field("baz").unwrap(), None),
(Some(&fields.field("baz").unwrap()), None),
];

struct MyVisitor;
Expand All @@ -1289,9 +1296,15 @@ mod test {
fn empty_fields_are_skipped() {
let fields = TEST_META_1.fields();
let values = &[
(&fields.field("foo").unwrap(), Some(&Empty as &dyn Value)),
(&fields.field("bar").unwrap(), Some(&57 as &dyn Value)),
(&fields.field("baz").unwrap(), Some(&Empty as &dyn Value)),
(
Some(&fields.field("foo").unwrap()),
Some(&Empty as &dyn Value),
),
(Some(&fields.field("bar").unwrap()), Some(&57 as &dyn Value)),
(
Some(&fields.field("baz").unwrap()),
Some(&Empty as &dyn Value),
),
];

struct MyVisitor;
Expand All @@ -1308,9 +1321,9 @@ mod test {
fn record_debug_fn() {
let fields = TEST_META_1.fields();
let values = &[
(&fields.field("foo").unwrap(), Some(&1 as &dyn Value)),
(&fields.field("bar").unwrap(), Some(&2 as &dyn Value)),
(&fields.field("baz").unwrap(), Some(&3 as &dyn Value)),
(Some(&fields.field("foo").unwrap()), Some(&1 as &dyn Value)),
(Some(&fields.field("bar").unwrap()), Some(&2 as &dyn Value)),
(Some(&fields.field("baz").unwrap()), Some(&3 as &dyn Value)),
];
let valueset = fields.value_set(values);
let mut result = String::new();
Expand All @@ -1328,9 +1341,18 @@ mod test {
let err: Box<dyn std::error::Error + Send + Sync + 'static> =
std::io::Error::new(std::io::ErrorKind::Other, "lol").into();
let values = &[
(&fields.field("foo").unwrap(), Some(&err as &dyn Value)),
(&fields.field("bar").unwrap(), Some(&Empty as &dyn Value)),
(&fields.field("baz").unwrap(), Some(&Empty as &dyn Value)),
(
Some(&fields.field("foo").unwrap()),
Some(&err as &dyn Value),
),
(
Some(&fields.field("bar").unwrap()),
Some(&Empty as &dyn Value),
),
(
Some(&fields.field("baz").unwrap()),
Some(&Empty as &dyn Value),
),
];
let valueset = fields.value_set(values);
let mut result = String::new();
Expand All @@ -1347,9 +1369,18 @@ mod test {
let first = &b"abc"[..];
let second: &[u8] = &[192, 255, 238];
let values = &[
(&fields.field("foo").unwrap(), Some(&first as &dyn Value)),
(&fields.field("bar").unwrap(), Some(&" " as &dyn Value)),
(&fields.field("baz").unwrap(), Some(&second as &dyn Value)),
(
Some(&fields.field("foo").unwrap()),
Some(&first as &dyn Value),
),
(
Some(&fields.field("bar").unwrap()),
Some(&" " as &dyn Value),
),
(
Some(&fields.field("baz").unwrap()),
Some(&second as &dyn Value),
),
];
let valueset = fields.value_set(values);
let mut result = String::new();
Expand Down
4 changes: 2 additions & 2 deletions tracing-core/tests/missed_register_callsite.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ fn subscriber_thread(idx: usize, register_sleep_micros: u64) -> JoinHandle<()> {
let _interest = CALLSITE.interest();

let meta = CALLSITE.metadata();
let field = meta.fields().field("message").unwrap();
let field = meta.fields().field("message");
let message = format!("event-from-{idx}", idx = idx);
let values = [(&field, Some(&message as &dyn Value))];
let values = [(field.as_ref(), Some(&message as &dyn Value))];
let value_set = CALLSITE.metadata().fields().value_set(&values);

Event::dispatch(meta, &value_set);
Expand Down
13 changes: 8 additions & 5 deletions tracing-log/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,14 @@ pub(crate) fn dispatch_record(record: &log::Record<'_>) {
dispatch.event(&Event::new(
meta,
&meta.fields().value_set(&[
(&keys.message, Some(record.args() as &dyn field::Value)),
(&keys.target, Some(&record.target())),
(&keys.module, module),
(&keys.file, file),
(&keys.line, line),
(
Some(&keys.message),
Some(record.args() as &dyn field::Value),
),
(Some(&keys.target), Some(&record.target())),
(Some(&keys.module), module),
(Some(&keys.file), file),
(Some(&keys.line), line),
]),
));
});
Expand Down
16 changes: 8 additions & 8 deletions tracing-subscriber/src/field/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,16 +255,16 @@ pub(in crate::field) mod test_util {
let fieldset = TEST_META_1.fields();
let values = &[
(
&fieldset.field("question").unwrap(),
Some(&fieldset.field("question").unwrap()),
Some(&"life, the universe, and everything" as &dyn Value),
),
(&fieldset.field("question.answer").unwrap(), None),
(Some(&fieldset.field("question.answer").unwrap()), None),
(
&fieldset.field("tricky").unwrap(),
Some(&fieldset.field("tricky").unwrap()),
Some(&true as &dyn Value),
),
(
&fieldset.field("can_you_do_it").unwrap(),
Some(&fieldset.field("can_you_do_it").unwrap()),
Some(&true as &dyn Value),
),
];
Expand All @@ -280,19 +280,19 @@ pub(in crate::field) mod test_util {
let none = tracing_core::field::debug(&Option::<&str>::None);
let values = &[
(
&fieldset.field("question").unwrap(),
Some(&fieldset.field("question").unwrap()),
Some(&none as &dyn Value),
),
(
&fieldset.field("question.answer").unwrap(),
Some(&fieldset.field("question.answer").unwrap()),
Some(&42 as &dyn Value),
),
(
&fieldset.field("tricky").unwrap(),
Some(&fieldset.field("tricky").unwrap()),
Some(&true as &dyn Value),
),
(
&fieldset.field("can_you_do_it").unwrap(),
Some(&fieldset.field("can_you_do_it").unwrap()),
Some(&false as &dyn Value),
),
];
Expand Down
11 changes: 5 additions & 6 deletions tracing-subscriber/src/fmt/fmt_layer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,12 +813,11 @@ macro_rules! with_event_from_span {
let fs = field::FieldSet::new(&[$($field),*], cs);
#[allow(unused)]
let mut iter = fs.iter();
let v = [$(
(&iter.next().unwrap(), ::core::option::Option::Some(&$value as &dyn field::Value)),
)*];
let vs = fs.value_set(&v);
let $event = Event::new_child_of($id, meta, &vs);
$code
match Event::new_child_of($id, meta, &fs.value_set(&[$(
(Some(&iter.next().unwrap()), ::core::option::Option::Some(&$value as &dyn field::Value)),
)*])) {
$event => $code,
}
};
}

Expand Down
Loading
Loading