-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Conversation
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 | ||
|
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this 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
… MongoDB queries (#90756) Allows the N+1 detector to operate on MongoDB queries. The example JSON comes from my [prod trace](https://sentry-leander.sentry.io/issues/trace/d1cfb683e06b415b9f89742214dcbc12/?groupId=6566277807&node=ag-1b956e6208c12234&node=txn-a9240785a11d46d393aea0c8bd137f11&pageEnd=2025-04-25T06%3A02%3A16.355&pageStart=2025-04-24T06%3A02%3A16.355&project=4509203466944512&query=&referrer=issue-stream&sort=date&source=issue_details&stream_index=0×tamp=1745517736) 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.
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.