From b2ee290a895926eca5704aec129d41dcf5743e2b Mon Sep 17 00:00:00 2001 From: nikkikapadia Date: Tue, 25 Nov 2025 16:32:07 -0500 Subject: [PATCH 1/5] accept strings for extrapolation mode in validator + adjust logic --- .../organization_alert_rule_details.py | 5 +- src/sentry/incidents/metric_issue_detector.py | 30 +++++++- src/sentry/snuba/snuba_query_validator.py | 12 ++- src/sentry/snuba/subscriptions.py | 2 +- .../endpoints/validators/test_validators.py | 77 ++++++++++++++++++- 5 files changed, 115 insertions(+), 11 deletions(-) diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_details.py b/src/sentry/incidents/endpoints/organization_alert_rule_details.py index 27c941d8ee9a13..4516151a9d6b5c 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_details.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_details.py @@ -125,7 +125,10 @@ def update_alert_rule( partial=True, ) if validator.is_valid(): - if data.get("dataset") == Dataset.EventsAnalyticsPlatform.value: + if ( + data.get("dataset", alert_rule.snuba_query.dataset) + == Dataset.EventsAnalyticsPlatform.value + ): if data.get("extrapolation_mode"): old_extrapolation_mode = ExtrapolationMode( alert_rule.snuba_query.extrapolation_mode diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index 30290a45370bfe..1c39d70ccf23ad 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -161,6 +161,8 @@ def is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode 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 not in [name for name, value in ExtrapolationMode.as_text_choices()]: + return True if ( new_extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.name.lower() and old_extrapolation_mode != ExtrapolationMode.SERVER_WEIGHTED.name.lower() @@ -169,6 +171,20 @@ def is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode return False +def format_extrapolation_mode(extrapolation_mode) -> ExtrapolationMode: + if type(extrapolation_mode) is int: + return extrapolation_mode + extrapolation_mode_options = ExtrapolationMode.as_text_choices() + numerical_extrapolation_mode = None + for name, value in extrapolation_mode_options: + if name == extrapolation_mode: + numerical_extrapolation_mode = value + break + if type(numerical_extrapolation_mode) is not int: + return None + return numerical_extrapolation_mode + + class MetricIssueDetectorValidator(BaseDetectorTypeValidator): data_sources = serializers.ListField( child=SnubaQueryValidator(timeWindowSeconds=True), required=False @@ -197,7 +213,11 @@ 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: + if extrapolation_mode is not None and extrapolation_mode not in [ + name for name, value in ExtrapolationMode.as_text_choices() + ]: + raise serializers.ValidationError(f"Invalid extrapolation mode: {extrapolation_mode}") + if extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.name.lower(): raise serializers.ValidationError( "server_weighted extrapolation mode is not supported for new detectors." ) @@ -288,6 +308,10 @@ def update_data_source( "Failed to send data to Seer, cannot update detector" ) + numerical_extrapolation_mode = format_extrapolation_mode( + data_source.get("extrapolation_mode", snuba_query.extrapolation_mode) + ) + update_snuba_query( snuba_query=snuba_query, query_type=data_source.get("query_type", snuba_query.type), @@ -298,9 +322,7 @@ def update_data_source( 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 - ), + extrapolation_mode=numerical_extrapolation_mode, ) def update_anomaly_detection(self, instance: Detector, validated_data: dict[str, Any]) -> bool: diff --git a/src/sentry/snuba/snuba_query_validator.py b/src/sentry/snuba/snuba_query_validator.py index a78295d4d2610b..8a5c4d81905383 100644 --- a/src/sentry/snuba/snuba_query_validator.py +++ b/src/sentry/snuba/snuba_query_validator.py @@ -87,7 +87,7 @@ class SnubaQueryValidator(BaseDataSourceValidator[QuerySubscription]): required=False, allow_empty=False, ) - extrapolation_mode = serializers.IntegerField(required=False, allow_null=True) + extrapolation_mode = serializers.CharField(required=False, allow_null=True) class Meta: model = QuerySubscription @@ -408,7 +408,15 @@ def _validate_group_by(self, value: Sequence[str] | None) -> Sequence[str] | Non def create_source(self, validated_data) -> QuerySubscription: extrapolation_mode = validated_data.get("extrapolation_mode") if extrapolation_mode is not None: - extrapolation_mode = ExtrapolationMode(extrapolation_mode) + extrapolation_mode_options = ExtrapolationMode.as_text_choices() + for name, value in extrapolation_mode_options: + if name == extrapolation_mode: + extrapolation_mode = ExtrapolationMode(value) + break + if type(extrapolation_mode) is not ExtrapolationMode: + raise serializers.ValidationError( + f"Invalid extrapolation mode: {extrapolation_mode}" + ) snuba_query = create_snuba_query( query_type=validated_data["query_type"], dataset=validated_data["dataset"], diff --git a/src/sentry/snuba/subscriptions.py b/src/sentry/snuba/subscriptions.py index 271c593a6d45e1..81c0e19e8e74d1 100644 --- a/src/sentry/snuba/subscriptions.py +++ b/src/sentry/snuba/subscriptions.py @@ -133,7 +133,7 @@ def update_snuba_query( extrapolation_mode=( extrapolation_mode if extrapolation_mode is not None - else ExtrapolationMode.UNKNOWN.value + else snuba_query.extrapolation_mode ), ) if new_event_types: diff --git a/tests/sentry/incidents/endpoints/validators/test_validators.py b/tests/sentry/incidents/endpoints/validators/test_validators.py index 854cce28387a22..1f53271cf7a5ff 100644 --- a/tests/sentry/incidents/endpoints/validators/test_validators.py +++ b/tests/sentry/incidents/endpoints/validators/test_validators.py @@ -1238,7 +1238,7 @@ def test_invalid_extrapolation_mode_create(self) -> None: "timeWindow": 3600, "environment": self.environment.name, "eventTypes": [SnubaQueryEventType.EventType.TRACE_ITEM_SPAN.name.lower()], - "extrapolation_mode": ExtrapolationMode.SERVER_WEIGHTED.value, + "extrapolation_mode": ExtrapolationMode.SERVER_WEIGHTED.name.lower(), }, ], } @@ -1263,7 +1263,7 @@ def test_invalid_extrapolation_mode_update(self) -> None: "timeWindow": 3600, "environment": self.environment.name, "eventTypes": [SnubaQueryEventType.EventType.TRACE_ITEM_SPAN.name.lower()], - "extrapolation_mode": ExtrapolationMode.CLIENT_AND_SERVER_WEIGHTED.value, + "extrapolation_mode": ExtrapolationMode.CLIENT_AND_SERVER_WEIGHTED.name.lower(), }, ], } @@ -1283,7 +1283,78 @@ def test_invalid_extrapolation_mode_update(self) -> None: "timeWindow": 3600, "environment": self.environment.name, "eventTypes": [SnubaQueryEventType.EventType.TRACE_ITEM_SPAN.name.lower()], - "extrapolation_mode": ExtrapolationMode.SERVER_WEIGHTED.value, + "extrapolation_mode": ExtrapolationMode.SERVER_WEIGHTED.name.lower(), + } + ], + } + 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 detector type.", + ): + update_validator.save() + + def test_nonexistent_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": "blah", + }, + ], + } + + validator = MetricIssueDetectorValidator(data=data, context=self.context) + assert validator.is_valid(), validator.errors + with self.assertRaisesMessage( + ValidationError, + expected_message="Invalid extrapolation mode: blah", + ): + validator.save() + + def test_nonexistent_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.name.lower(), + }, + ], + } + + 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": "blah", } ], } From 6d45c578bd6bd6be73bfe5145ee2340b57b09447 Mon Sep 17 00:00:00 2001 From: nikkikapadia Date: Wed, 26 Nov 2025 12:35:10 -0500 Subject: [PATCH 2/5] address comments --- src/sentry/incidents/metric_issue_detector.py | 24 ++++++------------- src/sentry/snuba/models.py | 7 ++++++ src/sentry/snuba/snuba_query_validator.py | 22 ++++++++--------- src/sentry/snuba/subscriptions.py | 4 ++-- .../endpoints/validators/test_validators.py | 23 +++++++++--------- 5 files changed, 37 insertions(+), 43 deletions(-) diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index 1c39d70ccf23ad..96e97fd934e1fe 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -173,16 +173,10 @@ def is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode def format_extrapolation_mode(extrapolation_mode) -> ExtrapolationMode: if type(extrapolation_mode) is int: + return ExtrapolationMode(extrapolation_mode) + if type(extrapolation_mode) is ExtrapolationMode: return extrapolation_mode - extrapolation_mode_options = ExtrapolationMode.as_text_choices() - numerical_extrapolation_mode = None - for name, value in extrapolation_mode_options: - if name == extrapolation_mode: - numerical_extrapolation_mode = value - break - if type(numerical_extrapolation_mode) is not int: - return None - return numerical_extrapolation_mode + return ExtrapolationMode.from_str(extrapolation_mode) class MetricIssueDetectorValidator(BaseDetectorTypeValidator): @@ -212,12 +206,8 @@ 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 is not None and extrapolation_mode not in [ - name for name, value in ExtrapolationMode.as_text_choices() - ]: - raise serializers.ValidationError(f"Invalid extrapolation mode: {extrapolation_mode}") - if extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.name.lower(): + def _validate_extrapolation_mode(self, extrapolation_mode: ExtrapolationMode) -> None: + if extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED: raise serializers.ValidationError( "server_weighted extrapolation mode is not supported for new detectors." ) @@ -308,7 +298,7 @@ def update_data_source( "Failed to send data to Seer, cannot update detector" ) - numerical_extrapolation_mode = format_extrapolation_mode( + extrapolation_mode = format_extrapolation_mode( data_source.get("extrapolation_mode", snuba_query.extrapolation_mode) ) @@ -322,7 +312,7 @@ def update_data_source( 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=numerical_extrapolation_mode, + extrapolation_mode=extrapolation_mode, ) def update_anomaly_detection(self, instance: Detector, validated_data: dict[str, Any]) -> bool: diff --git a/src/sentry/snuba/models.py b/src/sentry/snuba/models.py index 03fa3989a8c433..7a35dfec100658 100644 --- a/src/sentry/snuba/models.py +++ b/src/sentry/snuba/models.py @@ -43,6 +43,13 @@ def as_choices(cls): def as_text_choices(cls): return tuple((mode.name.lower(), mode.value) for mode in cls) + @classmethod + def from_str(cls, name: str): + for mode in cls: + if mode.name.lower() == name: + return mode + return None + @region_silo_model class SnubaQuery(Model): diff --git a/src/sentry/snuba/snuba_query_validator.py b/src/sentry/snuba/snuba_query_validator.py index 8a5c4d81905383..ccc3819d5e618e 100644 --- a/src/sentry/snuba/snuba_query_validator.py +++ b/src/sentry/snuba/snuba_query_validator.py @@ -167,6 +167,15 @@ def validate_event_types(self, value: Sequence[str]) -> list[SnubaQueryEventType return validated + def validate_extrapolation_mode(self, extrapolation_mode: str) -> ExtrapolationMode | None: + if extrapolation_mode is not None: + extrapolation_mode_enum = ExtrapolationMode.from_str(extrapolation_mode) + if extrapolation_mode_enum is None: + raise serializers.ValidationError( + f"Invalid extrapolation mode: {extrapolation_mode}" + ) + return extrapolation_mode_enum + def validate(self, data): data = super().validate(data) self._validate_aggregate(data) @@ -406,17 +415,6 @@ 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 is not None: - extrapolation_mode_options = ExtrapolationMode.as_text_choices() - for name, value in extrapolation_mode_options: - if name == extrapolation_mode: - extrapolation_mode = ExtrapolationMode(value) - break - if type(extrapolation_mode) is not ExtrapolationMode: - raise serializers.ValidationError( - f"Invalid extrapolation mode: {extrapolation_mode}" - ) snuba_query = create_snuba_query( query_type=validated_data["query_type"], dataset=validated_data["dataset"], @@ -427,7 +425,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=extrapolation_mode, + 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 81c0e19e8e74d1..d52b7c14367149 100644 --- a/src/sentry/snuba/subscriptions.py +++ b/src/sentry/snuba/subscriptions.py @@ -89,7 +89,7 @@ def update_snuba_query( resolution, environment, event_types, - extrapolation_mode=None, + extrapolation_mode: ExtrapolationMode | None = None, ): """ Updates a SnubaQuery. Triggers updates to any related QuerySubscriptions. @@ -131,7 +131,7 @@ def update_snuba_query( resolution=int(resolution.total_seconds()), environment=environment, extrapolation_mode=( - extrapolation_mode + extrapolation_mode.value if extrapolation_mode is not None else snuba_query.extrapolation_mode ), diff --git a/tests/sentry/incidents/endpoints/validators/test_validators.py b/tests/sentry/incidents/endpoints/validators/test_validators.py index 1f53271cf7a5ff..5637e3bbef0d1a 100644 --- a/tests/sentry/incidents/endpoints/validators/test_validators.py +++ b/tests/sentry/incidents/endpoints/validators/test_validators.py @@ -1315,12 +1315,11 @@ def test_nonexistent_extrapolation_mode_create(self) -> None: } validator = MetricIssueDetectorValidator(data=data, context=self.context) - assert validator.is_valid(), validator.errors - with self.assertRaisesMessage( - ValidationError, - expected_message="Invalid extrapolation mode: blah", - ): - validator.save() + assert not validator.is_valid(), validator.errors + assert ( + validator.errors["dataSources"]["extrapolationMode"][0] + == "Invalid extrapolation mode: blah" + ) def test_nonexistent_extrapolation_mode_update(self) -> None: data = { @@ -1361,9 +1360,9 @@ def test_nonexistent_extrapolation_mode_update(self) -> None: 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 detector type.", - ): - update_validator.save() + + assert not update_validator.is_valid(), update_validator.errors + assert ( + update_validator.errors["dataSources"]["extrapolationMode"][0] + == "Invalid extrapolation mode: blah" + ) From 7a11ead5058a461b83dfa7d1e598b12e6e1c75cc Mon Sep 17 00:00:00 2001 From: nikkikapadia Date: Wed, 26 Nov 2025 13:40:14 -0500 Subject: [PATCH 3/5] change validation with from_str --- src/sentry/incidents/metric_issue_detector.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index 96e97fd934e1fe..13cd209bbb0c43 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -159,9 +159,13 @@ def validate_conditions(self, 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(new_extrapolation_mode) is ExtrapolationMode: + new_extrapolation_mode = 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 not in [name for name, value in ExtrapolationMode.as_text_choices()]: + if type(old_extrapolation_mode) is ExtrapolationMode: + old_extrapolation_mode = old_extrapolation_mode.name.lower() + if ExtrapolationMode.from_str(new_extrapolation_mode) is None: return True if ( new_extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.name.lower() From 04caca137dc927f2c39fd3d07bed11d30898edf0 Mon Sep 17 00:00:00 2001 From: nikkikapadia Date: Wed, 26 Nov 2025 13:46:57 -0500 Subject: [PATCH 4/5] another fix --- src/sentry/incidents/metric_issue_detector.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index 13cd209bbb0c43..2fc86bbc6189de 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -175,7 +175,9 @@ def is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode return False -def format_extrapolation_mode(extrapolation_mode) -> ExtrapolationMode: +def format_extrapolation_mode(extrapolation_mode) -> ExtrapolationMode | None: + if extrapolation_mode is None: + return None if type(extrapolation_mode) is int: return ExtrapolationMode(extrapolation_mode) if type(extrapolation_mode) is ExtrapolationMode: From a5a520ad44fe526999d1accf05b207afb1f36c6a Mon Sep 17 00:00:00 2001 From: nikkikapadia Date: Wed, 26 Nov 2025 13:56:31 -0500 Subject: [PATCH 5/5] another fix --- src/sentry/incidents/metric_issue_detector.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index 2fc86bbc6189de..dff5cee6b97bf3 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -165,7 +165,10 @@ def is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode old_extrapolation_mode = ExtrapolationMode(old_extrapolation_mode).name.lower() if type(old_extrapolation_mode) is ExtrapolationMode: old_extrapolation_mode = old_extrapolation_mode.name.lower() - if ExtrapolationMode.from_str(new_extrapolation_mode) is None: + if ( + new_extrapolation_mode is not None + and ExtrapolationMode.from_str(new_extrapolation_mode) is None + ): return True if ( new_extrapolation_mode == ExtrapolationMode.SERVER_WEIGHTED.name.lower()