Browse Source

feat(alert): Migrate certain conditions to be filters (#20572)

* migrated conditions

* use feature flag for project

* migration for project flag

* additional validation

* fix migration code

* fix test
David Wang 4 years ago
parent
commit
cb4629dd3c

+ 1 - 1
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: 0102_collect_relay_analytics
+sentry: 0103_project_has_alert_filters
 sessions: 0001_initial
 sites: 0002_alter_domain_unique
 social_auth: 0001_initial

+ 6 - 0
src/sentry/api/endpoints/project_rules_configuration.py

@@ -1,6 +1,8 @@
 from __future__ import absolute_import
 
+from sentry import features
 from sentry.api.bases.project import ProjectEndpoint
+from sentry.constants import SENTRY_RULES_WITH_MIGRATED_FILTERS
 from sentry.rules import rules
 from rest_framework.response import Response
 
@@ -15,9 +17,13 @@ class ProjectRulesConfigurationEndpoint(ProjectEndpoint):
         condition_list = []
         filter_list = []
 
+        project_has_filters = features.has("projects:alert-filters", project)
         # TODO: conditions need to be based on actions
         for rule_type, rule_cls in rules:
             node = rule_cls(project)
+            # skip over conditions if they are not in the migrated set for a project with alert-filters
+            if project_has_filters and node.id not in SENTRY_RULES_WITH_MIGRATED_FILTERS:
+                continue
             context = {"id": node.id, "label": node.label, "enabled": node.is_enabled()}
             if hasattr(node, "prompt"):
                 context["prompt"] = node.prompt

+ 18 - 1
src/sentry/api/serializers/rest_framework/rule.py

@@ -4,6 +4,8 @@ import six
 
 from rest_framework import serializers
 
+from sentry import features
+from sentry.constants import MIGRATED_CONDITIONS
 from sentry.models import Environment
 from sentry.rules import rules
 
@@ -115,7 +117,22 @@ class RuleSerializer(serializers.Serializer):
             if not filter_match:
                 raise serializers.ValidationError(
                     {
-                        "filterMatch": u"Must select a filter match (all, any, none) if filters are supplied"
+                        "filterMatch": u"Must select a filter match (all, any, none) if filters are supplied."
+                    }
+                )
+
+        # ensure that if a user has alert-filters enabled, they do not use old conditions
+        project = self.context["project"]
+        conditions = attrs.get("conditions", tuple())
+        project_has_filters = features.has("projects:alert-filters", project)
+        if project_has_filters:
+            old_conditions = [
+                condition for condition in conditions if condition["id"] in MIGRATED_CONDITIONS
+            ]
+            if old_conditions:
+                raise serializers.ValidationError(
+                    {
+                        "conditions": u"Conditions evaluating an event attribute, tag, or level are outdated please use an appropriate filter instead."
                     }
                 )
 

+ 16 - 0
src/sentry/constants.py

@@ -233,6 +233,22 @@ SENTRY_RULES = (
     "sentry.rules.filters.issue_occurrences.IssueOccurrencesFilter",
     "sentry.rules.filters.assigned_to.AssignedToFilter",
     "sentry.rules.filters.latest_release.LatestReleaseFilter",
+    # The following filters are duplicates of their respective conditions and are conditionally shown if the user has issue alert-filters
+    "sentry.rules.filters.event_attribute.EventAttributeFilter",
+    "sentry.rules.filters.tagged_event.TaggedEventFilter",
+    "sentry.rules.filters.level.LevelFilter",
+)
+
+MIGRATED_CONDITIONS = frozenset(
+    [
+        "sentry.rules.conditions.tagged_event.TaggedEventCondition",
+        "sentry.rules.conditions.event_attribute.EventAttributeCondition",
+        "sentry.rules.conditions.level.LevelCondition",
+    ]
+)
+
+SENTRY_RULES_WITH_MIGRATED_FILTERS = frozenset(
+    [rule for rule in SENTRY_RULES if rule not in MIGRATED_CONDITIONS]
 )
 
 # methods as defined by http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html + PATCH

+ 1 - 0
src/sentry/features/__init__.py

@@ -105,6 +105,7 @@ default_manager.add("organizations:invite-members-rate-limits", OrganizationFeat
 #       them alphabetically! The order features are registered is not important.
 
 # Project scoped features
+default_manager.add("projects:alert-filters", ProjectFeature)  # NOQA
 default_manager.add("projects:custom-inbound-filters", ProjectFeature)  # NOQA
 default_manager.add("projects:data-forwarding", ProjectFeature)  # NOQA
 default_manager.add("projects:discard-groups", ProjectFeature)  # NOQA

+ 42 - 0
src/sentry/migrations/0103_project_has_alert_filters.py

@@ -0,0 +1,42 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.29 on 2020-09-16 02:03
+from __future__ import unicode_literals
+
+import bitfield.models
+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', '0102_collect_relay_analytics'),
+    ]
+
+    operations = [
+        migrations.SeparateDatabaseAndState(
+            state_operations=[
+                migrations.AlterField(
+                    model_name='project',
+                    name='flags',
+                    field=bitfield.models.BitField(((b'has_releases', b'This Project has sent release data'), (b'has_issue_alerts_targeting', b'This Project has issue alerts targeting'), (b'has_transactions', b'This Project has sent transactions'), (b'has_alert_filters', b'This Project has filters')), default=10, null=True),
+                ),
+            ]
+        )
+    ]

+ 2 - 1
src/sentry/models/project.py

@@ -107,8 +107,9 @@ class Project(Model, PendingDeletionMixin):
             ("has_releases", "This Project has sent release data"),
             ("has_issue_alerts_targeting", "This Project has issue alerts targeting"),
             ("has_transactions", "This Project has sent transactions"),
+            ("has_alert_filters", "This Project has filters"),
         ),
-        default=2,
+        default=10,
         null=True,
     )
 

+ 7 - 0
src/sentry/rules/filters/event_attribute.py

@@ -0,0 +1,7 @@
+from __future__ import absolute_import
+
+from sentry.rules.conditions.event_attribute import EventAttributeCondition
+
+
+class EventAttributeFilter(EventAttributeCondition):
+    rule_type = "filter/event"

+ 7 - 0
src/sentry/rules/filters/level.py

@@ -0,0 +1,7 @@
+from __future__ import absolute_import
+
+from sentry.rules.conditions.level import LevelCondition
+
+
+class LevelFilter(LevelCondition):
+    rule_type = "filter/event"

+ 7 - 0
src/sentry/rules/filters/tagged_event.py

@@ -0,0 +1,7 @@
+from __future__ import absolute_import
+
+from sentry.rules.conditions.tagged_event import TaggedEventCondition
+
+
+class TaggedEventFilter(TaggedEventCondition):
+    rule_type = "filter/event"

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