Browse Source

conditional Activated AlertRule creation (#65837)

per [implementation
details](https://www.notion.so/sentry/Alert-Threshold-Implementation-Details-Activated-Alert-Rules-f1e9eaa6c26d41b4898b95f3c8e4411a)

This PR taps into the `ReleaseProject` model `post_save` (which
represents a project being added to a release - or release being created
for a project), and determines whether there is an Activated AlertRule
for that project, and whether the activation trigger is on Release
creation. If so - it will subscribe that project to the alert rule and
construct a custom `query_extra` with the release specified, and the
event timestamp hinged off the current datetime.

edit:
I've moved the "subscribe project to alert rule" to be a model method,
and i've moved the conditional subscription to the AlertRuleManager

NOTE: we'll need to look in to the possibility of multiple
QuerySubscriptions being created for a single AlertRule
Nathan Hsieh 1 year ago
parent
commit
3ab3d1412a

+ 4 - 18
src/sentry/incidents/logic.py

@@ -21,7 +21,6 @@ from sentry.incidents import tasks
 from sentry.incidents.models import (
     AlertRule,
     AlertRuleActivationCondition,
-    AlertRuleActivationConditionType,
     AlertRuleActivity,
     AlertRuleActivityType,
     AlertRuleExcludedProjects,
@@ -42,6 +41,7 @@ from sentry.incidents.models import (
     IncidentTrigger,
     TriggerStatus,
 )
+from sentry.incidents.utils.types import AlertRuleActivationConditionType
 from sentry.models.actor import Actor
 from sentry.models.notificationaction import ActionService, ActionTarget
 from sentry.models.project import Project
@@ -69,7 +69,6 @@ from sentry.snuba.metrics.extraction import should_use_on_demand_metrics
 from sentry.snuba.metrics.naming_layer.mri import get_available_operations, is_mri, parse_mri
 from sentry.snuba.models import SnubaQuery
 from sentry.snuba.subscriptions import (
-    bulk_create_snuba_subscriptions,
     bulk_delete_snuba_subscriptions,
     bulk_disable_snuba_subscriptions,
     bulk_enable_snuba_subscriptions,
@@ -586,8 +585,8 @@ def create_alert_rule(
             AlertRuleProjects.objects.bulk_create(arps)
 
         # NOTE: This constructs the query in snuba
-        # TODO: only construct `CONTINUOUS` monitor type AlertRule queries in snuba
-        subscribe_projects_to_alert_rule(alert_rule, projects)
+        # NOTE: Will only subscribe if AlertRule.monitor_type === 'CONTINUOUS'
+        alert_rule.subscribe_projects(projects=projects)
 
         # Activity is an audit log of what's happened with this alert rule
         AlertRuleActivity.objects.create(
@@ -828,7 +827,7 @@ def update_alert_rule(
             ]
 
         if new_projects:
-            subscribe_projects_to_alert_rule(alert_rule, new_projects)
+            alert_rule.subscribe_projects(projects=new_projects)
 
         if deleted_subs:
             bulk_delete_snuba_subscriptions(deleted_subs)
@@ -848,19 +847,6 @@ def update_alert_rule(
     return alert_rule
 
 
-def subscribe_projects_to_alert_rule(alert_rule: AlertRule, projects: list[Project]):
-    """
-    Subscribes a list of projects to an alert rule
-    :return: The list of created subscriptions
-
-    TODO: consolidate `bulk_create_snuba_subscriptions` with this in between method
-    TODO: only create subscription if AlertRule.monitor_type === 'CONTINUOUS'
-    """
-    return bulk_create_snuba_subscriptions(
-        projects, tasks.INCIDENTS_SNUBA_SUBSCRIPTION_TYPE, alert_rule.snuba_query
-    )
-
-
 def enable_alert_rule(alert_rule):
     if alert_rule.status != AlertRuleStatus.DISABLED.value:
         return

+ 85 - 6
src/sentry/incidents/models.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import logging
 from collections import namedtuple
 from collections.abc import Callable
 from datetime import timedelta
@@ -10,6 +11,7 @@ from uuid import uuid4
 from django.conf import settings
 from django.core.cache import cache
 from django.db import IntegrityError, models, router, transaction
+from django.db.models import QuerySet
 from django.db.models.signals import post_delete, post_save
 from django.utils import timezone
 
@@ -30,13 +32,16 @@ from sentry.db.models import (
 )
 from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
 from sentry.db.models.manager import BaseManager
+from sentry.incidents.utils.constants import INCIDENTS_SNUBA_SUBSCRIPTION_TYPE
+from sentry.incidents.utils.types import AlertRuleActivationConditionType
 from sentry.models.actor import Actor
 from sentry.models.notificationaction import AbstractNotificationAction, ActionService, ActionTarget
 from sentry.models.organization import Organization
+from sentry.models.project import Project
 from sentry.models.team import Team
 from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.snuba.models import QuerySubscription
-from sentry.snuba.subscriptions import delete_snuba_subscription
+from sentry.snuba.subscriptions import bulk_create_snuba_subscriptions, delete_snuba_subscription
 from sentry.utils import metrics
 from sentry.utils.retries import TimedRetryPolicy
 
@@ -66,6 +71,9 @@ def invoke_alert_subscription_callback(
     return callback(subscription)
 
 
+logger = logging.getLogger(__name__)
+
+
 @region_silo_only_model
 class IncidentProject(Model):
     __relocation_scope__ = RelocationScope.Excluded
@@ -427,6 +435,52 @@ class AlertRuleManager(BaseManager["AlertRule"]):
                 for sub_id in subscription_ids
             )
 
+    def conditionally_subscribe_project_to_alert_rules(
+        self,
+        project: Project,
+        activation_condition: AlertRuleActivationConditionType,
+        query_extra: str,
+        trigger: str,
+    ) -> list[QuerySubscription]:
+        """
+        Subscribes a project to an alert rule given activation condition
+        """
+        try:
+            project_alert_rules: QuerySet[AlertRule] = self.filter(
+                projects=project,
+                monitor_type=AlertRuleMonitorType.ACTIVATED.value,
+            )
+            created_subscriptions = []
+            for alert_rule in project_alert_rules:
+                if alert_rule.activation_conditions.filter(
+                    condition_type=activation_condition.value
+                ).exists():
+                    logger.info(
+                        "Attempt subscribe project to activated alert rule",
+                        extra={
+                            "trigger": trigger,
+                            "query_extra": query_extra,
+                            "condition": activation_condition,
+                        },
+                    )
+                    created_subscriptions.extend(
+                        alert_rule.subscribe_projects(
+                            projects=[project],
+                            monitor_type=AlertRuleMonitorType.ACTIVATED,
+                            query_extra=query_extra,
+                        )
+                    )
+            return created_subscriptions
+        except Exception as e:
+            logger.exception(
+                "Failed to subscribe project to activated alert rule",
+                extra={
+                    "trigger": trigger,
+                    "exception": e,
+                },
+            )
+        return []
+
 
 @region_silo_only_model
 class AlertRuleExcludedProjects(Model):
@@ -567,6 +621,36 @@ class AlertRule(Model):
 
         return old_pk
 
+    def subscribe_projects(
+        self,
+        projects: list[Project],
+        monitor_type: AlertRuleMonitorType = AlertRuleMonitorType.CONTINUOUS,
+        query_extra: str | None = None,
+    ) -> list[QuerySubscription]:
+        """
+        Subscribes a list of projects to the alert rule instance
+        :return: The list of created subscriptions
+        """
+
+        logger.info(
+            "Subscribing projects to alert rule",
+            extra={
+                "alert_rule.monitor_type": self.monitor_type,
+                "conditional_monitor_type": monitor_type.value,
+                "query_extra": query_extra,
+            },
+        )
+        # NOTE: AlertRuleMonitorType.ACTIVATED will be conditionally subscribed given activation triggers
+        # On activated subscription, additional query parameters will be added to the constructed query in Snuba
+        if self.monitor_type == monitor_type.value:
+            return bulk_create_snuba_subscriptions(
+                projects,
+                INCIDENTS_SNUBA_SUBSCRIPTION_TYPE,
+                self.snuba_query,
+                query_extra,
+            )
+        return []
+
 
 class TriggerStatus(Enum):
     ACTIVE = 0
@@ -687,11 +771,6 @@ class AlertRuleTrigger(Model):
         unique_together = (("alert_rule", "label"),)
 
 
-class AlertRuleActivationConditionType(Enum):
-    RELEASE_CREATION = 0
-    DEPLOY_CREATION = 1
-
-
 @region_silo_only_model
 class AlertRuleActivationCondition(Model):
     """

+ 2 - 3
src/sentry/incidents/receivers.py

@@ -15,8 +15,6 @@ def add_project_to_include_all_rules(instance, created, **kwargs):
 
     NOTE: This feature is not currently utilized and may break the UX flow
     """
-    from sentry.incidents.logic import subscribe_projects_to_alert_rule
-
     if not created:
         return
 
@@ -24,7 +22,8 @@ def add_project_to_include_all_rules(instance, created, **kwargs):
         organization=instance.organization, include_all_projects=True
     )
     for alert_rule in alert_rules:
-        subscribe_projects_to_alert_rule(alert_rule, [instance])
+        # NOTE: defaults to only subscribe if AlertRule.monitor_type === 'CONTINUOUS'
+        alert_rule.subscribe_projects(projects=[instance])
 
 
 @receiver(pre_save, sender=IncidentTrigger)

+ 4 - 4
src/sentry/incidents/tasks.py

@@ -17,6 +17,10 @@ from sentry.incidents.models import (
     IncidentStatus,
     IncidentStatusMethod,
 )
+from sentry.incidents.utils.constants import (
+    INCIDENTS_SNUBA_SUBSCRIPTION_TYPE,
+    SUBSCRIPTION_METRICS_LOGGER,
+)
 from sentry.incidents.utils.types import QuerySubscriptionUpdate
 from sentry.models.project import Project
 from sentry.services.hybrid_cloud.user import RpcUser
@@ -32,10 +36,6 @@ from sentry.utils.http import absolute_uri
 
 logger = logging.getLogger(__name__)
 
-INCIDENTS_SNUBA_SUBSCRIPTION_TYPE = "incidents"
-INCIDENT_SNAPSHOT_BATCH_SIZE = 50
-SUBSCRIPTION_METRICS_LOGGER = "subscription_metrics_logger"
-
 
 @instrumented_task(
     name="sentry.incidents.tasks.send_subscriber_notifications",

+ 3 - 0
src/sentry/incidents/utils/constants.py

@@ -0,0 +1,3 @@
+INCIDENTS_SNUBA_SUBSCRIPTION_TYPE = "incidents"
+INCIDENT_SNAPSHOT_BATCH_SIZE = 50
+SUBSCRIPTION_METRICS_LOGGER = "subscription_metrics_logger"

+ 6 - 0
src/sentry/incidents/utils/types.py

@@ -1,4 +1,5 @@
 from datetime import datetime
+from enum import Enum
 from typing import Any, TypedDict
 
 
@@ -7,3 +8,8 @@ class QuerySubscriptionUpdate(TypedDict):
     subscription_id: str
     values: Any
     timestamp: datetime
+
+
+class AlertRuleActivationConditionType(Enum):
+    RELEASE_CREATION = 0
+    DEPLOY_CREATION = 1

+ 14 - 17
src/sentry/models/release.py

@@ -3,7 +3,6 @@ from __future__ import annotations
 import itertools
 import logging
 import re
-from collections import namedtuple
 from collections.abc import Mapping, Sequence
 from time import time
 from typing import ClassVar
@@ -46,7 +45,7 @@ from sentry.models.releases.constants import (
 )
 from sentry.models.releases.exceptions import ReleaseCommitError, UnsafeReleaseDeletion
 from sentry.models.releases.release_project import ReleaseProject
-from sentry.models.releases.util import ReleaseQuerySet, SemverFilter
+from sentry.models.releases.util import ReleaseQuerySet, SemverFilter, SemverVersion
 from sentry.services.hybrid_cloud.user import RpcUser
 from sentry.signals import issue_resolved
 from sentry.utils import metrics
@@ -60,12 +59,6 @@ from sentry.utils.strings import truncatechars
 logger = logging.getLogger(__name__)
 
 
-class SemverVersion(
-    namedtuple("SemverVersion", "major minor patch revision prerelease_case prerelease")
-):
-    pass
-
-
 class ReleaseStatus:
     OPEN = 0
     ARCHIVED = 1
@@ -477,6 +470,7 @@ class Release(Model):
                         organization_id=project.organization_id, version=version
                     )
 
+                # NOTE: `add_project` creates a ReleaseProject instance
                 release.add_project(project)
                 if not project.flags.has_releases:
                     project.flags.has_releases = True
@@ -689,15 +683,18 @@ class Release(Model):
             raise ReleaseCommitError
         with TimedRetryPolicy(10)(lock.acquire):
             start = time()
-            with atomic_transaction(
-                using=(
-                    router.db_for_write(type(self)),
-                    router.db_for_write(ReleaseCommit),
-                    router.db_for_write(Repository),
-                    router.db_for_write(CommitAuthor),
-                    router.db_for_write(Commit),
-                )
-            ), in_test_hide_transaction_boundary():
+            with (
+                atomic_transaction(
+                    using=(
+                        router.db_for_write(type(self)),
+                        router.db_for_write(ReleaseCommit),
+                        router.db_for_write(Repository),
+                        router.db_for_write(CommitAuthor),
+                        router.db_for_write(Commit),
+                    )
+                ),
+                in_test_hide_transaction_boundary(),
+            ):
                 # TODO(dcramer): would be good to optimize the logic to avoid these
                 # deletes but not overly important
                 ReleaseCommit.objects.filter(release=self).delete()

+ 34 - 2
src/sentry/models/releases/release_project.py

@@ -1,9 +1,10 @@
 from __future__ import annotations
 
 import logging
-from typing import ClassVar
+from typing import TYPE_CHECKING, ClassVar
 
 from django.db import models
+from django.utils import timezone
 
 from sentry import features
 from sentry.backup.scopes import RelocationScope
@@ -14,10 +15,16 @@ from sentry.db.models import (
     region_silo_only_model,
 )
 from sentry.db.models.manager import BaseManager
+from sentry.incidents.utils.types import AlertRuleActivationConditionType
 from sentry.tasks.relay import schedule_invalidate_project_config
 
 logger = logging.getLogger(__name__)
 
+if TYPE_CHECKING:
+    from sentry.models.project import Project
+    from sentry.models.release import Release
+    from sentry.snuba.models import QuerySubscription
+
 
 class ReleaseProjectModelManager(BaseManager["ReleaseProject"]):
     @staticmethod
@@ -33,8 +40,33 @@ class ReleaseProjectModelManager(BaseManager["ReleaseProject"]):
         ):
             schedule_invalidate_project_config(project_id=project.id, trigger=trigger)
 
-    def post_save(self, instance, **kwargs):
+    @staticmethod
+    def _subscribe_project_to_alert_rule(
+        project: Project, release: Release, trigger: str
+    ) -> list[QuerySubscription]:
+        """
+        TODO: potentially enable custom query_extra to be passed on ReleaseProject creation (on release/deploy)
+        NOTE: import AlertRule model here to avoid circular dependency
+        TODO: move once AlertRule has been split into separate subdirectory files
+        """
+        from sentry.incidents.models import AlertRule
+
+        query_extra = f"release:{release.version} AND event.timestamp:>{timezone.now().isoformat()}"
+        return AlertRule.objects.conditionally_subscribe_project_to_alert_rules(
+            project=project,
+            activation_condition=AlertRuleActivationConditionType.RELEASE_CREATION,
+            query_extra=query_extra,
+            trigger=trigger,
+        )
+
+    def post_save(self, instance, created, **kwargs):
         self._on_post(project=instance.project, trigger="releaseproject.post_save")
+        if created:
+            self._subscribe_project_to_alert_rule(
+                project=instance.project,
+                release=instance.release,
+                trigger="releaseproject.post_save",
+            )
 
     def post_delete(self, instance, **kwargs):
         self._on_post(project=instance.project, trigger="releaseproject.post_delete")

+ 7 - 0
src/sentry/models/releases/util.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import logging
+from collections import namedtuple
 from collections.abc import Sequence
 from dataclasses import dataclass
 
@@ -18,6 +19,12 @@ from sentry.utils.numbers import validate_bigint
 logger = logging.getLogger(__name__)
 
 
+class SemverVersion(
+    namedtuple("SemverVersion", "major minor patch revision prerelease_case prerelease")
+):
+    pass
+
+
 @dataclass
 class SemverFilter:
     operator: str

+ 1 - 1
src/sentry/snuba/models.py

@@ -112,7 +112,7 @@ class QuerySubscription(Model):
     date_updated = models.DateTimeField(default=timezone.now, null=True)
     query_extra = models.TextField(
         null=True
-    )  # additional query filters to attach to the query created in Snuba
+    )  # additional query filters to attach to the query created in Snuba such as datetime filters, or release/deploy tags
 
     objects: ClassVar[BaseManager[Self]] = BaseManager(
         cache_fields=("pk", "subscription_id"), cache_ttl=int(timedelta(hours=1).total_seconds())

Some files were not shown because too many files changed in this diff