Add errorsSampler for Sentry config to match functionality with tracesSampler (re-request)

[IgnusG]

Problem Statement

This issue is a plea to revisit the feature request #4800.

I came across this request while researching options for changing the sampling rate dynamically without reinitializing the client. During my research I found Sentry discouraged doing this re-initialization and while looking for alternative configuration options I came across tracesSampler. To my disappointment it was however not available for errors. After a bit of googling I found it was actually dismissed as not planned in the above feature request.

I've asked about alternative designs to this issue in this discussion and listed a few alternatives sentry offers, unfortunately none satisfactory I could find. I believe this is something that is hard enough to byo in user land and would be better done by this new configuration option in sentry.

This errorsSampler would be a perfect fit as it would allow users to pull in a dynamic value from anywhere (eg. fetching some dynamic configuration from the server) and return it to select a new sample rate. It could also be used to sample certain errors or errors from critical code blocks higher than other errors for example. Imagine a critical payment block where we want to receive 100% of errors, a semi critical search code block where we can get away with 50% to preserve quotas and a non important bloopers page where we still want to get some errors but only if they really occur frequently - let's say 1%.

None of this is possible with beforeSend unless the client writes a new sampler entirely (completely bypassing sentry's sampler). In that case they could do it with beforeSend but since tracesSampler already exists as well this feels like redundant work.

An argument was raised in the original thread that tracesSampler was required because traces could not be stopped from being sent in beforeSend. I'm not sure I understand that argument since we also have beforeSendTransaction which can be used exactly as beforeSend to block a trace from being sent. From that one could argue tracesSampler is then nothing more than syntactic sugar around beforeSendTransaction as well.

Solution Brainstorm

The following is an example of the proposed api:

import { fetchDynamicSampleRate } from "somewhere";

Sentry.init({
	errorsSampler: async () => {
		return await fetchDynamicSampleRate();
	}
});

This would allow truly dynamic sampling controlled by the client in addition to all the other sampling sentry is already doing.

Alternatively if there are technical limitations or api opposition towards making this method support asynchronous functions a slight adjustment (albeit slightly more cumbersome to work with) on the client's side would still make this possible:

import { pollDynamicSampleRate } from "somewhere";

let dynamicSampleRate = 1;

pollDynamicSampleRate((nextSampleRate) => {
	dynamicSampleRate = nextSampleRate;
});

Sentry.init({
	errorsSampler: () => {
		return dynamicSampleRate;
	}
});

I hope the team could at least give some guidance in the docs on how to approach such situations if the decision to not support this configuration option is final. Thank you Sentry Team 🙏

[mydea]

Hi @IgnusG ,

thanks for writing in!

An #4800 (comment) that tracesSampler was required because traces could not be stopped from being sent in beforeSend. I'm not sure I understand that argument since we also have beforeSendTransaction which can be used exactly as beforeSend to block a trace from being sent. From that one could argue tracesSampler is then nothing more than syntactic sugar around beforeSendTransaction as well.

Actually beforeSendTransaction is a more recent addition to our API, where tracesSampler precedes it.

I don't think having an async sampler is something we'll be adding anytime soon. I wonder, why can't you solve this with an async global event processor - you can register this whenever you want, and can use it to discard any errors you do not care about:

Sentry.addGlobalEventProcessor(async function (event, hint) {
  // returning `null` will drop the event
  // returning `event` will send the event

  // If you only care about errors (this triggers for all kinds of events!), filter for event.type (it is undefined for errors)
  if (event.type) {
    return event;
  }

  // filter and maybe return null
  return event;
});

Note that we discourage using async event processors - in most cases, it should be preferred to first do async lookups, then register the global event processor when you've got everything loaded you need.

I am closing this for now as we are not looking to expand our API surface in this area further (we already have rather too many ways to do things, not too little). feel free to reopen the issue if you think my proposal does not solve your use case suitably!

[IgnusG]

Hi @mydea. Thank you for the prompt reply. If the team discourages async sampling, it could be dropped in favour of the alternative approach I mentioned in the examples above (just like you mention retrieving the async information first and then using it in the global event processor).

Thank you for the solution you mentioned as well. Unfortunately this will still require you to bring your own sampler in order to use the global event processor. The question is not do we want to send this specific event or not, but rather what percentage of this type of errors do we want to send.

In your example we would have to create a sampler and then manually track all events as they come in. The custom sampler could then decide whether or not to return an event or not. Sentry's sampler on the other hand would have to be set to 1 in order to not interfere with the custom sampler, effectively bypassing the work sentry already knows how to do best and replacing it with a custom implementation.

Here's an example of how that could look:

// we have to bring our own sampler since we are unable to modify Sentry's
import { Sampler } from "probably-something-custom";

const customSampler = new Sampler();

customSampler.forErrorRegisterSampleRate({ errorMatcher: /Critical Payment Flow/, sampleRate: 1 });
customSampler.forErrorRegisterSampleRate({ errorMatcher: /Search Flow/, sampleRate: 0.5 });
customSampler.forErrorRegisterSampleRate({ errorMatcher: /Bloopers Page/, sampleRate: 0.01 });

Sentry.addGlobalEventProcessor(async function (event, hint) {
	// event is not an error, return it as is
	if (event.type !== undefined) return event;

	const hit = customSampler.sampleEvent(event.originalError);
	
	if (hit) return event;

	return null;
});

Btw. I'm unable to re-open the issue even if I wanted to since there's no option to do that on my side

[IgnusG]

Alternatively if Sentry would expose its sampler which the user could use inside of the global event processor that would also help. But I'm not sure how interconnected the sampler is in Sentry and if it's even feasible to offer it as something the user can import and use on their own.

[mydea]

I think you may be overestimating what the default sampling behavior does ;) The sampling function based on the sampleRate is just:

Math.random() > sampleRate

So what you need to do is:

Sentry.addGlobalEventProcessor(function (event, hint) {
	// event is not an error, return it as is
	if (event.type !== undefined) return event;

        const sampleRate = getExpectedSampleRateForError(event);
        return Math.random() > sampleRate ? null : event;
});

There are also other steps for sampling (e.g. deduping events etc.) but all of this would still happen regardless of what you do in there. So this solution should do what you want (+ set sampleRate: 1 overall, as you mentioned).

For reference, you can think of the sampling as a pipe:

1. Event comes in
2. Check Math.random() > sampleRate. If matches, discard
3. Pass event through all registered global event processors. if any returns null, discard.
4. Pass event to beforeSend. if it returns null, discard
5. Send event

[IgnusG]

And here I was thinking of some sophisticated statistical engine sentry's running on the client 😅 But I guess it makes somewhat sense, you wouldn't want to slow down the client just to perform complex sampling.

I still think it might be good to include this option sometime in the future (at least if the sampling engine ever gets more complex, thus the users would not have to worry about keeping their own sampling implementation sound). But in the meantime this should be enough to do it.

Thanks for the help @mydea 🙏