Skip to content

Commit bb72dbb

Browse files
authored
fix(dashboards-comparison): Add proper handling for environment filters (#93822)
We were getting a bunch of `'str' object has no attribute 'name'` errors in both metrics and eap queries on the de job run. I think it may be due to how we previously handled the environment filters, so i've fixed that in the script and added a test for it.
1 parent 413f76b commit bb72dbb

File tree

2 files changed

+47
-1
lines changed

2 files changed

+47
-1
lines changed

src/sentry/discover/compare_tables.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@
1414
)
1515
from sentry.models.dashboard import Dashboard
1616
from sentry.models.dashboard_widget import DashboardWidget, DashboardWidgetQuery
17+
from sentry.models.environment import Environment
1718
from sentry.models.organization import Organization
1819
from sentry.models.project import Project
1920
from sentry.organizations.absolute_url import generate_organization_url
2021
from sentry.search.eap.types import EAPResponse, SearchResolverConfig
22+
from sentry.search.events.filter import to_list
2123
from sentry.search.events.types import EventsResponse, SnubaParams
2224
from sentry.snuba import metrics_enhanced_performance, spans_rpc
2325

@@ -143,7 +145,17 @@ def compare_tables_for_dashboard_widget_queries(
143145
equations = [equation for equation in _get_equation_list(widget_query.fields or []) if equation]
144146
query = widget_query.conditions
145147

146-
environments = dashboard.filters.get("environment", []) if dashboard.filters is not None else []
148+
environment_names: str | list[str] = (
149+
dashboard.filters.get("environment", []) if dashboard.filters else []
150+
)
151+
if environment_names:
152+
environments = list(
153+
Environment.objects.filter(
154+
name__in=to_list(environment_names), organization_id=organization.id
155+
)
156+
)
157+
else:
158+
environments = []
147159

148160
snuba_params = SnubaParams(
149161
environments=environments,

tests/sentry/discover/test_compare_tables.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,15 @@ def setUp(self):
3535
self.dashboard_2 = self.create_dashboard(
3636
organization=self.organization, filters={"environment": []}
3737
)
38+
self.environment = self.create_environment(
39+
organization=self.organization, project=self.project
40+
)
41+
self.dashboard_3 = self.create_dashboard(
42+
organization=self.organization, filters={"environment": [self.environment.name]}
43+
)
3844
self.dashboard.projects.set([self.project])
3945
self.dashboard_2.projects.set([])
46+
self.dashboard_3.projects.set([self.project])
4047

4148
self.successful_widget_2 = DashboardWidget.objects.create(
4249
dashboard=self.dashboard_2,
@@ -147,6 +154,25 @@ def setUp(self):
147154
fields=["count()", "http.status_code"],
148155
)
149156

157+
self.widget_with_environment_filter = DashboardWidget.objects.create(
158+
dashboard=self.dashboard_3,
159+
title="Test Erroring Widget",
160+
order=6,
161+
display_type=DashboardWidgetDisplayTypes.TABLE,
162+
widget_type=DashboardWidgetTypes.TRANSACTION_LIKE,
163+
)
164+
165+
self.widget_with_environment_filter_query = DashboardWidgetQuery.objects.create(
166+
widget=self.widget_with_environment_filter,
167+
name="Test Widget Query With Environment Filter",
168+
order=6,
169+
fields=["transaction", "count()"],
170+
conditions="!event.type:error",
171+
orderby="-count()",
172+
aggregates=["count()"],
173+
columns=["transaction"],
174+
)
175+
150176
self.triple_write_segment(
151177
project=self.project,
152178
trace_id=uuid4().hex,
@@ -328,3 +354,11 @@ def test_compare_widget_query_with_no_project(self):
328354
)
329355
assert comparison_result["passed"] is True
330356
assert comparison_result["mismatches"] == []
357+
358+
def test_compare_widget_query_that_errors_out(self):
359+
comparison_result = compare_tables_for_dashboard_widget_queries(
360+
self.widget_with_environment_filter_query
361+
)
362+
assert comparison_result["passed"] is False
363+
# assert that both queries don't fail due to the environment filter
364+
assert comparison_result["reason"] != CompareTableResult.BOTH_FAILED

0 commit comments

Comments
 (0)