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..dff5cee6b97bf3 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -159,8 +159,17 @@ 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 type(old_extrapolation_mode) is ExtrapolationMode: + old_extrapolation_mode = old_extrapolation_mode.name.lower() + 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() and old_extrapolation_mode != ExtrapolationMode.SERVER_WEIGHTED.name.lower() @@ -169,6 +178,16 @@ def is_invalid_extrapolation_mode(old_extrapolation_mode, new_extrapolation_mode return False +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: + return extrapolation_mode + return ExtrapolationMode.from_str(extrapolation_mode) + + class MetricIssueDetectorValidator(BaseDetectorTypeValidator): data_sources = serializers.ListField( child=SnubaQueryValidator(timeWindowSeconds=True), required=False @@ -196,8 +215,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 == ExtrapolationMode.SERVER_WEIGHTED.value: + 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." ) @@ -288,6 +307,10 @@ def update_data_source( "Failed to send data to Seer, cannot update detector" ) + 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 +321,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=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 a78295d4d2610b..ccc3819d5e618e 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 @@ -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,9 +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 = ExtrapolationMode(extrapolation_mode) snuba_query = create_snuba_query( query_type=validated_data["query_type"], dataset=validated_data["dataset"], @@ -419,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 271c593a6d45e1..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,9 +131,9 @@ 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 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..5637e3bbef0d1a 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,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.SERVER_WEIGHTED.value, + "extrapolation_mode": ExtrapolationMode.SERVER_WEIGHTED.name.lower(), } ], } @@ -1296,3 +1296,73 @@ def test_invalid_extrapolation_mode_update(self) -> None: 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 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 = { + **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", + } + ], + } + update_validator = MetricIssueDetectorValidator( + instance=detector, data=update_data, context=self.context, partial=True + ) + + assert not update_validator.is_valid(), update_validator.errors + assert ( + update_validator.errors["dataSources"]["extrapolationMode"][0] + == "Invalid extrapolation mode: blah" + )