From d38f79336d43eec28f5720caabaf3d1ff5ae224e Mon Sep 17 00:00:00 2001
From: akinnane <17098249+akinnane@users.noreply.github.com>
Date: Wed, 31 Mar 2021 21:12:20 +0100
Subject: [PATCH] subscriber: fix `on_event` serialization when no fields set
 on span (#1333)

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 change aligns the size hint with the actual serialized data.

Refs: /~https://github.com/tokio-rs/tracing/issues/829#issuecomment-661984255

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
---
 tracing-serde/src/fields.rs               |  2 +-
 tracing-subscriber/src/fmt/format/json.rs | 19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/tracing-serde/src/fields.rs b/tracing-serde/src/fields.rs
index f6bab1ab53..eedc72d6b1 100644
--- a/tracing-serde/src/fields.rs
+++ b/tracing-serde/src/fields.rs
@@ -21,7 +21,7 @@ impl<'a> Serialize for SerializeFieldMap<'a, Event<'_>> {
     where
         S: Serializer,
     {
-        let len = self.0.metadata().fields().len();
+        let len = self.0.fields().count();
         let serializer = serializer.serialize_map(Some(len))?;
         let mut visitor = SerdeMapVisitor::new(serializer);
         self.0.record(&mut visitor);
diff --git a/tracing-subscriber/src/fmt/format/json.rs b/tracing-subscriber/src/fmt/format/json.rs
index 80a11d8f7d..a5e2d40ba1 100644
--- a/tracing-subscriber/src/fmt/format/json.rs
+++ b/tracing-subscriber/src/fmt/format/json.rs
@@ -496,7 +496,7 @@ impl<'a> fmt::Debug for WriteAdaptor<'a> {
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::fmt::{test::MockMakeWriter, time::FormatTime, CollectorBuilder};
+    use crate::fmt::{format::FmtSpan, test::MockMakeWriter, time::FormatTime, CollectorBuilder};
     use tracing::{self, collect::with_default};
 
     use std::fmt;
@@ -653,6 +653,23 @@ mod test {
         });
     }
 
+    #[test]
+    fn json_span_event() {
+        // Check span events serialize correctly.
+        // Discussion: /~https://github.com/tokio-rs/tracing/issues/829#issuecomment-661984255
+        //
+        let expected = r#"{"timestamp":"fake time","level":"INFO","fields":{"message":"enter"},"target":"tracing_subscriber::fmt::format::json::test"}"#;
+        let collector = collector()
+            .flatten_event(false)
+            .with_current_span(false)
+            .with_span_list(false)
+            .with_span_events(FmtSpan::ENTER);
+
+        test_json(expected, collector, || {
+            tracing::info_span!("valid_json").in_scope(|| {});
+        });
+    }
+
     fn test_json<T>(
         expected: &str,
         builder: crate::fmt::CollectorBuilder<JsonFields, Format<Json>>,