Pass slices into FieldSet::value_set instead of array references

[str4d]

Feature Request

Motivation

I'm trying to write FFI wrapper methods around tracing, and the simplest API is to pass a slice of values across as (values: *const *const c_char, len: usize). However, FieldSet::value_set takes a reference to a fixed-size array, and it isn't possible to generically construct a ValueSet (or have more than 32 fields when not using the macro).

Proposal

Change:

pub fn value_set<'v, V>(&'v self, values: &'v V) -> ValueSet<'v> where V: ValidLen<'v>

to

pub fn value_set<'v, V>(&'v self, values: &'v V) -> ValueSet<'v> where V: Borrow<[(&'a Field, Option<&'a (dyn Value + 'a)>)]>

(or the correct variant of this, as defined by people who actually know how this is used by the rest of the tracing internals 😄).

Alternatives

• Use TryInto from the slice to a fixed-size array.
  • I think this requires FFI-compatible const generics.
• Have 32 separate FFI methods for each concrete array size
  • This is more awkward to work with, but maybe not too much more complex to implement in e.g. C preprocessor macros over the generic version (I haven't implemented that part yet).
  • Don't need a zero-size method as there is always at least one filled field - message.
• Use a [T; 32] inside the FFI method, and fill in unused positions with (placeholder_field, None).
  • This requires having fields that are never used in real logging, which could maybe be implemented by instantiating a 32-field callsite inside the FFI method and then never using it for any real event.

[hawkw]

The reason this method takes arrays rather than slices is in order to validate that their lengths are less than 32 elements statically. This is used to emit an error from the macros at compile time when too many fields are passed.

However, the fields are stored as a slice internally:

tracing/tracing-core/src/field.rs

Line 85 in c4a6a6d

values: &'a [(&'a Field, Option<&'a (dyn Value + 'a)>)],

We could consider adding a separate constructor that takes a slice and validates the length at runtime (either with an assertion or by making the method fallible). This could be used in cases like FFI. We wouldn't want to change the existing method, as the macros would no longer be able to check lengths statically, but it would be fine to add a new one for this use case.

What do you think?

[str4d]

I'd be happy with a separate slice-compatible constructor. It would also be useful to document how to construct a compatible slice; Boxing is one option (and likely the direction I'll be going in to be able to handle arbitrary values across FFI), but hints on how else to satisfy the slice constraints (that don't require delving into how the valueset! macro is implemented) would be handy.

[mjte-riot]

For what it's worth, I would also be very interested in this being solved in 0.2, which I see is part of #922. We have an FFI API that exposes event and span calls into tracing. Even after getting dynamic fields working, the 32-field limitation is the next step.

From reading the comments for ValidLen, it looks as though the reason for it is just an optimization based on the static allocation restriction, and '32" is likely arbitrary? Any idea if there is a deeper reasoning behind the 32-field limit? At the other end of our log/event pipeline would be a sink for sending data to our metrics systems, which can handle hundreds of fields in a given event.

[nagisa]

The reason this method takes arrays rather than slices is in order to validate that their lengths are less than 32 elements statically. This is used to emit an error from the macros at compile time when too many fields are passed.

What is the reason for the 32-field limit?

[BrynCooke]

As length validation has been removed in #2508 would that make a difference to allowing slices in 1.x? The ValidLen trait is documented as:

/// Restrictions on ValueSet lengths were removed in #2508 but this type remains for backwards compatibility.

Maybe ValidLen can be implemented for all slices instead of just arrays with fixed size.