tracing: fix record_all panic

[conradludgate]

Motivation

Fixes #3431

Solution

This is horrible. We rename valueset! to valueset_all!, then we re-introduce the original valueset! with some notable changes.

The new valueset! doesn't use fields from the fieldset iterator, it instead uses FieldSet::field(...) to find the correct field. If the field isn't found, we set a fallback field that will be ignored (callsites are not equal).

[conradludgate]

Please suggest a better way to create a fake Field. I hate this

[conradludgate]

An alternative could be some construction based on ArrayVec, we only push the fields that are found.

[conradludgate]

Rather than using another dep on arrayvec, we could use MaybeUninit and the relevant unsafe. Neither Field nor Option<&dyn Value> need to be dropped, which simplifies the setup.

[hds]

Why does this need to be different from the original implementation before #3398?

[conradludgate]

The original implementation iterates over fields in the span metadata. It doesn't construct the field name based on the actual current field given. recordall! simply never worked beyond the case of the exact same spans in the exact order.

[conradludgate]

You can see that it calls $next and discards $k. So whatever is in the field name provided in recordall! is simply discarded.

tracing/tracing/src/macros.rs

Line 3003 in c47c777

$crate::__macro_support::Iterator::next(&mut iter).expect("FieldSet corrupted (this is a bug)"),

tracing/tracing/src/macros.rs

Lines 2982 to 2987 in c47c777

	(@ { $(,)* $($out:expr),* }, $next:expr, { $k:expr } = $val:expr) => {
	$crate::valueset!(
	@ { $($out),*, (&$next, Some(&$val as &dyn $crate::field::Value)) },
	$next,
	)
	};

[hds]

Thanks for this PR!

I think it would be good to have a little more test coverage. Especially because these macros are frequent sources of unintentional breaking changes.