Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: Selective disabling option for N+1 warnings #4920

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

Open
Dibbu-cell wants to merge 4 commits into getsentry:master
base: master
Choose a base branch
Loading
from Dibbu-cell:change5

Conversation

@Dibbu-cell
Copy link

@Dibbu-cell Dibbu-cell commented Oct 13, 2025

Description:-
This PR introduces an opt-out mechanism for N+1 detection in the Python SDK so users can selectively mark code paths where N+1-style queries are intentionally acceptable.

Issues:-
resolves: #4887

Reminders:-

Please add tests to validate your changes, and lint your code using tox -e linters.
Add GH Issue ID & Linear ID (if applicable)
PR title should use conventional commit style (feat:, fix:, ref:, meta:)
For external contributors: CONTRIBUTING.md, Sentry SDK development docs, Discord community

Hi! This PR is for Hacktoberfest.

Please add the hacktoberfest-accepted label if it cannot be merged soon. Thank you!

ulgens reacted with heart emoji
@Dibbu-cell Dibbu-cell requested a review from a team as a code owner October 13, 2025 13:52
cursor[bot]

This comment was marked as outdated.

Comment on lines 19 to 27
tx = sentry_sdk.get_current_span()
if tx is not None:
try:
tx.set_tag("sentry.n_plus_one.ignore", True)
if reason:
tx.set_tag("sentry.n_plus_one.reason", reason)
except Exception:
# best-effort
pass
Copy link

@seer-by-sentry seer-by-sentry bot Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential bug: The allow_n_plus_one() helper only sets tags on the current span, not the parent transaction, when called within a nested span.
  • Description: The allow_n_plus_one() function sets tags only on the span returned by get_current_span(). When this function is called within a nested span context, get_current_span() returns the child span, not the transaction. The tags are therefore not propagated to the containing transaction. Since the server-side N+1 detector checks for these tags at the transaction level, the feature will fail to ignore N+1 patterns in the common use case where database operations are wrapped in their own child spans.

  • Suggested fix: In allow_n_plus_one(), after setting the tag on the current span, check if span.containing_transaction exists. If it does, set the same tag on the transaction object to ensure the tag is propagated correctly for server-side detection.
    severity: 0.7, confidence: 0.95

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Author

i wanted to insist please see my Pr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seer-by-sentry seer-by-sentry[bot] seer-by-sentry[bot] left review comments

@cursor cursor[bot] cursor[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Selective disabling option for N+1 warnings

1 participant

AltStyle によって変換されたページ (->オリジナル) /