Просмотр исходного кода

fix(snql) Turn SnQL on by default (#27750)

Have this be on by default. I talked to the open source team to make sure this
wouldn't impact those users. This required a couple small changes to tests and
some corresponding fixes in the snuba SDK.
Evan Hicks 3 лет назад
Родитель
Сommit
0d4a7793c7

+ 1 - 1
requirements-base.txt

@@ -51,7 +51,7 @@ rfc3986-validator==0.1.1
 # [end] jsonschema format validators
 sentry-relay==0.8.8
 sentry-sdk>=1.0.0,<1.2.0
-snuba-sdk>=0.0.22,<1.0.0
+snuba-sdk>=0.0.23,<1.0.0
 simplejson==3.17.2
 statsd==3.3
 structlog==21.1.0

+ 1 - 1
src/sentry/options/defaults.py

@@ -213,7 +213,7 @@ 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=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)

+ 9 - 3
src/sentry/utils/snuba.py

@@ -761,11 +761,17 @@ def _bulk_snuba_query(
         # 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 = _snuba_query
+        query_fn, query_type = _snuba_query, "legacy"
         if isinstance(snuba_param_list[0][0], Query):
-            query_fn = _snql_query
+            query_fn, query_type = _snql_query, "snql"
         elif use_snql:
-            query_fn = _legacy_snql_query
+            query_fn, query_type = _legacy_snql_query, "translated"
+
+        metrics.incr(
+            "snuba.snql.query.type",
+            tags={"type": query_type, "referrer": query_referrer},
+        )
+        span.set_tag("snuba.query.type", query_type)
 
         if len(snuba_param_list) > 1:
             query_results = list(

+ 1 - 1
tests/snuba/api/endpoints/test_discover_query.py

@@ -173,7 +173,7 @@ class DiscoverQueryTest(APITestCase, SnubaTestCase):
                     "conditionFields": [
                         [
                             "if",
-                            [["in", ["release", "tuple", ["'foo'"]]], "release", "'other'"],
+                            [["in", ["release", ["tuple", ["'foo'"]]]], "release", "'other'"],
                             "release",
                         ]
                     ],

+ 1 - 1
tests/snuba/search/test_backend.py

@@ -1940,7 +1940,7 @@ class EventsSnubaSearchTest(TestCase, SnubaTestCase):
             elif key in issue_search_config.numeric_keys:
                 val = "123"
             elif key in issue_search_config.date_keys:
-                val = "2019-01-01"
+                val = self.base_datetime.isoformat()
             elif key in issue_search_config.boolean_keys:
                 val = "true"
             else:

+ 41 - 55
tests/snuba/test_snuba.py

@@ -6,6 +6,7 @@ from unittest import mock
 
 import pytest
 from django.utils import timezone
+from snuba_sdk.column import InvalidColumn
 
 from sentry.testutils import SnubaTestCase, TestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
@@ -13,8 +14,6 @@ from sentry.utils import snuba
 
 
 class SnubaTest(TestCase, SnubaTestCase):
-    should_use_snql = None
-
     def _insert_event_for_time(self, ts, hash="a" * 32, group_id=None):
         self.snuba_insert(
             (
@@ -56,31 +55,27 @@ class SnubaTest(TestCase, SnubaTestCase):
         ]
 
         self.snuba_insert(events)
-        snql_option = 1.0 if self.should_use_snql else 0.0
-        with self.options({"snuba.snql.referrer-rate": snql_option}):
-            assert (
-                snuba.query(
-                    start=now - timedelta(days=1),
-                    end=now + timedelta(days=1),
-                    groupby=["project_id"],
-                    filter_keys={"project_id": [self.project.id]},
-                    referrer="testing.test" if self.should_use_snql else "",
-                )
-                == {self.project.id: 1}
+        assert (
+            snuba.query(
+                start=now - timedelta(days=1),
+                end=now + timedelta(days=1),
+                groupby=["project_id"],
+                filter_keys={"project_id": [self.project.id]},
+                referrer="testing.test",
             )
+            == {self.project.id: 1}
+        )
 
     def test_fail(self) -> None:
         now = datetime.now()
-        with pytest.raises(snuba.SnubaError):
-            snql_option = 1.0 if self.should_use_snql else 0.0
-            with self.options({"snuba.snql.referrer-rate": snql_option}):
-                snuba.query(
-                    start=now - timedelta(days=1),
-                    end=now + timedelta(days=1),
-                    filter_keys={"project_id": [self.project.id]},
-                    groupby=[")("],
-                    referrer="testing.test" if self.should_use_snql else "",
-                )
+        with pytest.raises(InvalidColumn):
+            snuba.query(
+                start=now - timedelta(days=1),
+                end=now + timedelta(days=1),
+                filter_keys={"project_id": [self.project.id]},
+                groupby=[")("],
+                referrer="testing.test",
+            )
 
     def test_organization_retention_respected(self) -> None:
         base_time = datetime.utcnow()
@@ -90,15 +85,13 @@ class SnubaTest(TestCase, SnubaTestCase):
 
         def _get_event_count():
             # attempt to query back 90 days
-            snql_option = 1.0 if self.should_use_snql else 0.0
-            with self.options({"snuba.snql.referrer-rate": snql_option}):
-                return snuba.query(
-                    start=base_time - timedelta(days=90),
-                    end=base_time + timedelta(days=1),
-                    groupby=["project_id"],
-                    filter_keys={"project_id": [self.project.id]},
-                    referrer="testing.test" if self.should_use_snql else "",
-                )
+            return snuba.query(
+                start=base_time - timedelta(days=90),
+                end=base_time + timedelta(days=1),
+                groupby=["project_id"],
+                filter_keys={"project_id": [self.project.id]},
+                referrer="testing.test",
+            )
 
         assert _get_event_count() == {self.project.id: 2}
         with self.options({"system.event-retention-days": 1}):
@@ -108,38 +101,31 @@ class SnubaTest(TestCase, SnubaTestCase):
         base_time = datetime.utcnow()
 
         with self.options({"system.event-retention-days": 1}):
-            snql_option = 1.0 if self.should_use_snql else 0.0
-            with self.options({"snuba.snql.referrer-rate": snql_option}):
-                assert (
-                    snuba.query(
-                        start=base_time - timedelta(days=90),
-                        end=base_time - timedelta(days=60),
-                        groupby=["project_id"],
-                        filter_keys={"project_id": [self.project.id]},
-                        referrer="testing.test" if self.should_use_snql else "",
-                    )
-                    == {}
-                )
-
-    def test_should_use_snql(self) -> None:
-        base_time = datetime.utcnow()
-
-        with self.options({"snuba.snql.snql_only": 1.0}):
             assert (
                 snuba.query(
-                    start=base_time - timedelta(days=1),
-                    end=base_time,
-                    aggregations=[["count", None, "count"]],
+                    start=base_time - timedelta(days=90),
+                    end=base_time - timedelta(days=60),
                     groupby=["project_id"],
                     filter_keys={"project_id": [self.project.id]},
-                    referrer="sessions.stability-sort",
+                    referrer="testing.test",
                 )
                 == {}
             )
 
+    def test_should_use_snql(self) -> None:
+        base_time = datetime.utcnow()
 
-class SnQLSnubaTest(SnubaTest):
-    should_use_snql = True
+        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):