From 6c2af38cc188ea6a959f9329b2c8103788adb019 Mon Sep 17 00:00:00 2001 From: Skyler Ferris Date: Sun, 6 Jul 2025 04:44:32 -0700 Subject: [PATCH 1/3] Correctly state dependency on JsonFields. The JSON formatter depends on the JsonFields FieldFormatter being in use. When .json() is called on a SubscriberBuilder directly this is handled automatically. However, when .json() is called on the Format given to .event_format() method of SubscriberBuilder a runtime error occurs unless the user also calls .json() on the SubscriberBuilder itself. This change correctly declares that the json code is only implemented for the JsonFields formatter. This results in compile-time errors, instead of a runtime error, unless the user calls .json() on the SubscriberBuilder *before* calling .event_format(). This improves the situation described in issue 3327, but a full resolution would involve either implementing the json code for the DefaultFields formatter as well (and by extension all future formatters added to the project), improving the error message so that the compiler recommends calling .json() on the SubscriberBuilder (it currently recommends removing the .json() call from the Format object), or making the SubscriberBuilder automatically use JsonFields when using the JSON output format. --- tracing-subscriber/src/fmt/format/json.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/tracing-subscriber/src/fmt/format/json.rs b/tracing-subscriber/src/fmt/format/json.rs index 3ef0fcd653..795de2f30f 100644 --- a/tracing-subscriber/src/fmt/format/json.rs +++ b/tracing-subscriber/src/fmt/format/json.rs @@ -119,10 +119,9 @@ where Span: Subscriber + for<'lookup> crate::registry::LookupSpan<'lookup>, N: for<'writer> FormatFields<'writer> + 'static; -impl serde::ser::Serialize for SerializableContext<'_, '_, Span, N> +impl serde::ser::Serialize for SerializableContext<'_, '_, Span, JsonFields> where Span: Subscriber + for<'lookup> crate::registry::LookupSpan<'lookup>, - N: for<'writer> FormatFields<'writer> + 'static, { fn serialize(&self, serializer_o: Ser) -> Result where @@ -149,10 +148,9 @@ where Span: for<'lookup> crate::registry::LookupSpan<'lookup>, N: for<'writer> FormatFields<'writer> + 'static; -impl serde::ser::Serialize for SerializableSpan<'_, '_, Span, N> +impl serde::ser::Serialize for SerializableSpan<'_, '_, Span, JsonFields> where Span: for<'lookup> crate::registry::LookupSpan<'lookup>, - N: for<'writer> FormatFields<'writer> + 'static, { fn serialize(&self, serializer: Ser) -> Result where @@ -162,7 +160,7 @@ where let ext = self.0.extensions(); let data = ext - .get::>() + .get::>() .expect("Unable to find FormattedFields in extensions; this is a bug"); // TODO: let's _not_ do this, but this resolves @@ -210,15 +208,14 @@ where } } -impl FormatEvent for Format +impl FormatEvent for Format where S: Subscriber + for<'lookup> LookupSpan<'lookup>, - N: for<'writer> FormatFields<'writer> + 'static, T: FormatTime, { fn format_event( &self, - ctx: &FmtContext<'_, S, N>, + ctx: &FmtContext<'_, S, JsonFields>, mut writer: Writer<'_>, event: &Event<'_>, ) -> fmt::Result @@ -248,7 +245,7 @@ where serializer.serialize_entry("level", &meta.level().as_serde())?; } - let format_field_marker: std::marker::PhantomData = std::marker::PhantomData; + let format_field_marker: std::marker::PhantomData = std::marker::PhantomData; let current_span = if self.format.display_current_span || self.format.display_span_list { From bb3ea9447af6ecbd86fc0d018d7c0bc197142688 Mon Sep 17 00:00:00 2001 From: Skyler Ferris Date: Sun, 13 Jul 2025 03:43:40 -0700 Subject: [PATCH 2/3] Re-introduce genericity For two of the methods the only constraint is that SerializableSpan has to implement Serialize for N/JsonFields. Make them generic again by adding this constraint, which will reduce code duplication when adding support for DefaultFields. --- tracing-subscriber/src/fmt/format/json.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tracing-subscriber/src/fmt/format/json.rs b/tracing-subscriber/src/fmt/format/json.rs index 795de2f30f..13978d22a0 100644 --- a/tracing-subscriber/src/fmt/format/json.rs +++ b/tracing-subscriber/src/fmt/format/json.rs @@ -119,9 +119,11 @@ where Span: Subscriber + for<'lookup> crate::registry::LookupSpan<'lookup>, N: for<'writer> FormatFields<'writer> + 'static; -impl serde::ser::Serialize for SerializableContext<'_, '_, Span, JsonFields> +impl serde::ser::Serialize for SerializableContext<'_, '_, Span, N> where Span: Subscriber + for<'lookup> crate::registry::LookupSpan<'lookup>, + N: for<'writer> FormatFields<'writer> + 'static, + for<'inner, 'outer> SerializableSpan<'inner, 'outer, Span, N>: serde::Serialize, { fn serialize(&self, serializer_o: Ser) -> Result where @@ -208,14 +210,16 @@ where } } -impl FormatEvent for Format +impl FormatEvent for Format where S: Subscriber + for<'lookup> LookupSpan<'lookup>, + N: for<'writer> FormatFields<'writer> + 'static, + for<'inner, 'outer> SerializableSpan<'inner, 'outer, S, N>: serde::Serialize, T: FormatTime, { fn format_event( &self, - ctx: &FmtContext<'_, S, JsonFields>, + ctx: &FmtContext<'_, S, N>, mut writer: Writer<'_>, event: &Event<'_>, ) -> fmt::Result @@ -245,7 +249,7 @@ where serializer.serialize_entry("level", &meta.level().as_serde())?; } - let format_field_marker: std::marker::PhantomData = std::marker::PhantomData; + let format_field_marker: std::marker::PhantomData = std::marker::PhantomData; let current_span = if self.format.display_current_span || self.format.display_span_list { From df3d0da7af7f726babfa2661f95bdaaddbf85c8e Mon Sep 17 00:00:00 2001 From: Skyler Ferris Date: Sun, 13 Jul 2025 04:03:52 -0700 Subject: [PATCH 3/3] Add JSON support for DefaultFields --- tracing-subscriber/src/fmt/format/json.rs | 33 +++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tracing-subscriber/src/fmt/format/json.rs b/tracing-subscriber/src/fmt/format/json.rs index 13978d22a0..ceb7a6a3aa 100644 --- a/tracing-subscriber/src/fmt/format/json.rs +++ b/tracing-subscriber/src/fmt/format/json.rs @@ -3,6 +3,7 @@ use crate::{ field::{RecordFields, VisitOutput}, fmt::{ fmt_layer::{FmtContext, FormattedFields}, + format::DefaultFields, writer::WriteAdaptor, }, registry::LookupSpan, @@ -150,6 +151,38 @@ where Span: for<'lookup> crate::registry::LookupSpan<'lookup>, N: for<'writer> FormatFields<'writer> + 'static; +impl serde::ser::Serialize for SerializableSpan<'_, '_, Span, DefaultFields> +where + Span: for<'lookup> crate::registry::LookupSpan<'lookup>, +{ + fn serialize(&self, serializer: Ser) -> Result + where + Ser: serde::ser::Serializer, + { + let mut serializer = serializer.serialize_map(None)?; + + let ext = self.0.extensions(); + let data = ext + .get::>() + .expect("Unable to find FormattedFields in extensions; this is a bug"); + + let fields = data.split(" "); + + for key_value_pair in fields { + let mut kvp_iter = key_value_pair.split("="); + if let (Some(key), Some(value)) = (kvp_iter.next(), kvp_iter.next()) { + // The default formatter surrounds the values in literal quotation marks; + // remove these from the serialization so they don't end up in the JSON + // output. + serializer.serialize_entry(&key, &value[1..value.len() - 1]); + } + } + + serializer.serialize_entry("name", self.0.metadata().name())?; + serializer.end() + } +} + impl serde::ser::Serialize for SerializableSpan<'_, '_, Span, JsonFields> where Span: for<'lookup> crate::registry::LookupSpan<'lookup>,