From 690a9a68098aa1eb86d1c5c4aad085ef12245bca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Ml=C3=A1dek?= <david.mladek.cz@gmail.com>
Date: Fri, 3 May 2024 19:17:38 +0200
Subject: [PATCH] tracing: allow `&[u8]` to be recorded as event/span field
 (#2954)

## Motivation

Users may want to pass data to `Subscribe`s which is not valid UTF-8. Currently, it would have to be encoded into `&str` to be passed as a field value.

## Solution

This branch adds a `record_bytes` method to `Visit`. It has an implementation falling back to `record_debug` so it is not be a breaking change.

`JsonVisitor` got an overridden implementation as it should not use `Debug` output and encode it as a string, but rather store the bytes as an array.

`PrettyVisitor` go an overridden implementation to make the output more pretty.

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
---
 tracing-core/src/field.rs                 | 54 ++++++++++++++++++++++-
 tracing-subscriber/src/fmt/format/json.rs | 15 ++++++-
 2 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/tracing-core/src/field.rs b/tracing-core/src/field.rs
index 663a98b49a..fbaa50dec4 100644
--- a/tracing-core/src/field.rs
+++ b/tracing-core/src/field.rs
@@ -38,7 +38,7 @@
 use crate::callsite;
 use core::{
     borrow::Borrow,
-    fmt,
+    fmt::{self, Write},
     hash::{Hash, Hasher},
     num,
     ops::Range,
@@ -224,6 +224,11 @@ pub trait Visit {
         self.record_debug(field, &value)
     }
 
+    /// Visit a byte slice.
+    fn record_bytes(&mut self, field: &Field, value: &[u8]) {
+        self.record_debug(field, &HexBytes(value))
+    }
+
     /// Records a type implementing `Error`.
     ///
     /// <div class="example-wrap" style="display:inline-block">
@@ -283,6 +288,26 @@ where
     DebugValue(t)
 }
 
+struct HexBytes<'a>(&'a [u8]);
+
+impl<'a> fmt::Debug for HexBytes<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_char('[')?;
+
+        let mut bytes = self.0.iter();
+
+        if let Some(byte) = bytes.next() {
+            f.write_fmt(format_args!("{byte:02x}"))?;
+        }
+
+        for byte in bytes {
+            f.write_fmt(format_args!(" {byte:02x}"))?;
+        }
+
+        f.write_char(']')
+    }
+}
+
 // ===== impl Visit =====
 
 impl<'a, 'b> Visit for fmt::DebugStruct<'a, 'b> {
@@ -443,6 +468,14 @@ impl Value for str {
     }
 }
 
+impl crate::sealed::Sealed for [u8] {}
+
+impl Value for [u8] {
+    fn record(&self, key: &Field, visitor: &mut dyn Visit) {
+        visitor.record_bytes(key, self)
+    }
+}
+
 #[cfg(feature = "std")]
 impl crate::sealed::Sealed for dyn std::error::Error + 'static {}
 
@@ -1131,4 +1164,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#"[61 62 63]" "[c0 ff ee]"#));
+    }
 }
diff --git a/tracing-subscriber/src/fmt/format/json.rs b/tracing-subscriber/src/fmt/format/json.rs
index f4e61fb123..1f045d93b5 100644
--- a/tracing-subscriber/src/fmt/format/json.rs
+++ b/tracing-subscriber/src/fmt/format/json.rs
@@ -488,6 +488,11 @@ impl<'a> field::Visit for JsonVisitor<'a> {
             .insert(field.name(), serde_json::Value::from(value));
     }
 
+    fn record_bytes(&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
@@ -528,13 +533,19 @@ mod test {
     #[test]
     fn json() {
         let expected =
-        "{\"timestamp\":\"fake time\",\"level\":\"INFO\",\"span\":{\"answer\":42,\"name\":\"json_span\",\"number\":3},\"spans\":[{\"answer\":42,\"name\":\"json_span\",\"number\":3}],\"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\":[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";
         let collector = collector()
             .flatten_event(false)
             .with_current_span(true)
             .with_span_list(true);
         test_json(expected, collector, || {
-            let span = tracing::span!(tracing::Level::INFO, "json_span", answer = 42, number = 3);
+            let span = tracing::span!(
+                tracing::Level::INFO,
+                "json_span",
+                answer = 42,
+                number = 3,
+                slice = &b"abc"[..]
+            );
             let _guard = span.enter();
             tracing::info!("some json test");
         });