From 41ae55afe51faa8ebc8a8b8c29fe094df9b15dda Mon Sep 17 00:00:00 2001
From: Eliza Weisman <eliza@buoyant.io>
Date: Wed, 4 Aug 2021 09:47:43 -0700
Subject: [PATCH] feat(console): don't display fields with "" values

Now that Tokio supports optional user-provided task names
(in tokio-rs/tokio#3881), the task spans it generates may contain empty
fields (if the user does not provide a name for a spawned task).
However, the console always displays the `task.name` field, even when
they are empty. This wastes space, and looks kind of ugly.

This branch updates the console to skip displaying fields whose value is
the empty string.

### Before:
![image](https://user-images.githubusercontent.com/2796466/128227118-7ee3b4d7-634f-410f-a1d0-4e5afd423566.png)

### After:
![image](https://user-images.githubusercontent.com/2796466/128227203-248bd059-839a-4254-8470-2761f726c885.png)

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
---
 console/src/tasks.rs | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/console/src/tasks.rs b/console/src/tasks.rs
index 666119e9d..820880fc9 100644
--- a/console/src/tasks.rs
+++ b/console/src/tasks.rs
@@ -161,24 +161,37 @@ impl State {
                 .drain(..)
                 .filter_map(|f| {
                     let field_name = f.name.as_ref()?;
-                    let name: Option<Arc<str>> = match field_name {
-                        proto::field::Name::StrName(n) => Some(n.clone().into()),
+                    let name: Arc<str> = match field_name {
+                        proto::field::Name::StrName(n) => n.clone().into(),
                         proto::field::Name::NameIdx(idx) => {
                             debug_assert_eq!(
                                 f.metadata_id.map(|m| m.id),
                                 Some(meta_id),
                                 "malformed field name: metadata ID mismatch!"
                             );
-                            meta.field_names.get(*idx as usize).cloned()
+                            let name = meta.field_names.get(*idx as usize).cloned();
+                            if name.is_none() {
+                                tracing::warn!(index = idx, "missing field name for");
+                            };
+                            name?
                         }
                     };
-                    let mut value: FieldValue = f.value.as_ref().expect("no value").clone().into();
-                    name.map(|name| {
-                        if &*name == "spawn.location" {
-                            value = value.truncate_registry_path();
-                        }
-                        Field { name, value }
-                    })
+
+                    let value = f.value;
+                    debug_assert!(
+                        value.is_some(),
+                        "missing field value for field `{:?}`",
+                        name
+                    );
+                    let mut value = FieldValue::from(value?)
+                        // if the value is an empty string, just skip it.
+                        .ensure_nonempty()?;
+
+                    if &*name == "spawn.location" {
+                        value = value.truncate_registry_path();
+                    }
+
+                    Some(Field { name, value })
                 })
                 .collect();
 
@@ -494,4 +507,12 @@ impl FieldValue {
         };
         FieldValue::Debug(s)
     }
+
+    /// If `self` is an empty string, returns `None`. Otherwise, returns `Some(self)`.
+    fn ensure_nonempty(self) -> Option<Self> {
+        match self {
+            FieldValue::Debug(s) | FieldValue::Str(s) if s.is_empty() => None,
+            val => Some(val),
+        }
+    }
 }