sentry-rust icon indicating copy to clipboard operation
sentry-rust copied to clipboard

Span management is costly even when not sampling

Open Ten0 opened this issue 1 year ago • 1 comments
trafficstars

Span management operations seem to be relatively costly, making them unsuited for profiling otherwise fast ~zero-alloc code.

This wouldn't matter much if it was only for sampled transactions, but this is actually for every one of them: Notably after fetching the current scope: https://github.com/getsentry/sentry-rust/blob/e828ca28a53994a64ef91e72f1f6fe7e10c6ea3c/sentry-tracing/src/layer.rs#L210

There's no way to check whether it will eventually be sampled.

This means that integrations such as that of tracing but also custom integrations (I'm writing one at the moment) can't avoid generating a bunch of Strings, Arcs, serde_json::Value for every Span that the framework may decide to sample, even though the framework already knows that it won't be sampled, e.g.: https://github.com/getsentry/sentry-rust/blob/e828ca28a53994a64ef91e72f1f6fe7e10c6ea3c/sentry-tracing/src/layer.rs#L218-L222 (Where in most integrations, one could avoid generating a Value, if the trace won't be sampled.)

It looks like one could benefit of being able to:

  • Identify from a TransactionOrSpan whether it will be sampled, allowing to not create sub-spans and all their resources unless necessary
  • When creating new transactions, getting access to the sampling API so that we can know in advance whether the transaction will be sampled, and if not, not allocate e.g. Strings to build description, that then even get re-allocated in TransactionContext.

Ten0 avatar Jul 12 '24 13:07 Ten0

You are absolutely right, the Sentry tracing/span types / APIs are unfortunately not tuned to be very high performant. I very much appreciate contributions to make them more so.

This was also very obvious to me a while back when I was profiling the tracing / metrics overhead. While doing so, I also noticed that tracing itself suffers from a similar problem where all of the span attributes are eagerly collected, even if you are never interested in those. My proposed PR to work around that however did not get any attention :-( https://github.com/tokio-rs/tracing/pull/2881

Swatinem avatar Jul 15 '24 08:07 Swatinem

I am going to close this issue for now. While we consider this to be a legitimate request, our team lacks the capacity to implement this improvement anytime in the foreseeable future, and we would like to avoid giving folks unrealistic expectations.

We would of course gladly accept PRs; this is just not something we have the capacity to work on.

szokeasaurusrex avatar Dec 13 '24 13:12 szokeasaurusrex