ref(snql) Remove legacy API code (#28631)

ref(snql) Remove legacy API code

The Legacy API hasn't been used in production for at least two releases now, so removing this code should be safe now.

src/sentry/options/defaults.py

@@ -212,8 +212,6 @@ register("snuba.search.max-chunk-size", default=2000)
 register("snuba.search.max-total-chunk-time-seconds", default=30.0)
 register("snuba.search.hits-sample-size", default=100)
 register("snuba.track-outcomes-sample-rate", default=0.0)
-register("snuba.snql.referrer-rate", default=0.0)
-register("snuba.snql.snql_only", default=1.0)
 
 # The percentage of tagkeys that we want to cache. Set to 1.0 in order to cache everything, <=0.0 to stop caching
 register("snuba.tagstore.cache-tagkeys-rate", default=0.0, flags=FLAG_PRIORITIZE_DISK)

src/sentry/utils/snql.py

@@ -1,15 +0,0 @@
-import random
-from typing import Optional
-
-from sentry import options
-
-
-def should_use_snql(referrer: Optional[str]) -> bool:
-    snql_rate = options.get("snuba.snql.snql_only")
-    if not isinstance(snql_rate, float):
-        return False
-
-    if snql_rate == 1.0:
-        return True
-
-    return random.random() <= snql_rate

src/sentry/utils/snuba.py

@@ -39,7 +39,6 @@ from sentry.snuba.events import Columns
 from sentry.utils import json, metrics
 from sentry.utils.compat import map
 from sentry.utils.dates import outside_retention_with_modified_start, to_timestamp
-from sentry.utils.snql import should_use_snql
 
 logger = logging.getLogger(__name__)
 
@@ -648,14 +647,7 @@ def raw_query(
         **kwargs,
     )
 
-    use_snql = should_use_snql(referrer)
-
-    return bulk_raw_query(
-        [snuba_params],
-        referrer=referrer,
-        use_cache=use_cache,
-        use_snql=use_snql,
-    )[0]
+    return bulk_raw_query([snuba_params], referrer=referrer, use_cache=use_cache)[0]
 
 
 SnubaQuery = Union[Query, MutableMapping[str, Any]]
@@ -691,19 +683,15 @@ def bulk_raw_query(
     snuba_param_list: Sequence[SnubaQueryParams],
     referrer: Optional[str] = None,
     use_cache: Optional[bool] = False,
-    use_snql: Optional[bool] = None,
 ) -> ResultSet:
     params = map(_prepare_query_params, snuba_param_list)
-    return _apply_cache_and_build_results(
-        params, referrer=referrer, use_cache=use_cache, use_snql=use_snql
-    )
+    return _apply_cache_and_build_results(params, referrer=referrer, use_cache=use_cache)
 
 
 def _apply_cache_and_build_results(
     snuba_param_list: Sequence[SnubaQueryBody],
     referrer: Optional[str] = None,
     use_cache: Optional[bool] = False,
-    use_snql: Optional[bool] = None,
 ) -> ResultSet:
     headers = {}
     if referrer:
@@ -731,7 +719,7 @@ def _apply_cache_and_build_results(
         to_query = [(query_pos, query_params, None) for query_pos, query_params in query_param_list]
 
     if to_query:
-        query_results = _bulk_snuba_query(map(itemgetter(1), to_query), headers, use_snql)
+        query_results = _bulk_snuba_query(map(itemgetter(1), to_query), headers)
         for result, (query_pos, _, cache_key) in zip(query_results, to_query):
             if cache_key:
                 cache.set(cache_key, json.dumps(result), settings.SENTRY_SNUBA_CACHE_TTL_SECONDS)
@@ -746,7 +734,6 @@ def _apply_cache_and_build_results(
 def _bulk_snuba_query(
     snuba_param_list: Sequence[SnubaQueryBody],
     headers: Mapping[str, str],
-    use_snql: Optional[bool] = None,
 ) -> ResultSet:
     with sentry_sdk.start_span(
         op="start_snuba_query",
@@ -757,15 +744,12 @@ def _bulk_snuba_query(
         # but we still want to know a general sense of how referrers impact performance
         span.set_tag("query.referrer", query_referrer)
         sentry_sdk.set_tag("query.referrer", query_referrer)
-        # This is confusing because this function is overloaded right now with three cases:
-        # 1. A legacy JSON query (_snuba_query)
-        # 2. A SnQL query of a legacy query (_legacy_snql_query)
-        # 3. A direct SnQL query using the new SDK (_snql_query)
-        query_fn, query_type = _snuba_query, "legacy"
+        # This is confusing because this function is overloaded right now with two cases:
+        # 1. A SnQL query of a legacy query (_legacy_snql_query)
+        # 2. A direct SnQL query using the new SDK (_snql_query)
+        query_fn, query_type = _legacy_snql_query, "translated"
         if isinstance(snuba_param_list[0][0], Query):
             query_fn, query_type = _snql_query, "snql"
-        elif use_snql:
-            query_fn, query_type = _legacy_snql_query, "translated"
 
         metrics.incr(
             "snuba.snql.query.type",
@@ -829,44 +813,6 @@ def _bulk_snuba_query(
 RawResult = Tuple[urllib3.response.HTTPResponse, Callable[[Any], Any], Callable[[Any], Any]]
 
 
-def _snuba_query(params: Tuple[SnubaQuery, Hub, Mapping[str, str]]) -> RawResult:
-    query_data, thread_hub, headers = params
-    query_params, forward, reverse = query_data
-    try:
-        with timer("snuba_query"):
-            referrer = headers.get("referer", "<unknown>")
-            if SNUBA_INFO:
-                # We want debug in the body, but not in the logger, so dump the json twice
-                logger.info(f"{referrer}.body: {json.dumps(query_params)}")
-                query_params["debug"] = True
-
-            with thread_hub.start_span(op="snuba", description=f"json encode query {referrer}"):
-                scope = thread_hub.scope
-                if scope.transaction:
-                    query_params["parent_api"] = scope.transaction.name
-
-                metrics.incr(
-                    "snuba.parent_api",
-                    tags={
-                        "parent_api": query_params.get("parent_api", "<unknown>"),
-                        "referrer": referrer,
-                    },
-                )
-                body = json.dumps(query_params)
-
-            with thread_hub.start_span(op="snuba", description=f"query {referrer}") as span:
-                span.set_tag("referrer", referrer)
-                for param_key, param_data in query_params.items():
-                    span.set_data(param_key, param_data)
-                return (
-                    _snuba_pool.urlopen("POST", "/query", body=body, headers=headers),
-                    forward,
-                    reverse,
-                )
-    except urllib3.exceptions.HTTPError as err:
-        raise SnubaError(err)
-
-
 def _snql_query(params: Tuple[SnubaQuery, Hub, Mapping[str, str]]) -> RawResult:
     # Eventually we can get rid of this wrapper, but for now it's cleaner to unwrap
     # the params here than in the calling function.

tests/sentry/snuba/test_discover.py

@@ -6324,45 +6324,6 @@ class ArithmeticTest(SnubaTestCase, TestCase):
         assert len(results["data"]) == 3
         assert [result["equation[0]"] for result in results["data"]] == [0.5, 0.2, 0.1]
 
-    # TODO: remove this once we're fully converted, this duplicate test is to test a specific bug with ordering and the
-    # new syntax
-    @patch("sentry.utils.snuba.should_use_snql", return_value=1)
-    def test_orderby_equation_with_snql(self, mock_use_snql):
-        for i in range(1, 3):
-            event_data = load_data("transaction")
-            # Half of duration so we don't get weird rounding differences when comparing the results
-            event_data["breakdowns"]["span_ops"]["ops.http"]["value"] = 300 * i
-            event_data["start_timestamp"] = iso_format(self.day_ago + timedelta(minutes=30))
-            event_data["timestamp"] = iso_format(self.day_ago + timedelta(minutes=30, seconds=3))
-            self.store_event(data=event_data, project_id=self.project.id)
-        query_params = {
-            "selected_columns": [
-                "spans.http",
-                "transaction.duration",
-            ],
-            "equations": [
-                "spans.http / transaction.duration",
-                "transaction.duration / spans.http",
-                "1500 + transaction.duration",
-            ],
-            "orderby": ["equation[0]"],
-            "query": self.query,
-            "params": self.params,
-        }
-        results = discover.query(**query_params)
-        assert len(results["data"]) == 3
-        assert [result["equation[0]"] for result in results["data"]] == [0.1, 0.2, 0.5]
-
-        query_params["orderby"] = ["equation[1]"]
-        results = discover.query(**query_params)
-        assert len(results["data"]) == 3
-        assert [result["equation[1]"] for result in results["data"]] == [2, 5, 10]
-
-        query_params["orderby"] = ["-equation[0]"]
-        results = discover.query(**query_params)
-        assert len(results["data"]) == 3
-        assert [result["equation[0]"] for result in results["data"]] == [0.5, 0.2, 0.1]
-
     def test_orderby_nonexistent_equation(self):
         with self.assertRaises(InvalidSearchQuery):
             discover.query(

tests/snuba/test_snuba.py

@@ -112,21 +112,6 @@ class SnubaTest(TestCase, SnubaTestCase):
                 == {}
             )
 
-    def test_should_use_snql(self) -> None:
-        base_time = datetime.utcnow()
-
-        assert (
-            snuba.query(
-                start=base_time - timedelta(days=1),
-                end=base_time,
-                aggregations=[["count", None, "count"]],
-                groupby=["project_id"],
-                filter_keys={"project_id": [self.project.id]},
-                referrer="sessions.stability-sort",
-            )
-            == {}
-        )
-
 
 class BulkRawQueryTest(TestCase, SnubaTestCase):
     def test_simple(self) -> None:
@@ -161,39 +146,6 @@ class BulkRawQueryTest(TestCase, SnubaTestCase):
             {(event_2.group.id, event_2.event_id)},
         ]
 
-    def test_simple_use_snql(self) -> None:
-        one_min_ago = iso_format(before_now(minutes=1))
-        event_1 = self.store_event(
-            data={"fingerprint": ["group-1"], "message": "hello", "timestamp": one_min_ago},
-            project_id=self.project.id,
-        )
-        event_2 = self.store_event(
-            data={"fingerprint": ["group-2"], "message": "hello", "timestamp": one_min_ago},
-            project_id=self.project.id,
-        )
-
-        results = snuba.bulk_raw_query(
-            [
-                snuba.SnubaQueryParams(
-                    start=timezone.now() - timedelta(days=1),
-                    end=timezone.now(),
-                    selected_columns=["event_id", "group_id", "timestamp"],
-                    filter_keys={"project_id": [self.project.id], "group_id": [event_1.group.id]},
-                ),
-                snuba.SnubaQueryParams(
-                    start=timezone.now() - timedelta(days=1),
-                    end=timezone.now(),
-                    selected_columns=["event_id", "group_id", "timestamp"],
-                    filter_keys={"project_id": [self.project.id], "group_id": [event_2.group.id]},
-                ),
-            ],
-            use_snql=True,
-        )
-        assert [{(item["group_id"], item["event_id"]) for item in r["data"]} for r in results] == [
-            {(event_1.group.id, event_1.event_id)},
-            {(event_2.group.id, event_2.event_id)},
-        ]
-
     @mock.patch("sentry.utils.snuba._bulk_snuba_query", side_effect=snuba._bulk_snuba_query)
     def test_cache(self, _bulk_snuba_query):
         one_min_ago = iso_format(before_now(minutes=1))