Skip to content

Commit 580e3e7

Browse files
scttcpershashjar
authored andcommitted
feat(issues): Include empty tags in group tags endpoint (#102049)
Add support for including empty tag values in the group tags endpoint (api/0/issues/{id}/tags/) when the organizations:issue-tags-include-empty-values feature flag is enabled. Makes a 2nd snuba query and attaches the empty values. I couldn't figure out how to make this work in a single snuba query, but i did figure out how to do this without N snuba queries. also needs some follow up frontend work <img width="454" height="256" alt="image" src="https://github.com/user-attachments/assets/6b935125-7ed5-4418-bcbb-f4bbc7349990" /> part of https://linear.app/getsentry/issue/RTC-1181/
1 parent d9770c5 commit 580e3e7

File tree

5 files changed

+164
-2
lines changed

5 files changed

+164
-2
lines changed

src/sentry/features/temporary.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,8 @@ def register_temporary_features(manager: FeatureManager) -> None:
193193
manager.add("organizations:issue-detection-sort-spans", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
194194
# Whether to allow issue only search on the issue list
195195
manager.add("organizations:issue-search-allow-postgres-only-search", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
196+
# Include empty tag count in issue tag totals/values
197+
manager.add("organizations:issue-tags-include-empty-values", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
196198
# Enable the new issue category mapping
197199
manager.add("organizations:issue-taxonomy", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
198200
manager.add("organizations:metric-issue-poc", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)

src/sentry/issues/endpoints/group_tags.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from rest_framework.request import Request
66
from rest_framework.response import Response
77

8-
from sentry import tagstore
8+
from sentry import features, tagstore
99
from sentry.api.api_publish_status import ApiPublishStatus
1010
from sentry.api.base import region_silo_endpoint
1111
from sentry.api.helpers.environments import get_environments
@@ -62,12 +62,19 @@ def get(self, request: Request, group: Group) -> Response:
6262

6363
environment_ids = [e.id for e in get_environments(request, group.project.organization)]
6464

65+
include_empty_values = features.has(
66+
"organizations:issue-tags-include-empty-values",
67+
group.project.organization,
68+
actor=request.user,
69+
)
70+
6571
tag_keys = backend.get_group_tag_keys_and_top_values(
6672
group,
6773
environment_ids,
6874
keys=keys,
6975
value_limit=value_limit,
7076
tenant_ids={"organization_id": group.project.organization_id},
77+
include_empty_values=include_empty_values,
7178
)
7279

7380
data = serialize(tag_keys, request.user)

src/sentry/tagstore/base.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ def get_group_tag_keys_and_top_values(
301301
keys: list[str] | None = None,
302302
value_limit=TOP_VALUES_DEFAULT_LIMIT,
303303
tenant_ids=None,
304+
include_empty_values=False,
304305
**kwargs,
305306
):
306307

src/sentry/tagstore/snuba/backend.py

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,7 @@ def get_group_tag_keys_and_top_values(
684684
keys: list[str] | None = None,
685685
value_limit: int = TOP_VALUES_DEFAULT_LIMIT,
686686
tenant_ids=None,
687+
include_empty_values=False,
687688
**kwargs,
688689
):
689690
# Similar to __get_tag_key_and_top_values except we get the top values
@@ -726,7 +727,28 @@ def get_group_tag_keys_and_top_values(
726727
tenant_ids=tenant_ids,
727728
)
728729

729-
# Then supplement the key objects with the top values for each.
730+
if include_empty_values:
731+
keys_to_check = list(values_by_key.keys()) or [k.key for k in keys_with_counts]
732+
empty_stats_map = self.__get_empty_value_stats_map(
733+
dataset=dataset,
734+
filters=filters,
735+
conditions=conditions,
736+
keys_to_check=keys_to_check,
737+
tenant_ids=tenant_ids,
738+
start=kwargs.get("start"),
739+
end=kwargs.get("end"),
740+
)
741+
for k, stats in empty_stats_map.items():
742+
if not stats or stats.get("count", 0) <= 0:
743+
continue
744+
vals = values_by_key.get(k, {})
745+
vals[""] = stats
746+
# Re-sort after adding empty count and trim to value_limit
747+
sorted_items = sorted(vals.items(), key=lambda kv: (-kv[1]["count"], str(kv[0])))[
748+
:value_limit
749+
]
750+
values_by_key[k] = {vk: vd for vk, vd in sorted_items}
751+
730752
for keyobj in keys_with_counts:
731753
key = keyobj.key
732754
values = values_by_key.get(key, dict())
@@ -744,6 +766,61 @@ def get_group_tag_keys_and_top_values(
744766

745767
return keys_with_counts
746768

769+
def __get_empty_value_stats_map(
770+
self,
771+
dataset: Dataset,
772+
filters: dict[str, list[Any]],
773+
conditions: list,
774+
keys_to_check: list[str],
775+
tenant_ids: dict[str, int | str] | None,
776+
start,
777+
end,
778+
) -> dict[str, dict[str, Any]]:
779+
if not keys_to_check:
780+
return {}
781+
782+
empty_alias_map: dict[str, dict[str, str]] = {}
783+
selected_columns_empty: list[list] = []
784+
for i, k in enumerate(keys_to_check):
785+
cnt_alias = f"empty_cnt_{i}"
786+
min_alias = f"empty_min_{i}"
787+
max_alias = f"empty_max_{i}"
788+
empty_alias_map[k] = {"count": cnt_alias, "first": min_alias, "last": max_alias}
789+
tag_expr = self.format_string.format(k)
790+
empty_predicate = ["equals", [tag_expr, ""]]
791+
selected_columns_empty.append(["countIf", [empty_predicate], cnt_alias])
792+
selected_columns_empty.append(["minIf", [SEEN_COLUMN, empty_predicate], min_alias])
793+
selected_columns_empty.append(["maxIf", [SEEN_COLUMN, empty_predicate], max_alias])
794+
795+
empty_filters = dict(filters)
796+
if self.key_column in empty_filters:
797+
# For empty-value stats, do not restrict by tags_key; events that
798+
# store an empty value may omit the key entirely. Removing this
799+
# filter ensures those events are counted.
800+
del empty_filters[self.key_column]
801+
802+
empty_results = snuba.query(
803+
dataset=dataset,
804+
start=start,
805+
end=end,
806+
groupby=None,
807+
conditions=conditions,
808+
filter_keys=empty_filters,
809+
aggregations=[],
810+
selected_columns=selected_columns_empty,
811+
referrer="tagstore._get_tag_keys_and_top_values_empty_counts",
812+
tenant_ids=tenant_ids,
813+
)
814+
stats_map: dict[str, dict[str, Any]] = {}
815+
for k in keys_to_check:
816+
aliases = empty_alias_map[k]
817+
stats_map[k] = {
818+
"count": empty_results.get(aliases["count"], 0),
819+
"first_seen": empty_results.get(aliases["first"]),
820+
"last_seen": empty_results.get(aliases["last"]),
821+
}
822+
return stats_map
823+
747824
def get_release_tags(self, organization_id, project_ids, environment_id, versions):
748825
filters = {"project_id": project_ids}
749826
if environment_id:

tests/sentry/issues/endpoints/test_group_tags.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,81 @@ def test_device_class(self) -> None:
304304
assert top_values[1]["value"] == "low"
305305
assert top_values[2]["value"] == "medium"
306306

307+
def test_include_empty_values_for_specific_key_with_feature(self) -> None:
308+
event = self.store_event(
309+
data={
310+
"fingerprint": ["group-empty-all"],
311+
"tags": {},
312+
"timestamp": before_now(minutes=1).isoformat(),
313+
},
314+
project_id=self.project.id,
315+
assert_no_errors=False,
316+
)
317+
self.store_event(
318+
data={
319+
"fingerprint": ["group-empty-all"],
320+
"tags": {"foo": "bar"},
321+
"timestamp": before_now(minutes=1).isoformat(),
322+
},
323+
project_id=self.project.id,
324+
)
325+
326+
self.login_as(user=self.user)
327+
328+
url = f"/api/0/issues/{event.group.id}/tags/?key=foo"
329+
330+
# Off by default: empty should be excluded from topValues
331+
response = self.client.get(url, format="json")
332+
assert response.status_code == 200
333+
assert len(response.data) == 1
334+
values = {v["value"] for v in response.data[0]["topValues"]}
335+
assert values == {"bar"}
336+
337+
# With feature: empty should be included
338+
with self.feature({"organizations:issue-tags-include-empty-values": True}):
339+
response = self.client.get(url, format="json")
340+
assert response.status_code == 200
341+
values = {v["value"] for v in response.data[0]["topValues"]}
342+
assert values == {"", "bar"}
343+
assert response.data[0]["topValues"][0]["count"] == 1
344+
345+
def test_include_empty_values_in_all_tags_with_feature(self) -> None:
346+
event = self.store_event(
347+
data={
348+
"fingerprint": ["group-empty-all-tags"],
349+
"tags": {"foo": ""},
350+
"timestamp": before_now(minutes=1).isoformat(),
351+
},
352+
project_id=self.project.id,
353+
assert_no_errors=False,
354+
)
355+
self.store_event(
356+
data={
357+
"fingerprint": ["group-empty-all-tags"],
358+
"tags": {"foo": "bar"},
359+
"timestamp": before_now(minutes=1).isoformat(),
360+
},
361+
project_id=self.project.id,
362+
)
363+
364+
self.login_as(user=self.user)
365+
url = f"/api/0/issues/{event.group.id}/tags/?limit=10"
366+
367+
# Off by default: empty should be excluded from the All Tags list
368+
response = self.client.get(url, format="json")
369+
assert response.status_code == 200
370+
foo = next((t for t in response.data if t["key"] == "foo"), None)
371+
assert foo is not None
372+
assert {v["value"] for v in foo["topValues"]} == {"bar"}
373+
374+
# With feature: empty should be included in the All Tags list
375+
with self.feature({"organizations:issue-tags-include-empty-values": True}):
376+
response = self.client.get(url, format="json")
377+
assert response.status_code == 200
378+
foo = next((t for t in response.data if t["key"] == "foo"), None)
379+
assert foo is not None
380+
assert {v["value"] for v in foo["topValues"]} == {"", "bar"}
381+
307382
def test_flags(self) -> None:
308383
event1 = self.store_event(
309384
data={

0 commit comments

Comments
 (0)