From 4adb069679228355c4c761c8aaaa742fb09408f3 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Mon, 9 Sep 2024 11:03:24 -0700 Subject: [PATCH] fix(search): hide feedback from issues search by default (#76984) Follow up to https://github.com/getsentry/sentry/pull/59505 to filter feedbacks from the default search when querying Postgres. As before, filtering on issue.category:feedback or a negation (ex !issue.category:error) will still return feedback issues. Fixes https://github.com/getsentry/sentry/issues/60697 Fixes https://github.com/getsentry/team-replay/issues/470 --- src/sentry/issues/grouptype.py | 3 + src/sentry/issues/search.py | 16 +++- src/sentry/search/snuba/executors.py | 31 ++++--- .../feedback/usecases/test_create_feedback.py | 37 ++++++++ .../test_organization_group_index.py | 85 +++++++++++++++++-- 5 files changed, 148 insertions(+), 24 deletions(-) diff --git a/src/sentry/issues/grouptype.py b/src/sentry/issues/grouptype.py index c9e6d548cc5c45..91887dceaa23e5 100644 --- a/src/sentry/issues/grouptype.py +++ b/src/sentry/issues/grouptype.py @@ -140,6 +140,8 @@ class GroupType: # If True this group type should be released everywhere. If False, fall back to features to # decide if this is released. released: bool = False + # If False this group is excluded from default searches, when there are no filters on issue.category or issue.type. + in_default_search: bool = True # Allow automatic resolution of an issue type, using the project-level option. enable_auto_resolve: bool = True @@ -580,6 +582,7 @@ class FeedbackGroup(GroupType): creation_quota = Quota(3600, 60, 1000) # 1000 per hour, sliding window of 60 seconds default_priority = PriorityLevel.MEDIUM notification_config = NotificationConfig(context=[]) + in_default_search = False # hide from issues stream @dataclass(frozen=True) diff --git a/src/sentry/issues/search.py b/src/sentry/issues/search.py index b060999729eac4..3566bfc5bdd32f 100644 --- a/src/sentry/issues/search.py +++ b/src/sentry/issues/search.py @@ -8,7 +8,13 @@ from sentry.api.event_search import SearchFilter, SearchKey, SearchValue from sentry.issues import grouptype -from sentry.issues.grouptype import GroupCategory, get_all_group_type_ids, get_group_type_by_type_id +from sentry.issues.grouptype import ( + GroupCategory, + GroupType, + get_all_group_type_ids, + get_group_type_by_type_id, +) +from sentry.issues.grouptype import registry as GT_REGISTRY from sentry.models.environment import Environment from sentry.models.organization import Organization from sentry.search.events.filter import convert_search_filter_to_snuba_query @@ -98,14 +104,18 @@ def group_categories_from( def group_types_from( search_filters: Sequence[SearchFilter] | None, -) -> set[int] | None: +) -> set[int]: """ Return the set of group type ids to include in the query, or None if all group types should be included. """ # if no relevant filters, return none to signify we should query all group types if not any(sf.key.name in ("issue.category", "issue.type") for sf in search_filters or ()): - return None + # Filters some types from the default search + all_group_type_objs: list[GroupType] = [ + GT_REGISTRY.get_by_type_id(id) for id in GT_REGISTRY.get_all_group_type_ids() + ] + return {gt.type_id for gt in all_group_type_objs if gt.in_default_search} # start by including all group types include_group_types = set(get_all_group_type_ids()) diff --git a/src/sentry/search/snuba/executors.py b/src/sentry/search/snuba/executors.py index a0f38d55238395..91fa4f1b280475 100644 --- a/src/sentry/search/snuba/executors.py +++ b/src/sentry/search/snuba/executors.py @@ -1820,23 +1820,22 @@ def query( # handle types based on issue.type and issue.category if not is_errors: raw_group_types = group_types_from(search_filters) - if raw_group_types is not None: - # no possible groups, return empty - if len(raw_group_types) == 0: - metrics.incr( - "snuba.search.group_attributes.no_possible_groups", skip_internal=False - ) - return self.empty_result - - # filter out the group types that are not visible to the org/user - group_types = [ - gt.type_id - for gt in grouptype.registry.get_visible(organization, actor) - if gt.type_id in raw_group_types - ] - where_conditions.append( - Condition(Column("occurrence_type_id", joined_entity), Op.IN, group_types) + # no possible groups, return empty + if len(raw_group_types) == 0: + metrics.incr( + "snuba.search.group_attributes.no_possible_groups", skip_internal=False ) + return self.empty_result + + # filter out the group types that are not visible to the org/user + group_types = [ + gt.type_id + for gt in grouptype.registry.get_visible(organization, actor) + if gt.type_id in raw_group_types + ] + where_conditions.append( + Condition(Column("occurrence_type_id", joined_entity), Op.IN, group_types) + ) sort_func = self.get_sort_defs(joined_entity)[sort_by] diff --git a/tests/sentry/feedback/usecases/test_create_feedback.py b/tests/sentry/feedback/usecases/test_create_feedback.py index 986a4141e025df..80bfb2ea02b1a4 100644 --- a/tests/sentry/feedback/usecases/test_create_feedback.py +++ b/tests/sentry/feedback/usecases/test_create_feedback.py @@ -1,6 +1,7 @@ from __future__ import annotations import time +from datetime import datetime from typing import Any from unittest.mock import Mock @@ -16,6 +17,7 @@ ) from sentry.models.group import Group, GroupStatus from sentry.testutils.helpers import Feature +from sentry.testutils.helpers.datetime import iso_format from sentry.testutils.pytest.fixtures import django_db_all from sentry.types.group import GroupSubStatus @@ -70,6 +72,41 @@ def create_dummy_response(*args, **kwargs): ) +def mock_feedback_event(project_id: int, dt: datetime): + return { + "project_id": project_id, + "request": { + "url": "https://sentry.sentry.io/feedback/?statsPeriod=14d", + "headers": { + "User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/118.0.0.0 Safari/537.36" + }, + }, + "event_id": "56b08cf7852c42cbb95e4a6998c66ad6", + "timestamp": dt.timestamp(), + "received": iso_format(dt), + "environment": "prod", + "release": "frontend@daf1316f209d961443664cd6eb4231ca154db502", + "user": { + "ip_address": "72.164.175.154", + "email": "josh.ferge@sentry.io", + "id": 880461, + "isStaff": False, + "name": "Josh Ferge", + }, + "contexts": { + "feedback": { + "contact_email": "josh.ferge@sentry.io", + "name": "Josh Ferge", + "message": "Testing!!", + "replay_id": "3d621c61593c4ff9b43f8490a78ae18e", + "url": "https://sentry.sentry.io/feedback/?statsPeriod=14d", + }, + }, + "breadcrumbs": [], + "platform": "javascript", + } + + def test_fix_for_issue_platform(): event: dict[str, Any] = { "project_id": 1, diff --git a/tests/sentry/issues/endpoints/test_organization_group_index.py b/tests/sentry/issues/endpoints/test_organization_group_index.py index 357e6173e9dda8..121da0177d7260 100644 --- a/tests/sentry/issues/endpoints/test_organization_group_index.py +++ b/tests/sentry/issues/endpoints/test_organization_group_index.py @@ -9,12 +9,15 @@ from django.utils import timezone from sentry import options +from sentry.feedback.usecases.create_feedback import FeedbackCreationSource, create_feedback_issue from sentry.integrations.models.external_issue import ExternalIssue from sentry.integrations.models.organization_integration import OrganizationIntegration from sentry.issues.grouptype import ( + FeedbackGroup, PerformanceNPlusOneGroupType, PerformanceRenderBlockingAssetSpanGroupType, PerformanceSlowDBQueryGroupType, + ProfileFileIOGroupType, ) from sentry.models.activity import Activity from sentry.models.apitoken import ApiToken @@ -57,13 +60,14 @@ from sentry.testutils.cases import APITestCase, SnubaTestCase from sentry.testutils.helpers import parse_link_header from sentry.testutils.helpers.datetime import before_now, iso_format -from sentry.testutils.helpers.features import apply_feature_flag_on_cls, with_feature +from sentry.testutils.helpers.features import Feature, apply_feature_flag_on_cls, with_feature from sentry.testutils.helpers.options import override_options from sentry.testutils.silo import assume_test_silo_mode from sentry.types.activity import ActivityType from sentry.types.group import GroupSubStatus, PriorityLevel from sentry.users.models.user_option import UserOption from sentry.utils import json +from tests.sentry.feedback.usecases.test_create_feedback import mock_feedback_event from tests.sentry.issues.test_utils import SearchIssueTestMixin @@ -3145,10 +3149,11 @@ def test_snuba_perf_issue(self, mock_query: MagicMock) -> None: self.login_as(user=self.user) # give time for consumers to run and propogate changes to clickhouse sleep(1) - response = self.get_success_response( - sort="new", - query="user.email:myemail@example.com", - ) + with self.feature([ProfileFileIOGroupType.build_visible_feature_name()]): + response = self.get_success_response( + sort="new", + query="user.email:myemail@example.com", + ) assert len(response.data) == 2 assert {r["id"] for r in response.data} == { str(perf_group_id), @@ -3917,6 +3922,76 @@ def test_snuba_heavy_error_handled_boolean(self, _: MagicMock) -> None: assert len(response_handled_0.data) == 1 assert int(response_handled_0.data[0]["id"]) == handled_event.group.id + def run_feedback_filtered_by_default_test(self, use_group_snuba_dataset: bool): + with Feature( + { + FeedbackGroup.build_visible_feature_name(): True, + FeedbackGroup.build_ingest_feature_name(): True, + "organizations:issue-search-snuba": use_group_snuba_dataset, + } + ): + event = self.store_event( + data={"event_id": uuid4().hex, "timestamp": iso_format(before_now(seconds=1))}, + project_id=self.project.id, + ) + assert event.group is not None + + feedback_event = mock_feedback_event(self.project.id, before_now(seconds=1)) + create_feedback_issue( + feedback_event, self.project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE + ) + self.login_as(user=self.user) + res = self.get_success_response(useGroupSnubaDataset=use_group_snuba_dataset) + + # test that the issue returned is NOT the feedback issue. + assert len(res.data) == 1 + issue = res.data[0] + feedback_group = Group.objects.get(type=FeedbackGroup.type_id) + assert int(issue["id"]) != feedback_group.id + assert issue["issueCategory"] != "feedback" + + def test_feedback_filtered_by_default_no_snuba_search(self, _): + self.run_feedback_filtered_by_default_test(False) + + def test_feedback_filtered_by_default_use_snuba_search(self, _): + self.run_feedback_filtered_by_default_test(True) + + def run_feedback_category_filter_test(self, use_group_snuba_dataset: bool): + with Feature( + { + FeedbackGroup.build_visible_feature_name(): True, + FeedbackGroup.build_ingest_feature_name(): True, + "organizations:issue-search-snuba": use_group_snuba_dataset, + } + ): + event = self.store_event( + data={"event_id": uuid4().hex, "timestamp": iso_format(before_now(seconds=1))}, + project_id=self.project.id, + ) + assert event.group is not None + + feedback_event = mock_feedback_event(self.project.id, before_now(seconds=1)) + create_feedback_issue( + feedback_event, self.project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE + ) + self.login_as(user=self.user) + res = self.get_success_response( + query="issue.category:feedback", useGroupSnubaDataset=use_group_snuba_dataset + ) + + # test that the issue returned IS the feedback issue. + assert len(res.data) == 1 + issue = res.data[0] + feedback_group = Group.objects.get(type=FeedbackGroup.type_id) + assert int(issue["id"]) == feedback_group.id + assert issue["issueCategory"] == "feedback" + + def test_feedback_category_filter_no_snuba_search(self, _): + self.run_feedback_category_filter_test(False) + + def test_feedback_category_filter_use_snuba_search(self, _): + self.run_feedback_category_filter_test(True) + class GroupUpdateTest(APITestCase, SnubaTestCase): endpoint = "sentry-api-0-organization-group-index"