diff --git a/src/sentry/features/temporary.py b/src/sentry/features/temporary.py index 507c5a0ce52e..95e464d0a73a 100644 --- a/src/sentry/features/temporary.py +++ b/src/sentry/features/temporary.py @@ -351,6 +351,8 @@ def register_temporary_features(manager: FeatureManager) -> None: manager.add("organizations:hard-delete-derived-data-invalidation", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) # Enable double-reading from EAP for issue feed search queries manager.add("organizations:issue-feed.eap-search", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) + # Merge issue platform category searches into a single Snuba query + manager.add("organizations:issue-search-merged-generic-query", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) # Batch latest-event attachment lookups in the issue stream manager.add("organizations:issue-stream-batched-latest-event-attachments", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) # Remove trace and breadcrumbs from issue summary input diff --git a/src/sentry/issues/search.py b/src/sentry/issues/search.py index 6cd55f210278..cefbe8ec7c62 100644 --- a/src/sentry/issues/search.py +++ b/src/sentry/issues/search.py @@ -225,14 +225,15 @@ def _query_params_for_generic( def get_search_strategy( - group_category: GroupCategory, + group_categories: Sequence[GroupCategory], visible_group_type_ids: Collection[int], ) -> GroupSearchStrategy: - if group_category == GroupCategory.ERROR: + if len(group_categories) == 1 and group_categories[0] == GroupCategory.ERROR: return _query_params_for_error + assert GroupCategory.ERROR not in group_categories return functools.partial( _query_params_for_generic, - categories=[group_category], + categories=group_categories, visible_group_type_ids=visible_group_type_ids, ) diff --git a/src/sentry/search/snuba/executors.py b/src/sentry/search/snuba/executors.py index 4cc9bb7edf08..cfbde3a118a6 100644 --- a/src/sentry/search/snuba/executors.py +++ b/src/sentry/search/snuba/executors.py @@ -343,9 +343,9 @@ def _prepare_aggregations( return aggregations - def _prepare_params_for_category( + def _prepare_params_for_categories( self, - group_category: int, + group_categories: Sequence[int], visible_group_type_ids: Collection[int], query_partial: IntermediateSearchQueryPartial, organization: Organization, @@ -362,14 +362,6 @@ def _prepare_params_for_category( actor: Any | None = None, aggregate_kwargs: TrendsSortWeights | None = None, ) -> SnubaQueryParams | None: - """ - :raises UnsupportedSearchQuery: when search_filters includes conditions on a dataset that doesn't support it - """ - - if group_category in SEARCH_FILTER_UPDATERS: - # remove filters not relevant to the group_category - search_filters = SEARCH_FILTER_UPDATERS[group_category](search_filters) - # convert search_filters to snuba format converted_filters = self._convert_search_filters( organization.id, project_ids, environments, search_filters @@ -386,7 +378,7 @@ def _prepare_params_for_category( else: conditions.append(converted_filter) - use_issue_platform = group_category is not GroupCategory.ERROR.value + use_issue_platform = GroupCategory.ERROR.value not in group_categories aggregations = self._prepare_aggregations( sort_field, start, end, having, aggregate_kwargs, use_issue_platform ) @@ -402,7 +394,8 @@ def _prepare_params_for_category( else: # Get the top matching groups by score, i.e. the actual search results # in the order that we want them. - orderby = [f"-{sort_field}", "group_id"] # ensure stable sort within the same score + group_id_sort = "-group_id" if len(group_categories) > 1 else "group_id" + orderby = [f"-{sort_field}", group_id_sort] pinned_query_partial: SearchQueryPartial = cast( SearchQueryPartial, @@ -414,7 +407,10 @@ def _prepare_params_for_category( ), ) - query_builder = get_search_strategy(GroupCategory(group_category), visible_group_type_ids) + query_builder = get_search_strategy( + [GroupCategory(group_category) for group_category in group_categories], + visible_group_type_ids, + ) snuba_query_params = query_builder( pinned_query_partial, selected_columns, @@ -502,12 +498,38 @@ def snuba_search( if group_categories - {GroupCategory.ERROR.value} else set() ) - query_params_for_categories = {} + merge_generic_categories = features.has( + "organizations:issue-search-merged-generic-query", organization, actor=actor + ) + category_filter_groups: list[tuple[list[int], Sequence[SearchFilter]]] = [] + for group_category in sorted(group_categories): + try: + category_search_filters = ( + SEARCH_FILTER_UPDATERS[group_category](snuba_search_filters) + if group_category in SEARCH_FILTER_UPDATERS + else snuba_search_filters + ) + except UnsupportedSearchQuery: + continue + + if merge_generic_categories and group_category != GroupCategory.ERROR.value: + for grouped_categories, grouped_search_filters in category_filter_groups: + if ( + GroupCategory.ERROR.value not in grouped_categories + and category_search_filters == grouped_search_filters + ): + grouped_categories.append(group_category) + break + else: + category_filter_groups.append(([group_category], category_search_filters)) + else: + category_filter_groups.append(([group_category], category_search_filters)) - for gc in group_categories: + query_params_for_categories: dict[tuple[int, ...], SnubaQueryParams] = {} + for grouped_categories, category_search_filters in category_filter_groups: try: - query_params = self._prepare_params_for_category( - gc, + query_params = self._prepare_params_for_categories( + grouped_categories, visible_group_type_ids, query_partial, organization, @@ -515,7 +537,7 @@ def snuba_search( environments, group_ids, filters, - snuba_search_filters, + category_search_filters, sort_field, start, end, @@ -524,15 +546,13 @@ def snuba_search( actor, aggregate_kwargs, ) - except UnsupportedSearchQuery: - pass except EmptyGroupIdIntersectionError: # Postgres candidates and the snuba group_id condition are # disjoint for this category — it can't match anything. Skip it. - pass - else: - if query_params is not None: - query_params_for_categories[gc] = query_params + continue + + if query_params is not None: + query_params_for_categories[tuple(grouped_categories)] = query_params callsite = "PostgresSnubaQueryExecutor.snuba_search" @@ -546,14 +566,16 @@ def _run_snuba_query() -> tuple[list[tuple[int, Any]], int]: "snuba.search.group_category_bulk", tags={ GroupCategory(gc_val).name.lower(): True - for gc_val, _ in query_params_for_categories.items() + for group_category_values in query_params_for_categories + for gc_val in group_category_values }, ) # one of the parallel bulk raw queries failed (maybe the issue platform dataset), # we'll fallback to querying for errors only - if GroupCategory.ERROR.value in query_params_for_categories.keys(): + error_query_params = query_params_for_categories.get((GroupCategory.ERROR.value,)) + if error_query_params is not None: bulk_query_results = bulk_raw_query( - [query_params_for_categories[GroupCategory.ERROR.value]], + [error_query_params], referrer=referrer, ) else: diff --git a/tests/snuba/search/test_backend.py b/tests/snuba/search/test_backend.py index 2cedf50a3d4e..423a04448a78 100644 --- a/tests/snuba/search/test_backend.py +++ b/tests/snuba/search/test_backend.py @@ -3621,6 +3621,28 @@ def setUp(self) -> None: ) self.error_group_2 = error_event_2.group + def _create_performance_issue(self, tag_value: str) -> Group: + group_type = PerformanceNPlusOneGroupType + with ( + mock.patch.object(group_type, "noise_config", new=NoiseConfig(0, timedelta(minutes=1))), + self.feature(group_type.build_ingest_feature_name()), + ): + _, group_info = self.process_occurrence( + event_id=uuid.uuid4().hex, + project_id=self.project.id, + type=group_type.type_id, + fingerprint=[uuid.uuid4().hex], + event_data={ + "title": "some problem", + "platform": "python", + "tags": {"my_tag": tag_value}, + "timestamp": before_now(minutes=1).isoformat(), + "received": before_now(minutes=1).isoformat(), + }, + ) + assert group_info is not None + return group_info.group + def test_generic_query(self) -> None: results = self.make_query( search_filter_query=f"issue.type:{ProfileFileIOGroupType.slug} my_tag:1" @@ -3636,34 +3658,138 @@ def test_generic_query_message(self) -> None: assert list(results) == [self.profile_group_1, self.profile_group_2] def test_generic_query_perf(self) -> None: - event_id = uuid.uuid4().hex group_type = PerformanceNPlusOneGroupType + performance_group = self._create_performance_issue("3") + with self.feature(group_type.build_visible_feature_name()): + results = self.make_query( + search_filter_query=f"issue.type:{PerformanceNPlusOneGroupType.slug} my_tag:3" + ) - with mock.patch.object( - PerformanceNPlusOneGroupType, "noise_config", new=NoiseConfig(0, timedelta(minutes=1)) + assert list(results) == [performance_group] + + def test_merge_default_category_queries(self) -> None: + with ( + self.feature("organizations:issue-search-merged-generic-query"), + mock.patch( + "sentry.search.snuba.executors.bulk_raw_query", wraps=snuba.bulk_raw_query + ) as bulk_query, ): - with self.feature(group_type.build_ingest_feature_name()): - _, group_info = self.process_occurrence( - event_id=event_id, - project_id=self.project.id, - type=group_type.type_id, - fingerprint=["some perf issue"], - event_data={ - "title": "some problem", - "platform": "python", - "tags": {"my_tag": "3"}, - "timestamp": before_now(minutes=1).isoformat(), - "received": before_now(minutes=1).isoformat(), - }, - ) - assert group_info is not None + results = self.make_query(search_filter_query="my_tag:1") - with self.feature(group_type.build_visible_feature_name()): - results = self.make_query( - search_filter_query=f"issue.type:{PerformanceNPlusOneGroupType.slug} my_tag:3" - ) + assert set(results) == { + self.profile_group_1, + self.profile_group_2, + self.error_group_1, + self.error_group_2, + } + query_params = bulk_query.call_args.args[0] + assert len(query_params) == 2 + assert {params.dataset for params in query_params} == { + Dataset.Events, + Dataset.IssuePlatform, + } - assert list(results) == [group_info.group] + def test_merge_generic_category_queries(self) -> None: + group_type = PerformanceNPlusOneGroupType + performance_group = self._create_performance_issue("1") + + query = ( + f"issue.type:[{ProfileFileIOGroupType.slug}," + f"{PerformanceNPlusOneGroupType.slug},error] my_tag:1" + ) + expected_groups = { + self.profile_group_1, + self.profile_group_2, + performance_group, + self.error_group_1, + self.error_group_2, + } + + with ( + self.feature(group_type.build_visible_feature_name()), + mock.patch( + "sentry.search.snuba.executors.bulk_raw_query", wraps=snuba.bulk_raw_query + ) as control_bulk_query, + ): + control_results = self.make_query(search_filter_query=query) + + with ( + self.feature(group_type.build_visible_feature_name()), + self.feature("organizations:issue-search-merged-generic-query"), + mock.patch( + "sentry.search.snuba.executors.bulk_raw_query", wraps=snuba.bulk_raw_query + ) as merged_bulk_query, + ): + merged_results = self.make_query(search_filter_query=query) + + assert set(control_results) == expected_groups + assert set(merged_results) == expected_groups + + control_query_params = control_bulk_query.call_args.args[0] + assert len(control_query_params) == 3 + assert all( + params.kwargs["orderby"] == ["-last_seen", "group_id"] + for params in control_query_params + ) + + merged_query_params = merged_bulk_query.call_args.args[0] + assert len(merged_query_params) == 2 + assert {params.dataset for params in merged_query_params} == { + Dataset.Events, + Dataset.IssuePlatform, + } + + issue_platform_params = next( + params for params in merged_query_params if params.dataset == Dataset.IssuePlatform + ) + assert issue_platform_params.kwargs["orderby"] == ["-last_seen", "-group_id"] + assert set(issue_platform_params.filter_keys["occurrence_type_id"]) >= { + ProfileFileIOGroupType.type_id, + PerformanceNPlusOneGroupType.type_id, + } + + def test_merge_generic_category_query_pagination(self) -> None: + group_type = PerformanceNPlusOneGroupType + performance_group = self._create_performance_issue("1") + query = ( + f"issue.type:[{ProfileFileIOGroupType.slug}," + f"{PerformanceNPlusOneGroupType.slug},error] my_tag:1" + ) + expected_groups = { + self.profile_group_1, + self.profile_group_2, + performance_group, + self.error_group_1, + self.error_group_2, + } + + with ( + self.feature(group_type.build_visible_feature_name()), + self.feature("organizations:issue-search-merged-generic-query"), + ): + first_page = self.make_query( + search_filter_query=query, + limit=2, + count_hits=True, + ) + second_page = self.make_query( + search_filter_query=query, + limit=2, + count_hits=True, + cursor=first_page.next, + ) + third_page = self.make_query( + search_filter_query=query, + limit=2, + count_hits=True, + cursor=second_page.next, + ) + + assert first_page.hits == len(expected_groups) + assert second_page.hits == len(expected_groups) + assert third_page.hits == len(expected_groups) + assert set([*first_page, *second_page, *third_page]) == expected_groups + assert len([*first_page, *second_page, *third_page]) == len(expected_groups) def test_error_generic_query(self) -> None: results = self.make_query(search_filter_query="my_tag:1")