a sketch for how task naming might work for tracing

[jbr]

Motivation

Currently, there is no way to attach a name to a task span for tokio-rs/console, but it would be great if that could be a predictably-present field that describes the task to a human reader. This is a quick sketch of one way this might look.

Solution

This PR adds a cfg_trace-gated unstable function tokio::task::spawn_named, which allows users to annotate a task at runtime with an appropriate name. Completing this PR would likely involve adding similar …_named spawn interfaces for spawn_blocking and spawn_local.

Example Usage

Modified from the console-subscriber example app:

use std::time::Duration;

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
    console_subscriber::init();

    let task1 = tokio::task::spawn_named("first", spawn_tasks(1, 10));
    let task2 = tokio::task::spawn_named("second", spawn_tasks(10, 100));
    let result = tokio::try_join! {
        task1,
        task2,
    };
    result?;

    Ok(())
}

#[tracing::instrument]
async fn spawn_tasks(min: u64, max: u64) {
    loop {
        for i in min..max {
            tokio::task::spawn_named(&format!("spawned {}", i), wait(i));
            tokio::time::sleep(Duration::from_secs(max) - Duration::from_secs(i)).await;
        }
    }
}

#[tracing::instrument]
async fn wait(seconds: u64) {
    tokio::time::sleep(Duration::from_secs(seconds)).await;
}

[carllerche]

Looks like a good start, left an inline comment.

[carllerche]

Couple of thoughts.

First, is there a reason to avoid the builder patter? Generally, we try to match the std API and specifying a name matches up w/ the thread::Builder pattern. Also, using a builder could allow us to add Builder::spawn_blocking as a separate API.

Then, what was the motivation to use impl Into<std::borrow::Cow<'static, str>>? Looking at the code, it seems like &str should work?

[carllerche]

To elaborate a bit, we can deviate from copying std APIs and patterns, but we should have a good reason to do so.

[jbr]

The Cow was just to avoid people having to type the ampersand in &format!("{}", ..) instead of format!("{}", ..), but &str is simpler. Fixed.

I'll take a look at doing a builder, it just seemed like a lot of ceremony for a single property that can be set, unlike std::thread::Builder which also has stack_size. The only reason someone would reach for the builder is to provide a name, unless there have been other proposed/future properties on a task (eg priority).

tokio::task::Builder::new().name("some task").spawn(async { … })
// vs
tokio::task::spawn_named("some task", async { … })

[carllerche]

The Cow was just to avoid people having to type the ampersand in &format!("{}", ..) instead of format!("{}", ..), but &str is simpler. Fixed.

I'll take a look at doing a builder, it just seemed like a lot of ceremony for a single property that can be set, unlike std::thread::Builder which also has stack_size. The only reason someone would reach for the builder is to provide a name, unless there have been other proposed/future properties on a task (eg priority).

tokio::task::Builder::new().name("some task").spawn(async { … })
// vs
tokio::task::spawn_named("some task", async { … })

The Cow was just to avoid people having to type the ampersand in &format!("{}", ..) instead of format!("{}", ..), but &str is simpler. Fixed.

Ah, would impl AsRef<str> work?

I'll take a look at doing a builder, it just seemed like a lot of ceremony for a single property that can be set, unlike std::thread::Builder which also has stack_size. The only reason someone would reach for the builder is to provide a name, unless there have been other proposed/future properties on a task (eg priority).

tokio::task::Builder::new().name("some task").spawn(async { … })
// vs
tokio::task::spawn_named("some task", async { … })

I expect we will have additional fields. For example, we will need a way to specify attributes. e.g.

tokio::task::Builder::new()
    .name("connection-handler")
    .attribute("peer-addr", socket.peer_addr())
    .spawn(async { ... });

Also, as I think about it, it would be an ideal place to add other configuration, like priority (if we do that).

[jbr]

Backfilling explanation: The reason I thought we needed a cow was I had assumed tracing needed attribute values to be 'static, but that doesn't seem to be the case, which makes sense in retrospect

[carllerche]

Do you plan on opening a new PR (is that why it is closed?)

[jbr]

Yep, but need to put some more thought into how this should work. I checked with @hawkw and attributes names need to be known at compile time, so it might be that annotating the spawn call with a span is actually more flexible (and therefore more ergonomic) than a builder. I'm happy to throw together a quick builder PR with names only, though.

[carllerche]

The initial PR should be name only, but the design direction does depend on details of things like attributes. I will ping @hawkw to weigh in here.

[jbr]

Continued at #3881