subscriber: fix on_event serialization when no fields set on span

[akinnane]

Motivation

Serializing a spans on_ACTION events, when no fields are set on the span, results in invalid JSON. This is because serializier_map was getting a size hint for self.0.metadata().fields().len() then serializing self.0.fields.field_set() instead. This resulted in the fields key being set to an empty object, then Serde serializes the k/v pairs from field_set(). This was causing an erroneous closing brace } to be added after the serialized fields.

This is discussed here: #829 (comment)

[dependencies]
tracing = "=0.1.25"
tracing-serde = "=0.1.2"
tracing-subscriber = "=0.2.17"

use tracing_subscriber::fmt::format::FmtSpan;

fn main() {
    tracing_subscriber::fmt()
        .json()
        .with_span_events(FmtSpan::FULL)
        .init();

    tracing::info_span!("broken_json").in_scope(|| {
    });

    tracing::info_span!("valid_json", "").in_scope(|| {
    });
}

{"timestamp":"Mar 28 19:08:44.323","level":"INFO","fields":{},"message":"new"},"target":"json_events"}
{"timestamp":"Mar 28 19:08:44.323","level":"INFO","fields":{},"message":"enter"},"target":"json_events","span":{"name":"broken_json"},"spans":[{"name":"broken_json"}]}
{"timestamp":"Mar 28 19:08:44.323","level":"INFO","fields":{},"message":"exit"},"target":"json_events"}
{"timestamp":"Mar 28 19:08:44.323","level":"INFO","fields":{},"message":"close","time.busy":"61.7µs","time.idle":"113µs"},"target":"json_events"}
{"timestamp":"Mar 28 19:08:44.323","level":"INFO","fields":{"message":"new"},"target":"json_events"}
{"timestamp":"Mar 28 19:08:44.323","level":"INFO","fields":{"message":"enter"},"target":"json_events","span":{"message":"","name":"valid_json"},"spans":[{"message":"","name":"valid_json"}]}
{"timestamp":"Mar 28 19:08:44.323","level":"INFO","fields":{"message":"exit"},"target":"json_events"}
{"timestamp":"Mar 28 19:08:44.323","level":"INFO","fields":{"message":"close","time.busy":"70.6µs","time.idle":"70.7µs"},"target":"json_events"}

Solution

This change aligns the size hint with the size of actual serialized data.

[hawkw]

thanks for the fix, this looks good to me!

i had one minor suggestion about testing, but i'd be fine to merge this as it is currently if that change seems like a lot of unnecessary effort. up to you!

[hawkw]

take it or leave it, but since the issue here is that we were generating invalid JSON, it might be worth rewriting this test in the same style as the record_works test? this test actually attempts to parse the output as JSON and then makes assertions about its structure. this could be worth doing, since it would also implicitly assert that the output definitely does parse as JSON.

of course, since the hard-coded JSON string is valid JSON as well, this test is also fine as-is.

[hawkw]

@akinnane this fixes the issue described in #829, correct?