feat(cdc_search): Implement cdc search for is:unresolved query.
#28006
Conversation
This implements the basics of `CdcPostgresSnubaQueryExecutor` to handle a single query - `is:unresolved`. This should work correctly for all existing sorts, filtering by projects, environments, etc. The results should be equivalent to the existing executor, but we can test that in the `ServiceDelegator` callback. This is reasonably rough. As we start to implement more features I'll work to factor this into methods.
b46371c to
4cd7c0b
Compare
src/sentry/search/snuba/executors.py
Outdated
| "passing through to base class", | ||
| extra={"search_filters": search_filters}, | ||
| ) | ||
| return self.postres_executor.query( |
There was a problem hiding this comment.
Why do you still want to run the non-cdc executor here? I think returning an error here should be ok (for now, I guess return self.empty_result)? Wouldn't this just mean we're running it twice for queries we don't really care about comparing or would this skew stats in the service delegator?
I'm wondering if it's better to let the executors be "dumber", especially if we have tests that can verify the right executor is used for certain queries (lol...or query for now). Leave it up to the search backend to determine what executor to use. Perhaps we can define a set supported query "rules" for each executor, and check them in the backend in order of executor priority, choosing the first one that passes all requirements.
I just realized this would be pretty similar to how the current PostgresSnubaQueryExecutor chooses between just postgres or postgres+snuba...but brought into the backend class a little bit more neatly defined, and not deeply integrated in the query function
There was a problem hiding this comment.
I was just being overcautious here, so that if some query accidentally that shouldn't makes it to this executor then we'd still be fine. We're probably better off just throwing an error though, so that tests will catch the problem more easily.
My plan for routing between backends was mostly to let the ServiceDelegator make that call, and have the backends not really know about each other at all. It could make sense for the backends to define which queries they allow though, so that the ServiceDelegator doesn't have to store the specific logic on which queries are allowed where. I'll think about that, anyway
* master: (109 commits) ref(js): Add notice to use useApi when applicable (#28365) fix(tests): Temporarily disable test for migration 0223 (#28363) fix(ui): Use theme border for form footer border (#28359) ref(ts): Authenticators have names (#28362) feat(cdc_search): Implement cdc search for `is:unresolved` query. (#28006) ref(rrweb): Upgrade to latest player (#28333) feat(issue-alerts): Add Sentry Apps Alert Rule Action as an Action (#28338) feat(notifications): Remove NotificationSettings when Slack is uninstalled (#28216) py38(django 2.2): bump django-crispy-forms to 1.8.1 (#28344) types(python): Slack Integration Types (#28236) ref(jira ac): Rm fk constraints from JiraTenant (#28349) chore(js): Upgrade storybook from 6.3.6 -> 6.3.7 (#28323) feat(symbol-sources): Redact symbol source secrets from audit logs (#28334) feat(notifications): Add `actor_id` to Analytics Events (#28347) ref(performance): Separate header from content in transaction events (#28346) Adding in new text styles for the codeowners sync CTA in the issues detail. (#28133) fix(django 2.2): rename view method to avoid clashing (#28312) fix(alerts): Project breadcrumb link to project rule list (#28339) ref(performance): Separate header from content in transaction vitals (#28343) feat(dev_env): Dev env bootstrap support for two Python versions (#28213) ...
…8006) This implements the basics of `CdcPostgresSnubaQueryExecutor` to handle a single query - `is:unresolved`. This should work correctly for all existing sorts, filtering by projects, environments, etc. The results should be equivalent to the existing executor, but we can test that in the `ServiceDelegator` callback. This is reasonably rough. As we start to implement more features I'll work to factor this into methods.
This implements the basics of
CdcPostgresSnubaQueryExecutorto handle a single query -is:unresolved. This should work correctly for all existing sorts, filtering by projects,environments, etc. The results should be equivalent to the existing executor, but we can test that
in the
ServiceDelegatorcallback.This is reasonably rough. As we start to implement more features I'll work to factor this into
methods.