subscriber: Consider making event formatters own field formatters

[hawkw]

Feature Request

Crates

• tracing-subscriber

Motivation

Currently, the tracing-subscriber fmt module has separate traits for event and field formatters. This allows users to override just the way fields are formatted while keeping the overall event format, or vise versa. This is good.

However, in some cases, event and field formatters are more tightly coupled. For example, the JSON event formatter requires the use of the JSON field formatter, and won't compile without it. That's good, but the compiler error doesn't make it immediately obvious that the JSON field formatter is also needed --- users will call .event_format(Json::default()) and expect it to work. This can be confusing.

Ideally, we should consider changes so that the event and field formatters remain decoupled, but event formatters which require a specific field formatter can set both.

Proposal

I think we should replace the separate fields for event and field formatters on the fmt::Layer with a single type which is required to implement both. The default event formatter would own a field formatter, which it would expose for modification, and it would implement FormatFields by delegating to that field formatter. That way, when using an event formatter that does't care about how fields are formatted, it would be possible to override one or the other. However, when using an event formatter that does require a specific field formatter, like JSON, switching to the JSON event formatter would automatically also switch to the JSON field formatter.

Unfortunately, I don't think it's possible to do this without a breaking change, but we should consider it in tracing-subscriber 0.3.

Alternatives

We could also not do this.

[hawkw]

@samschlegel I think this should fix the issue you hit yesterday.

[davidbarsky]

Maybe somewhat related to this, but someone gave me this suggestion:

It seems like one convenience inside the tracing_subscriber::fmt stuff would be to allow for fmt strings like the standard logging fmt strings that make reference to span variables [such that] the following is supported:

{d} {spans[].name} [{spans.0.target}] [{spans.-1.request_id}] - {L} - {m}{n}

This might tie in with nicely with a rewrittten filtering syntax, such that some filters could be written construtively in terms of their grammar. I'm not sure everything can be presented in that way, but there might be a nice symmetry there.

[hawkw]

Maybe somewhat related to this, but someone gave me this suggestion:

It seems like one convenience inside the tracing_subscriber::fmt stuff would be to allow for fmt strings like the standard logging fmt strings that make reference to span variables [such that] the following is supported:

{d} {spans[].name} [{spans.0.target}] [{spans.-1.request_id}] - {L} - {m}{n}

This might tie in with nicely with a rewrittten filtering syntax, such that some filters could be written construtively in terms of their grammar. I'm not sure everything can be presented in that way, but there might be a nice symmetry there.

That seems like a great idea, but is not really related to this issue at all — maybe open a new feature request to propose it?

[hawkw]

Some more notes:

• A few people (including @nagisa and other folks on discord) have asked about using the fmt::format traits in other Layers. Currently, the FormatEvent trait cannot be used outside of tracing_subscriber::fmt, since the FmtContext type doesn't have a public constructor. I'd really like to change this so that the existing {matters and the traits are usable in other Layers. The main reason that FmtContext exists as a separate type is to pass in the field formatter to the FormatEvent::format_event method. If we change these methods to take a regular layer::Context, they would be much easier to use elsewhere.
• Currently, there's an impl of FormatEvent for fns:
  

  tracing/tracing-subscriber/src/fmt/format/mod.rs

  Lines 57 to 61 in 68556f2

  	impl<S, N> FormatEvent<S, N>
  	for fn(ctx: &FmtContext<'_, S, N>, &mut dyn fmt::Write, &Event<'_>) -> fmt::Result
  	where
  	S: Subscriber + for<'a> LookupSpan<'a>,
  	N: for<'a> FormatFields<'a> + 'static,

  
  This makes it hard to add new methods to the trait (such as for getting/setting a field formatter), since a bare fn won't have those methods. However, since this is a breaking change, we can probably introduce a wrapper type format_fn or something, instead, and it could also be aware of a formatter (and pass it into the wrapped function!).
  • This would also allow us to take big-F Fns as well as little-f fns. Accepting only little-f fns as we do currently is a bit of a pain, because non-capturing closures often need to be cast (i believe @davidbarsky hit this a couple days ago), and capturing closures cannot be used at all...
• If we do a big rewrite of this API, it might be worth trying to replace more of the generics with trait objects --- a lot of people have had issues with the number of generics in this API making it difficult to represent dynamic subscriber configurations. We should do before/after benchmarking to see if this hurts performance enough to matter.

[hawkw]

#1651 should also help with this