Skip to content

Commit

Permalink
fix test, move feature check
Browse files Browse the repository at this point in the history
  • Loading branch information
ceorourke committed Sep 27, 2024
1 parent 627d3fa commit 744d477
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 16 deletions.
18 changes: 13 additions & 5 deletions src/sentry/incidents/logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
from django.utils import timezone as django_timezone
from snuba_sdk import Column, Condition, Limit, Op

from sentry import analytics, audit_log, quotas
from sentry import analytics, audit_log, features, quotas
from sentry.api.exceptions import ResourceDoesNotExist
from sentry.auth.access import SystemAccess
from sentry.constants import CRASH_RATE_ALERT_AGGREGATE_ALIAS, ObjectStatus
from sentry.db.models import Model
Expand Down Expand Up @@ -646,8 +647,13 @@ def create_alert_rule(
AlertRuleExcludedProjects.objects.bulk_create(exclusions)

if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC.value:
if not features.has("organizations:anomaly-detection-alerts", organization):
alert_rule.delete()
raise ResourceDoesNotExist(
"Your organization does not have access to this feature."
)
# NOTE: if adding a new metric alert type, take care to check that it's handled here
send_new_rule_data(organization, alert_rule, projects[0])
send_new_rule_data(alert_rule, projects[0])

if user:
create_audit_entry_from_user(
Expand Down Expand Up @@ -903,11 +909,13 @@ def update_alert_rule(
updated_fields["team_id"] = alert_rule.team_id

if detection_type == AlertRuleDetectionType.DYNAMIC:
if not features.has("organizations:anomaly-detection-alerts", organization):
raise ResourceDoesNotExist(
"Your organization does not have access to this feature."
)
# NOTE: if adding a new metric alert type, take care to check that it's handled here
project = projects[0] if projects else alert_rule.projects.get()
update_rule_data(
organization, alert_rule, project, updated_fields, updated_query_fields
)
update_rule_data(alert_rule, project, updated_fields, updated_query_fields)
else:
# if this was a dynamic rule, delete the data in Seer
if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC:
Expand Down
11 changes: 1 addition & 10 deletions src/sentry/seer/anomaly_detection/store_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,9 @@
from parsimonious.exceptions import ParseError
from urllib3.exceptions import MaxRetryError, TimeoutError

from sentry import features
from sentry.api.bases.organization_events import get_query_columns
from sentry.api.exceptions import ResourceDoesNotExist
from sentry.conf.server import SEER_ANOMALY_DETECTION_STORE_DATA_URL
from sentry.incidents.models.alert_rule import AlertRule, AlertRuleDetectionType, AlertRuleStatus
from sentry.models.organization import Organization
from sentry.models.project import Project
from sentry.net.http import connection_from_url
from sentry.seer.anomaly_detection.types import (
Expand Down Expand Up @@ -78,10 +75,7 @@ def handle_send_historical_data_to_seer(alert_rule: AlertRule, project: Project,
raise ValidationError(f"Failed to send data to Seer - cannot {method} alert rule.")


def send_new_rule_data(organization: Organization, alert_rule: AlertRule, project: Project) -> None:
if not features.has("organizations:anomaly-detection-alerts", organization):
alert_rule.delete()
raise ResourceDoesNotExist("Your organization does not have access to this feature.")
def send_new_rule_data(alert_rule: AlertRule, project: Project) -> None:
try:
handle_send_historical_data_to_seer(alert_rule, project, "create")
except (TimeoutError, MaxRetryError, ParseError, ValidationError):
Expand All @@ -92,14 +86,11 @@ def send_new_rule_data(organization: Organization, alert_rule: AlertRule, projec


def update_rule_data(
organization: Organization,
alert_rule: AlertRule,
project: Project,
updated_fields: dict[str, Any],
updated_query_fields: dict[str, Any],
) -> None:
if not features.has("organizations:anomaly-detection-alerts", organization):
raise ResourceDoesNotExist("Your organization does not have access to this feature.")
# if the rule previously wasn't a dynamic type but it is now, we need to send Seer data for the first time
# OR it's dynamic but the query or aggregate is changing so we need to update the data Seer has
if updated_fields.get("detection_type") == AlertRuleDetectionType.DYNAMIC and (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from django.test.utils import override_settings
from httpx import HTTPError
from rest_framework import status
from rest_framework.exceptions import ErrorDetail
from urllib3.exceptions import MaxRetryError, TimeoutError
from urllib3.response import HTTPResponse

Expand Down Expand Up @@ -400,7 +401,10 @@ def test_anomaly_detection_alert_other_error(self, mock_seer_request):
**data,
)
assert not AlertRule.objects.filter(detection_type=AlertRuleDetectionType.DYNAMIC).exists()
assert resp.data["detail"]["message"] == "Invalid request"
assert resp.data[0] == ErrorDetail(
string="Failed to send data to Seer - cannot create alert rule.", code="invalid"
)

assert mock_seer_request.call_count == 1

@with_feature("organizations:anomaly-detection-alerts")
Expand Down

0 comments on commit 744d477

Please sign in to comment.