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

Too Many IDs #407

Open
hds opened this issue Apr 4, 2023 · 0 comments
Open

Too Many IDs #407

hds opened this issue Apr 4, 2023 · 0 comments
Labels
S-feature Severity: feature. This is adding a new feature.

Comments

@hds
Copy link
Collaborator

hds commented Apr 4, 2023

What problem are you trying to solve?

Currently Console has 3 separate classes of IDs that it uses across different objects. This is probably 1 more than is necessary, especially inside tokio-console itself.

All the IDs

Span ID

This could also be called the instrumentation ID, as it's the one generated by the instrumentation.

All objects have a Span ID, because the span is how the object llifetime is tracked.

Console ID

The Span ID incorporates the thread that a span is created on, meaning that it creates very large numbers, even early in an application's execution time. For this reason Console creates its own display ID which is nice an sequential.

The issue with the Console ID is that it gets reset if tokio-console is restarted and so it isn't stable across restarts of Console, even when the monitored application keeps running. This was already brough up in #385 and is why for tasks, another ID is used.

Task ID

This is the ID which is assigned to a task by the runtime. It could be called the Tokio ID, the Runtime ID (although this becomes problematic once we start tracking runtimes as described in #130), or even the Native ID if we want to be generic.

This ID is nice for tasks because it's stable across tokio-console restarts and it's globally sequential across all (1.x Tokio) runtimes, so you get small pretty numbers. The downside is that neither Resources, nor AsyncOps have a comparable ID.

Naming

There is also an issue with naming. Strictly speaking, this is an issue in Tokio, but it's worth mentioning here and then perhaps an issue can be created over there.

There are 2 traces which have a task.id field:

  • runtime.spawn span: In this case, the field refers to the Task ID as described above.
  • tokio::task::waker: event: The Span ID of the task being woken.
  • runtime.resource.async_op.poll event: In this case, the field refers to the Span ID of the task which is being polled.

I'm not even 100% sure this list is complete or correct, which is kind of part of the problem.

How should the problem be solved?

Part of the issue with naming, is that in console, on the Task struct we have fields id, task_id, span_id. Then on the the AsyncOpStats struct we have a field called task_id, but it's not clear which ID on the task it refers to.

While we have fixed #385 by using the ID from Tokio, the same issue exists for resources and async ops.

Display ID gets maintained in the console subscriber

The first step is to maintain the display ID in the console subscriber. This will fix #385 for resoruces and async ops as well. In fact, it would fix it for tasks, but we probably want to continue to use the Tokio task ids.

By keeping the display ID in the subscriber, the console only needs to know that it's a display ID, not how it's generated. We can use the Tokio task ID for tasks and a generated ID for resources and async ops.

This reduces the number of IDs that need to be kept at any point to 2. Span ID and Display ID. The Display ID would be the tokio generated Task ID in the case of tasks, or a console-subscriber sequentially generated ID otherwise.

Fix the naming in spans created in Tokio

I think it's also worth considering fixing the task.id field name in waker events in Tokio. I'm not sure how to do this in a backwards compatible way though - aside from doubling up on the name for some period of time before removing the previous one.

This would be to keep past versions of console-subscriber working for a while with newer versions of tokio more than anything else.

My proposal would be to rename the field to one of:

  • task.sid for Task Span ID
  • task.iid for Task Instrumentation ID (to be more generic)
  • task.span.id to be really explicit
@hds hds added the S-feature Severity: feature. This is adding a new feature. label Apr 4, 2023
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

1 participant