From ea7178b8c66a700ab2a173812fef7a2f0c544ba1 Mon Sep 17 00:00:00 2001
From: Eliza Weisman <eliza@buoyant.io>
Date: Thu, 28 Mar 2019 14:47:06 -0700
Subject: [PATCH] trace-core: Add slightly more useful debug impls (#1014)

This branch improves the `fmt::Debug` implementation for `Metadata`,
and adds `fmt::Display` implementations for `FieldSet` and `ValueSet`.

When formatting a `Metadata`, only present fields are formatted --- if
optional fields, such as the file, line number, and module path are
`None`, they will be excluded. In addition, `Metadata` now formats its
`FieldSet` using `FieldSet`'s `fmt::Display` implementation, which is a
bit less noisy. Finally, the `Debug` output for `Metadata` now includes
the callsite that the metadata originates from.

The intention behind these changes is to make the output from failed
tests somewhat easier to interpret.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
---
 tokio-trace/tokio-trace-core/src/field.rs    | 23 ++++++++++++++
 tokio-trace/tokio-trace-core/src/metadata.rs | 32 +++++++++++++++-----
 2 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/tokio-trace/tokio-trace-core/src/field.rs b/tokio-trace/tokio-trace-core/src/field.rs
index 0d85e6715..9bc5649c1 100644
--- a/tokio-trace/tokio-trace-core/src/field.rs
+++ b/tokio-trace/tokio-trace-core/src/field.rs
@@ -538,6 +538,14 @@ impl fmt::Debug for FieldSet {
     }
 }
 
+impl fmt::Display for FieldSet {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_set()
+            .entries(self.names.iter().map(|n| display(n)))
+            .finish()
+    }
+}
+
 // ===== impl Iter =====
 
 impl Iterator for Iter {
@@ -614,6 +622,21 @@ impl<'a> fmt::Debug for ValueSet<'a> {
                 }
                 dbg
             })
+            .field("callsite", &self.callsite())
+            .finish()
+    }
+}
+
+impl<'a> fmt::Display for ValueSet<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        self.values
+            .iter()
+            .fold(&mut f.debug_map(), |dbg, (key, v)| {
+                if let Some(val) = v {
+                    val.record(key, dbg);
+                }
+                dbg
+            })
             .finish()
     }
 }
diff --git a/tokio-trace/tokio-trace-core/src/metadata.rs b/tokio-trace/tokio-trace-core/src/metadata.rs
index d003c156e..e7c656f34 100644
--- a/tokio-trace/tokio-trace-core/src/metadata.rs
+++ b/tokio-trace/tokio-trace-core/src/metadata.rs
@@ -209,14 +209,32 @@ impl<'a> Metadata<'a> {
 
 impl<'a> fmt::Debug for Metadata<'a> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        f.debug_struct("Metadata")
-            .field("name", &self.name)
+        let mut meta = f.debug_struct("Metadata");
+        meta.field("name", &self.name)
             .field("target", &self.target)
-            .field("level", &self.level)
-            .field("module_path", &self.module_path)
-            .field("file", &self.file)
-            .field("line", &self.line)
-            .field("field_names", &self.fields)
+            .field("level", &self.level);
+
+        if let Some(path) = self.module_path() {
+            meta.field("module_path", &path);
+        }
+
+        match (self.file(), self.line()) {
+            (Some(file), Some(line)) => {
+                meta.field("location", &format_args!("{}:{}", file, line));
+            }
+            (Some(file), None) => {
+                meta.field("file", &format_args!("{}", file));
+            }
+
+            // Note: a line num with no file is a kind of weird case that _probably_ never occurs...
+            (None, Some(line)) => {
+                meta.field("line", &line);
+            }
+            (None, None) => {}
+        };
+
+        meta.field("fields", &format_args!("{}", self.fields))
+            .field("callsite", &self.callsite())
             .finish()
     }
 }