Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
8 changes: 2 additions & 6 deletions src/sentry/incidents/serializers/alert_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,12 +272,8 @@ def create(self, validated_data):

self._handle_triggers(alert_rule, triggers)

# NOTE (mifu67): skip dual writing anomaly detection alerts until we figure out how to handle them
should_dual_write = (
features.has(
"organizations:workflow-engine-metric-alert-dual-write", alert_rule.organization
)
and alert_rule.detection_type != AlertRuleDetectionType.DYNAMIC
should_dual_write = features.has(
"organizations:workflow-engine-metric-alert-dual-write", alert_rule.organization
)
if should_dual_write:
try:
Expand Down
65 changes: 50 additions & 15 deletions src/sentry/workflow_engine/migration_helpers/alert_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from sentry.incidents.grouptype import MetricIssue
from sentry.incidents.models.alert_rule import (
AlertRule,
AlertRuleDetectionType,
AlertRuleThresholdType,
AlertRuleTrigger,
AlertRuleTriggerAction,
Expand Down Expand Up @@ -285,12 +286,24 @@ def migrate_metric_data_conditions(
)
condition_result = PRIORITY_MAP.get(alert_rule_trigger.label, DetectorPriorityLevel.HIGH)

detector_trigger = DataCondition.objects.create(
comparison=alert_rule_trigger.alert_threshold,
condition_result=condition_result,
type=threshold_type,
condition_group=detector_data_condition_group,
)
if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC:
detector_trigger = DataCondition.objects.create(
type=Condition.ANOMALY_DETECTION,
comparison={
"sensitivity": alert_rule.sensitivity,
"seasonality": alert_rule.seasonality,
"threshold_type": alert_rule.threshold_type,
},
condition_result=condition_result,
condition_group=detector_data_condition_group,
)
else:
detector_trigger = DataCondition.objects.create(
comparison=alert_rule_trigger.alert_threshold,
condition_result=condition_result,
type=threshold_type,
condition_group=detector_data_condition_group,
)
DataConditionAlertRuleTrigger.objects.create(
data_condition=detector_trigger,
alert_rule_trigger_id=alert_rule_trigger.id,
Expand Down Expand Up @@ -584,7 +597,9 @@ def dual_write_alert_rule(alert_rule: AlertRule, user: RpcUser | None = None) ->
for trigger_action in trigger_actions:
migrate_metric_action(trigger_action)
# step 4: migrate alert rule resolution
migrate_resolve_threshold_data_condition(alert_rule)
# if the alert rule is an anomaly detection alert, then this is handled by the anomaly detection data condition
if alert_rule.detection_type != AlertRuleDetectionType.DYNAMIC:
migrate_resolve_threshold_data_condition(alert_rule)


def dual_update_alert_rule(alert_rule: AlertRule) -> None:
Expand Down Expand Up @@ -699,20 +714,28 @@ def dual_update_resolve_condition(alert_rule: AlertRule) -> DataCondition | None
)
raise MissingDataConditionGroup

data_conditions = DataCondition.objects.filter(condition_group=detector_data_condition_group)
resolve_condition = data_conditions.filter(condition_result=DetectorPriorityLevel.OK).first()

# changing detector priority level for anomaly detection alerts is handled by the data condition
# so we should delete the explicit resolution condition if it exists
Comment on lines +720 to +721
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean this was a regular alert that was changed to become an anomaly detection alert?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what is happening here

if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC:
if resolve_condition is not None:
Comment on lines +722 to +723
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC:
if resolve_condition is not None:
if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC and resolve_condition is not None:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote it this way because we should always return before the resolve detector trigger logic for anomaly detection alerts!

resolve_condition.delete()
return None

# if we're changing from anomaly detection alert to regular metric alert, we need to recreate the resolve condition
if resolve_condition is None:
resolve_condition = migrate_resolve_threshold_data_condition(alert_rule)
return resolve_condition

if alert_rule.resolve_threshold is not None:
resolve_threshold = alert_rule.resolve_threshold
else:
resolve_threshold = get_resolve_threshold(detector_data_condition_group)
if resolve_threshold == -1:
raise UnresolvableResolveThreshold

data_conditions = DataCondition.objects.filter(condition_group=detector_data_condition_group)
try:
resolve_condition = data_conditions.get(condition_result=DetectorPriorityLevel.OK)
except DataCondition.DoesNotExist:
# In the serializer, we call handle triggers before migrating the resolve data condition,
# so the resolve condition may not exist yet. Return early.
return None
resolve_condition.update(comparison=resolve_threshold)

return resolve_condition
Expand All @@ -732,14 +755,26 @@ def dual_update_migrated_alert_rule_trigger(
condition_group=action_filter.condition_group,
type=Condition.ISSUE_PRIORITY_DEESCALATING,
).first()

updated_detector_trigger_fields: dict[str, Any] = {}
updated_action_filter_fields: dict[str, Any] = {}
label = alert_rule_trigger.label
updated_detector_trigger_fields["condition_result"] = PRIORITY_MAP.get(
label, DetectorPriorityLevel.HIGH
)
updated_action_filter_fields["comparison"] = PRIORITY_MAP.get(label, DetectorPriorityLevel.HIGH)
updated_detector_trigger_fields["comparison"] = alert_rule_trigger.alert_threshold
alert_rule = alert_rule_trigger.alert_rule

# if we're changing to anomaly detection, we need to set the comparison JSON
if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC:
updated_detector_trigger_fields["type"] = Condition.ANOMALY_DETECTION
updated_detector_trigger_fields["comparison"] = {
"sensitivity": alert_rule.sensitivity,
"seasonality": alert_rule.seasonality,
"threshold_type": alert_rule.threshold_type,
}
else:
updated_detector_trigger_fields["comparison"] = alert_rule_trigger.alert_threshold

detector_trigger.update(**updated_detector_trigger_fields)
if updated_action_filter_fields:
Expand Down
Loading
Loading