Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUST-2032 track task spawn location for tokio instrumentation #1201

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

hds
Copy link
Contributor

@hds hds commented Sep 11, 2024

The tracing instrumentation present in tokio (which feeds into
tokio-console) tracks the spawning location of each task. This can be
useful in understanding what each task is doing, especially if the tasks
aren't named (which is a tokio_unstable feature so not usually present
in published crates).

The location tracking isn't as useful when tasks are spawned from some
central location like the mongodb crate does, because all tasks have
the same spawn location. However, since the instrumentation uses the
panic:Location to determine the spawn location, the #[track_caller]
attribute macro can be used to ensure that the actual location is picked
up instead of the common one.

A side effect of using the #[track_caller] attribute is that the
single line output in the case of a panic (when RUST_BACKTRACE isn't
set) will point to the callsite in the code, and not the common site -
although this is often desirable anyway.

This change adds #[track_caller] to the pub(crate) function
crate::runtime::spawn as well as the method AsyncJoinHandle::spawn.

The tracing instrumentation present in tokio (which feeds into
tokio-console) tracks the spawning location of each task. This can be
useful in understanding what each task is doing, especially if the tasks
aren't named (which is a `tokio_unstable` feature so not usually present
in published crates).

The location tracking isn't as useful when tasks are spawned from some
central location like the `mongodb` crate does, because all tasks have
the same spawn location. However, since the instrumentation uses the
`panic:Location` to determine the spawn location, the `#[track_caller]`
attribute macro can be used to ensure that the actual location is picked
up instead of the common one.

A side effect of using the `#[track_caller]` attribute is that the
single line output in the case of a panic (when `RUST_BACKTRACE` isn't
set) will point to the callsite in the code, and not the common site -
although this is often desirable anyway.

This change adds `#[track_caller]` to the `pub(crate)` function
`crate::runtime::spawn` as well as the method `AsyncJoinHandle::spawn`.
@hds
Copy link
Contributor Author

hds commented Sep 11, 2024

A couple of notes on the MR. This came about after receiving a question about the behaviour of an application which was using the mongodb crate and had this screenshot from tokio-console:

console-mongodb_tasks

All those tasks would have a more useful value in the Location column with this patch.

No tests are included because testing #[track_caller] is a little bit involved - and testing it with the tokio instrumentation enabled even more so. However, I'm happy to add tests by forcing a panic and recording the panic location, they would look something like the ones here:

https://github.com/tokio-rs/tokio/blob/91169992b2ed0cf8844dbaa0b4024f9db588a629/tokio/tests/rt_panic.rs

with the following helper function:

https://github.com/tokio-rs/tokio/blob/91169992b2ed0cf8844dbaa0b4024f9db588a629/tokio/tests/support/panic.rs

@abr-egn abr-egn self-requested a review September 11, 2024 15:18
@abr-egn
Copy link
Contributor

abr-egn commented Sep 11, 2024

This looks like a very nice enhancement, thank you! I've kicked off the test run and if there aren't any known failures I'll merge this in :)

@abr-egn abr-egn merged commit 6502b5f into mongodb:main Sep 11, 2024
13 of 15 checks passed
@abr-egn abr-egn changed the title track task spawn location for tokio instrumentation RUST-2032 track task spawn location for tokio instrumentation Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants