feat(dashboards): Default widget split decision to errors (#74723)

In the case that neither side has data, or both sides have data, force
the dataset to errors and record the reason in the source field. This
way we can ensure the split occurs and also warn the user that they may
need to select a dataset.

Note: To avoid changing behaviour for existing on-demand users by
forcing the split to use errors, I feature flagged the default scenario
so without the feature flag the default is still discover.

src/sentry/api/bases/organization_events.py

@@ -240,33 +240,39 @@ def handle_on_demand(self, request: Request) -> tuple[bool, MetricSpecType]:
 
  return use_on_demand_metrics, on_demand_metric_type
 
- def get_split_decision(self, has_errors, has_transactions_data):
+ def save_split_decision(self, widget, has_errors, has_transactions_data, organization, user):
  """This can be removed once the discover dataset has been fully split"""
+ source = DashboardDatasetSourcesTypes.INFERRED.value
  if has_errors and not has_transactions_data:
  decision = DashboardWidgetTypes.ERROR_EVENTS
+ sentry_sdk.set_tag("discover.split_reason", "query_result")
  elif not has_errors and has_transactions_data:
  decision = DashboardWidgetTypes.TRANSACTION_LIKE
- elif has_errors and has_transactions_data:
- decision = DashboardWidgetTypes.DISCOVER
+ sentry_sdk.set_tag("discover.split_reason", "query_result")
  else:
- # In the case that neither side has data, we do not need to split this yet and can make multiple queries to check each time.
- # This will help newly created widgets or infrequent count widgets that shouldn't be prematurely assigned a side.
- decision = None
- sentry_sdk.set_tag("split_decision", decision)
- return decision
+ if features.has(
+ "organizations:performance-discover-dataset-selector", organization, actor=user
+ ):
+ # In the case that neither side has data, or both sides have data, default to errors.
+ decision = DashboardWidgetTypes.ERROR_EVENTS
+ source = DashboardDatasetSourcesTypes.FORCED.value
+ sentry_sdk.set_tag("discover.split_reason", "default")
+ else:
+ # This branch can be deleted once the feature flag for the discover split is removed
+ if has_errors and has_transactions_data:
+ decision = DashboardWidgetTypes.DISCOVER
+ else:
+ # In the case that neither side has data, we do not need to split this yet and can make multiple queries to check each time.
+ # This will help newly created widgets or infrequent count widgets that shouldn't be prematurely assigned a side.
+ decision = None
 
- def save_split_decision(self, widget, has_errors, has_transactions_data):
- """This can be removed once the discover dataset has been fully split"""
- new_discover_widget_split = self.get_split_decision(has_errors, has_transactions_data)
- if (
- new_discover_widget_split is not None
- and widget.discover_widget_split != new_discover_widget_split
- ):
- widget.discover_widget_split = new_discover_widget_split
- widget.dataset_source = DashboardDatasetSourcesTypes.INFERRED.value
+ sentry_sdk.set_tag("discover.split_decision", decision)
+ if decision is not None and widget.discover_widget_split != decision:
+ widget.discover_widget_split = decision
+ widget.dataset_source = source
  widget.save()
 
- return new_discover_widget_split
+ return decision
 
  def save_discover_saved_query_split_decision(
  self, query, dataset_inferred_from_query, has_errors, has_transactions_data

src/sentry/api/endpoints/organization_events.py

@@ -438,7 +438,7 @@ def _dashboards_data_fn(scoped_dataset, offset, limit, scoped_query, dashboard_w
  error_results = None
 
  original_results = _data_fn(scoped_dataset, offset, limit, scoped_query)
- if original_results.get("data"):
+ if original_results.get("data") is not None:
  dataset_meta = original_results.get("meta", {})
  else:
  dataset_meta = list(original_results.values())[0].get("data").get("meta", {})
@@ -460,7 +460,9 @@ def _dashboards_data_fn(scoped_dataset, offset, limit, scoped_query, dashboard_w
  transaction_results = _data_fn(discover, offset, limit, transactions_only_query)
  has_transactions = len(transaction_results["data"]) > 0
 
- decision = self.save_split_decision(widget, has_errors, has_transactions)
+ decision = self.save_split_decision(
+ widget, has_errors, has_transactions, organization, request.user
+ )
 
  if decision == DashboardWidgetTypes.DISCOVER:
  return _data_fn(discover, offset, limit, scoped_query)

src/sentry/api/endpoints/organization_events_stats.py

@@ -421,7 +421,9 @@ def fn(
  )
  has_transactions = self.check_if_results_have_data(transaction_results)
 
- decision = self.save_split_decision(widget, has_errors, has_transactions)
+ decision = self.save_split_decision(
+ widget, has_errors, has_transactions, organization, request.user
+ )
 
  if decision == DashboardWidgetTypes.DISCOVER:
  # The user needs to be warned to split in this case.

tests/snuba/api/endpoints/test_organization_events_mep.py

@@ -3725,20 +3725,23 @@ def test_split_decision_for_ambiguous_widget_without_data(self):
 
  response = self.do_request(
  {
- "field": ["count()", "transaction.name", "error.type"],
+ "field": ["count()", "transaction.op", "error.type"],
  "query": "",
  "dataset": "metricsEnhanced",
  "per_page": 50,
  "dashboardWidgetId": widget.id,
- }
+ },
+ features={"organizations:performance-discover-dataset-selector": True},
  )
 
  assert response.status_code == 200, response.content
- assert response.data.get("meta").get("discoverSplitDecision") is None
+ assert response.data.get("meta").get(
+ "discoverSplitDecision"
+ ) == DashboardWidgetTypes.get_type_name(DashboardWidgetTypes.ERROR_EVENTS)
 
  widget.refresh_from_db()
- assert widget.discover_widget_split is None
- assert widget.dataset_source == DatasetSourcesTypes.UNKNOWN.value
+ assert widget.discover_widget_split == DashboardWidgetTypes.ERROR_EVENTS
+ assert widget.dataset_source == DatasetSourcesTypes.FORCED.value
 
  def test_split_decision_for_ambiguous_widget_with_data(self):
  # Store a transaction
@@ -3775,14 +3778,18 @@ def test_split_decision_for_ambiguous_widget_with_data(self):
  "dataset": "metricsEnhanced",
  "per_page": 50,
  "dashboardWidgetId": widget.id,
- }
+ },
+ features={"organizations:performance-discover-dataset-selector": True},
  )
 
  assert response.status_code == 200, response.content
- assert response.data.get("meta").get("discoverSplitDecision") is None
+ assert response.data.get("meta").get(
+ "discoverSplitDecision"
+ ) == DashboardWidgetTypes.get_type_name(DashboardWidgetTypes.ERROR_EVENTS)
 
  widget.refresh_from_db()
- assert widget.discover_widget_split is DashboardWidgetTypes.DISCOVER
+ assert widget.discover_widget_split == DashboardWidgetTypes.ERROR_EVENTS
+ assert widget.dataset_source == DatasetSourcesTypes.FORCED.value
 
 
 class OrganizationEventsMetricsEnhancedPerformanceEndpointTestWithMetricLayer(

tests/snuba/api/endpoints/test_organization_events_stats_mep.py

@@ -8,6 +8,7 @@
 from django.urls import reverse
 from rest_framework.response import Response
 
+from sentry.discover.models import DatasetSourcesTypes
 from sentry.models.dashboard_widget import DashboardWidget, DashboardWidgetTypes
 from sentry.models.environment import Environment
 from sentry.sentry_metrics.use_case_id_registry import UseCaseID
@@ -791,6 +792,7 @@ def test_split_decision_for_errors_widget(self):
 
  widget.refresh_from_db()
  assert widget.discover_widget_split == DashboardWidgetTypes.ERROR_EVENTS
+ assert widget.dataset_source == DatasetSourcesTypes.INFERRED.value
 
  def test_split_decision_for_transactions_widget(self):
  self.store_transaction_metric(
@@ -820,6 +822,7 @@ def test_split_decision_for_transactions_widget(self):
 
  widget.refresh_from_db()
  assert widget.discover_widget_split == DashboardWidgetTypes.TRANSACTION_LIKE
+ assert widget.dataset_source == DatasetSourcesTypes.INFERRED.value
 
  def test_split_decision_for_top_events_errors_widget(self):
  error_data = load_data("python", timestamp=before_now(minutes=1))
@@ -860,6 +863,7 @@ def test_split_decision_for_top_events_errors_widget(self):
 
  widget.refresh_from_db()
  assert widget.discover_widget_split == DashboardWidgetTypes.ERROR_EVENTS
+ assert widget.dataset_source == DatasetSourcesTypes.INFERRED.value
 
  def test_split_decision_for_top_events_transactions_widget(self):
  self.store_transaction_metric(
@@ -896,6 +900,7 @@ def test_split_decision_for_top_events_transactions_widget(self):
 
  widget.refresh_from_db()
  assert widget.discover_widget_split == DashboardWidgetTypes.TRANSACTION_LIKE
+ assert widget.dataset_source == DatasetSourcesTypes.INFERRED.value
 
  def test_split_decision_for_ambiguous_widget_without_data(self):
  _, widget, __ = create_widget(
@@ -913,14 +918,18 @@ def test_split_decision_for_ambiguous_widget_without_data(self):
  "dataset": "metricsEnhanced",
  "per_page": 50,
  "dashboardWidgetId": widget.id,
- }
+ },
+ features={"organizations:performance-discover-dataset-selector": True},
  )
 
  assert response.status_code == 200, response.content
- assert response.data.get("meta").get("discoverSplitDecision") is None
+ assert response.data.get("meta").get(
+ "discoverSplitDecision"
+ ) == DashboardWidgetTypes.get_type_name(DashboardWidgetTypes.ERROR_EVENTS)
 
  widget.refresh_from_db()
- assert widget.discover_widget_split is None
+ assert widget.discover_widget_split == DashboardWidgetTypes.ERROR_EVENTS
+ assert widget.dataset_source == DatasetSourcesTypes.FORCED.value
 
 
 class OrganizationEventsStatsMetricsEnhancedPerformanceEndpointTestWithMetricLayer(