subscriber: Reorder JSON fields

[bryanburgers]

Motivation

JSON is a structured format, so the order of the fields should be
unimportant.

However, sometimes the JSON output is read in non-structured or
semi-structured contexts, and sometimes the output is truncated. In
those situations, when the spans and the current span are serialized
before the fields and the message, it makes it very difficult to see
what is going on.

Solution

By serializing the "fields" key before the "spans" and "span" keys, this
information is more likely visible when the payload is shown raw and
gets truncated.

Systems that parse the JSON as structured data should be unaffected
because of the nature of JSON.

Fixes #891

[hawkw]

I had one note about the behavior when flatten_event is true. Other than that, this looks good to me!

[hawkw]

If we are not flattening the event's fields into the root JSON object, it looks like we will now emit the fields first, llike this:

{
    "timestamp": "Feb 20 11:28:15.096",
    "level": "INFO",
    "fields": {"message":"some message","key":"value"},
    "target":" mycrate",
    "span": {"name": "leaf"},
    "spans": [{"name":"root"},{"name":"leaf"}],
}

But if we are flattening the event fields, it looks like the fields are still last, so it will look like this:

{
    "timestamp":"Feb 20 11:28:15.096",
    "level":"INFO",
    "target":"mycrate",
    "span":{"name": "leaf"},
    "spans":[{"name": "root"},{"name": "leaf"}],
    "message":"some message",
    "key":"value"
}

Do you think we should maybe move the code from the else here

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

Lines 266 to 271 in be827b1

	} else {
	let mut visitor = tracing_serde::SerdeMapVisitor::new(serializer);
	event.record(&mut visitor);
	visitor.finish()
	}

to up here, as well, so that the ordering is consistent regardless of flattening?

[bryanburgers]

Sure! I had to introduce a new function to SerdeMapVisitor to return the underlying serializer back to the caller to make this work – hopefully that's not a problem!

---

I'm not sure if the JSON standard specifies any particular action when a key appears twice. Both Firefox and Chrome seem to use the last specified value:

• JSON.parse('{ "target": "mycrate", "target": "practice" }') will return { "target": "practice" }
• JSON.parse('{ "target": "practice", "target": "mycrate" }') will return { "target": "mycrate" }

So this could be considered a breaking change in the pathological case where a consumer with flattening on emits a key that is one of the known keys.

event!(Level::INFO, archer = "Bob", bow = "yew", arrow = "dx-487", target = "practice", hit = "bullseye");

[hawkw]

Sure! I had to introduce a new function to SerdeMapVisitor to return the underlying serializer back to the caller to make this work – hopefully that's not a problem!

Okay, that's fine, we'll just have to make sure to publish a new tracing-serde release before we can publish a new tracing-subscriber. Not a problem, just making a note of this.

I'm not sure if the JSON standard specifies any particular action when a key appears twice. Both Firefox and Chrome seem to use the last specified value:

* `JSON.parse('{ "target": "mycrate", "target": "practice" }')` will return `{ "target": "practice" }`

* `JSON.parse('{ "target": "practice", "target": "mycrate" }')` will return `{ "target": "mycrate" }`

So this could be considered a breaking change in the pathological case where a consumer with flattening on emits a key that is one of the known keys.

Hmm. That could be an issue. The names "target" and "file", which appear in the metadata, are both potentially common enough that I think potential clashes with a field name might be worth worrying about. We might want to make other changes (out of scope for this PR) to handle this when the fields are being flattened...maybe prefixing the metadata fields with "metadata_" or something? Of course, that would break the JSON format as well, so it would have to wait for 0.3 before we can address it.

[hawkw]

Looks good to me! @LucioFranco, do you wanna take a look as well?