Browse Source

fix(ondemand): Pass projects in correctly (#67728)

- Projects weren't being passed to QueryBuilder correctly which meant we
were querying without projects
- Add a test that actually runs a query so we know its working as
expected
William Mak 11 months ago
parent
commit
fb60947f69

+ 3 - 1
src/sentry/tasks/on_demand_metrics.py

@@ -393,6 +393,7 @@ def _get_widget_query_low_cardinality(
     return all(field_cardinality.values())
 
 
+@sentry_sdk.tracing.trace
 def check_field_cardinality(
     query_columns: list[str] | None,
     organization: Organization,
@@ -459,6 +460,7 @@ def check_field_cardinality(
     return {key: cardinality_map.get(value, True) for key, value in cache_keys.items()}
 
 
+@sentry_sdk.tracing.trace
 def _query_cardinality(
     query_columns: list[str], organization: Organization, period: str = "30m"
 ) -> tuple[EventsResponse, list[str]]:
@@ -468,7 +470,7 @@ def _query_cardinality(
     params: ParamsType = {
         "statsPeriod": period,
         "organization_id": organization.id,
-        "projects": Project.objects.filter(organization=organization),
+        "project_objects": Project.objects.filter(organization=organization),
     }
     start, end = get_date_range_from_params(params)
     params["start"] = start

+ 47 - 0
tests/sentry/api/endpoints/test_organization_dashboard_widget_details.py

@@ -2,6 +2,7 @@ from unittest import mock
 
 from django.urls import reverse
 
+from sentry import options
 from sentry.models.dashboard_widget import (
     DashboardWidget,
     DashboardWidgetDisplayTypes,
@@ -1034,3 +1035,49 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
         warnings = response.data["warnings"]
         assert len(warnings["queries"]) == 2
         assert response.data == {"warnings": {"columns": {}, "queries": [None, None]}}
+
+    def test_widget_cardinality(self):
+        self.store_event(
+            data={
+                "event_id": "a" * 32,
+                "transaction": "/example",
+                "message": "how to make fast",
+                "timestamp": iso_format(before_now(minutes=2)),
+                "tags": {"sometag": "foo"},
+            },
+            project_id=self.project.id,
+        )
+        project = self.create_project()
+        self.create_environment(project=project, name="mock_env")
+        data = {
+            "title": "Test Query",
+            "displayType": "table",
+            "widgetType": "discover",
+            "limit": 5,
+            "queries": [
+                {
+                    "name": "",
+                    "conditions": "release.stage:adopted",
+                    "columns": ["sometag"],
+                    "fields": [],
+                    "aggregates": ["count()"],
+                }
+            ],
+        }
+
+        option_get = options.get
+
+        def mock_options(option_name):
+            if option_name == "on_demand.max_widget_cardinality.on_query_count":
+                return 0
+            else:
+                return option_get(option_name)
+
+        with mock.patch("sentry.options.get", side_effect=mock_options):
+            with self.feature(ONDEMAND_FEATURES):
+                response = self.client.post(f"{self.url()}?environment=mock_env", data)
+        assert response.status_code == 200, response.data
+        warnings = response.data["warnings"]
+        assert "columns" in warnings
+        assert len(warnings["columns"]) == 1
+        assert warnings["columns"]["sometag"] == "disabled:high-cardinality"

+ 0 - 12
tests/sentry/tasks/test_on_demand_metrics.py

@@ -10,7 +10,6 @@ from sentry.models.project import Project
 from sentry.models.user import User
 from sentry.tasks import on_demand_metrics
 from sentry.tasks.on_demand_metrics import (
-    _query_cardinality,
     get_field_cardinality_cache_key,
     process_widget_specs,
     schedule_on_demand_check,
@@ -566,14 +565,3 @@ def assert_on_demand_model(
         return
 
     assert model.spec_hashes == expected_hashes[model.spec_version]  # Still include hashes
-
-
-@mock.patch("sentry.tasks.on_demand_metrics.QueryBuilder")
-@django_db_all
-def test_query_cardinality_called_with_projects(
-    raw_snql_query: Any, project: Project, organization: Organization
-) -> None:
-    _query_cardinality(["sometag"], organization)
-    raw_snql_query.assert_called_once()
-    mock_call = raw_snql_query.mock_calls[0]
-    assert [proj.id for proj in mock_call.kwargs["params"]["projects"]] == [project.id]