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

Standardize on span naming #41

Closed
seanmonstar opened this issue May 28, 2021 · 5 comments
Closed

Standardize on span naming #41

seanmonstar opened this issue May 28, 2021 · 5 comments
Assignees
Labels
S-feature Severity: feature. This is adding a new feature.
Milestone

Comments

@seanmonstar
Copy link
Member

The prototype currently just looks for a span named tokio::task::spawn, but if want to work for any "runtime"-like thingy, we probably should have a more generic name. It still needs to be unique enough that a user would be very unlikely to create a span with the same name.

Besides task spawning, we also need to do this for wakers and resources too.

@hawkw
Copy link
Member

hawkw commented Jun 15, 2021

I think we should probably care about just a standardized span name, rather than a standardized span name and tracing target. I think it would be good to allow runtimes to have the target differ,

For example, @nikomatsakis mentioned wanting to instrument rayon with the console. So, if you're using tokio and rayon in the same program, we could distinguish between spans named spawn with the tokio::task target, and spans named spawn with the rayon target. Although, on the other hand, we could use a custom target but also display the module path.

The other thing is whether spawn is the correct name. We may want to use something less generic, to ensure that people don't accidentally create spans with the spawn name that aren't actually supposed to correspond to tasks.

Perhaps one option would be to require overridden targets beginning with console, as well as the spawn span naming? I dunno, just throwing out ideas here.

@hawkw hawkw added the S-feature Severity: feature. This is adding a new feature. label Jun 15, 2021
@hawkw hawkw self-assigned this Jun 15, 2021
@carllerche
Copy link
Member

To help avoid name collision, we can probably include a prefix. e.g. "runtime.spawn".

@hawkw
Copy link
Member

hawkw commented Jun 16, 2021

+1 for @carllerche's suggestion of runtime., that seems like a good naming convention to indicate that the span is provided by the runtime's instrumentation rather than user code, and avoid colliding with user spans, but without tying the naming scheme to an individual runtime implementation.

@carllerche
Copy link
Member

Ref: tokio-rs/tokio#3954 with regards to figuring out how to name resource spans. My guess is we need a "kind" annotation to identify resource spans vs. operation spans as well.

hawkw added a commit to hawkw/rayon that referenced this issue Jul 12, 2021
The instrumentation conforms to the standardized naming requirements for
`tokio-console` (see tokio-rs/console#41), so they will show up in the
console.

The `tracing` dependency is an opt-in feature flag.

Signed-off-by: Eliza Weisman <[email protected]>
@hawkw
Copy link
Member

hawkw commented Jul 22, 2021

I believe #68 completed the console side work for this. We still need to update tokio to use the new naming scheme, though.

@hawkw hawkw closed this as completed Jul 22, 2021
@seanmonstar seanmonstar added this to the 0.1 milestone Aug 13, 2021
hawkw added a commit to tokio-rs/tokio that referenced this issue Aug 16, 2021
Currently, the per-task spans generated by Tokio's `tracing` feature
have the span name "task" and the target "tokio::task". This is because
the console subscriber identified tasks by looking specifically for the
"tokio::task" target.

In tokio-rs/console#41, it was proposed that the console change to a
more generic system for identifying the spans that correspond to tasks,
to allow recording tasks belonging to multiple runtime crates (e.g. an
application that uses Tokio for async tasks and Rayon for CPU-bound
tasks). PR tokio-rs/console#68 changed the console to track any spans
"runtime.spawn", regardless of target (so that the target can be used to
identify the runtime a task came from), with "tokio::task/task" tracked
for backwards-compatibility with the current release version of tokio.

This branch changes Tokio's span naming to the new convention. I also
rearranged a couple fields so that the task's kind field always comes
before the name and spawn location, since it's likely to be the
shortest, and renamed the `function` field on blocking tasks to `fn`,
for brevity's sake.

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit to tokio-rs/tokio that referenced this issue Aug 16, 2021
Currently, the per-task spans generated by Tokio's `tracing` feature
have the span name "task" and the target "tokio::task". This is because
the console subscriber identified tasks by looking specifically for the
"tokio::task" target.

In tokio-rs/console#41, it was proposed that the console change to a
more generic system for identifying the spans that correspond to tasks,
to allow recording tasks belonging to multiple runtime crates (e.g. an
application that uses Tokio for async tasks and Rayon for CPU-bound
tasks). PR tokio-rs/console#68 changed the console to track any spans
"runtime.spawn", regardless of target (so that the target can be used to
identify the runtime a task came from), with "tokio::task/task" tracked
for backwards-compatibility with the current release version of tokio.

This branch changes Tokio's span naming to the new convention. I also
rearranged a couple fields so that the task's kind field always comes
before the name and spawn location, since it's likely to be the
shortest, and renamed the `function` field on blocking tasks to `fn`,
for brevity's sake.

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit to hawkw/rayon that referenced this issue Feb 14, 2022
The instrumentation conforms to the standardized naming requirements for
`tokio-console` (see tokio-rs/console#41), so they will show up in the
console.

The `tracing` dependency is an opt-in feature flag.

Signed-off-by: Eliza Weisman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-feature Severity: feature. This is adding a new feature.
Projects
None yet
Development

No branches or pull requests

3 participants