From c518e1a7d0e4aa1597847f8f51f19a2093c61786 Mon Sep 17 00:00:00 2001
From: Bryan Burgers <bryan@burgers.io>
Date: Thu, 20 Aug 2020 12:21:02 -0500
Subject: [PATCH] subscriber: Reorder JSON fields (#892)

## 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
---
 tracing-serde/src/lib.rs                  | 26 +++++++-----
 tracing-subscriber/src/fmt/format/json.rs | 51 ++++++++++++-----------
 2 files changed, 42 insertions(+), 35 deletions(-)

diff --git a/tracing-serde/src/lib.rs b/tracing-serde/src/lib.rs
index 8acf4d6f..21712b0b 100644
--- a/tracing-serde/src/lib.rs
+++ b/tracing-serde/src/lib.rs
@@ -333,6 +333,22 @@ where
             state: Ok(()),
         }
     }
+
+    /// Completes serializing the visited object, returning `Ok(())` if all
+    /// fields were serialized correctly, or `Error(S::Error)` if a field could
+    /// not be serialized.
+    pub fn finish(self) -> Result<S::Ok, S::Error> {
+        self.state?;
+        self.serializer.end()
+    }
+
+    /// Completes serializing the visited object, returning ownership of the underlying serializer
+    /// if all fields were serialized correctly, or `Err(S::Error)` if a field could not be
+    /// serialized.
+    pub fn take_serializer(self) -> Result<S, S::Error> {
+        self.state?;
+        Ok(self.serializer)
+    }
 }
 
 impl<S> Visit for SerdeMapVisitor<S>
@@ -374,16 +390,6 @@ where
     }
 }
 
-impl<S: SerializeMap> SerdeMapVisitor<S> {
-    /// Completes serializing the visited object, returning `Ok(())` if all
-    /// fields were serialized correctly, or `Error(S::Error)` if a field could
-    /// not be serialized.
-    pub fn finish(self) -> Result<S::Ok, S::Error> {
-        self.state?;
-        self.serializer.end()
-    }
-}
-
 /// Implements `tracing_core::field::Visit` for some `serde::ser::SerializeStruct`.
 #[derive(Debug)]
 pub struct SerdeStructVisitor<S: SerializeStruct> {
diff --git a/tracing-subscriber/src/fmt/format/json.rs b/tracing-subscriber/src/fmt/format/json.rs
index 9d02d7d4..d26c822d 100644
--- a/tracing-subscriber/src/fmt/format/json.rs
+++ b/tracing-subscriber/src/fmt/format/json.rs
@@ -31,10 +31,10 @@ use tracing_log::NormalizeEvent;
 /// {
 ///     "timestamp":"Feb 20 11:28:15.096",
 ///     "level":"INFO",
-///     "spans":[{"name":"root"},{"name":"leaf"}],
-///     "span":{name":"leaf"},
-///     "target":"mycrate",
 ///     "fields":{"message":"some message","key":"value"}
+///     "target":"mycrate",
+///     "span":{name":"leaf"},
+///     "spans":[{"name":"root"},{"name":"leaf"}],
 /// }
 /// ```
 ///
@@ -217,6 +217,28 @@ where
                 None
             };
 
+            if self.format.flatten_event {
+                let mut visitor = tracing_serde::SerdeMapVisitor::new(serializer);
+                event.record(&mut visitor);
+
+                serializer = visitor.take_serializer()?;
+            } else {
+                use tracing_serde::fields::AsMap;
+                serializer.serialize_entry("fields", &event.field_map())?;
+            };
+
+            if self.display_target {
+                serializer.serialize_entry("target", meta.target())?;
+            }
+
+            if self.format.display_current_span {
+                if let Some(ref span) = current_span {
+                    serializer
+                        .serialize_entry("span", &SerializableSpan(span, format_field_marker))
+                        .unwrap_or(());
+                }
+            }
+
             if self.format.display_span_list && current_span.is_some() {
                 serializer.serialize_entry(
                     "spans",
@@ -224,18 +246,6 @@ where
                 )?;
             }
 
-            if self.format.display_current_span {
-                if let Some(span) = current_span {
-                    serializer
-                        .serialize_entry("span", &SerializableSpan(&span, format_field_marker))
-                        .unwrap_or(());
-                }
-            }
-
-            if self.display_target {
-                serializer.serialize_entry("target", meta.target())?;
-            }
-
             if self.display_thread_name {
                 let current_thread = std::thread::current();
                 match current_thread.name() {
@@ -256,16 +266,7 @@ where
                     .serialize_entry("threadId", &format!("{:?}", std::thread::current().id()))?;
             }
 
-            if !self.format.flatten_event {
-                use tracing_serde::fields::AsMap;
-                serializer.serialize_entry("fields", &event.field_map())?;
-                serializer.end()
-            } else {
-                let mut visitor = tracing_serde::SerdeMapVisitor::new(serializer);
-                event.record(&mut visitor);
-
-                visitor.finish()
-            }
+            serializer.end()
         };
 
         visit().map_err(|_| fmt::Error)?;