Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 17 additions & 5 deletions src/sentry/incidents/metric_issue_detector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -169,6 +171,14 @@ 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 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
Expand Down Expand Up @@ -196,8 +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 == 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."
)
Expand Down Expand Up @@ -288,6 +298,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),
Expand All @@ -298,9 +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=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:
Expand Down
7 changes: 7 additions & 0 deletions src/sentry/snuba/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
16 changes: 11 additions & 5 deletions src/sentry/snuba/snuba_query_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"],
Expand All @@ -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"],
Expand Down
6 changes: 3 additions & 3 deletions src/sentry/snuba/subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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:
Expand Down
76 changes: 73 additions & 3 deletions tests/sentry/incidents/endpoints/validators/test_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
],
}
Expand All @@ -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(),
},
],
}
Expand All @@ -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(),
}
],
}
Expand All @@ -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"
)
Loading