Skip to content

Conversation

Dibbu-cell
Copy link

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!

@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

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.

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

Successfully merging this pull request may close these issues.

Selective disabling option for N+1 warnings

1 participant