diff --git a/src/sentry/api/bases/organization_events.py b/src/sentry/api/bases/organization_events.py index ee9302e6c304c4..b2608373a9a7ce 100644 --- a/src/sentry/api/bases/organization_events.py +++ b/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 diff --git a/src/sentry/api/endpoints/organization_events.py b/src/sentry/api/endpoints/organization_events.py index 6b14bbe8c1fe6e..c997f1021bd354 100644 --- a/src/sentry/api/endpoints/organization_events.py +++ b/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) diff --git a/src/sentry/api/endpoints/organization_events_stats.py b/src/sentry/api/endpoints/organization_events_stats.py index 5fb5fb247e3dc2..a9af5af442fb0f 100644 --- a/src/sentry/api/endpoints/organization_events_stats.py +++ b/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. diff --git a/tests/snuba/api/endpoints/test_organization_events_mep.py b/tests/snuba/api/endpoints/test_organization_events_mep.py index 398856c353d4aa..8e644620b2a776 100644 --- a/tests/snuba/api/endpoints/test_organization_events_mep.py +++ b/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( diff --git a/tests/snuba/api/endpoints/test_organization_events_stats_mep.py b/tests/snuba/api/endpoints/test_organization_events_stats_mep.py index 2aab9c89017ce6..048cf1142a9a9c 100644 --- a/tests/snuba/api/endpoints/test_organization_events_stats_mep.py +++ b/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(