Skip to content

Commit d4cc44e

Browse files
authored
feat(error_upsampling): Enable error upsampling for tag counts on issues (#95961)
Tag counts use the legacy snuba query path, changed it so every legacy query for Events dataset now converts count() aggregations to use sample_weight for allowlisted projects. This takes care of all tag store queries.
1 parent cee1934 commit d4cc44e

File tree

2 files changed

+119
-1
lines changed

2 files changed

+119
-1
lines changed

src/sentry/utils/snuba.py

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@
2727
from snuba_sdk.legacy import json_to_snql
2828
from snuba_sdk.query import SelectableExpression
2929

30-
from sentry.api.helpers.error_upsampling import UPSAMPLED_ERROR_AGGREGATION
30+
from sentry.api.helpers.error_upsampling import (
31+
UPSAMPLED_ERROR_AGGREGATION,
32+
are_any_projects_error_upsampled,
33+
)
3134
from sentry.models.environment import Environment
3235
from sentry.models.group import Group
3336
from sentry.models.grouprelease import GroupRelease
@@ -1405,6 +1408,13 @@ def query(
14051408
selected_columns = selected_columns or []
14061409
groupby = groupby or []
14071410

1411+
if dataset == Dataset.Events and filter_keys.get("project_id"):
1412+
project_filter = filter_keys.get("project_id")
1413+
project_ids = (
1414+
project_filter if isinstance(project_filter, (list, tuple)) else [project_filter]
1415+
)
1416+
_convert_count_aggregations_for_error_upsampling(aggregations, project_ids)
1417+
14081418
try:
14091419
body = raw_query(
14101420
dataset=dataset,
@@ -2099,3 +2109,27 @@ def get_upsampled_count_snql_with_alias(alias: str) -> list[SelectableExpression
20992109
],
21002110
alias=alias,
21012111
)
2112+
2113+
2114+
def _convert_count_aggregations_for_error_upsampling(
2115+
aggregations: list[list[Any]], project_ids: Sequence[int]
2116+
) -> None:
2117+
"""
2118+
Converts count() aggregations to upsampled_count() for error upsampled projects.
2119+
2120+
This function modifies the aggregations list in-place, swapping any "count()"
2121+
or "count" aggregation functions to "upsampled_count" when any of the projects
2122+
are configured for error upsampling.
2123+
2124+
Args:
2125+
aggregations: List of aggregation specifications in format [function, column, alias]
2126+
project_ids: List of project IDs being queried
2127+
"""
2128+
if not are_any_projects_error_upsampled(project_ids):
2129+
return
2130+
2131+
for aggregation in aggregations:
2132+
if len(aggregation) >= 1:
2133+
# Handle both "count()" and "count" formats
2134+
if aggregation[0] in ("count()", "count"):
2135+
aggregation[0] = "toInt64(sum(ifNull(sample_weight, 1)))"

tests/snuba/tagstore/test_tagstore_backend.py

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,6 +1109,90 @@ def test_get_group_tag_value_paginator_sort_by_last_seen(self):
11091109
# top key should be "quux" as it's the most recent than "bar"
11101110
assert top_key.value == "quux"
11111111

1112+
def test_error_upsampling_tag_value_counts(self):
1113+
"""Test that tag value counts are properly weighted when projects use error upsampling."""
1114+
1115+
# Set up allowlisted project for error upsampling
1116+
with self.options({"issues.client_error_sampling.project_allowlist": [self.proj1.id]}):
1117+
# Create first event with sample_weight=10 and tag value "alpha"
1118+
event1 = self.store_event(
1119+
data={
1120+
"event_id": "a1" * 16,
1121+
"message": "Error event with high sample weight",
1122+
"type": "error",
1123+
"exception": exception,
1124+
"timestamp": (self.now - timedelta(seconds=1)).isoformat(),
1125+
"fingerprint": ["error-upsampling-group"],
1126+
"tags": {
1127+
"custom_tag": "alpha",
1128+
"environment": "test",
1129+
},
1130+
# This creates a sample_weight of 10 (1/0.1)
1131+
"contexts": {"error_sampling": {"client_sample_rate": 0.1}},
1132+
},
1133+
project_id=self.proj1.id,
1134+
)
1135+
1136+
# Create second event with sample_weight=5 and tag value "beta"
1137+
self.store_event(
1138+
data={
1139+
"event_id": "b2" * 16,
1140+
"message": "Error event with medium sample weight",
1141+
"type": "error",
1142+
"exception": exception,
1143+
"timestamp": (self.now - timedelta(seconds=2)).isoformat(),
1144+
"fingerprint": ["error-upsampling-group"],
1145+
"tags": {
1146+
"custom_tag": "beta",
1147+
"environment": "test",
1148+
},
1149+
# This creates a sample_weight of 5 (1/0.2)
1150+
"contexts": {"error_sampling": {"client_sample_rate": 0.2}},
1151+
},
1152+
project_id=self.proj1.id,
1153+
)
1154+
1155+
# Get the group from one of the events
1156+
error_upsampling_group = event1.group
1157+
1158+
# Test get_top_group_tag_values with error upsampling
1159+
resp = self.ts.get_top_group_tag_values(
1160+
error_upsampling_group,
1161+
self.proj1env1.id,
1162+
"custom_tag",
1163+
10, # limit
1164+
tenant_ids={"referrer": "r", "organization_id": 1234},
1165+
)
1166+
1167+
# Verify we get both tag values
1168+
assert len(resp) == 2
1169+
1170+
# Sort by times_seen descending to get consistent order
1171+
resp = sorted(resp, key=lambda x: x.times_seen, reverse=True)
1172+
1173+
# First tag value should be "alpha" with times_seen=10 (sample_weight)
1174+
assert resp[0].key == "custom_tag"
1175+
assert resp[0].value == "alpha"
1176+
assert resp[0].times_seen == 10
1177+
assert resp[0].group_id == error_upsampling_group.id
1178+
1179+
# Second tag value should be "beta" with times_seen=5 (sample_weight)
1180+
assert resp[1].key == "custom_tag"
1181+
assert resp[1].value == "beta"
1182+
assert resp[1].times_seen == 5
1183+
assert resp[1].group_id == error_upsampling_group.id
1184+
1185+
# Also test get_group_tag_value_count for total count
1186+
total_count = self.ts.get_group_tag_value_count(
1187+
error_upsampling_group,
1188+
self.proj1env1.id,
1189+
"custom_tag",
1190+
tenant_ids={"referrer": "r", "organization_id": 1234},
1191+
)
1192+
1193+
# Total should be 10 + 5 = 15 (weighted sum, not 2 raw events)
1194+
assert total_count == 15
1195+
11121196

11131197
class ProfilingTagStorageTest(TestCase, SnubaTestCase, SearchIssueTestMixin):
11141198
def setUp(self):

0 commit comments

Comments
 (0)