Merge 35ab3a3 into 37b9207

evanpurkhiser · web-flow · commit be91152172ba · 2026-03-16T19:36:22.000Z
diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt
@@ -39,6 +39,6 @@ social_auth: 0003_social_auth_json_field
 
 tempest: 0003_use_encrypted_char_field
 
-uptime: 0054_delete_bad_assertions
+uptime: 0055_backfill_2xx_status_assertion
 
 workflow_engine: 0110_owner_team_break_fk
diff --git a/src/sentry/uptime/migrations/0055_backfill_2xx_status_assertion.py b/src/sentry/uptime/migrations/0055_backfill_2xx_status_assertion.py
@@ -0,0 +1,95 @@
+# Generated by Django 5.2.11 on 2026-02-20 10:53
+import copy
+import logging
+from typing import Any
+
+from django.db import migrations
+from django.db.backends.base.schema import BaseDatabaseSchemaEditor
+from django.db.migrations.state import StateApps
+
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.utils.query import RangeQuerySetWrapperWithProgressBar
+
+logger = logging.getLogger(__name__)
+
+DEFAULT_STATUS_CODE_CHECKS = [
+    {"op": "status_code_check", "operator": {"cmp": "greater_than"}, "value": 199},
+    {"op": "status_code_check", "operator": {"cmp": "less_than"}, "value": 300},
+]
+
+
+def _has_any_status_code_check(node: dict[str, Any]) -> bool:
+    op = node.get("op")
+    if op == "status_code_check":
+        return True
+    elif op in ("and", "or"):
+        return any(_has_any_status_code_check(child) for child in node.get("children", []))
+    elif op == "not":
+        operand = node.get("operand")
+        return _has_any_status_code_check(operand) if operand else False
+    return False
+
+
+def ensure_2xx_status_checks(
+    assertion: dict[str, Any] | None,
+) -> tuple[dict[str, Any], bool]:
+    """
+    Ensure the assertion contains at least one status_code_check. If none exists,
+    add the default >199 and <300 pair. Returns (assertion, was_modified).
+    """
+    if not assertion or not assertion.get("root"):
+        return {"root": {"op": "and", "children": copy.deepcopy(DEFAULT_STATUS_CODE_CHECKS)}}, True
+
+    root = assertion["root"]
+    if _has_any_status_code_check(root):
+        return assertion, False
+
+    # No status code check found — add the pair
+    if root.get("op") == "and":
+        new_children = list(root.get("children", [])) + copy.deepcopy(DEFAULT_STATUS_CODE_CHECKS)
+        return {"root": {**root, "children": new_children}}, True
+    else:
+        # Root is not an "and" group — wrap it
+        return {
+            "root": {"op": "and", "children": [root, *copy.deepcopy(DEFAULT_STATUS_CODE_CHECKS)]}
+        }, True
+
+
+def backfill_2xx_status_assertion(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None:
+    UptimeSubscription = apps.get_model("uptime", "UptimeSubscription")
+
+    subscriptions = UptimeSubscription.objects.all()
+
+    for subscription in RangeQuerySetWrapperWithProgressBar(subscriptions):
+        new_assertion, was_modified = ensure_2xx_status_checks(subscription.assertion)
+        if was_modified:
+            subscription.assertion = new_assertion
+            subscription.save(update_fields=["assertion"])
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production.
+    # This should only be used for operations where it's safe to run the migration after your
+    # code has deployed. So this should not be used for most operations that alter the schema
+    # of a table.
+    # Here are some things that make sense to mark as post deployment:
+    # - Large data migrations. Typically we want these to be run manually so that they can be
+    #   monitored and not block the deploy for a long period of time while they run.
+    # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
+    #   run this outside deployments so that we don't block them. Note that while adding an index
+    #   is a schema change, it's completely safe to run the operation after the code has deployed.
+    # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment
+
+    is_post_deployment = True
+
+    dependencies = [
+        ("uptime", "0054_delete_bad_assertions"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            backfill_2xx_status_assertion,
+            migrations.RunPython.noop,
+            hints={"tables": ["uptime_uptimesubscription"]},
+        )
+    ]
diff --git a/tests/sentry/uptime/migrations/test_0055_backfill_2xx_status_assertion.py b/tests/sentry/uptime/migrations/test_0055_backfill_2xx_status_assertion.py
@@ -0,0 +1,245 @@
+import importlib
+import uuid
+
+import pytest
+
+from sentry.testutils.cases import TestMigrations
+
+migration = importlib.import_module("sentry.uptime.migrations.0055_backfill_2xx_status_assertion")
+ensure_2xx_status_checks = migration.ensure_2xx_status_checks
+
+GT_199 = {"op": "status_code_check", "operator": {"cmp": "greater_than"}, "value": 199}
+LT_300 = {"op": "status_code_check", "operator": {"cmp": "less_than"}, "value": 300}
+
+
+class TestEnsure2xxStatusChecks:
+    def test_none_assertion(self) -> None:
+        result, was_modified = ensure_2xx_status_checks(None)
+        assert was_modified
+        assert result == {"root": {"op": "and", "children": [GT_199, LT_300]}}
+
+    def test_empty_dict(self) -> None:
+        result, was_modified = ensure_2xx_status_checks({})
+        assert was_modified
+        assert result == {"root": {"op": "and", "children": [GT_199, LT_300]}}
+
+    def test_already_has_status_code_check_at_root(self) -> None:
+        assertion = {
+            "root": {"op": "status_code_check", "operator": {"cmp": "equals"}, "value": 200}
+        }
+        result, was_modified = ensure_2xx_status_checks(assertion)
+        assert not was_modified
+        assert result == assertion
+
+    def test_already_has_status_code_check_nested_in_and(self) -> None:
+        assertion = {
+            "root": {
+                "op": "and",
+                "children": [
+                    {"op": "status_code_check", "operator": {"cmp": "equals"}, "value": 200},
+                    {
+                        "op": "json_path",
+                        "operator": {"cmp": "equals"},
+                        "value": "$.status",
+                        "operand": {"jsonpath_op": "literal", "value": "ok"},
+                    },
+                ],
+            }
+        }
+        result, was_modified = ensure_2xx_status_checks(assertion)
+        assert not was_modified
+        assert result == assertion
+
+    def test_already_has_status_code_check_nested_in_or(self) -> None:
+        assertion = {
+            "root": {
+                "op": "or",
+                "children": [
+                    {"op": "status_code_check", "operator": {"cmp": "equals"}, "value": 200},
+                    {"op": "status_code_check", "operator": {"cmp": "equals"}, "value": 204},
+                ],
+            }
+        }
+        result, was_modified = ensure_2xx_status_checks(assertion)
+        assert not was_modified
+        assert result == assertion
+
+    def test_already_has_status_code_check_inside_not(self) -> None:
+        assertion = {
+            "root": {
+                "op": "not",
+                "operand": {"op": "status_code_check", "operator": {"cmp": "equals"}, "value": 404},
+            }
+        }
+        result, was_modified = ensure_2xx_status_checks(assertion)
+        assert not was_modified
+        assert result == assertion
+
+    def test_already_has_status_code_check_deeply_nested(self) -> None:
+        assertion = {
+            "root": {
+                "op": "and",
+                "children": [
+                    {
+                        "op": "or",
+                        "children": [
+                            {
+                                "op": "status_code_check",
+                                "operator": {"cmp": "equals"},
+                                "value": 200,
+                            },
+                            {
+                                "op": "status_code_check",
+                                "operator": {"cmp": "equals"},
+                                "value": 204,
+                            },
+                        ],
+                    }
+                ],
+            }
+        }
+        result, was_modified = ensure_2xx_status_checks(assertion)
+        assert not was_modified
+        assert result == assertion
+
+    def test_no_status_code_check_and_root(self) -> None:
+        json_path_check = {
+            "op": "json_path",
+            "operator": {"cmp": "equals"},
+            "value": "$.status",
+            "operand": {"jsonpath_op": "literal", "value": "ok"},
+        }
+        assertion = {"root": {"op": "and", "children": [json_path_check]}}
+        result, was_modified = ensure_2xx_status_checks(assertion)
+        assert was_modified
+        assert result == {"root": {"op": "and", "children": [json_path_check, GT_199, LT_300]}}
+
+    def test_no_status_code_check_non_and_root(self) -> None:
+        json_path_check = {
+            "op": "json_path",
+            "operator": {"cmp": "equals"},
+            "value": "$.status",
+            "operand": {"jsonpath_op": "literal", "value": "ok"},
+        }
+        assertion = {"root": json_path_check}
+        result, was_modified = ensure_2xx_status_checks(assertion)
+        assert was_modified
+        assert result == {"root": {"op": "and", "children": [json_path_check, GT_199, LT_300]}}
+
+
+@pytest.mark.skip(reason="TestMigrations is slow; run explicitly")
+class Backfill2xxStatusAssertionMigrationTest(TestMigrations):
+    migrate_from = "0054_delete_bad_assertions"
+    migrate_to = "0055_backfill_2xx_status_assertion"
+    app = "uptime"
+
+    def setup_initial_state(self) -> None:
+        self.organization = self.create_organization(name="test-org")
+        self.project = self.create_project(organization=self.organization)
+
+        self.null_assertion_sub = self.create_uptime_subscription(
+            subscription_id=uuid.uuid4().hex,
+            interval_seconds=300,
+            region_slugs=["default"],
+        )
+
+        self.existing_status_code_sub = self.create_uptime_subscription(
+            subscription_id=uuid.uuid4().hex,
+            interval_seconds=300,
+            region_slugs=["default"],
+            assertion={
+                "root": {
+                    "op": "and",
+                    "children": [
+                        {"op": "status_code_check", "operator": {"cmp": "equals"}, "value": 200}
+                    ],
+                }
+            },
+        )
+
+        self.json_path_only_sub = self.create_uptime_subscription(
+            subscription_id=uuid.uuid4().hex,
+            interval_seconds=300,
+            region_slugs=["default"],
+            assertion={
+                "root": {
+                    "op": "and",
+                    "children": [
+                        {
+                            "op": "json_path",
+                            "operator": {"cmp": "equals"},
+                            "value": "$.status",
+                            "operand": {"jsonpath_op": "literal", "value": "ok"},
+                        }
+                    ],
+                }
+            },
+        )
+
+        self.non_and_root_sub = self.create_uptime_subscription(
+            subscription_id=uuid.uuid4().hex,
+            interval_seconds=300,
+            region_slugs=["default"],
+            assertion={
+                "root": {
+                    "op": "json_path",
+                    "operator": {"cmp": "equals"},
+                    "value": "$.status",
+                    "operand": {"jsonpath_op": "literal", "value": "ok"},
+                }
+            },
+        )
+
+    def test_migration(self) -> None:
+        # null assertion gets the default >199 AND <300 pair
+        self.null_assertion_sub.refresh_from_db()
+        assert self.null_assertion_sub.assertion == {
+            "root": {"op": "and", "children": [GT_199, LT_300]}
+        }
+
+        # existing status_code_check is left untouched
+        self.existing_status_code_sub.refresh_from_db()
+        assert self.existing_status_code_sub.assertion == {
+            "root": {
+                "op": "and",
+                "children": [
+                    {"op": "status_code_check", "operator": {"cmp": "equals"}, "value": 200}
+                ],
+            }
+        }
+
+        # json_path only — pair appended, existing child preserved
+        self.json_path_only_sub.refresh_from_db()
+        assert self.json_path_only_sub.assertion == {
+            "root": {
+                "op": "and",
+                "children": [
+                    {
+                        "op": "json_path",
+                        "operator": {"cmp": "equals"},
+                        "value": "$.status",
+                        "operand": {"jsonpath_op": "literal", "value": "ok"},
+                    },
+                    GT_199,
+                    LT_300,
+                ],
+            }
+        }
+
+        # non-and root — wrapped in a new and group
+        self.non_and_root_sub.refresh_from_db()
+        assert self.non_and_root_sub.assertion == {
+            "root": {
+                "op": "and",
+                "children": [
+                    {
+                        "op": "json_path",
+                        "operator": {"cmp": "equals"},
+                        "value": "$.status",
+                        "operand": {"jsonpath_op": "literal", "value": "ok"},
+                    },
+                    GT_199,
+                    LT_300,
+                ],
+            }
+        }
