Skip to content
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

fix(search): hide feedback from issues search by default #76984

Merged
merged 9 commits into from
Sep 9, 2024
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
aliu39 marked this conversation as resolved.
Show resolved Hide resolved
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
aliu39 marked this conversation as resolved.
Show resolved Hide resolved
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
Loading