feat(discover): Prevent calling Snuba with an empty list of projects (#…

…69577)

I discovered that we can make calls to Snuba with an empty list of `project_ids` (which is a required field) and that _always_ returns no data. This may not be happening in production, however, during development, this can take a lot of time to debug.

In production, we will raise an error rather than calling Snuba, thus, helping us find any issues in production.

During development, we will call Snuba _without_ `project_ids` which will cause an error from Snuba and help a developer notice the problem. We're doing this since there are a lot of tests in our codebase which require including the list of projects before calling Snuba (You can see some of the failures in #69575 ). We should eventually fix it, however, it requires significant effort.

src/sentry/search/events/builder/discover.py

@@ -72,8 +72,10 @@
 from sentry.snuba.dataset import Dataset
 from sentry.snuba.metrics.utils import MetricMeta
 from sentry.utils.dates import outside_retention_with_modified_start
+from sentry.utils.env import in_test_environment
 from sentry.utils.snuba import (
     QueryOutsideRetentionError,
+    UnqualifiedQueryError,
     is_duration_measurement,
     is_measurement,
     is_numeric_measurement,
@@ -583,13 +585,25 @@ def resolve_params(self) -> list[WhereType]:
         if self.end:
             conditions.append(Condition(self.column("timestamp"), Op.LT, self.end))
 
-        conditions.append(
-            Condition(
-                self.column("project_id"),
-                Op.IN,
-                self.params.project_ids,
+        # project_ids is a required column for most datasets, however, Snuba does not
+        # complain on an empty list which results on no data being returned.
+        # This change will prevent calling Snuba when no projects are selected.
+        # Snuba will complain with UnqualifiedQueryError: validation failed for entity...
+        if not self.params.project_ids:
+            # TODO: Fix the tests and always raise the error
+            # In development, we will let Snuba complain about the lack of projects
+            # so the developer can write their tests with a non-empty project list
+            # In production, we will raise an error
+            if not in_test_environment():
+                raise UnqualifiedQueryError("You need to specify at least one project.")
+        else:
+            conditions.append(
+                Condition(
+                    self.column("project_id"),
+                    Op.IN,
+                    self.params.project_ids,
+                )
             )
-        )
 
         if len(self.params.environments) > 0:
             term = event_search.SearchFilter(

src/sentry/utils/snuba.py

@@ -1022,6 +1022,8 @@ def _bulk_snuba_query(
                     raise RateLimitExceeded(error["message"])
                 elif error["type"] == "schema":
                     raise SchemaValidationError(error["message"])
+                elif error["type"] == "invalid_query":
+                    raise UnqualifiedQueryError(error["message"])
                 elif error["type"] == "clickhouse":
                     raise clickhouse_error_codes_map.get(error["code"], QueryExecutionError)(
                         error["message"]

tests/sentry/api/endpoints/test_organization_dashboard_widget_details.py

@@ -638,6 +638,8 @@ def test_save_with_invalid_orderby_not_from_columns_or_aggregates(self):
         assert "queries" in response.data, response.data
 
     def test_save_with_total_count(self):
+        # We cannot query the Discover entity without a project being defined for the org
+        self.create_project()
         data = {
             "title": "Test Query",
             "displayType": "table",

tests/sentry/dynamic_sampling/tasks/test_custom_rule_notifications.py

@@ -49,6 +49,8 @@ def test_get_num_samples(self):
         """
         Tests that the num_samples function returns the correct number of samples
         """
+        # We cannot query the discover_transactions entity without a project being defined for the org
+        self.create_project()
         num_samples = get_num_samples(self.rule)
         assert num_samples == 0
         self.create_transaction()

tests/sentry/search/events/builder/test_discover.py

@@ -16,8 +16,9 @@
 from sentry.search.events.builder import QueryBuilder
 from sentry.search.events.types import ParamsType, QueryBuilderConfig
 from sentry.snuba.dataset import Dataset
+from sentry.snuba.referrer import Referrer
 from sentry.testutils.cases import TestCase
-from sentry.utils.snuba import QueryOutsideRetentionError
+from sentry.utils.snuba import QueryOutsideRetentionError, UnqualifiedQueryError, bulk_snuba_queries
 from sentry.utils.validators import INVALID_ID_DETAILS
 
 pytestmark = pytest.mark.sentry_metrics
@@ -67,6 +68,27 @@ def test_simple_query(self):
         )
         query.get_snql_query().validate()
 
+    def test_query_without_project_ids(self):
+        params: ParamsType = {
+            "start": self.params["start"],
+            "end": self.params["end"],
+            "organization_id": self.organization.id,
+        }
+        with pytest.raises(UnqualifiedQueryError):
+            query = QueryBuilder(Dataset.Discover, params, query="foo", selected_columns=["id"])
+            bulk_snuba_queries([query.get_snql_query()], referrer=Referrer.TESTING_TEST.value)
+
+    def test_query_with_empty_project_ids(self):
+        params: ParamsType = {
+            "start": self.params["start"],
+            "end": self.params["end"],
+            "project_id": [],  # We add an empty project_id list
+            "organization_id": self.organization.id,
+        }
+        with pytest.raises(UnqualifiedQueryError):
+            query = QueryBuilder(Dataset.Discover, params, query="foo", selected_columns=["id"])
+            bulk_snuba_queries([query.get_snql_query()], referrer=Referrer.TESTING_TEST.value)
+
     def test_simple_orderby(self):
         query = QueryBuilder(
             Dataset.Discover,