Skip to content

Commit 11e7079

Browse files
authored
fix(eap): Handle nans in other timeseries for EAP (#82131)
This fixes the nans in the other timeseries for EAP when some numeric attribute does not exist for a time bucket, the avg aggregate will produce a nan. It does not fix it for the spans indexed dataset because the query there is actually wrong and treats entries without the attribute as 0. It also does not fix it for the rpc calls because rpc calls are failing with a division by 0 error.
1 parent b372cd5 commit 11e7079

File tree

2 files changed

+68
-1
lines changed

2 files changed

+68
-1
lines changed

src/sentry/snuba/spans_eap.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import logging
22
from collections.abc import Mapping, Sequence
33
from datetime import timedelta
4+
from typing import Any, TypedDict
45

56
import sentry_sdk
67
from snuba_sdk import Column, Condition
@@ -244,14 +245,16 @@ def top_events_timeseries(
244245
snuba_params.end_date,
245246
rollup,
246247
)
248+
247249
with sentry_sdk.start_span(op="spans_indexed", name="top_events.transform_results") as span:
248250
span.set_data("result_count", len(result.get("data", [])))
249251
result = top_events_builder.process_results(result)
252+
other_result = top_events_builder.process_results(other_result)
250253

251254
issues: Mapping[int, str | None] = {}
252255
translated_groupby = top_events_builder.translated_groupby
253256

254-
results = (
257+
results: dict[str, TimeseriesResult] = (
255258
{discover.OTHER_KEY: {"order": limit, "data": other_result["data"]}}
256259
if len(other_result.get("data", []))
257260
else {}
@@ -292,3 +295,8 @@ def top_events_timeseries(
292295
)
293296

294297
return top_events_results
298+
299+
300+
class TimeseriesResult(TypedDict):
301+
order: int
302+
data: list[dict[str, Any]]

tests/snuba/api/endpoints/test_organization_events_stats_span_indexed.py

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,61 @@ def test_handle_nans_from_snuba(self):
9090
)
9191
assert response.status_code == 200, response.content
9292

93+
def test_handle_nans_from_snuba_top_n(self):
94+
self.store_spans(
95+
[
96+
self.create_span(
97+
{
98+
"description": "foo",
99+
"measurements": {"lcp": {"value": 1}},
100+
},
101+
start_ts=self.day_ago,
102+
),
103+
self.create_span({"description": "foo"}, start_ts=self.day_ago),
104+
self.create_span({"description": "foo"}, start_ts=self.two_days_ago),
105+
self.create_span(
106+
{
107+
"description": "bar",
108+
"measurements": {"lcp": {"value": 2}},
109+
},
110+
start_ts=self.day_ago,
111+
),
112+
self.create_span({"description": "bar"}, start_ts=self.day_ago),
113+
self.create_span({"description": "bar"}, start_ts=self.two_days_ago),
114+
],
115+
is_eap=self.is_eap,
116+
)
117+
118+
response = self._do_request(
119+
data={
120+
"field": ["span.description", "p50(measurements.lcp)", "avg(measurements.lcp)"],
121+
"yAxis": ["p50(measurements.lcp)", "avg(measurements.lcp)"],
122+
"project": self.project.id,
123+
"dataset": self.dataset,
124+
"excludeOther": 0,
125+
"topEvents": 1,
126+
"partial": 1,
127+
"per_page": 50,
128+
"interval": "1d",
129+
"statsPeriod": "7d",
130+
"orderby": "-avg_measurements_lcp",
131+
"sort": "-avg_measurements_lcp",
132+
},
133+
)
134+
assert response.status_code == 200, response.content
135+
136+
# We cannot actually assert the value because the `spans_indexed` is
137+
# producing the wrong result and treating missing values as 0s which
138+
# skews the final aggregation.
139+
# This is also the reason it never errored because snuba never returns
140+
# nans in this situation.
141+
#
142+
# for k in response.data:
143+
# for agg in ["p50(measurements.lcp)", "avg(measurements.lcp)"]:
144+
# for row in response.data[k][agg]["data"]:
145+
# assert row[1][0]["count"] in {0, 1, 2}
146+
# assert response.data["Other"]
147+
93148
def test_count_unique(self):
94149
event_counts = [6, 0, 6, 3, 0, 3]
95150
for hour, count in enumerate(event_counts):
@@ -857,3 +912,7 @@ def test_invalid_intervals(self):
857912
},
858913
)
859914
assert response.status_code == 400, response.content
915+
916+
@pytest.mark.xfail(reason="division by 0 error in snuba")
917+
def test_handle_nans_from_snuba_top_n(self):
918+
super().test_handle_nans_from_snuba_top_n()

0 commit comments

Comments
 (0)