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

Sampling based Transaction.NoOp to easy memory pressure #3636

Open
bitsandfoxes opened this issue Sep 27, 2024 · 1 comment
Open

Sampling based Transaction.NoOp to easy memory pressure #3636

bitsandfoxes opened this issue Sep 27, 2024 · 1 comment

Comments

@bitsandfoxes
Copy link
Contributor

What

Came up in #3581 (comment)
It does not matter whether a transaction gets sampled or not, they still require the same amount of resources - i.e. memory.

How

We could expand on this on the NoOp. Instead of a NoOpTransaction singleton we could pass around something like a SampledTransaction that are functionally NoOp but hold the data required to provide a client report - i.e. span count. I guess the same could work for spans? That way we'd get rid of all the data we know will get discarded in the end anyway.

Context

We introduced the NoOp transaction with the SDK disabled here: https://github.com/getsentry/sentry-dotnet/pull/3581/files and we could expand on this.

When starting a transaction it either has its sampling decision already set or it goes through the TracesSampler (if provided)

if (tracesSampler(samplingContext) is { } sampleRate)
{
transaction.IsSampled = _randomValuesFactory.NextBool(sampleRate);
transaction.SampleRate = sampleRate;
}

or it gets it's sampling state set randomly

transaction.IsSampled = _randomValuesFactory.NextBool(sampleRate);

So we know right from the beginning if a transaction ends up in a client report or not.

@Joshhua5
Copy link

Thanks for creating this, we've had to slow down our Sentry adoption on some high throughput API's due to the overhead causing OOM in Kubernetes, reducing the sampling rate wasn't effective at curbing the overhead of adding sentry. Which this ticket targets directly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Status: No status
Development

No branches or pull requests

2 participants