Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/sentry/issues/endpoints/group_tagkey_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from rest_framework.request import Request
from rest_framework.response import Response

from sentry import tagstore
from sentry import features, tagstore
from sentry.api.api_owners import ApiOwner
from sentry.api.api_publish_status import ApiPublishStatus
from sentry.api.base import region_silo_endpoint
Expand Down Expand Up @@ -69,6 +69,11 @@ def get(self, request: Request, group, key) -> Response:
"""
lookup_key = tagstore.backend.prefix_reserved_key(key)
tenant_ids = {"organization_id": group.project.organization_id}
include_empty_values = features.has(
"organizations:issue-tags-include-empty-values",
group.project.organization,
actor=request.user,
)
try:
environment_id = get_environment_id(request, group.project.organization_id)
except Environment.DoesNotExist:
Expand All @@ -81,18 +86,27 @@ def get(self, request: Request, group, key) -> Response:
environment_id,
lookup_key,
tenant_ids=tenant_ids,
include_empty_values=include_empty_values,
)
except tagstore.GroupTagKeyNotFound:
raise ResourceDoesNotExist

if group_tag_key.count is None:
group_tag_key.count = tagstore.backend.get_group_tag_value_count(
group, environment_id, lookup_key, tenant_ids=tenant_ids
group,
environment_id,
lookup_key,
tenant_ids=tenant_ids,
include_empty_values=include_empty_values,
)

if group_tag_key.top_values is None:
group_tag_key.top_values = tagstore.backend.get_top_group_tag_values(
group, environment_id, lookup_key, tenant_ids=tenant_ids
group,
environment_id,
lookup_key,
tenant_ids=tenant_ids,
include_empty_values=include_empty_values,
)

return Response(serialize(group_tag_key, request.user, serializer=TagKeySerializer()))
15 changes: 13 additions & 2 deletions src/sentry/issues/endpoints/group_tagkey_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from rest_framework.request import Request
from rest_framework.response import Response

from sentry import analytics, tagstore
from sentry import analytics, features, tagstore
from sentry.analytics.events.eventuser_endpoint_request import EventUserEndpointRequest
from sentry.api.api_owners import ApiOwner
from sentry.api.api_publish_status import ApiPublishStatus
Expand Down Expand Up @@ -81,12 +81,18 @@ def get(self, request: Request, group, key) -> Response:

environment_ids = [e.id for e in get_environments(request, group.project.organization)]
tenant_ids = {"organization_id": group.project.organization_id}
include_empty_values = features.has(
"organizations:issue-tags-include-empty-values",
group.project.organization,
actor=request.user,
)
try:
tagstore.backend.get_group_tag_key(
group,
None,
lookup_key,
tenant_ids=tenant_ids,
include_empty_values=include_empty_values,
)
except tagstore.GroupTagKeyNotFound:
raise ResourceDoesNotExist
Expand All @@ -106,7 +112,12 @@ def get(self, request: Request, group, key) -> Response:
serializer_cls = None

paginator = tagstore.backend.get_group_tag_value_paginator(
group, environment_ids, lookup_key, order_by=order_by, tenant_ids=tenant_ids
group,
environment_ids,
lookup_key,
order_by=order_by,
tenant_ids=tenant_ids,
include_empty_values=include_empty_values,
)

return self.paginate(
Expand Down
35 changes: 31 additions & 4 deletions src/sentry/tagstore/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,14 @@ def get_tag_values(self, project_id, environment_id, key: str, tenant_ids=None):
"""
raise NotImplementedError

def get_group_tag_key(self, group, environment_id, key: str, tenant_ids=None):
def get_group_tag_key(
self,
group,
environment_id,
key: str,
tenant_ids=None,
include_empty_values=False,
):
"""
>>> get_group_tag_key(group, 3, "key1")
"""
Expand Down Expand Up @@ -220,14 +227,21 @@ def get_group_tag_value_iter(
limit: int = 1000,
offset: int = 0,
tenant_ids: dict[str, int | str] | None = None,
include_empty_values=False,
) -> list[GroupTagValue]:
"""
>>> get_group_tag_value_iter(group, 2, 3, 'environment')
"""
raise NotImplementedError

def get_group_tag_value_paginator(
self, group, environment_ids, key: str, order_by="-id", tenant_ids=None
self,
group,
environment_ids,
key: str,
order_by="-id",
tenant_ids=None,
include_empty_values=False,
):
"""
>>> get_group_tag_value_paginator(group, 3, 'environment')
Expand Down Expand Up @@ -262,14 +276,27 @@ def get_generic_groups_user_counts(
) -> dict[int, int]:
raise NotImplementedError

def get_group_tag_value_count(self, group, environment_id, key: str, tenant_ids=None):
def get_group_tag_value_count(
self,
group,
environment_id,
key: str,
tenant_ids=None,
include_empty_values=False,
):
"""
>>> get_group_tag_value_count(group, 3, 'key1')
"""
raise NotImplementedError

def get_top_group_tag_values(
self, group, environment_id, key: str, limit=TOP_VALUES_DEFAULT_LIMIT, tenant_ids=None
self,
group,
environment_id,
key: str,
limit=TOP_VALUES_DEFAULT_LIMIT,
tenant_ids=None,
include_empty_values=False,
):
"""
>>> get_top_group_tag_values(group, 3, 'key1')
Expand Down
66 changes: 57 additions & 9 deletions src/sentry/tagstore/snuba/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ def __get_tag_key_and_top_values(
limit=3,
raise_on_empty=True,
tenant_ids=None,
include_empty_values=False,
**kwargs,
):
tag = self.format_string.format(key)
Expand All @@ -180,7 +181,8 @@ def __get_tag_key_and_top_values(
aggregations = kwargs.get("aggregations", [])

dataset, filters = self.apply_group_filters(group, filters)
conditions.append([tag, "!=", ""])
if not include_empty_values:
conditions.append([tag, "!=", ""])
aggregations += [
["uniq", tag, "values_seen"],
["count()", "", "count"],
Expand Down Expand Up @@ -500,14 +502,22 @@ def get_tag_values(self, project_id, environment_id, key, tenant_ids=None):
)
return set(key.top_values)

def get_group_tag_key(self, group, environment_id, key, tenant_ids=None):
def get_group_tag_key(
self,
group,
environment_id,
key,
tenant_ids=None,
include_empty_values=None,
):
return self.__get_tag_key_and_top_values(
group.project_id,
group,
environment_id,
key,
limit=TOP_VALUES_DEFAULT_LIMIT,
tenant_ids=tenant_ids,
include_empty_values=include_empty_values,
)

def get_group_tag_keys(
Expand Down Expand Up @@ -651,12 +661,21 @@ def apply_group_filters(self, group: Group | None, filters):
dataset = Dataset.IssuePlatform
return dataset, filters

def get_group_tag_value_count(self, group, environment_id, key: str, tenant_ids=None):
def get_group_tag_value_count(
self,
group,
environment_id,
key: str,
tenant_ids=None,
include_empty_values=False,
):
tag = self.format_string.format(key)
filters = {"project_id": get_project_list(group.project_id)}
if environment_id:
filters["environment"] = [environment_id]
conditions = [[tag, "!=", ""]]
if include_empty_values:
conditions = []
aggregations = [["count()", "", "count"]]
dataset, filters = self.apply_group_filters(group, filters)

Expand All @@ -670,10 +689,22 @@ def get_group_tag_value_count(self, group, environment_id, key: str, tenant_ids=
)

def get_top_group_tag_values(
self, group, environment_id, key: str, limit=TOP_VALUES_DEFAULT_LIMIT, tenant_ids=None
self,
group,
environment_id,
key: str,
limit=TOP_VALUES_DEFAULT_LIMIT,
tenant_ids=None,
include_empty_values=False,
):
tag = self.__get_tag_key_and_top_values(
group.project_id, group, environment_id, key, limit, tenant_ids=tenant_ids
group.project_id,
group,
environment_id,
key,
limit,
tenant_ids=tenant_ids,
include_empty_values=include_empty_values,
)
return tag.top_values

Expand Down Expand Up @@ -1452,18 +1483,24 @@ def get_group_tag_value_iter(
limit: int = 1000,
offset: int = 0,
tenant_ids: dict[str, int | str] | None = None,
include_empty_values=False,
) -> list[GroupTagValue]:
filters = {
filters: dict[str, list[Any]] = {
"project_id": get_project_list(group.project_id),
self.key_column: [key],
}
# When you filter by tags_key = ["foo"], we're filtering to where the tags_key array contains "foo".
# In order to get the total count with empty values, we need to remove the filter.
if include_empty_values:
del filters[self.key_column]
dataset, filters = self.apply_group_filters(group, filters)

if environment_ids:
filters["environment"] = environment_ids
tag_expression = self.format_string.format(key)
results = snuba.query(
dataset=dataset,
groupby=[self.value_column],
groupby=[tag_expression],
filter_keys=filters,
conditions=[],
aggregations=[
Expand All @@ -1486,7 +1523,13 @@ def get_group_tag_value_iter(
return group_tag_values

def get_group_tag_value_paginator(
self, group, environment_ids, key: str, order_by="-id", tenant_ids=None
self,
group,
environment_ids,
key: str,
order_by="-id",
tenant_ids=None,
include_empty_values=False,
):
from sentry.api.paginator import SequencePaginator

Expand All @@ -1499,7 +1542,12 @@ def get_group_tag_value_paginator(
raise ValueError("Unsupported order_by: %s" % order_by)

group_tag_values = self.get_group_tag_value_iter(
group, environment_ids, key, orderby="-last_seen", tenant_ids=tenant_ids
group,
environment_ids,
key,
orderby="-last_seen",
tenant_ids=tenant_ids,
include_empty_values=include_empty_values,
)

desc = order_by.startswith("-")
Expand Down
63 changes: 63 additions & 0 deletions tests/sentry/issues/endpoints/test_group_tagkey_details.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from sentry.models.group import Group
from sentry.testutils.cases import APITestCase, PerformanceIssueTestCase, SnubaTestCase
from sentry.testutils.helpers.datetime import before_now
from sentry.testutils.helpers.features import with_feature


class GroupTagDetailsTest(APITestCase, SnubaTestCase, PerformanceIssueTestCase):
Expand Down Expand Up @@ -44,3 +45,65 @@ def test_simple_perf(self) -> None:
assert response.status_code == 200, response.content
assert response.data["key"] == "foo"
assert response.data["totalValues"] == 2

def test_excludes_empty_values_by_default(self) -> None:
for i in range(2):
self.store_event(
data={
"tags": {"foo": ""},
"fingerprint": ["group-empty-default"],
"timestamp": before_now(seconds=1 + i).isoformat(),
},
project_id=self.project.id,
assert_no_errors=False,
)

event = self.store_event(
data={
"tags": {"foo": "baz"},
"fingerprint": ["group-empty-default"],
"timestamp": before_now(seconds=1).isoformat(),
},
project_id=self.project.id,
)

self.login_as(user=self.user)

url = f"/api/0/issues/{event.group.id}/tags/foo/"
response = self.client.get(url, format="json")
assert response.status_code == 200, response.content
assert response.data["totalValues"] == 1
assert "" not in {value["value"] for value in response.data["topValues"]}

@with_feature({"organizations:issue-tags-include-empty-values": True})
def test_includes_empty_values_with_feature(self) -> None:
for i in range(2):
self.store_event(
data={
"tags": {"foo": ""},
"fingerprint": ["group-empty-flag"],
"timestamp": before_now(seconds=1 + i).isoformat(),
},
project_id=self.project.id,
assert_no_errors=False,
)

event = self.store_event(
data={
"tags": {"foo": "baz"},
"fingerprint": ["group-empty-flag"],
"timestamp": before_now(seconds=1).isoformat(),
},
project_id=self.project.id,
)

self.login_as(user=self.user)

url = f"/api/0/issues/{event.group.id}/tags/foo/"
response = self.client.get(url, format="json")

assert response.status_code == 200, response.content
assert response.data["totalValues"] == 3
top_values = {value["value"]: value["count"] for value in response.data["topValues"]}
assert top_values.get("") == 2
assert top_values.get("baz") == 1
Loading
Loading