Skip to content

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

Closed
irfansharif wants to merge 2 commits intocockroachdb:masterfrom
irfansharif:210122.span-registry
Closed

[prototype] tracing: centralize storage of structured events#59310
irfansharif wants to merge 2 commits intocockroachdb:masterfrom
irfansharif:210122.span-registry

Conversation

@irfansharif
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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