Skip to content

Commit

Permalink
subscriber: fix on_event serialization when no fields set on span (#…
Browse files Browse the repository at this point in the history
…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: #829 (comment)

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
akinnane and hawkw committed Mar 31, 2021
1 parent 51965bf commit f2c1825
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 2 deletions.
2 changes: 1 addition & 1 deletion tracing-serde/src/fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
35 changes: 34 additions & 1 deletion tracing-subscriber/src/fmt/format/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ impl<'a> fmt::Debug for WriteAdaptor<'a> {

#[cfg(test)]
mod test {
use crate::fmt::{test::MockWriter, time::FormatTime};
use crate::fmt::{format::FmtSpan, test::MockWriter, time::FormatTime};
use lazy_static::lazy_static;
use tracing::{self, subscriber::with_default};

Expand Down Expand Up @@ -672,6 +672,39 @@ mod test {
});
}

#[test]
fn json_span_event() {
// Check span events serialize correctly.
// Discussion: /~https://github.com/tokio-rs/tracing/issues/829#issuecomment-661984255
//
lazy_static! {
static ref BUF: Mutex<Vec<u8>> = Mutex::new(vec![]);
}
let expected = r#"{"timestamp":"fake time","level":"INFO","fields":{"message":"enter"},"target":"tracing_subscriber::fmt::format::json::test"}"#;

let make_writer = || MockWriter::new(&BUF);
let subscriber = crate::fmt::Subscriber::builder()
.json()
.flatten_event(false)
.with_current_span(false)
.with_span_list(false)
.with_span_events(FmtSpan::ENTER)
.with_writer(make_writer)
.with_timer(MockTime)
.finish();

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

let actual = String::from_utf8(BUF.try_lock().unwrap().to_vec()).unwrap();
assert_eq!(
serde_json::from_str::<std::collections::HashMap<&str, serde_json::Value>>(expected)
.unwrap(),
serde_json::from_str(actual.as_str()).unwrap()
);
}

#[cfg(feature = "json")]
fn test_json<T, U>(
make_writer: T,
Expand Down

0 comments on commit f2c1825

Please sign in to comment.