fix(uptime): Don't consume a seat or create audit log when onboarding an auto detected monitor

Users don't see onboarding monitors, and so they shouldn't consume a quota seat or create an audit log. This pr moves this to the point where the monitor graduates from onboarding.

Author: wedamija

src/sentry/uptime/detectors/result_handler.py

@@ -9,6 +9,7 @@
     CheckResult,
 )
 
+from sentry import audit_log
 from sentry.uptime.detectors.ranking import _get_cluster
 from sentry.uptime.detectors.tasks import set_failed_url
 from sentry.uptime.models import UptimeSubscription, get_project_subscription
@@ -18,6 +19,7 @@
 )
 from sentry.uptime.types import ProjectUptimeSubscriptionMode
 from sentry.utils import metrics
+from sentry.utils.audit import create_system_audit_entry
 from sentry.workflow_engine.models.detector import Detector
 
 logger = logging.getLogger(__name__)
@@ -90,6 +92,13 @@ def handle_onboarding_result(
                 interval_seconds=int(AUTO_DETECTED_ACTIVE_SUBSCRIPTION_INTERVAL.total_seconds()),
                 mode=ProjectUptimeSubscriptionMode.AUTO_DETECTED_ACTIVE,
             )
+            create_system_audit_entry(
+                organization=detector.project.organization,
+                target_object=project_subscription.id,
+                event=audit_log.get_event_id("UPTIME_MONITOR_ADD"),
+                data=project_subscription.get_audit_log_data(),
+            )
+
             metrics.incr(
                 "uptime.result_processor.autodetection.graduated_onboarding",
                 sample_rate=1.0,

src/sentry/uptime/detectors/tasks.py

@@ -7,7 +7,7 @@
 from dateutil.parser import parse as parse_datetime
 from django.utils import timezone
 
-from sentry import audit_log, features
+from sentry import features
 from sentry.locks import locks
 from sentry.models.organization import Organization
 from sentry.models.project import Project
@@ -34,7 +34,6 @@
 )
 from sentry.uptime.types import ProjectUptimeSubscriptionMode
 from sentry.utils import metrics
-from sentry.utils.audit import create_system_audit_entry
 from sentry.utils.hashlib import md5_text
 from sentry.utils.locking import UnableToAcquireLock
 
@@ -240,16 +239,10 @@ def process_candidate_url(
         "organizations:uptime-automatic-subscription-creation", project.organization
     ) and features.has("organizations:uptime", project.organization):
         # If we hit this point, then the url looks worth monitoring. Create an uptime subscription in monitor mode.
-        uptime_monitor = monitor_url_for_project(project, url)
+        monitor_url_for_project(project, url)
         # Disable auto-detection on this project and organization now that we've successfully found a hostname
         project.update_option("sentry:uptime_autodetection", False)
         project.organization.update_option("sentry:uptime_autodetection", False)
-        create_system_audit_entry(
-            organization=project.organization,
-            target_object=uptime_monitor.id,
-            event=audit_log.get_event_id("UPTIME_MONITOR_ADD"),
-            data=uptime_monitor.get_audit_log_data(),
-        )
 
     metrics.incr("uptime.detectors.candidate_url.succeeded", sample_rate=1.0)
     return True

src/sentry/uptime/subscriptions/subscriptions.py

@@ -244,20 +244,22 @@ def create_project_uptime_subscription(
         )
         detector = create_detector_from_project_subscription(uptime_monitor)
 
-        # Update status. This may have the side effect of removing or creating a
-        # remote subscription. When a new monitor is created we will ensure seat
-        # assignment, which may cause the monitor to be disabled if there are no
-        # available seat assignments.
-        match status:
-            case ObjectStatus.ACTIVE:
-                try:
-                    enable_uptime_detector(detector, ensure_assignment=True)
-                except UptimeMonitorNoSeatAvailable:
-                    # No need to do anything if we failed to handle seat
-                    # assignment. The monitor will be created, but not enabled
-                    pass
-            case ObjectStatus.DISABLED:
-                disable_uptime_detector(detector)
+        # Don't consume a seat if we're still in onboarding mode
+        if mode != ProjectUptimeSubscriptionMode.AUTO_DETECTED_ONBOARDING:
+            # Update status. This may have the side effect of removing or creating a
+            # remote subscription. When a new monitor is created we will ensure seat
+            # assignment, which may cause the monitor to be disabled if there are no
+            # available seat assignments.
+            match status:
+                case ObjectStatus.ACTIVE:
+                    try:
+                        enable_uptime_detector(detector, ensure_assignment=True)
+                    except UptimeMonitorNoSeatAvailable:
+                        # No need to do anything if we failed to handle seat
+                        # assignment. The monitor will be created, but not enabled
+                        pass
+                case ObjectStatus.DISABLED:
+                    disable_uptime_detector(detector)
 
     # ProjectUptimeSubscription may have been updated as part of
     # {enable,disable}_uptime_detector
@@ -333,14 +335,16 @@ def update_project_uptime_subscription(
             },
         )
 
-        # Update status. This may have the side effect of removing or creating a
-        # remote subscription. Will raise a UptimeMonitorNoSeatAvailable if seat
-        # assignment fails.
-        match status:
-            case ObjectStatus.DISABLED:
-                disable_uptime_detector(detector)
-            case ObjectStatus.ACTIVE:
-                enable_uptime_detector(detector)
+        # Don't consume a seat if we're still in onboarding mode
+        if mode != ProjectUptimeSubscriptionMode.AUTO_DETECTED_ONBOARDING:
+            # Update status. This may have the side effect of removing or creating a
+            # remote subscription. Will raise a UptimeMonitorNoSeatAvailable if seat
+            # assignment fails.
+            match status:
+                case ObjectStatus.DISABLED:
+                    disable_uptime_detector(detector)
+                case ObjectStatus.ACTIVE:
+                    enable_uptime_detector(detector)
 
     # ProjectUptimeSubscription may have been updated as part of
     # {enable,disable}_uptime_detector

tests/sentry/uptime/subscriptions/test_subscriptions.py

@@ -361,6 +361,19 @@ def test_status_disable(self, mock_disable_uptime_detector):
         )
         mock_disable_uptime_detector.assert_called()
 
+    @mock.patch("sentry.uptime.subscriptions.subscriptions.disable_uptime_detector")
+    def test_status_disable_not_called_onboarding(self, mock_disable_uptime_detector):
+        create_project_uptime_subscription(
+            self.project,
+            self.environment,
+            url="https://sentry.io",
+            interval_seconds=3600,
+            timeout_ms=1000,
+            mode=ProjectUptimeSubscriptionMode.AUTO_DETECTED_ONBOARDING,
+            status=ObjectStatus.DISABLED,
+        )
+        mock_disable_uptime_detector.assert_not_called()
+
     @mock.patch("sentry.uptime.subscriptions.subscriptions.enable_uptime_detector")
     def test_status_enable(self, mock_enable_uptime_detector):
         with self.tasks():
@@ -377,6 +390,20 @@ def test_status_enable(self, mock_enable_uptime_detector):
             assert detector
             mock_enable_uptime_detector.assert_called_with(detector, ensure_assignment=True)
 
+    @mock.patch("sentry.uptime.subscriptions.subscriptions.enable_uptime_detector")
+    def test_status_enable_not_called_onboarding(self, mock_enable_uptime_detector):
+        with self.tasks():
+            create_project_uptime_subscription(
+                self.project,
+                self.environment,
+                url="https://sentry.io",
+                interval_seconds=3600,
+                timeout_ms=1000,
+                mode=ProjectUptimeSubscriptionMode.AUTO_DETECTED_ONBOARDING,
+                status=ObjectStatus.ACTIVE,
+            )
+            mock_enable_uptime_detector.assert_not_called()
+
     @mock.patch(
         "sentry.quotas.backend.check_assign_seat",
         return_value=SeatAssignmentResult(assignable=False, reason="Testing"),