Browse Source

mep: Introduce dataset parameter (#33892)

- This introduces a dataset parameter, so that we can eventually
  explicitly state we want to back the results with metrics and error
  for incompatible queries instead of the implicit fallback we have
  today
William Mak 2 years ago
parent
commit
4cdb8a4f45

+ 14 - 1
src/sentry/api/bases/organization_events.py

@@ -21,13 +21,20 @@ from sentry.models.group import Group
 from sentry.search.events.constants import TIMEOUT_ERROR_MESSAGE
 from sentry.search.events.constants import TIMEOUT_ERROR_MESSAGE
 from sentry.search.events.fields import get_function_alias
 from sentry.search.events.fields import get_function_alias
 from sentry.search.events.filter import get_filter
 from sentry.search.events.filter import get_filter
-from sentry.snuba import discover
+from sentry.snuba import discover, metrics_enhanced_performance
 from sentry.utils import snuba
 from sentry.utils import snuba
 from sentry.utils.cursors import Cursor
 from sentry.utils.cursors import Cursor
 from sentry.utils.dates import get_interval_from_range, get_rollup_from_request, parse_stats_period
 from sentry.utils.dates import get_interval_from_range, get_rollup_from_request, parse_stats_period
 from sentry.utils.http import absolute_uri
 from sentry.utils.http import absolute_uri
 from sentry.utils.snuba import MAX_FIELDS, SnubaTSResult
 from sentry.utils.snuba import MAX_FIELDS, SnubaTSResult
 
 
+# Doesn't map 1:1 with real datasets, but rather what we present to users
+# ie. metricsEnhanced is not a real dataset
+DATASET_OPTIONS = {
+    "discover": discover,
+    "metricsEnhanced": metrics_enhanced_performance,
+}
+
 
 
 def resolve_axis_column(column: str, index: int = 0) -> str:
 def resolve_axis_column(column: str, index: int = 0) -> str:
     return cast(
     return cast(
@@ -60,6 +67,12 @@ class OrganizationEventsEndpointBase(OrganizationEndpoint):  # type: ignore
 
 
         return [team.id for team in teams]
         return [team.id for team in teams]
 
 
+    def get_dataset(self, request: Request) -> Any:
+        dataset_label = request.GET.get("dataset", "discover")
+        if dataset_label not in DATASET_OPTIONS:
+            raise ParseError(detail=f"dataset must be one of: {', '.join(DATASET_OPTIONS.keys())}")
+        return DATASET_OPTIONS[dataset_label]
+
     def get_snuba_params(
     def get_snuba_params(
         self, request: Request, organization: Organization, check_global_views: bool = True
         self, request: Request, organization: Organization, check_global_views: bool = True
     ) -> Dict[str, Any]:
     ) -> Dict[str, Any]:

+ 9 - 2
src/sentry/api/endpoints/organization_events.py

@@ -59,7 +59,15 @@ class OrganizationEventsV2Endpoint(OrganizationEventsV2EndpointBase):
         performance_dry_run_mep = features.has(
         performance_dry_run_mep = features.has(
             "organizations:performance-dry-run-mep", organization=organization, actor=request.user
             "organizations:performance-dry-run-mep", organization=organization, actor=request.user
         )
         )
-        metrics_enhanced = request.GET.get("metricsEnhanced") == "1" and performance_use_metrics
+
+        # This param will be deprecated in favour of dataset
+        if "metricsEnhanced" in request.GET:
+            metrics_enhanced = request.GET.get("metricsEnhanced") == "1" and performance_use_metrics
+            dataset = discover if not metrics_enhanced else metrics_enhanced_performance
+        else:
+            dataset = self.get_dataset(request) if performance_use_metrics else discover
+            metrics_enhanced = dataset != discover
+
         sentry_sdk.set_tag("performance.metrics_enhanced", metrics_enhanced)
         sentry_sdk.set_tag("performance.metrics_enhanced", metrics_enhanced)
         allow_metric_aggregates = request.GET.get("preventMetricAggregates") != "1"
         allow_metric_aggregates = request.GET.get("preventMetricAggregates") != "1"
         referrer = (
         referrer = (
@@ -67,7 +75,6 @@ class OrganizationEventsV2Endpoint(OrganizationEventsV2EndpointBase):
         )
         )
 
 
         def data_fn(offset, limit):
         def data_fn(offset, limit):
-            dataset = discover if not metrics_enhanced else metrics_enhanced_performance
             query_details = {
             query_details = {
                 "selected_columns": self.get_field_list(organization, request),
                 "selected_columns": self.get_field_list(organization, request),
                 "query": request.GET.get("query"),
                 "query": request.GET.get("query"),

+ 10 - 2
src/sentry/api/endpoints/organization_events_stats.py

@@ -144,7 +144,16 @@ class OrganizationEventsStatsEndpoint(OrganizationEventsV2EndpointBase):  # type
                 "organizations:performance-dry-run-mep", False
                 "organizations:performance-dry-run-mep", False
             )
             )
 
 
-            metrics_enhanced = request.GET.get("metricsEnhanced") == "1" and performance_use_metrics
+            # This param will be deprecated in favour of dataset
+            if "metricsEnhanced" in request.GET:
+                metrics_enhanced = (
+                    request.GET.get("metricsEnhanced") == "1" and performance_use_metrics
+                )
+                dataset = discover if not metrics_enhanced else metrics_enhanced_performance
+            else:
+                dataset = self.get_dataset(request) if performance_use_metrics else discover
+                metrics_enhanced = dataset != discover
+
             allow_metric_aggregates = request.GET.get("preventMetricAggregates") != "1"
             allow_metric_aggregates = request.GET.get("preventMetricAggregates") != "1"
             sentry_sdk.set_tag("performance.metrics_enhanced", metrics_enhanced)
             sentry_sdk.set_tag("performance.metrics_enhanced", metrics_enhanced)
 
 
@@ -172,7 +181,6 @@ class OrganizationEventsStatsEndpoint(OrganizationEventsV2EndpointBase):  # type
                     zerofill_results=zerofill_results,
                     zerofill_results=zerofill_results,
                     include_other=include_other,
                     include_other=include_other,
                 )
                 )
-            dataset = discover if not metrics_enhanced else metrics_enhanced_performance
             query_details = {
             query_details = {
                 "selected_columns": query_columns,
                 "selected_columns": query_columns,
                 "query": query,
                 "query": query,

+ 33 - 22
tests/snuba/api/endpoints/test_organization_events_v2.py

@@ -4994,12 +4994,23 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
     def test_no_projects(self):
     def test_no_projects(self):
         response = self.do_request(
         response = self.do_request(
             {
             {
-                "metricsEnhanced": "1",
+                "dataset": "metricsEnhanced",
             }
             }
         )
         )
 
 
         assert response.status_code == 200, response.content
         assert response.status_code == 200, response.content
 
 
+    def test_invalid_dataset(self):
+        response = self.do_request(
+            {
+                "dataset": "aFakeDataset",
+                "project": self.project.id,
+            }
+        )
+
+        assert response.status_code == 400, response.content
+        assert response.data["detail"] == "dataset must be one of: discover, metricsEnhanced"
+
     def test_out_of_retention(self):
     def test_out_of_retention(self):
         self.create_project()
         self.create_project()
         with self.options({"system.event-retention-days": 10}):
         with self.options({"system.event-retention-days": 10}):
@@ -5009,7 +5020,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                 "query": "event.type:transaction",
                 "query": "event.type:transaction",
                 "start": iso_format(before_now(days=20)),
                 "start": iso_format(before_now(days=20)),
                 "end": iso_format(before_now(days=15)),
                 "end": iso_format(before_now(days=15)),
-                "metricsEnhanced": "1",
+                "dataset": "metricsEnhanced",
             }
             }
             response = self.do_request(query)
             response = self.do_request(query)
         assert response.status_code == 400, response.content
         assert response.status_code == 400, response.content
@@ -5021,7 +5032,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                 "field": ["epm()"],
                 "field": ["epm()"],
                 "query": "hi \n there",
                 "query": "hi \n there",
                 "project": self.project.id,
                 "project": self.project.id,
-                "metricsEnhanced": "1",
+                "dataset": "metricsEnhanced",
             }
             }
         )
         )
         assert response.status_code == 400, response.content
         assert response.status_code == 400, response.content
@@ -5041,7 +5052,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
             {
             {
                 "field": ["project.name", "environment", "epm()"],
                 "field": ["project.name", "environment", "epm()"],
                 "query": "event.type:transaction",
                 "query": "event.type:transaction",
-                "metricsEnhanced": "1",
+                "dataset": "metricsEnhanced",
                 "per_page": 50,
                 "per_page": 50,
             }
             }
         )
         )
@@ -5071,7 +5082,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
             {
             {
                 "field": ["title", "p50()"],
                 "field": ["title", "p50()"],
                 "query": "event.type:transaction",
                 "query": "event.type:transaction",
-                "metricsEnhanced": "1",
+                "dataset": "metricsEnhanced",
                 "per_page": 50,
                 "per_page": 50,
             }
             }
         )
         )
@@ -5104,7 +5115,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
             {
             {
                 "field": ["transaction", "project", "p50(transaction.duration)"],
                 "field": ["transaction", "project", "p50(transaction.duration)"],
                 "query": "event.type:transaction p50(transaction.duration):<50",
                 "query": "event.type:transaction p50(transaction.duration):<50",
-                "metricsEnhanced": "1",
+                "dataset": "metricsEnhanced",
                 "per_page": 50,
                 "per_page": 50,
             }
             }
         )
         )
@@ -5138,7 +5149,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
             {
             {
                 "field": ["transaction", "project", "p50(transaction.duration)"],
                 "field": ["transaction", "project", "p50(transaction.duration)"],
                 "query": "event.type:transaction p50(transaction.duration):<50",
                 "query": "event.type:transaction p50(transaction.duration):<50",
-                "metricsEnhanced": "1",
+                "dataset": "metricsEnhanced",
                 "preventMetricAggregates": "1",
                 "preventMetricAggregates": "1",
                 "per_page": 50,
                 "per_page": 50,
             }
             }
@@ -5169,7 +5180,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
             {
             {
                 "field": ["transaction", "project", "p50(transaction.duration)"],
                 "field": ["transaction", "project", "p50(transaction.duration)"],
                 "query": "event.type:transaction p75(transaction.duration):<50",
                 "query": "event.type:transaction p75(transaction.duration):<50",
-                "metricsEnhanced": "1",
+                "dataset": "metricsEnhanced",
                 "per_page": 50,
                 "per_page": 50,
             }
             }
         )
         )
@@ -5198,7 +5209,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
             {
             {
                 "field": ["test", "p50(transaction.duration)"],
                 "field": ["test", "p50(transaction.duration)"],
                 "query": "event.type:transaction",
                 "query": "event.type:transaction",
-                "metricsEnhanced": "1",
+                "dataset": "metricsEnhanced",
                 "per_page": 50,
                 "per_page": 50,
             }
             }
         )
         )
@@ -5262,7 +5273,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                     "user_misery()",
                     "user_misery()",
                 ],
                 ],
                 "query": "event.type:transaction",
                 "query": "event.type:transaction",
-                "metricsEnhanced": "1",
+                "dataset": "metricsEnhanced",
                 "per_page": 50,
                 "per_page": 50,
             }
             }
         )
         )
@@ -5311,7 +5322,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                 "p95()",
                 "p95()",
             ],
             ],
             "per_page": 50,
             "per_page": 50,
-            "metricsEnhanced": "1",
+            "dataset": "metricsEnhanced",
         }
         }
         response = self.do_request(query)
         response = self.do_request(query)
 
 
@@ -5366,7 +5377,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                 "p95()",
                 "p95()",
             ],
             ],
             "per_page": 50,
             "per_page": 50,
-            "metricsEnhanced": "1",
+            "dataset": "metricsEnhanced",
         }
         }
 
 
         query["orderby"] = ["team_key_transaction", "p95()"]
         query["orderby"] = ["team_key_transaction", "p95()"]
@@ -5400,7 +5411,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                 "p95()",
                 "p95()",
             ],
             ],
             "per_page": 50,
             "per_page": 50,
-            "metricsEnhanced": "1",
+            "dataset": "metricsEnhanced",
         }
         }
 
 
         query["orderby"] = ["team_key_transaction", "p95()"]
         query["orderby"] = ["team_key_transaction", "p95()"]
@@ -5456,7 +5467,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                 "p95()",
                 "p95()",
             ],
             ],
             "per_page": 50,
             "per_page": 50,
-            "metricsEnhanced": "1",
+            "dataset": "metricsEnhanced",
         }
         }
 
 
         # test ascending order
         # test ascending order
@@ -5534,7 +5545,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                 "p95()",
                 "p95()",
             ],
             ],
             "per_page": 50,
             "per_page": 50,
-            "metricsEnhanced": "1",
+            "dataset": "metricsEnhanced",
         }
         }
 
 
         # key transactions
         # key transactions
@@ -5642,7 +5653,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                     "failure_rate()",
                     "failure_rate()",
                     "p95()",
                     "p95()",
                 ],
                 ],
-                "metricsEnhanced": "1",
+                "dataset": "metricsEnhanced",
                 "per_page": 50,
                 "per_page": 50,
             }
             }
 
 
@@ -5701,7 +5712,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                     "count_web_vitals(measurements.cls, good)",
                     "count_web_vitals(measurements.cls, good)",
                 ],
                 ],
                 "query": "event.type:transaction",
                 "query": "event.type:transaction",
-                "metricsEnhanced": "1",
+                "dataset": "metricsEnhanced",
                 "per_page": 50,
                 "per_page": 50,
             }
             }
         )
         )
@@ -5735,7 +5746,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
             {
             {
                 "field": ["transaction", "count_web_vitals(measurements.lcp, poor)"],
                 "field": ["transaction", "count_web_vitals(measurements.lcp, poor)"],
                 "query": "event.type:transaction",
                 "query": "event.type:transaction",
-                "metricsEnhanced": "1",
+                "dataset": "metricsEnhanced",
                 "per_page": 50,
                 "per_page": 50,
             }
             }
         )
         )
@@ -5755,7 +5766,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                 "count_web_vitals(measurements.foo, poor)",
                 "count_web_vitals(measurements.foo, poor)",
             ],
             ],
             "project": [self.project.id],
             "project": [self.project.id],
-            "metricsEnhanced": "1",
+            "dataset": "metricsEnhanced",
         }
         }
         response = self.do_request(query)
         response = self.do_request(query)
         assert response.status_code == 400, response.content
         assert response.status_code == 400, response.content
@@ -5765,7 +5776,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                 "count_web_vitals(tags[lcp], poor)",
                 "count_web_vitals(tags[lcp], poor)",
             ],
             ],
             "project": [self.project.id],
             "project": [self.project.id],
-            "metricsEnhanced": "1",
+            "dataset": "metricsEnhanced",
         }
         }
         response = self.do_request(query)
         response = self.do_request(query)
         assert response.status_code == 400, response.content
         assert response.status_code == 400, response.content
@@ -5775,7 +5786,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                 "count_web_vitals(transaction.duration, poor)",
                 "count_web_vitals(transaction.duration, poor)",
             ],
             ],
             "project": [self.project.id],
             "project": [self.project.id],
-            "metricsEnhanced": "1",
+            "dataset": "metricsEnhanced",
         }
         }
         response = self.do_request(query)
         response = self.do_request(query)
         assert response.status_code == 400, response.content
         assert response.status_code == 400, response.content
@@ -5785,7 +5796,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
                 "count_web_vitals(measurements.lcp, bad)",
                 "count_web_vitals(measurements.lcp, bad)",
             ],
             ],
             "project": [self.project.id],
             "project": [self.project.id],
-            "metricsEnhanced": "1",
+            "dataset": "metricsEnhanced",
         }
         }
         response = self.do_request(query)
         response = self.do_request(query)
         assert response.status_code == 400, response.content
         assert response.status_code == 400, response.content