refs(alert_rules): Stop using query related fields on AlertRule and QuerySubscription. Remove unused columns from models, and unused Models (#18805)

This removes all query related fields from both `AlertRule` and `QuerySubscription`, since we can
now rely solely on `SnubaQuery`. This also removes the *Environment many to many tables, since these
are also unused. This migration only removes them from the state - we'll add another migration later to really remove the columns and unused tables.

As part of this pr we remove any final outstanding uses of these columns and tables as well.

migrations_lockfile.txt

@@ -10,7 +10,7 @@ auth: 0008_alter_user_username_max_length
 contenttypes: 0002_remove_content_type_name
 jira_ac: 0001_initial
 nodestore: 0001_initial
-sentry: 0076_alert_rules_disable_constraints
+sentry: 0077_alert_query_col_drop_state
 sessions: 0001_initial
 sites: 0002_alter_domain_unique
 social_auth: 0001_initial

src/sentry/api/endpoints/organization_incident_index.py

@@ -68,7 +68,7 @@ class OrganizationIncidentIndexEndpoint(OrganizationEndpoint):
 
         envs = self.get_environments(request, organization)
         if envs:
-            incidents = incidents.filter(alert_rule__environment__in=envs)
+            incidents = incidents.filter(alert_rule__snuba_query__environment__in=envs)
 
         query_status = request.GET.get("status")
         if query_status is not None:

src/sentry/incidents/endpoints/serializers.py

@@ -274,6 +274,12 @@ class AlertRuleSerializer(CamelSnakeModelSerializer):
     projects = serializers.ListField(child=ProjectField(), required=False)
     excluded_projects = serializers.ListField(child=ProjectField(), required=False)
     triggers = serializers.ListField(required=True)
+    query = serializers.CharField(required=True, allow_blank=True)
+    time_window = serializers.IntegerField(
+        required=True, min_value=1, max_value=int(timedelta(days=1).total_seconds() / 60)
+    )
+    threshold_period = serializers.IntegerField(default=1, min_value=1, max_value=20)
+    aggregation = serializers.IntegerField(required=False)
 
     class Meta:
         model = AlertRule
@@ -290,14 +296,6 @@ class AlertRuleSerializer(CamelSnakeModelSerializer):
             "triggers",
         ]
         extra_kwargs = {
-            "query": {"allow_blank": True, "required": True},
-            "threshold_period": {"default": 1, "min_value": 1, "max_value": 20},
-            "time_window": {
-                "min_value": 1,
-                "max_value": int(timedelta(days=1).total_seconds() / 60),
-                "required": True,
-            },
-            "aggregation": {"required": False},
             "name": {"min_length": 1, "max_length": 64},
             "include_all_projects": {"default": False},
         }

src/sentry/incidents/logic.py

@@ -14,7 +14,6 @@ from sentry.api.event_search import get_filter
 from sentry.incidents import tasks
 from sentry.incidents.models import (
     AlertRule,
-    AlertRuleEnvironment,
     AlertRuleExcludedProjects,
     AlertRuleStatus,
     AlertRuleTrigger,
@@ -35,7 +34,7 @@ from sentry.incidents.models import (
 )
 from sentry.models import Integration, Project
 from sentry.snuba.discover import resolve_discover_aliases
-from sentry.snuba.models import query_aggregation_to_snuba, QueryAggregations, QueryDatasets
+from sentry.snuba.models import query_aggregation_to_snuba, QueryDatasets
 from sentry.snuba.subscriptions import (
     aggregate_to_query_aggregation,
     bulk_create_snuba_subscriptions,
@@ -593,11 +592,6 @@ def create_alert_rule(
             organization=organization,
             snuba_query=snuba_query,
             name=name,
-            dataset=dataset.value,
-            query=query,
-            aggregation=aggregation.value,
-            time_window=time_window,
-            resolution=resolution,
             threshold_period=threshold_period,
             include_all_projects=include_all_projects,
         )
@@ -613,9 +607,6 @@ def create_alert_rule(
             ]
             AlertRuleExcludedProjects.objects.bulk_create(exclusions)
 
-        if environment:
-            AlertRuleEnvironment.objects.create(alert_rule=alert_rule, environment=environment)
-
         subscribe_projects_to_alert_rule(alert_rule, projects)
 
     return alert_rule
@@ -699,12 +690,10 @@ def update_alert_rule(
         updated_fields["name"] = name
     if query is not None:
         validate_alert_rule_query(query)
-        updated_query_fields["query"] = updated_fields["query"] = query
+        updated_query_fields["query"] = query
     if aggregation is not None:
-        updated_fields["aggregation"] = aggregation.value
         updated_query_fields["aggregation"] = aggregation
     if time_window:
-        updated_fields["time_window"] = time_window
         updated_query_fields["time_window"] = timedelta(minutes=time_window)
     if threshold_period:
         updated_fields["threshold_period"] = threshold_period
@@ -723,7 +712,7 @@ def update_alert_rule(
             # XXX: We use the alert rule aggregation here since currently we're
             # expecting the enum value to be passed.
             updated_query_fields.setdefault(
-                "aggregation", QueryAggregations(alert_rule.aggregation)
+                "aggregation", aggregate_to_query_aggregation[snuba_query.aggregate]
             )
             updated_query_fields.setdefault(
                 "time_window", timedelta(seconds=snuba_query.time_window)
@@ -798,17 +787,6 @@ def update_alert_rule(
         if deleted_subs:
             bulk_delete_snuba_subscriptions(deleted_subs)
 
-        if environment:
-            # Delete rows we don't have present in the updated data.
-            AlertRuleEnvironment.objects.filter(alert_rule=alert_rule).exclude(
-                environment=environment
-            ).delete()
-            AlertRuleEnvironment.objects.get_or_create(
-                alert_rule=alert_rule, environment=environment
-            )
-        else:
-            AlertRuleEnvironment.objects.filter(alert_rule=alert_rule).delete()
-
     return alert_rule
 
 

src/sentry/incidents/models.py

@@ -352,31 +352,6 @@ class AlertRuleManager(BaseManager):
             )
 
 
-class AlertRuleEnvironment(Model):
-    __core__ = True
-
-    environment = FlexibleForeignKey("sentry.Environment", db_constraint=False)
-    alert_rule = FlexibleForeignKey("sentry.AlertRule", db_constraint=False)
-
-    class Meta:
-        app_label = "sentry"
-        db_table = "sentry_alertruleenvironment"
-        unique_together = (("alert_rule", "environment"),)
-
-
-class AlertRuleQuerySubscription(Model):
-    __core__ = True
-
-    query_subscription = FlexibleForeignKey(
-        "sentry.QuerySubscription", db_constraint=False, unique=True
-    )
-    alert_rule = FlexibleForeignKey("sentry.AlertRule", db_constraint=False)
-
-    class Meta:
-        app_label = "sentry"
-        db_table = "sentry_alertrulequerysubscription"
-
-
 class AlertRuleExcludedProjects(Model):
     __core__ = True
 
@@ -398,26 +373,14 @@ class AlertRule(Model):
 
     organization = FlexibleForeignKey("sentry.Organization", null=True)
     snuba_query = FlexibleForeignKey("sentry.SnubaQuery", null=True, unique=True)
-    query_subscriptions = models.ManyToManyField(
-        "sentry.QuerySubscription", related_name="alert_rules", through=AlertRuleQuerySubscription
-    )
     excluded_projects = models.ManyToManyField(
         "sentry.Project", related_name="alert_rule_exclusions", through=AlertRuleExcludedProjects
     )
     name = models.TextField()
     status = models.SmallIntegerField(default=AlertRuleStatus.PENDING.value)
-    dataset = models.TextField(null=True)
-    query = models.TextField(null=True)
-    environment = models.ManyToManyField(
-        "sentry.Environment", related_name="alert_rule_environment", through=AlertRuleEnvironment
-    )
     # Determines whether we include all current and future projects from this
     # organization in this rule.
     include_all_projects = models.BooleanField(default=False)
-    # TODO: Remove this default after we migrate
-    aggregation = models.IntegerField(default=QueryAggregations.TOTAL.value, null=True)
-    time_window = models.IntegerField(null=True)
-    resolution = models.IntegerField(null=True)
     threshold_period = models.IntegerField()
     date_modified = models.DateTimeField(default=timezone.now)
     date_added = models.DateTimeField(default=timezone.now)

src/sentry/integrations/slack/utils.py

@@ -13,6 +13,7 @@ from sentry.api.fields.actor import Actor
 from sentry.incidents.logic import get_incident_aggregates
 from sentry.incidents.models import IncidentStatus, IncidentTrigger
 from sentry.snuba.models import QueryAggregations
+from sentry.snuba.subscriptions import aggregate_to_query_aggregation
 from sentry.utils import json
 from sentry.utils.assets import get_asset_url
 from sentry.utils.dates import to_timestamp
@@ -314,11 +315,12 @@ def build_incident_attachment(incident):
         status = "Critical"
         color = LEVEL_TO_COLOR["fatal"]
 
-    agg_text = QUERY_AGGREGATION_DISPLAY[alert_rule.aggregation]
+    query_aggregation = aggregate_to_query_aggregation[alert_rule.snuba_query.aggregate]
+    agg_text = QUERY_AGGREGATION_DISPLAY[query_aggregation.value]
 
     agg_value = (
         aggregates["count"]
-        if alert_rule.aggregation == QueryAggregations.TOTAL.value
+        if query_aggregation == QueryAggregations.TOTAL
         else aggregates["unique_users"]
     )
     time_window = alert_rule.snuba_query.time_window / 60
@@ -326,7 +328,7 @@ def build_incident_attachment(incident):
     text = "{} {} in the last {} minutes".format(agg_value, agg_text, time_window)
 
     if alert_rule.snuba_query.query != "":
-        text = text + "\nFilter: {}".format(alert_rule.query)
+        text = text + "\nFilter: {}".format(alert_rule.snuba_query.query)
 
     ts = incident.date_started
 

src/sentry/migrations/0077_alert_query_col_drop_state.py

@@ -0,0 +1,67 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.29 on 2020-05-13 22:23
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    # This flag is used to mark that a migration shouldn't be automatically run in
+    # production. We set this to True for operations that we think are risky and want
+    # someone from ops to run manually and monitor.
+    # General advice is that if in doubt, mark your migration as `is_dangerous`.
+    # Some things you should always mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that
+    #   they can be monitored. Since data migrations will now hold a transaction open
+    #   this is even more important.
+    # - Adding columns to highly active tables, even ones that are NULL.
+    is_dangerous = False
+
+    # This flag is used to decide whether to run this migration in a transaction or not.
+    # By default we prefer to run in a transaction, but for migrations where you want
+    # to `CREATE INDEX CONCURRENTLY` this needs to be set to False. Typically you'll
+    # want to create an index concurrently when adding one to an existing table.
+    atomic = True
+
+    dependencies = [("sentry", "0076_alert_rules_disable_constraints")]
+
+    operations = [
+        migrations.SeparateDatabaseAndState(
+            state_operations=[
+                migrations.AlterUniqueTogether(
+                    name="alertruleenvironment", unique_together=set([])
+                ),
+                migrations.RemoveField(model_name="alertruleenvironment", name="alert_rule"),
+                migrations.RemoveField(model_name="alertruleenvironment", name="environment"),
+                migrations.RemoveField(model_name="alertrulequerysubscription", name="alert_rule"),
+                migrations.RemoveField(
+                    model_name="alertrulequerysubscription", name="query_subscription"
+                ),
+                migrations.AlterUniqueTogether(
+                    name="querysubscriptionenvironment", unique_together=set([])
+                ),
+                migrations.RemoveField(
+                    model_name="querysubscriptionenvironment", name="environment"
+                ),
+                migrations.RemoveField(
+                    model_name="querysubscriptionenvironment", name="query_subscription"
+                ),
+                migrations.RemoveField(model_name="alertrule", name="aggregation"),
+                migrations.RemoveField(model_name="alertrule", name="dataset"),
+                migrations.RemoveField(model_name="alertrule", name="environment"),
+                migrations.RemoveField(model_name="alertrule", name="query"),
+                migrations.RemoveField(model_name="alertrule", name="query_subscriptions"),
+                migrations.RemoveField(model_name="alertrule", name="resolution"),
+                migrations.RemoveField(model_name="alertrule", name="time_window"),
+                migrations.RemoveField(model_name="querysubscription", name="aggregation"),
+                migrations.RemoveField(model_name="querysubscription", name="dataset"),
+                migrations.RemoveField(model_name="querysubscription", name="environments"),
+                migrations.RemoveField(model_name="querysubscription", name="query"),
+                migrations.RemoveField(model_name="querysubscription", name="resolution"),
+                migrations.RemoveField(model_name="querysubscription", name="time_window"),
+                migrations.DeleteModel(name="AlertRuleEnvironment"),
+                migrations.DeleteModel(name="AlertRuleQuerySubscription"),
+                migrations.DeleteModel(name="QuerySubscriptionEnvironment"),
+            ]
+        )
+    ]

src/sentry/snuba/models.py

@@ -25,19 +25,6 @@ class QueryDatasets(Enum):
     EVENTS = "events"
 
 
-class QuerySubscriptionEnvironment(Model):
-    __core__ = True
-
-    query_subscription = FlexibleForeignKey("sentry.QuerySubscription", db_constraint=False)
-    environment = FlexibleForeignKey("sentry.Environment", db_constraint=False)
-    date_added = models.DateTimeField(default=timezone.now)
-
-    class Meta:
-        app_label = "sentry"
-        db_table = "sentry_querysubscriptionenvironment"
-        unique_together = (("query_subscription", "environment"),)
-
-
 class SnubaQuery(Model):
     __core__ = True
 
@@ -64,18 +51,10 @@ class QuerySubscription(Model):
         DELETING = 3
 
     project = FlexibleForeignKey("sentry.Project", db_constraint=False)
-    environments = models.ManyToManyField(
-        "sentry.Environment", through=QuerySubscriptionEnvironment
-    )
     snuba_query = FlexibleForeignKey("sentry.SnubaQuery", null=True, related_name="subscriptions")
     type = models.TextField()
     status = models.SmallIntegerField(default=Status.ACTIVE.value)
     subscription_id = models.TextField(unique=True, null=True)
-    dataset = models.TextField(null=True)
-    query = models.TextField(null=True)
-    aggregation = models.IntegerField(default=0, null=True)
-    time_window = models.IntegerField(null=True)
-    resolution = models.IntegerField(null=True)
     date_added = models.DateTimeField(default=timezone.now)
 
     objects = BaseManager(

src/sentry/snuba/subscriptions.py

@@ -4,12 +4,7 @@ import logging
 
 from django.db import transaction
 
-from sentry.snuba.models import (
-    QueryAggregations,
-    QuerySubscription,
-    QuerySubscriptionEnvironment,
-    SnubaQuery,
-)
+from sentry.snuba.models import QueryAggregations, QuerySubscription, SnubaQuery
 from sentry.snuba.tasks import (
     create_subscription_in_snuba,
     delete_subscription_from_snuba,
@@ -118,17 +113,7 @@ def create_snuba_subscription(project, subscription_type, snuba_query):
         project=project,
         snuba_query=snuba_query,
         type=subscription_type,
-        dataset=snuba_query.dataset,
-        query=snuba_query.query,
-        aggregation=aggregate_to_query_aggregation[snuba_query.aggregate].value,
-        time_window=snuba_query.time_window,
-        resolution=snuba_query.resolution,
     )
-    if snuba_query.environment:
-        QuerySubscriptionEnvironment.objects.create(
-            query_subscription=subscription, environment=snuba_query.environment
-        )
-
     create_subscription_in_snuba.apply_async(
         kwargs={"query_subscription_id": subscription.id}, countdown=5
     )
@@ -163,20 +148,7 @@ def update_snuba_subscription(subscription, snuba_query):
     :return: The QuerySubscription representing the subscription
     """
     with transaction.atomic():
-        subscription.update(
-            status=QuerySubscription.Status.UPDATING.value,
-            query=snuba_query.query,
-            aggregation=aggregate_to_query_aggregation[snuba_query.aggregate].value,
-            time_window=snuba_query.time_window,
-            resolution=snuba_query.resolution,
-        )
-        QuerySubscriptionEnvironment.objects.filter(query_subscription=subscription).exclude(
-            environment=snuba_query.environment
-        ).delete()
-        if snuba_query.environment:
-            QuerySubscriptionEnvironment.objects.get_or_create(
-                query_subscription=subscription, environment=snuba_query.environment
-            )
+        subscription.update(status=QuerySubscription.Status.UPDATING.value)
 
         update_subscription_in_snuba.apply_async(
             kwargs={"query_subscription_id": subscription.id}, countdown=5

src/sentry/snuba/tasks.py

@@ -106,7 +106,9 @@ def delete_subscription_from_snuba(query_subscription_id):
         return
 
     if subscription.subscription_id is not None:
-        _delete_from_snuba(QueryDatasets(subscription.dataset), subscription.subscription_id)
+        _delete_from_snuba(
+            QueryDatasets(subscription.snuba_query.dataset), subscription.subscription_id
+        )
 
     subscription.delete()
 
@@ -125,7 +127,7 @@ def _create_in_snuba(subscription):
     )
     response = _snuba_pool.urlopen(
         "POST",
-        "/%s/subscriptions" % (subscription.dataset,),
+        "/%s/subscriptions" % (snuba_query.dataset,),
         body=json.dumps(
             {
                 "project_id": subscription.project_id,