Skip to content

Commit

Permalink
review fixes
Browse files Browse the repository at this point in the history
`record_byte_slice` renamed to `record_bytes`, default implementation provides a type that prints hex representation of the bytes in its debug impl, removed both overriding implementations (json and pretty) because hex is good enough for both
  • Loading branch information
mladedav committed Apr 26, 2024
1 parent bab9178 commit 28d6e7a
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 20 deletions.
37 changes: 34 additions & 3 deletions tracing-core/src/field.rs
Expand Up @@ -225,8 +225,8 @@ pub trait Visit {
}

/// Visit a byte slice.
fn record_byte_slice(&mut self, field: &Field, value: &[u8]) {
self.record_debug(field, &value)
fn record_bytes(&mut self, field: &Field, value: &[u8]) {
self.record_debug(field, &HexBytes(value))
}

/// Records a type implementing `Error`.
Expand Down Expand Up @@ -288,6 +288,18 @@ where
DebugValue(t)
}

struct HexBytes<'a>(&'a [u8]);

impl<'a> fmt::Debug for HexBytes<'a> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
for byte in self.0 {
f.write_fmt(format_args!("{byte:02x}"))?;
}

Ok(())
}
}

// ===== impl Visit =====

impl<'a, 'b> Visit for fmt::DebugStruct<'a, 'b> {
Expand Down Expand Up @@ -452,7 +464,7 @@ impl crate::sealed::Sealed for [u8] {}

impl Value for [u8] {
fn record(&self, key: &Field, visitor: &mut dyn Visit) {
visitor.record_byte_slice(key, self)
visitor.record_bytes(key, self)
}
}

Expand Down Expand Up @@ -1142,4 +1154,23 @@ mod test {
});
assert_eq!(result, format!("{}", err));
}

#[test]
fn record_bytes() {
let fields = TEST_META_1.fields();
let first = &b"abc"[..];
let second: &[u8] = &[192, 255, 238];
let values = &[
(&fields.field("foo").unwrap(), Some(&first as &dyn Value)),
(&fields.field("bar").unwrap(), Some(&" " as &dyn Value)),
(&fields.field("baz").unwrap(), Some(&second as &dyn Value)),
];
let valueset = fields.value_set(values);
let mut result = String::new();
valueset.record(&mut |_: &Field, value: &dyn fmt::Debug| {
use core::fmt::Write;
write!(&mut result, "{:?}", value).unwrap();
});
assert_eq!(result, format!("{}", r#"616263" "c0ffee"#));
}
}
7 changes: 1 addition & 6 deletions tracing-subscriber/src/fmt/format/json.rs
Expand Up @@ -488,11 +488,6 @@ impl<'a> field::Visit for JsonVisitor<'a> {
.insert(field.name(), serde_json::Value::from(value));
}

fn record_byte_slice(&mut self, field: &Field, value: &[u8]) {
self.values
.insert(field.name(), serde_json::Value::from(value));
}

fn record_debug(&mut self, field: &Field, value: &dyn fmt::Debug) {
match field.name() {
// Skip fields that are actually log metadata that have already been handled
Expand Down Expand Up @@ -533,7 +528,7 @@ mod test {
#[test]
fn json() {
let expected =
"{\"timestamp\":\"fake time\",\"level\":\"INFO\",\"span\":{\"answer\":42,\"name\":\"json_span\",\"number\":3,\"slice\":[97,98,99]},\"spans\":[{\"answer\":42,\"name\":\"json_span\",\"number\":3,\"slice\":[97,98,99]}],\"target\":\"tracing_subscriber::fmt::format::json::test\",\"fields\":{\"message\":\"some json test\"}}\n";
"{\"timestamp\":\"fake time\",\"level\":\"INFO\",\"span\":{\"answer\":42,\"name\":\"json_span\",\"number\":3,\"slice\":\"616263\"},\"spans\":[{\"answer\":42,\"name\":\"json_span\",\"number\":3,\"slice\":\"616263\"}],\"target\":\"tracing_subscriber::fmt::format::json::test\",\"fields\":{\"message\":\"some json test\"}}\n";
let collector = collector()
.flatten_event(false)
.with_current_span(true)
Expand Down
11 changes: 0 additions & 11 deletions tracing-subscriber/src/fmt/format/pretty.rs
Expand Up @@ -426,17 +426,6 @@ impl<'a> PrettyVisitor<'a> {
}

impl<'a> field::Visit for PrettyVisitor<'a> {
fn record_byte_slice(&mut self, field: &Field, value: &[u8]) {
let hex =
value
.iter()
.fold(String::with_capacity(value.len() * 2), |mut acc, byte| {
acc.push_str(&format!("{byte:02x}"));
acc
});
self.record_debug(field, &format_args!("{}", hex));
}

fn record_str(&mut self, field: &Field, value: &str) {
if self.result.is_err() {
return;
Expand Down

0 comments on commit 28d6e7a

Please sign in to comment.