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

Span management is costly even when not sampling #668

Open
Ten0 opened this issue Jul 12, 2024 · 1 comment
Open

Span management is costly even when not sampling #668

Ten0 opened this issue Jul 12, 2024 · 1 comment

Comments

@Ten0
Copy link
Contributor

Ten0 commented Jul 12, 2024

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:

let parent_sentry_span = sentry_core::configure_scope(|s| s.get_span());

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.:

// Add the data from the original span to the sentry span.
// This comes from typically the `fields` in `tracing::instrument`.
for (key, value) in data {
sentry_span.set_data(key, value);
}

(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.
@Swatinem
Copy link
Member

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 :-( tokio-rs/tracing#2881

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

No branches or pull requests

2 participants