Selective disabling option for N+1 warnings

[ulgens]

Problem Statement

We are currently having lots of false-ish positives from N+1 detection (with Django). We have multiple loops that we iterate over a queryset, make internal - external calls, update data on related tables, send notifications etc. Sentry detects them as N+1, which is technically true, but also there is no feasible way for us to solve them and we are perfectly happy with those loops.

Sentry has several settings about how N+1s are detected, but there is nothing that equivalent of # noqa

Solution Brainstorm

Sentry should support a # noqa style selective disabling mechanism for N+1 issue detection.

[linear[bot]]

PY-1873

[sentrivana]

Hey @ulgens, thanks for writing in. For context, N+1 issues are created on the server, the SDK just provides the spans.

This sounds like a usecase for archiving/deleting/ignoring issues in Sentry directly, see here. You can also opt out of performance issues completely. Do these solutions work for you, and if not, why not? I would like to avoid adding a whole new mechanism and complexity to the SDK (turning off features via code comments) for this if it can be solved differently.

[ulgens]

For context, N+1 issues are created on the server, the SDK just provides the spans.

Thanks for the info.

This sounds like a usecase for archiving/deleting/ignoring issues in Sentry directly, see here. You can also opt out of performance issues completely. Do these solutions work for you, and if not, why not?

Ignoring issues doesn't always work, for several reasons:

• Sentry's issue grouping is not always predictable. Sometimes the same block of code gets multiple issues/logs because it was called from different places. Sometimes different logs get grouped together because their messages are similar. The process doesn't feel deterministic, and controlling is almost impossible - the best you can do is trust Sentry or ignore the problem. "Ignore" feature doesn't work well in many scenarios because it leads to ignoring things I don't want, or not ignoring the things I want.
• Ignored issues still eat up from the quota.

Opting out doesn't work either because I don't want to fully disable the detection. It's just some specific cases that we expect and are okay with those N+1s, and in the code itself, we want to tell Sentry that those are okay.

I would like to avoid adding a whole new mechanism and complexity to the SDK (turning off features via code comments)

I totally understand being hesitant about a new mechanism, but I think this is not a "whole new mechanism"; this is a missing vital feature. All linting tools I'm currently using give an option to ignore specific cases. Some allow # noqa style comments, some require ignore/exclude=<file_name>, some allow both. Not having this option converts the "N+1 detection" mechanism into something that is too noisy, leading to alert fatigue.

[sentrivana]

Thanks for the follow up @ulgens.

I get your point about grouping being unstable -- this is definitely a concern. However, for N+1 issues, I'd have expected this to be much less of a problem as they come from database spans directly and thus should be much more resilient to things that usually make exception grouping a pain (the stacktrace is not involved at all, for instance).

You are correct though that ignoring issues still counts towards your quota (EDIT: this is incorrect, see https://github.com/getsentry/sentry-python/issues/4887#issuecomment-3371820225), because as I learned today:

There's also an option to Delete and Discard Forever, which will make it so that the issue is never seen again, even if it recurs. Any future events tied to the permanently deleted issue will be discarded automatically and won't count towards your quota.

You can only delete Error Issues. Other issue categories, such as Performance Issues, don't support this feature.

(EDIT: The above is also not correct, just a case of outdated docs, which we've now fixed.)

If this is time sensitive, there is one hacky workaround I can think of, which would be to modify the spans in a before_send_transaction such that the N+1 issue detector will not trigger on them. Looks like adding ... to the end of the span description would do the trick. 🙃

I'll check what the rest of the team thinks regarding a more proper solution.

[sentrivana]

Correction: Just got word from our performance team, performance issues like n+1 actually don't count towards any quota. Also, the docs bit about not being able to delete performance issues is outdated, we'll fix that.

[ulgens]

Just got word from our performance team, performance issues like n+1 actually don't count towards any quota.

@sentrivana Thank you. This is good to know, but I believe my original request for a "noqa" option is still valid.

Looks like adding ... to the end of the span description would do the trick. 🙃

Doesn't that mean there is already a way to selectively ignore spans, from the code? I mean, I'm not sure what exactly is query there but if it can process query.endswith("..."), I think it can also process query.endswith("# noqa: sentry:n+1")