Skip to content

Commit 2f4a0fe

Browse files
committed
refactor handle_error_sampling
fix issues with double conversion and moved logic into helper
1 parent 7d299ac commit 2f4a0fe

File tree

2 files changed

+62
-21
lines changed

2 files changed

+62
-21
lines changed

src/sentry/api/bases/organization_events.py

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818
from sentry.api.base import CURSOR_LINK_HEADER
1919
from sentry.api.bases import NoProjects
2020
from sentry.api.bases.organization import FilterParamsDateNotNull, OrganizationEndpoint
21-
from sentry.api.helpers.error_upsampling import are_any_projects_error_upsampled
21+
from sentry.api.helpers.error_upsampling import (
22+
are_any_projects_error_upsampled,
23+
convert_fields_for_upsampling,
24+
)
2225
from sentry.api.helpers.mobile import get_readable_device_name
2326
from sentry.api.helpers.teams import get_teams
2427
from sentry.api.serializers.snuba import SnubaTSResultSerializer
@@ -425,30 +428,14 @@ def handle_data(
425428

426429
def handle_error_upsampling(self, project_ids: Sequence[int], results: dict[str, Any]):
427430
"""
428-
If the query is for error upsampled projects, we need to rename the fields to include the ()
429-
and update the data and meta fields to reflect the new field names. This works around a limitation in
430-
how aliases are handled in the SnQL parser.
431+
If the query is for error upsampled projects, we convert various functions under the hood.
432+
We need to rename these fields before returning the results to the client, to hide the conversion.
433+
This is done here to work around a limitation in how aliases are handled in the SnQL parser.
431434
"""
432435
if are_any_projects_error_upsampled(project_ids):
433436
data = results.get("data", [])
434437
fields_meta = results.get("meta", {}).get("fields", {})
435-
436-
function_conversions = {
437-
"upsampled_count()": "count()",
438-
"upsampled_eps()": "eps()",
439-
"upsampled_epm()": "epm()",
440-
}
441-
442-
# Go over each both data and meta, and convert function names to the non-upsampled version
443-
for upsampled_function, count_function in function_conversions.items():
444-
for result in data:
445-
if upsampled_function in result:
446-
result[count_function] = result[upsampled_function]
447-
del result[upsampled_function]
448-
449-
if upsampled_function in fields_meta:
450-
fields_meta[count_function] = fields_meta[upsampled_function]
451-
del fields_meta[upsampled_function]
438+
convert_fields_for_upsampling(data, fields_meta)
452439

453440
def handle_issues(
454441
self, results: Sequence[Any], project_ids: Sequence[int], organization: Organization

src/sentry/api/helpers/error_upsampling.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,57 @@
1010

1111
UPSAMPLED_ERROR_AGGREGATION = "upsampled_count"
1212

13+
# Function key conversions for error upsampling results
14+
_FUNCTION_KEY_CONVERSIONS = {
15+
"count()": "sample_count()",
16+
"eps()": "sample_eps()",
17+
"epm()": "sample_epm()",
18+
"upsampled_count()": "count()",
19+
"upsampled_eps()": "eps()",
20+
"upsampled_epm()": "epm()",
21+
}
22+
23+
# Pre-computed ordered keys to handle conversion conflicts
24+
# Keys that are targets of other conversions must be processed first
25+
_conversion_targets = set(_FUNCTION_KEY_CONVERSIONS.values())
26+
_ORDERED_CONVERSION_KEYS = sorted(
27+
_FUNCTION_KEY_CONVERSIONS.keys(), key=lambda k: k not in _conversion_targets
28+
)
29+
30+
31+
def convert_fields_for_upsampling(data: list[dict[str, Any]], fields_meta: dict[str, str]) -> None:
32+
"""
33+
Convert field names in query results for error upsampled projects.
34+
This renames upsampled_* functions to their standard names and standard functions
35+
to sample_* equivalents to hide the conversion from the client.
36+
37+
Args:
38+
data: List of result dictionaries to modify in-place
39+
fields_meta: Meta fields dictionary to modify in-place
40+
"""
41+
# Collect keys that need conversion and exist in data
42+
all_present_keys = set()
43+
for result in data:
44+
all_present_keys.update(result.keys())
45+
46+
# Filter the pre-ordered list to only include keys actually present
47+
keys_to_convert = [key for key in _ORDERED_CONVERSION_KEYS if key in all_present_keys]
48+
49+
# Apply conversions to data
50+
for result in data:
51+
for original_key in keys_to_convert:
52+
if original_key in result:
53+
converted_key = _FUNCTION_KEY_CONVERSIONS[original_key]
54+
result[converted_key] = result[original_key]
55+
del result[original_key]
56+
57+
# Apply conversions to fields_meta
58+
for original_key in keys_to_convert:
59+
if original_key in fields_meta:
60+
converted_key = _FUNCTION_KEY_CONVERSIONS[original_key]
61+
fields_meta[converted_key] = fields_meta[original_key]
62+
del fields_meta[original_key]
63+
1364

1465
def is_errors_query_for_error_upsampled_projects(
1566
snuba_params: SnubaParams,
@@ -55,6 +106,9 @@ def transform_query_columns_for_error_upsampling(
55106
"count()": "upsampled_count()",
56107
"eps()": "upsampled_eps()",
57108
"epm()": "upsampled_epm()",
109+
"sample_count()": "count()",
110+
"sample_eps()": "eps()",
111+
"sample_epm()": "epm()",
58112
}
59113

60114
transformed_columns = []

0 commit comments

Comments
 (0)