Skip to content

feat(perf-issues): Allow experimental N+1 DB Span detector to pick up MongoDB queries #90756

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

Merged
merged 3 commits into from
May 6, 2025

Conversation

leeandher
Copy link
Member

Allows the N+1 detector to operate on MongoDB queries. The example JSON comes from my prod trace that i generated from playing around with empowerplant. Tests work but maybe there's more coverage I should add but I'm not too sure.

@leeandher leeandher requested review from a team as code owners May 1, 2025 19:04
@leeandher leeandher changed the title feat(perf-issues): Allow experimental detector to pick up MongoDB queries feat(perf-issues): Allow experimental N+1 DB Span detector to pick up MongoDB queries May 1, 2025
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 1, 2025
Comment on lines -240 to -245
def _contains_valid_repeating_query(self, span: Span) -> bool:
# Make sure we at least have a space, to exclude e.g. MongoDB and
# Prisma's `rawQuery`.
query = span.get("description", None)
return bool(query) and " " in query

Copy link
Member Author

Choose a reason for hiding this comment

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

So I took apart this function and moved it's check to the visit_span method via are_spans_equivalent.

b_relay_description = b.get("sentry_tags", {}).get("description")
return a_relay_description == b_relay_description and base_checks

return base_checks
Copy link
Member Author

Choose a reason for hiding this comment

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

Now, instead of just checking a.hash against b.hash we're also validating that the descriptions match, and aren't empty. These two checks used to only happen after the detector ran through, in _contains_valid_repeating_query and only for the first of the n_spans. Now we're checking every span against the previous explicitly.

For mongodb system spans we also check the relay descriptions which include collection names, for even more confidence.

Copy link
Member

@roggenkemper roggenkemper left a comment

Choose a reason for hiding this comment

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

thanks for cleaning up some of this code - definitely makes it easier to understand than before

@leeandher leeandher merged commit 390e1d9 into master May 6, 2025
62 checks passed
@leeandher leeandher deleted the leander/npo-db-exp branch May 6, 2025 17:06
@github-actions github-actions bot locked and limited conversation to collaborators May 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants