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

[prototype] tracing: centralize storage of structured events #59310

Closed

Conversation

irfansharif
Copy link
Contributor

I think it'll make addressing questions like #59145 easier. By
disaggregating storage from the spans themselves, questions around
"consuming" tracing events are a bit simpler to reason about. I think
we'll want to do something like this anyway if we want to cap the total
memory usage of structured events across active spans.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

...into it's own thing. We're still working out exactly
how structured payloads are stored, how their ownership flows
internally. See the recent discussion in cockroachdb#59145 for our state of
confusion.

Release note: None
I think it'll make addressing questions like cockroachdb#59145 easier. By
disaggregating storage from the spans themselves, questions around
"consuming" tracing events are a bit simpler to reason about. I think
we'll want to do something like this anyway if we want to cap the total
memory usage of structured events across active spans.

Release note: None
@irfansharif
Copy link
Contributor Author

irfansharif commented Jan 28, 2021

Nothing we'll get to soon, we're too close to the stability cutoff. I still think this is a good idea in general, and something we should move towards. It's a small change, and makes a few things a bit more natural:

  • Memory accounting: we'll have a centralized place for where it all happens, instead of threading it in through individual spans and child spans. It also lets us introduce a global debug switch to drop all structured recordings we’re holding onto, or control what the RSS is for the current set of observable structured events.
  • Historical introspection: by decoupling the garbage collection of structured events from the lifecycle of the span itself, we can go back in time (very briefly) and see what the collected events were. Right now we can only introspect in-flight spans. This might not come in super handy, but it also might, given spans are very short-lived in practice.
  • We can refer to child spans by Span ID, instead of building an in-memory graph to traverse through. It helps obviate memory issues and trickiness we're observing with forked spans that outlive parents, and parents that might still want to hold onto references to child spans for traversal. [dnm] tracing: clean up use of auto collection across tasks #59391.

@irfansharif irfansharif changed the title [wip] tracing: centralize storage of structured events [prototype] tracing: centralize storage of structured events Jan 28, 2021
@irfansharif
Copy link
Contributor Author

Another perk of having span-centric storage comes from #61353. At the time of writing, only root spans are accessible through the registry. So a built-in that's able to selectively toggle the verbosity of a span is of limited use; we can't isolate a random child span because it's only indirectly accessible (we'd need to traverse the tree of the root span).

@irfansharif irfansharif deleted the 210122.span-registry branch March 15, 2021 16:36
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