Browse Source

ref(metrics_extraction): auto fixes + do_request refactor (#64894)

Automatic formatting changes and move `do_request` to the parental
class.
Armen Zambrano G 1 year ago
parent
commit
a1ae8365cd

+ 20 - 12
src/sentry/search/events/builder/metrics.py

@@ -1024,9 +1024,11 @@ class MetricsQueryBuilder(QueryBuilder):
             # Need this otherwise top_events returns only 1 item
             groupbys = [Column(col) for col in self._get_group_bys()]
         groupby_aliases = [
-            groupby.alias
-            if isinstance(groupby, (AliasedExpression, CurriedFunction))
-            else groupby.name
+            (
+                groupby.alias
+                if isinstance(groupby, (AliasedExpression, CurriedFunction))
+                else groupby.name
+            )
             for groupby in groupbys
             if not (
                 isinstance(groupby, CurriedFunction) and groupby.function == "team_key_transaction"
@@ -1074,9 +1076,11 @@ class MetricsQueryBuilder(QueryBuilder):
                             Function(
                                 "tuple",
                                 [
-                                    groupby.exp
-                                    if isinstance(groupby, AliasedExpression)
-                                    else groupby
+                                    (
+                                        groupby.exp
+                                        if isinstance(groupby, AliasedExpression)
+                                        else groupby
+                                    )
                                     for groupby in self.groupby
                                     if not (
                                         isinstance(groupby, CurriedFunction)
@@ -1172,9 +1176,11 @@ class MetricsQueryBuilder(QueryBuilder):
                             Function(
                                 "tuple",
                                 [
-                                    groupby.exp
-                                    if isinstance(groupby, AliasedExpression)
-                                    else groupby
+                                    (
+                                        groupby.exp
+                                        if isinstance(groupby, AliasedExpression)
+                                        else groupby
+                                    )
                                     for groupby in self.groupby
                                 ],
                             ),
@@ -1845,9 +1851,11 @@ class TopMetricsQueryBuilder(TimeseriesMetricQueryBuilder):
                             get_series(
                                 projects=self.params.projects,
                                 metrics_query=metrics_query,
-                                use_case_id=UseCaseID.TRANSACTIONS
-                                if self.is_performance
-                                else UseCaseID.SESSIONS,
+                                use_case_id=(
+                                    UseCaseID.TRANSACTIONS
+                                    if self.is_performance
+                                    else UseCaseID.SESSIONS
+                                ),
                                 include_meta=True,
                                 tenant_ids=self.tenant_ids,
                             )

+ 13 - 9
src/sentry/search/events/datasets/metrics.py

@@ -918,9 +918,9 @@ class MetricsDatasetConfig(DatasetConfig):
             column_name_resolver=lambda _use_case_id, _org_id, value: self.builder.resolve_column_name(
                 value
             ),
-            org_id=self.builder.params.organization.id
-            if self.builder.params.organization
-            else None,
+            org_id=(
+                self.builder.params.organization.id if self.builder.params.organization else None
+            ),
             project_ids=self.builder.params.project_ids,
         )
 
@@ -1104,9 +1104,11 @@ class MetricsDatasetConfig(DatasetConfig):
             f"histogramIf({num_buckets})",
             [
                 Column("value"),
-                Function("and", [zoom_params, metric_condition])
-                if zoom_params
-                else metric_condition,
+                (
+                    Function("and", [zoom_params, metric_condition])
+                    if zoom_params
+                    else metric_condition
+                ),
             ],
             alias,
         )
@@ -1725,9 +1727,11 @@ class MetricsDatasetConfig(DatasetConfig):
                         condition,
                     ],
                 ),
-                args["interval"]
-                if interval is None
-                else Function("divide", [args["interval"], interval]),
+                (
+                    args["interval"]
+                    if interval is None
+                    else Function("divide", [args["interval"], interval])
+                ),
             ],
             alias,
         )

+ 21 - 15
src/sentry/snuba/discover.py

@@ -344,15 +344,17 @@ def timeseries_query(
         for snql_query, result in zip(query_list, query_results):
             results.append(
                 {
-                    "data": zerofill(
-                        result["data"],
-                        snql_query.params.start,
-                        snql_query.params.end,
-                        rollup,
-                        "time",
-                    )
-                    if zerofill_results
-                    else result["data"],
+                    "data": (
+                        zerofill(
+                            result["data"],
+                            snql_query.params.start,
+                            snql_query.params.end,
+                            rollup,
+                            "time",
+                        )
+                        if zerofill_results
+                        else result["data"]
+                    ),
                     "meta": result["meta"],
                 }
             )
@@ -509,9 +511,11 @@ def top_events_timeseries(
     ):
         return SnubaTSResult(
             {
-                "data": zerofill([], params["start"], params["end"], rollup, "time")
-                if zerofill_results
-                else [],
+                "data": (
+                    zerofill([], params["start"], params["end"], rollup, "time")
+                    if zerofill_results
+                    else []
+                ),
             },
             params["start"],
             params["end"],
@@ -553,9 +557,11 @@ def top_events_timeseries(
         for key, item in results.items():
             results[key] = SnubaTSResult(
                 {
-                    "data": zerofill(item["data"], params["start"], params["end"], rollup, "time")
-                    if zerofill_results
-                    else item["data"],
+                    "data": (
+                        zerofill(item["data"], params["start"], params["end"], rollup, "time")
+                        if zerofill_results
+                        else item["data"]
+                    ),
                     "order": item["order"],
                 },
                 params["start"],

+ 1 - 1
src/sentry/snuba/metrics/extraction.py

@@ -224,7 +224,7 @@ _SEARCH_TO_METRIC_AGGREGATES: dict[str, MetricOperationType] = {
     "p95": "p95",
     "p99": "p99",
     # p100 is not supported in the metrics layer, so we convert to max which is equivalent.
-    "p100": "max"
+    "p100": "max",
     # generic percentile is not supported by metrics layer.
 }
 

+ 9 - 0
src/sentry/testutils/cases.py

@@ -40,6 +40,7 @@ from django.utils.functional import cached_property
 from requests.utils import CaseInsensitiveDict, get_encoding_from_headers
 from rest_framework import status
 from rest_framework.request import Request
+from rest_framework.response import Response
 from rest_framework.test import APITestCase as BaseAPITestCase
 from sentry_relay.consts import SPAN_STATUS_NAME_TO_CODE
 from snuba_sdk import Granularity, Limit, Offset
@@ -2012,8 +2013,16 @@ class MetricsEnhancedPerformanceTestCase(BaseMetricsLayerTestCase, TestCase):
 
     def setUp(self):
         super().setUp()
+        self.login_as(user=self.user)
         self._index_metric_strings()
 
+    def do_request(self, data: dict[str, Any], features: dict[str, str] = None) -> Response:
+        """Set up self.features and self.url in the inheriting classes.
+        You can pass your own features if you do not want to use the default used by the subclass.
+        """
+        with self.feature(self.features if features is None else features):
+            return self.client.get(self.url, data=data, format="json")
+
     def _index_metric_strings(self):
         strings = [
             "transaction",

+ 2 - 9
tests/snuba/api/endpoints/test_organization_events_mep.py

@@ -3134,15 +3134,8 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTestWithOnDemandMetric
 
     def setUp(self) -> None:
         super().setUp()
-
-    def do_request(self, query: Any) -> Any:
-        self.login_as(user=self.user)
-        url = reverse(
-            self.viewname,
-            kwargs={"organization_slug": self.organization.slug},
-        )
-        with self.feature({"organizations:on-demand-metrics-extraction-widgets": True}):
-            return self.client.get(url, query, format="json")
+        self.url = reverse(self.viewname, kwargs={"organization_slug": self.organization.slug})
+        self.features = {"organizations:on-demand-metrics-extraction-widgets": True}
 
     def _on_demand_query_check(
         self,

+ 5 - 17
tests/snuba/api/endpoints/test_organization_events_stats_mep.py

@@ -43,13 +43,6 @@ class OrganizationEventsStatsMetricsEnhancedPerformanceEndpointTest(
 
         self.additional_params = dict()
 
-    def do_request(self, data, url=None, features=None):
-        if features is None:
-            features = {"organizations:discover-basic": True}
-        features.update(self.features)
-        with self.feature(features):
-            return self.client.get(self.url if url is None else url, data=data, format="json")
-
     # These throughput tests should roughly match the ones in OrganizationEventsStatsEndpointTest
     def test_throughput_epm_hour_rollup(self):
         # Each of these denotes how many events to create in each hour
@@ -976,14 +969,10 @@ class OrganizationEventsStatsMetricsEnhancedPerformanceEndpointTestWithOnDemandW
             "sentry-api-0-organization-events-stats",
             kwargs={"organization_slug": self.project.organization.slug},
         )
-        self.features = {"organizations:on-demand-metrics-extraction-widgets": True}
-
-    def do_request(self, data, url=None, features=None):
-        if features is None:
-            features = {"organizations:discover-basic": True}
-        features.update(self.features)
-        with self.feature(features):
-            return self.client.get(self.url if url is None else url, data=data, format="json")
+        self.features = {
+            "organizations:on-demand-metrics-extraction-widgets": True,
+            "organizations:on-demand-metrics-extraction": True,
+        }
 
     def test_top_events_wrong_on_demand_type(self):
         query = "transaction.duration:>=100"
@@ -1392,7 +1381,6 @@ class OrganizationEventsStatsMetricsEnhancedPerformanceEndpointTestWithOnDemandW
     def _test_is_metrics_extracted_data(
         self, params: dict[str, Any], expected_on_demand_query: bool, dataset: str
     ) -> None:
-        features = {"organizations:on-demand-metrics-extraction": True}
         spec = OnDemandMetricSpec(
             field="count()",
             query="transaction.duration:>1s",
@@ -1400,7 +1388,7 @@ class OrganizationEventsStatsMetricsEnhancedPerformanceEndpointTestWithOnDemandW
         )
 
         self.store_on_demand_metric(1, spec=spec)
-        response = self.do_request(params, features=features)
+        response = self.do_request(params)
 
         assert response.status_code == 200, response.content
         meta = response.data["meta"]