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 authored Mar 31, 2021
1 parent f40410f commit d38f793
Show file tree
Hide file tree
Showing 2 changed files with 19 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
19 changes: 18 additions & 1 deletion tracing-subscriber/src/fmt/format/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>>,
Expand Down

0 comments on commit d38f793

Please sign in to comment.