From 2d0a52e43f0a2c3bda8d493245fcf98eb27c2be3 Mon Sep 17 00:00:00 2001 From: nikkikapadia Date: Wed, 29 Oct 2025 15:28:01 -0400 Subject: [PATCH 1/6] wip post + put validation for extrapolation mode --- .../endpoints/organization_alert_rule_details.py | 11 +++++++++++ .../endpoints/organization_alert_rule_index.py | 4 ++++ src/sentry/incidents/metric_issue_detector.py | 16 +++++++++++++++- 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_details.py b/src/sentry/incidents/endpoints/organization_alert_rule_details.py index a9b70eae98c1ed..899ebb5bdebe83 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_details.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_details.py @@ -49,6 +49,7 @@ from sentry.models.rulesnooze import RuleSnooze from sentry.sentry_apps.services.app import app_service from sentry.sentry_apps.utils.errors import SentryAppBaseError +from sentry.snuba.models import ExtrapolationMode from sentry.users.services.user.service import user_service from sentry.workflow_engine.migration_helpers.alert_rule import dual_delete_migrated_alert_rule from sentry.workflow_engine.models import Detector @@ -121,6 +122,16 @@ def update_alert_rule( partial=True, ) if validator.is_valid(): + if data.get("extrapolation_mode"): + if ( + data.get("extrapolation_mode") == ExtrapolationMode.SERVER_WEIGHTED.name.lower() + and alert_rule.snuba_query.extrapolation_mode + != ExtrapolationMode.SERVER_WEIGHTED.name.lower() + ): + raise serializers.ValidationError( + "server_weighted extrapolation mode is not a valid mode for this alert type." + ) + try: trigger_sentry_app_action_creators_for_incidents(validator.validated_data) except SentryAppBaseError as e: diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_index.py b/src/sentry/incidents/endpoints/organization_alert_rule_index.py index bc838eef5eefb8..b73c4b1017474a 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_index.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_index.py @@ -73,6 +73,7 @@ from sentry.sentry_apps.services.app import app_service from sentry.sentry_apps.utils.errors import SentryAppBaseError from sentry.snuba.dataset import Dataset +from sentry.snuba.models import ExtrapolationMode from sentry.uptime.types import ( DATA_SOURCE_UPTIME_SUBSCRIPTION, GROUP_TYPE_UPTIME_DOMAIN_CHECK_FAILURE, @@ -100,6 +101,9 @@ def create_metric_alert( "Creation of transaction-based alerts is disabled, as we migrate to the span dataset. Create span-based alerts (dataset: events_analytics_platform) with the is_transaction:true filter instead." ) + if data.get("extrapolation_mode") == ExtrapolationMode.SERVER_WEIGHTED.name.lower(): + raise ValidationError("server_weighted extrapolation mode is not supported for new alerts.") + if project: data["projects"] = [project.slug] diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index 243302ed9148c6..00f22755fcde9f 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -11,7 +11,12 @@ from sentry.seer.anomaly_detection.store_data_workflow_engine import send_new_detector_data from sentry.snuba.dataset import Dataset from sentry.snuba.metrics.extraction import should_use_on_demand_metrics -from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType +from sentry.snuba.models import ( + ExtrapolationMode, + QuerySubscription, + SnubaQuery, + SnubaQueryEventType, +) from sentry.snuba.snuba_query_validator import SnubaQueryValidator from sentry.snuba.subscriptions import update_snuba_query from sentry.tasks.relay import schedule_invalidate_project_config @@ -163,6 +168,12 @@ def _validate_transaction_dataset_deprecation(self, dataset: Dataset) -> None: "Creation of transaction-based alerts is disabled, as we migrate to the span dataset. Create span-based alerts (dataset: events_analytics_platform) with the is_transaction:true filter instead." ) + def _validate_extrapolation_mode(self, extrapolation_mode: str) -> None: + if extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.name.lower(): + raise serializers.ValidationError( + "server_weighted extrapolation mode is not supported for new alerts." + ) + def get_quota(self) -> DetectorQuota: organization = self.context.get("organization") request = self.context.get("request") @@ -276,6 +287,9 @@ def create(self, validated_data: dict[str, Any]): for validated_data_source in validated_data["data_sources"]: self._validate_transaction_dataset_deprecation(validated_data_source.get("dataset")) + if "extrapolation_mode" in validated_data: + self._validate_extrapolation_mode(validated_data.get("extrapolation_mode")) + detector = super().create(validated_data) if detector.config.get("detection_type") == AlertRuleDetectionType.DYNAMIC.value: From 0842aae283bda1ce7e281193d71aaeb51700e7ca Mon Sep 17 00:00:00 2001 From: nikkikapadia Date: Fri, 7 Nov 2025 11:05:51 -0500 Subject: [PATCH 2/6] add validation for new detectors --- src/sentry/incidents/metric_issue_detector.py | 35 +++++++++++++++++-- src/sentry/snuba/snuba_query_validator.py | 2 ++ src/sentry/snuba/subscriptions.py | 11 +++++- 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index 49c7774f53e1ee..2e6e1afcd0ad14 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -212,6 +212,31 @@ def is_editing_transaction_dataset( return True return False + def is_invalid_extrapolation_mode( + self, snuba_query: SnubaQuery, data_source: SnubaQueryDataSourceType + ) -> bool: + if data_source.get("dataset") == Dataset.EventsAnalyticsPlatform: + new_extrapolation_mode = data_source.get( + "extrapolation_mode", snuba_query.extrapolation_mode + ) + old_extrapolation_mode = snuba_query.extrapolation_mode + if ( + new_extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.value + and old_extrapolation_mode != ExtrapolationMode.SERVER_WEIGHTED.value + ): + return True + if ( + new_extrapolation_mode == ExtrapolationMode.NONE.value + and old_extrapolation_mode != ExtrapolationMode.NONE.value + ): + return True + if ( + new_extrapolation_mode == ExtrapolationMode.CLIENT_AND_SERVER_WEIGHTED.value + or new_extrapolation_mode == ExtrapolationMode.UNKNOWN.value + ): + return False + return False + def update_data_source(self, instance: Detector, data_source: SnubaQueryDataSourceType): try: source_instance = DataSource.objects.get(detector=instance) @@ -235,6 +260,9 @@ def update_data_source(self, instance: Detector, data_source: SnubaQueryDataSour "Updates to transaction-based alerts is disabled, as we migrate to the span dataset. Create span-based alerts (dataset: events_analytics_platform) with the is_transaction:true filter instead." ) + if self.is_invalid_extrapolation_mode(snuba_query, data_source): + raise serializers.ValidationError("Invalid extrapolation mode") + update_snuba_query( snuba_query=snuba_query, query_type=data_source.get("query_type", snuba_query.type), @@ -245,6 +273,9 @@ def update_data_source(self, instance: Detector, data_source: SnubaQueryDataSour resolution=timedelta(seconds=data_source.get("resolution", snuba_query.resolution)), environment=data_source.get("environment", snuba_query.environment), event_types=data_source.get("event_types", [event_type for event_type in event_types]), + extrapolation_mode=data_source.get( + "extrapolation_mode", snuba_query.extrapolation_mode + ), ) def update(self, instance: Detector, validated_data: dict[str, Any]): @@ -278,9 +309,7 @@ def create(self, validated_data: dict[str, Any]): if "data_sources" in validated_data: for validated_data_source in validated_data["data_sources"]: self._validate_transaction_dataset_deprecation(validated_data_source.get("dataset")) - - if "extrapolation_mode" in validated_data: - self._validate_extrapolation_mode(validated_data.get("extrapolation_mode")) + self._validate_extrapolation_mode(validated_data_source.get("extrapolation_mode")) detector = super().create(validated_data) diff --git a/src/sentry/snuba/snuba_query_validator.py b/src/sentry/snuba/snuba_query_validator.py index 10b8fa8db896e8..f0077489a90b8c 100644 --- a/src/sentry/snuba/snuba_query_validator.py +++ b/src/sentry/snuba/snuba_query_validator.py @@ -86,6 +86,7 @@ class SnubaQueryValidator(BaseDataSourceValidator[QuerySubscription]): required=False, allow_empty=False, ) + extrapolation_mode = serializers.IntegerField(required=False, allow_null=True) class Meta: model = QuerySubscription @@ -412,6 +413,7 @@ def create_source(self, validated_data) -> QuerySubscription: environment=validated_data["environment"], event_types=validated_data["event_types"], group_by=validated_data.get("group_by"), + extrapolation_mode=validated_data.get("extrapolation_mode"), ) return create_snuba_subscription( project=self.context["project"], diff --git a/src/sentry/snuba/subscriptions.py b/src/sentry/snuba/subscriptions.py index a31069f9b46ce4..5389d89763b343 100644 --- a/src/sentry/snuba/subscriptions.py +++ b/src/sentry/snuba/subscriptions.py @@ -7,7 +7,12 @@ from sentry.models.environment import Environment from sentry.models.project import Project from sentry.snuba.dataset import Dataset -from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType +from sentry.snuba.models import ( + ExtrapolationMode, + QuerySubscription, + SnubaQuery, + SnubaQueryEventType, +) from sentry.snuba.tasks import ( create_subscription_in_snuba, delete_subscription_from_snuba, @@ -27,6 +32,7 @@ def create_snuba_query( environment: Environment | None, event_types: Collection[SnubaQueryEventType.EventType] = (), group_by: Sequence[str] | None = None, + extrapolation_mode: ExtrapolationMode | None = None, ): """ Constructs a SnubaQuery which is the postgres representation of a query in snuba @@ -52,6 +58,7 @@ def create_snuba_query( resolution=int(resolution.total_seconds()), environment=environment, group_by=group_by, + extrapolation_mode=extrapolation_mode, ) if not event_types: if dataset == Dataset.Events: @@ -78,6 +85,7 @@ def update_snuba_query( resolution, environment, event_types, + extrapolation_mode, ): """ Updates a SnubaQuery. Triggers updates to any related QuerySubscriptions. @@ -118,6 +126,7 @@ def update_snuba_query( time_window=int(time_window.total_seconds()), resolution=int(resolution.total_seconds()), environment=environment, + extrapolation_mode=extrapolation_mode, ) if new_event_types: SnubaQueryEventType.objects.bulk_create( From b037625579c0ed0a8b7a7e1eb8664d45fba0e735 Mon Sep 17 00:00:00 2001 From: nikkikapadia Date: Fri, 7 Nov 2025 12:34:52 -0500 Subject: [PATCH 3/6] make tests and edit code --- .../organization_alert_rule_details.py | 39 +++++++--- src/sentry/incidents/metric_issue_detector.py | 4 +- src/sentry/snuba/snuba_query_validator.py | 7 +- src/sentry/snuba/subscriptions.py | 14 +++- .../test_organization_alert_rule_details.py | 18 +++++ .../test_organization_alert_rule_index.py | 9 +++ .../endpoints/validators/test_validators.py | 72 +++++++++++++++++++ 7 files changed, 148 insertions(+), 15 deletions(-) diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_details.py b/src/sentry/incidents/endpoints/organization_alert_rule_details.py index 899ebb5bdebe83..24a5aef67fab0d 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_details.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_details.py @@ -49,6 +49,7 @@ from sentry.models.rulesnooze import RuleSnooze from sentry.sentry_apps.services.app import app_service from sentry.sentry_apps.utils.errors import SentryAppBaseError +from sentry.snuba.dataset import Dataset from sentry.snuba.models import ExtrapolationMode from sentry.users.services.user.service import user_service from sentry.workflow_engine.migration_helpers.alert_rule import dual_delete_migrated_alert_rule @@ -103,6 +104,25 @@ def fetch_alert_rule( return Response(serialized_rule) +def _is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode) -> bool: + if ( + new_extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.name.lower() + and old_extrapolation_mode != ExtrapolationMode.SERVER_WEIGHTED.name.lower() + ): + return True + if ( + new_extrapolation_mode == ExtrapolationMode.NONE.name.lower() + and old_extrapolation_mode != ExtrapolationMode.NONE.name.lower() + ): + return True + if ( + new_extrapolation_mode == ExtrapolationMode.CLIENT_AND_SERVER_WEIGHTED.name.lower() + or new_extrapolation_mode == ExtrapolationMode.UNKNOWN.name.lower() + ): + return False + return False + + def update_alert_rule( request: Request, organization: Organization, alert_rule: AlertRule ) -> Response: @@ -122,15 +142,16 @@ def update_alert_rule( partial=True, ) if validator.is_valid(): - if data.get("extrapolation_mode"): - if ( - data.get("extrapolation_mode") == ExtrapolationMode.SERVER_WEIGHTED.name.lower() - and alert_rule.snuba_query.extrapolation_mode - != ExtrapolationMode.SERVER_WEIGHTED.name.lower() - ): - raise serializers.ValidationError( - "server_weighted extrapolation mode is not a valid mode for this alert type." - ) + if data.get("dataset") == Dataset.EventsAnalyticsPlatform.value: + if data.get("extrapolation_mode"): + old_extrapolation_mode = ExtrapolationMode( + alert_rule.snuba_query.extrapolation_mode + ).name.lower() + new_extrapolation_mode = data.get("extrapolation_mode", old_extrapolation_mode) + if _is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode): + raise serializers.ValidationError( + "Invalid extrapolation mode for this alert type." + ) try: trigger_sentry_app_action_creators_for_incidents(validator.validated_data) diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index 2e6e1afcd0ad14..6ed11d1f7a6ab8 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -168,7 +168,7 @@ def _validate_transaction_dataset_deprecation(self, dataset: Dataset) -> None: ) def _validate_extrapolation_mode(self, extrapolation_mode: str) -> None: - if extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.name.lower(): + if extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.value: raise serializers.ValidationError( "server_weighted extrapolation mode is not supported for new alerts." ) @@ -261,7 +261,7 @@ def update_data_source(self, instance: Detector, data_source: SnubaQueryDataSour ) if self.is_invalid_extrapolation_mode(snuba_query, data_source): - raise serializers.ValidationError("Invalid extrapolation mode") + raise serializers.ValidationError("Invalid extrapolation mode for this alert type.") update_snuba_query( snuba_query=snuba_query, diff --git a/src/sentry/snuba/snuba_query_validator.py b/src/sentry/snuba/snuba_query_validator.py index f0077489a90b8c..94b1a4ff82cc2f 100644 --- a/src/sentry/snuba/snuba_query_validator.py +++ b/src/sentry/snuba/snuba_query_validator.py @@ -31,6 +31,7 @@ ) from sentry.snuba.metrics.naming_layer.mri import is_mri from sentry.snuba.models import ( + ExtrapolationMode, QuerySubscription, QuerySubscriptionDataSourceHandler, SnubaQuery, @@ -99,6 +100,7 @@ class Meta: "environment", "event_types", "group_by", + "extrapolation_mode", ] data_source_type_handler = QuerySubscriptionDataSourceHandler @@ -403,6 +405,9 @@ def _validate_group_by(self, value: Sequence[str] | None) -> Sequence[str] | Non @override def create_source(self, validated_data) -> QuerySubscription: + extrapolation_mode = validated_data.get("extrapolation_mode") + if extrapolation_mode: + extrapolation_mode = ExtrapolationMode(extrapolation_mode) snuba_query = create_snuba_query( query_type=validated_data["query_type"], dataset=validated_data["dataset"], @@ -413,7 +418,7 @@ def create_source(self, validated_data) -> QuerySubscription: environment=validated_data["environment"], event_types=validated_data["event_types"], group_by=validated_data.get("group_by"), - extrapolation_mode=validated_data.get("extrapolation_mode"), + extrapolation_mode=extrapolation_mode, ) return create_snuba_subscription( project=self.context["project"], diff --git a/src/sentry/snuba/subscriptions.py b/src/sentry/snuba/subscriptions.py index 5389d89763b343..271c593a6d45e1 100644 --- a/src/sentry/snuba/subscriptions.py +++ b/src/sentry/snuba/subscriptions.py @@ -58,7 +58,11 @@ def create_snuba_query( resolution=int(resolution.total_seconds()), environment=environment, group_by=group_by, - extrapolation_mode=extrapolation_mode, + extrapolation_mode=( + extrapolation_mode.value + if extrapolation_mode is not None + else ExtrapolationMode.UNKNOWN.value + ), ) if not event_types: if dataset == Dataset.Events: @@ -85,7 +89,7 @@ def update_snuba_query( resolution, environment, event_types, - extrapolation_mode, + extrapolation_mode=None, ): """ Updates a SnubaQuery. Triggers updates to any related QuerySubscriptions. @@ -126,7 +130,11 @@ def update_snuba_query( time_window=int(time_window.total_seconds()), resolution=int(resolution.total_seconds()), environment=environment, - extrapolation_mode=extrapolation_mode, + extrapolation_mode=( + extrapolation_mode + if extrapolation_mode is not None + else ExtrapolationMode.UNKNOWN.value + ), ) if new_event_types: SnubaQueryEventType.objects.bulk_create( diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py index af5ca3aba6b2b2..df74e72b79f045 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -1416,6 +1416,24 @@ def test_change_name_of_existing_alert(self) -> None: ) assert len(audit_log_entry) == 1 + def test_invalid_extrapolation_mode(self) -> None: + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) + self.login_as(self.user) + alert_rule = self.alert_rule + # We need the IDs to force update instead of create, so we just get the rule using our own API. Like frontend would. + alert_rule_dict = deepcopy(self.alert_rule_dict) + alert_rule_dict["dataset"] = "events_analytics_platform" + alert_rule_dict["alertType"] = "eap_metrics" + alert_rule_dict["extrapolation_mode"] = "server_weighted" + + with self.feature("organizations:incidents"): + resp = self.get_error_response( + self.organization.slug, alert_rule.id, status_code=400, **alert_rule_dict + ) + assert resp.data[0] == "Invalid extrapolation mode for this alert type." + class AlertRuleDetailsSlackPutEndpointTest(AlertRuleDetailsBase): method = "put" diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py index 01003ff6b873cf..97255185232a34 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py @@ -1705,6 +1705,15 @@ def test_generic_metrics_dataset_deprecation_validation(self) -> None: == "Creation of transaction-based alerts is disabled, as we migrate to the span dataset. Create span-based alerts (dataset: events_analytics_platform) with the is_transaction:true filter instead." ) + def test_invalid_extrapolation_mode(self) -> None: + data = deepcopy(self.alert_rule_dict) + data["dataset"] = "events_analytics_platform" + data["alertType"] = "eap_metrics" + data["extrapolation_mode"] = "server_weighted" + with self.feature(["organizations:incidents", "organizations:performance-view"]): + resp = self.get_error_response(self.organization.slug, status_code=400, **data) + assert resp.data[0] == "server_weighted extrapolation mode is not supported for new alerts." + @freeze_time() class AlertRuleCreateEndpointTestCrashRateAlert(AlertRuleIndexBase): diff --git a/tests/sentry/incidents/endpoints/validators/test_validators.py b/tests/sentry/incidents/endpoints/validators/test_validators.py index 562b3d97dbfc8b..94a31fa436befe 100644 --- a/tests/sentry/incidents/endpoints/validators/test_validators.py +++ b/tests/sentry/incidents/endpoints/validators/test_validators.py @@ -26,6 +26,7 @@ ) from sentry.snuba.dataset import Dataset from sentry.snuba.models import ( + ExtrapolationMode, QuerySubscription, QuerySubscriptionDataSourceHandler, SnubaQuery, @@ -617,3 +618,74 @@ def test_transaction_dataset_deprecation_generic_metrics_update(self) -> None: with_feature("organizations:discover-saved-queries-deprecation"), ): update_validator.save() + + def test_invalid_extrapolation_mode_create(self) -> None: + data = { + **self.valid_data, + "dataSources": [ + { + "queryType": SnubaQuery.Type.PERFORMANCE.value, + "dataset": Dataset.EventsAnalyticsPlatform.value, + "query": "test query", + "aggregate": "count()", + "timeWindow": 3600, + "environment": self.environment.name, + "eventTypes": [SnubaQueryEventType.EventType.TRACE_ITEM_SPAN.name.lower()], + "extrapolation_mode": ExtrapolationMode.SERVER_WEIGHTED.value, + }, + ], + } + + validator = MetricIssueDetectorValidator(data=data, context=self.context) + assert validator.is_valid(), validator.errors + with self.assertRaisesMessage( + ValidationError, + expected_message="server_weighted extrapolation mode is not supported for new alerts.", + ): + validator.save() + + def test_invalid_extrapolation_mode_update(self) -> None: + data = { + **self.valid_data, + "dataSources": [ + { + "queryType": SnubaQuery.Type.PERFORMANCE.value, + "dataset": Dataset.EventsAnalyticsPlatform.value, + "query": "test query", + "aggregate": "count()", + "timeWindow": 3600, + "environment": self.environment.name, + "eventTypes": [SnubaQueryEventType.EventType.TRACE_ITEM_SPAN.name.lower()], + "extrapolation_mode": ExtrapolationMode.CLIENT_AND_SERVER_WEIGHTED.value, + }, + ], + } + + validator = MetricIssueDetectorValidator(data=data, context=self.context) + assert validator.is_valid(), validator.errors + + detector = validator.save() + + update_data = { + "dataSources": [ + { + "queryType": SnubaQuery.Type.PERFORMANCE.value, + "dataset": Dataset.EventsAnalyticsPlatform.value, + "query": "updated query", + "aggregate": "count()", + "timeWindow": 3600, + "environment": self.environment.name, + "eventTypes": [SnubaQueryEventType.EventType.TRACE_ITEM_SPAN.name.lower()], + "extrapolation_mode": ExtrapolationMode.SERVER_WEIGHTED.value, + } + ], + } + update_validator = MetricIssueDetectorValidator( + instance=detector, data=update_data, context=self.context, partial=True + ) + assert update_validator.is_valid(), update_validator.errors + with self.assertRaisesMessage( + ValidationError, + expected_message="Invalid extrapolation mode for this alert type.", + ): + update_validator.save() From 6ae18337dc3f6e66c83224684471fefb6006b8ce Mon Sep 17 00:00:00 2001 From: nikkikapadia Date: Mon, 10 Nov 2025 10:46:03 -0500 Subject: [PATCH 4/6] make put validation less restrictive --- .../endpoints/organization_alert_rule_details.py | 10 ---------- src/sentry/incidents/metric_issue_detector.py | 10 ---------- 2 files changed, 20 deletions(-) diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_details.py b/src/sentry/incidents/endpoints/organization_alert_rule_details.py index 24a5aef67fab0d..b093befa68cefd 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_details.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_details.py @@ -110,16 +110,6 @@ def _is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mod and old_extrapolation_mode != ExtrapolationMode.SERVER_WEIGHTED.name.lower() ): return True - if ( - new_extrapolation_mode == ExtrapolationMode.NONE.name.lower() - and old_extrapolation_mode != ExtrapolationMode.NONE.name.lower() - ): - return True - if ( - new_extrapolation_mode == ExtrapolationMode.CLIENT_AND_SERVER_WEIGHTED.name.lower() - or new_extrapolation_mode == ExtrapolationMode.UNKNOWN.name.lower() - ): - return False return False diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index 6ed11d1f7a6ab8..0ea69b617777e6 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -225,16 +225,6 @@ def is_invalid_extrapolation_mode( and old_extrapolation_mode != ExtrapolationMode.SERVER_WEIGHTED.value ): return True - if ( - new_extrapolation_mode == ExtrapolationMode.NONE.value - and old_extrapolation_mode != ExtrapolationMode.NONE.value - ): - return True - if ( - new_extrapolation_mode == ExtrapolationMode.CLIENT_AND_SERVER_WEIGHTED.value - or new_extrapolation_mode == ExtrapolationMode.UNKNOWN.value - ): - return False return False def update_data_source(self, instance: Detector, data_source: SnubaQueryDataSourceType): From ca57cb3695556b475721a69e51899a5f42427ad5 Mon Sep 17 00:00:00 2001 From: nikkikapadia Date: Mon, 10 Nov 2025 15:50:34 -0500 Subject: [PATCH 5/6] fixes --- .../organization_alert_rule_details.py | 12 +----- src/sentry/incidents/metric_issue_detector.py | 41 +++++++++++-------- .../endpoints/validators/test_validators.py | 4 +- 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_details.py b/src/sentry/incidents/endpoints/organization_alert_rule_details.py index b093befa68cefd..c1290f56e5336f 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_details.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_details.py @@ -38,6 +38,7 @@ delete_alert_rule, get_slack_actions_with_async_lookups, ) +from sentry.incidents.metric_issue_detector import is_invalid_extrapolation_mode from sentry.incidents.models.alert_rule import AlertRule from sentry.incidents.serializers import AlertRuleSerializer as DrfAlertRuleSerializer from sentry.incidents.utils.sentry_apps import trigger_sentry_app_action_creators_for_incidents @@ -104,15 +105,6 @@ def fetch_alert_rule( return Response(serialized_rule) -def _is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode) -> bool: - if ( - new_extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.name.lower() - and old_extrapolation_mode != ExtrapolationMode.SERVER_WEIGHTED.name.lower() - ): - return True - return False - - def update_alert_rule( request: Request, organization: Organization, alert_rule: AlertRule ) -> Response: @@ -138,7 +130,7 @@ def update_alert_rule( alert_rule.snuba_query.extrapolation_mode ).name.lower() new_extrapolation_mode = data.get("extrapolation_mode", old_extrapolation_mode) - if _is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode): + if is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode): raise serializers.ValidationError( "Invalid extrapolation mode for this alert type." ) diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index 0ea69b617777e6..a22a25c40bdc80 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -140,6 +140,19 @@ def validate_conditions(self, value): return value +def is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode) -> bool: + if type(new_extrapolation_mode) is int: + new_extrapolation_mode = ExtrapolationMode(new_extrapolation_mode).name.lower() + if type(old_extrapolation_mode) is int: + old_extrapolation_mode = ExtrapolationMode(old_extrapolation_mode).name.lower() + if ( + new_extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.name.lower() + and old_extrapolation_mode != ExtrapolationMode.SERVER_WEIGHTED.name.lower() + ): + return True + return False + + class MetricIssueDetectorValidator(BaseDetectorTypeValidator): data_sources = serializers.ListField( child=SnubaQueryValidator(timeWindowSeconds=True), required=False @@ -170,7 +183,7 @@ def _validate_transaction_dataset_deprecation(self, dataset: Dataset) -> None: def _validate_extrapolation_mode(self, extrapolation_mode: str) -> None: if extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.value: raise serializers.ValidationError( - "server_weighted extrapolation mode is not supported for new alerts." + "server_weighted extrapolation mode is not supported for new detectors." ) def get_quota(self) -> DetectorQuota: @@ -212,21 +225,6 @@ def is_editing_transaction_dataset( return True return False - def is_invalid_extrapolation_mode( - self, snuba_query: SnubaQuery, data_source: SnubaQueryDataSourceType - ) -> bool: - if data_source.get("dataset") == Dataset.EventsAnalyticsPlatform: - new_extrapolation_mode = data_source.get( - "extrapolation_mode", snuba_query.extrapolation_mode - ) - old_extrapolation_mode = snuba_query.extrapolation_mode - if ( - new_extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.value - and old_extrapolation_mode != ExtrapolationMode.SERVER_WEIGHTED.value - ): - return True - return False - def update_data_source(self, instance: Detector, data_source: SnubaQueryDataSourceType): try: source_instance = DataSource.objects.get(detector=instance) @@ -250,8 +248,15 @@ def update_data_source(self, instance: Detector, data_source: SnubaQueryDataSour "Updates to transaction-based alerts is disabled, as we migrate to the span dataset. Create span-based alerts (dataset: events_analytics_platform) with the is_transaction:true filter instead." ) - if self.is_invalid_extrapolation_mode(snuba_query, data_source): - raise serializers.ValidationError("Invalid extrapolation mode for this alert type.") + old_extrapolation_mode = snuba_query.extrapolation_mode + new_extrapolation_mode = data_source.get( + "extrapolation_mode", snuba_query.extrapolation_mode + ) + if data_source.get("dataset") == Dataset.EventsAnalyticsPlatform: + if is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode): + raise serializers.ValidationError( + "Invalid extrapolation mode for this detector type." + ) update_snuba_query( snuba_query=snuba_query, diff --git a/tests/sentry/incidents/endpoints/validators/test_validators.py b/tests/sentry/incidents/endpoints/validators/test_validators.py index 94a31fa436befe..2a4a084d1679d8 100644 --- a/tests/sentry/incidents/endpoints/validators/test_validators.py +++ b/tests/sentry/incidents/endpoints/validators/test_validators.py @@ -640,7 +640,7 @@ def test_invalid_extrapolation_mode_create(self) -> None: assert validator.is_valid(), validator.errors with self.assertRaisesMessage( ValidationError, - expected_message="server_weighted extrapolation mode is not supported for new alerts.", + expected_message="server_weighted extrapolation mode is not supported for new detectors.", ): validator.save() @@ -686,6 +686,6 @@ def test_invalid_extrapolation_mode_update(self) -> None: assert update_validator.is_valid(), update_validator.errors with self.assertRaisesMessage( ValidationError, - expected_message="Invalid extrapolation mode for this alert type.", + expected_message="Invalid extrapolation mode for this detector type.", ): update_validator.save() From 4fcd9d2960a86b337e145e4a4d721c63d5a31e77 Mon Sep 17 00:00:00 2001 From: nikkikapadia Date: Mon, 10 Nov 2025 16:01:44 -0500 Subject: [PATCH 6/6] cursor fix --- src/sentry/snuba/snuba_query_validator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/snuba/snuba_query_validator.py b/src/sentry/snuba/snuba_query_validator.py index 94b1a4ff82cc2f..c5bc55d8d036e9 100644 --- a/src/sentry/snuba/snuba_query_validator.py +++ b/src/sentry/snuba/snuba_query_validator.py @@ -406,7 +406,7 @@ def _validate_group_by(self, value: Sequence[str] | None) -> Sequence[str] | Non @override def create_source(self, validated_data) -> QuerySubscription: extrapolation_mode = validated_data.get("extrapolation_mode") - if extrapolation_mode: + if extrapolation_mode is not None: extrapolation_mode = ExtrapolationMode(extrapolation_mode) snuba_query = create_snuba_query( query_type=validated_data["query_type"],