feat(console): replace target column with kind column in tasks view (#478)

hds · web-flow · commit 903d9fa9f9d2 · 2023-10-17T15:22:32.000+02:00
In the `tokio-console` tasks view, there are a fixed set of columns and
any remaining fields are included in a "Fields" column at the end. One
of the fields which is always present on a task, but doesn't receive a
dedicated column is the kind field, which currently takes one of the
following values:
* `task` (a "normal" async task)
* `blocking`
* `block_on`
* `local`

Meanwhile, there is a dedicated column for the task span's target, which
currently takes one of the following values:
* `tokio::task`
* `tokio::task::blocking`

The target for tasks with kind `block_on` and `local` is also
`tokio::task`.

This change replaces the target column with a kind column as it provides
more information in fewer characters. The target value is moved
(somewhat artificially) to the fields which appear in the final column.

The `target` is also left on the `state::Task` struct as we expect to
want to filter by it in the future.

Additionally, the `console-subscriber` examples have been updated so
that there are options to visualize `blocking`, `block_on`, and `local`
tasks. The `app` example has been updated to include an optional task
which calls `tokio::spawn_blocking`. A new example `local` has been
added which creates a `LocalSet` and spawns local tasks onto it.
diff --git a/console-subscriber/examples/app.rs b/console-subscriber/examples/app.rs
@@ -45,6 +45,12 @@ async fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
                     .spawn(no_yield(20))
                     .unwrap();
             }
+            "blocking" => {
+                tokio::task::Builder::new()
+                    .name("spawns_blocking")
+                    .spawn(spawn_blocking(5))
+                    .unwrap();
+            }
             "help" | "-h" => {
                 eprintln!("{}", HELP);
                 return Ok(());
@@ -135,3 +141,14 @@ async fn no_yield(seconds: u64) {
         _ = handle.await;
     }
 }
+
+#[tracing::instrument]
+async fn spawn_blocking(seconds: u64) {
+    loop {
+        let seconds = seconds;
+        _ = tokio::task::spawn_blocking(move || {
+            std::thread::sleep(Duration::from_secs(seconds));
+        })
+        .await;
+    }
+}
diff --git a/console-subscriber/examples/local.rs b/console-subscriber/examples/local.rs
@@ -0,0 +1,38 @@
+//! Local tasks
+//!
+//! This example shows the instrumentation on local tasks. Tasks spawned onto a
+//! `LocalSet` with `spawn_local` have the kind `local` in `tokio-console`.
+//!
+//! Additionally, because the `console-subscriber` is initialized before the
+//! tokio runtime is created, we will also see the `block_on` kind task.
+use std::time::Duration;
+use tokio::{runtime, task};
+
+fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
+    console_subscriber::init();
+
+    let rt = runtime::Builder::new_current_thread()
+        .enable_all()
+        .build()
+        .unwrap();
+    let local = task::LocalSet::new();
+    local.block_on(&rt, async {
+        loop {
+            let mut join_handles = Vec::new();
+            for _ in 0..10 {
+                let jh = task::spawn_local(async {
+                    tokio::time::sleep(Duration::from_millis(100)).await;
+
+                    std::thread::sleep(Duration::from_millis(100));
+                });
+                join_handles.push(jh);
+            }
+
+            for jh in join_handles {
+                _ = jh.await;
+            }
+        }
+    });
+
+    Ok(())
+}
diff --git a/tokio-console/src/intern.rs b/tokio-console/src/intern.rs
@@ -21,20 +21,17 @@ pub(crate) struct Strings {
 pub(crate) struct InternedStr(Rc<String>);
 
 impl Strings {
-    // NOTE(elzia): currently, we never need to use this, but we can always
-    // uncomment it if we do...
-
-    // pub(crate) fn string_ref<Q>(&mut self, string: &Q) -> InternedStr
-    // where
-    //     InternedStr: Borrow<Q>,
-    //     Q: Hash + Eq + ToOwned<Owned = String>,
-    // {
-    //     if let Some(s) = self.strings.get(string) {
-    //         return s.clone();
-    //     }
-
-    //     self.insert(string.to_owned())
-    // }
+    pub(crate) fn string_ref<Q>(&mut self, string: &Q) -> InternedStr
+    where
+        InternedStr: Borrow<Q>,
+        Q: Hash + Eq + ToOwned<Owned = String> + ?Sized,
+    {
+        if let Some(s) = self.strings.get(string) {
+            return s.clone();
+        }
+
+        self.insert(string.to_owned())
+    }
 
     pub(crate) fn string(&mut self, string: String) -> InternedStr {
         if let Some(s) = self.strings.get(&string) {
diff --git a/tokio-console/src/state/mod.rs b/tokio-console/src/state/mod.rs
@@ -274,9 +274,15 @@ impl Metadata {
 
 impl Field {
     const SPAWN_LOCATION: &'static str = "spawn.location";
+    const KIND: &'static str = "kind";
     const NAME: &'static str = "task.name";
     const TASK_ID: &'static str = "task.id";
 
+    /// Creates a new Field with a pre-interned `name` and a `FieldValue`.
+    fn new(name: InternedStr, value: FieldValue) -> Self {
+        Field { name, value }
+    }
+
     /// Converts a wire-format `Field` into an internal `Field` representation,
     /// using the provided `Metadata` for the task span that the field came
     /// from.
diff --git a/tokio-console/src/state/tasks.rs b/tokio-console/src/state/tasks.rs
@@ -88,14 +88,22 @@ pub(crate) struct Task {
     span_id: SpanId,
     /// A cached string representation of the Id for display purposes.
     id_str: String,
+    /// A precomputed short description string used in the async ops table
     short_desc: InternedStr,
+    /// Fields that don't have their own column, pre-formatted
     formatted_fields: Vec<Vec<Span<'static>>>,
+    /// The task statistics that are updated over the lifetime of the task
     stats: TaskStats,
+    /// The target of the span representing the task
     target: InternedStr,
+    /// The name of the task (when `tokio::task::Builder` is used)
     name: Option<InternedStr>,
     /// Currently active warnings for this task.
     warnings: Vec<Linter<Task>>,
+    /// The source file and line number the task was spawned from
     location: String,
+    /// The kind of task, currently one of task, blocking, block_on, local
+    kind: InternedStr,
 }
 
 #[derive(Debug)]
@@ -171,25 +179,38 @@ impl TasksState {
                 };
                 let mut name = None;
                 let mut task_id = None;
+                let mut kind = strings.string(String::new());
+                let target_field = Field::new(
+                    strings.string_ref("target"),
+                    FieldValue::Str(meta.target.to_string()),
+                );
                 let mut fields = task
                     .fields
                     .drain(..)
                     .filter_map(|pb| {
                         let field = Field::from_proto(pb, meta, strings)?;
                         // the `task.name` field gets its own column, if it's present.
-                        if &*field.name == Field::NAME {
-                            name = Some(strings.string(field.value.to_string()));
-                            return None;
+                        match &*field.name {
+                            Field::NAME => {
+                                name = Some(strings.string(field.value.to_string()));
+                                None
+                            }
+                            Field::TASK_ID => {
+                                task_id = match field.value {
+                                    FieldValue::U64(id) => Some(id as TaskId),
+                                    _ => None,
+                                };
+                                None
+                            }
+                            Field::KIND => {
+                                kind = strings.string(field.value.to_string());
+                                None
+                            }
+                            _ => Some(field),
                         }
-                        if &*field.name == Field::TASK_ID {
-                            task_id = match field.value {
-                                FieldValue::U64(id) => Some(id as TaskId),
-                                _ => None,
-                            };
-                            return None;
-                        }
-                        Some(field)
                     })
+                    // We wish to include the target in the fields as we won't give it a dedicated column.
+                    .chain([target_field])
                     .collect::<Vec<_>>();
 
                 let formatted_fields = Field::make_formatted(styles, &mut fields);
@@ -220,6 +241,7 @@ impl TasksState {
                     target: meta.target.clone(),
                     warnings: Vec::new(),
                     location,
+                    kind,
                 };
                 if let TaskLintResult::RequiresRecheck = task.lint(linters) {
                     next_pending_lint.insert(task.id);
@@ -307,6 +329,10 @@ impl Task {
         &self.target
     }
 
+    pub(crate) fn kind(&self) -> &str {
+        &self.kind
+    }
+
     pub(crate) fn short_desc(&self) -> &str {
         &self.short_desc
     }
diff --git a/tokio-console/src/view/tasks.rs b/tokio-console/src/view/tasks.rs
@@ -26,7 +26,7 @@ impl TableList<12> for TasksTable {
     type Context = ();
 
     const HEADER: &'static [&'static str; 12] = &[
-        "Warn", "ID", "State", "Name", "Total", "Busy", "Sched", "Idle", "Polls", "Target",
+        "Warn", "ID", "State", "Name", "Total", "Busy", "Sched", "Idle", "Polls", "Kind",
         "Location", "Fields",
     ];
 
@@ -78,15 +78,15 @@ impl TableList<12> for TasksTable {
         let mut id_width = view::Width::new(Self::WIDTHS[1] as u16);
         let mut name_width = view::Width::new(Self::WIDTHS[3] as u16);
         let mut polls_width = view::Width::new(Self::WIDTHS[7] as u16);
-        let mut target_width = view::Width::new(Self::WIDTHS[8] as u16);
+        let mut kind_width = view::Width::new(Self::WIDTHS[8] as u16);
         let mut location_width = view::Width::new(Self::WIDTHS[9] as u16);
 
         let mut num_idle = 0;
         let mut num_running = 0;
 
         let rows = {
             let id_width = &mut id_width;
-            let target_width = &mut target_width;
+            let kind_width = &mut kind_width;
             let location_width = &mut location_width;
             let name_width = &mut name_width;
             let polls_width = &mut polls_width;
@@ -134,7 +134,7 @@ impl TableList<12> for TasksTable {
                         dur_cell(task.scheduled(now)),
                         dur_cell(task.idle(now)),
                         Cell::from(polls_width.update_str(task.total_polls().to_string())),
-                        Cell::from(target_width.update_str(task.target()).to_owned()),
+                        Cell::from(kind_width.update_str(task.kind()).to_owned()),
                         Cell::from(location_width.update_str(task.location()).to_owned()),
                         Cell::from(Spans::from(
                             task.formatted_fields()
@@ -186,7 +186,7 @@ impl TableList<12> for TasksTable {
             Span::from(format!(" Idle ({})", num_idle)),
         ]);
 
-        /* TODO: use this to adjust the max size of name and target columns...
+        /* TODO: use this to adjust the max size of name and kind columns...
         // How many characters wide are the fixed-length non-field columns?
         let fixed_col_width = id_width.chars()
             + STATE_LEN
@@ -195,7 +195,7 @@ impl TableList<12> for TasksTable {
             + DUR_LEN as u16
             + DUR_LEN as u16
             + POLLS_LEN as u16
-            + target_width.chars();
+            + kind_width.chars();
         */
         let warnings = state
             .tasks_state()
@@ -257,7 +257,7 @@ impl TableList<12> for TasksTable {
             layout::Constraint::Length(DUR_LEN as u16),
             layout::Constraint::Length(DUR_LEN as u16),
             polls_width.constraint(),
-            target_width.constraint(),
+            kind_width.constraint(),
             location_width.constraint(),
             fields_width,
         ];
