Skip to content

Commit

Permalink
fix(search): hide feedback from issues search by default (#76984)
Browse files Browse the repository at this point in the history
Follow up to #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 #60697
Fixes getsentry/team-replay#470
  • Loading branch information
aliu39 authored and c298lee committed Sep 10, 2024
1 parent fa6b204 commit 4adb069
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 24 deletions.
3 changes: 3 additions & 0 deletions src/sentry/issues/grouptype.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 13 additions & 3 deletions src/sentry/issues/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
Expand Down
31 changes: 15 additions & 16 deletions src/sentry/search/snuba/executors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
37 changes: 37 additions & 0 deletions tests/sentry/feedback/usecases/test_create_feedback.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import time
from datetime import datetime
from typing import Any
from unittest.mock import Mock

Expand All @@ -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

Expand Down Expand Up @@ -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,
Expand Down
85 changes: 80 additions & 5 deletions tests/sentry/issues/endpoints/test_organization_group_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 4adb069

Please sign in to comment.