diff --git a/src/sentry/uptime/detectors/result_handler.py b/src/sentry/uptime/detectors/result_handler.py index c73c0a8c9c0b4a..f46f7f4e9f04b8 100644 --- a/src/sentry/uptime/detectors/result_handler.py +++ b/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, diff --git a/src/sentry/uptime/detectors/tasks.py b/src/sentry/uptime/detectors/tasks.py index 54d1297aebc989..31b6bbab2b89d7 100644 --- a/src/sentry/uptime/detectors/tasks.py +++ b/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 diff --git a/src/sentry/uptime/subscriptions/subscriptions.py b/src/sentry/uptime/subscriptions/subscriptions.py index e4996ea632f6b4..8dc975e5413277 100644 --- a/src/sentry/uptime/subscriptions/subscriptions.py +++ b/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 diff --git a/tests/sentry/uptime/subscriptions/test_subscriptions.py b/tests/sentry/uptime/subscriptions/test_subscriptions.py index ba097c9298de17..7323a571d351a4 100644 --- a/tests/sentry/uptime/subscriptions/test_subscriptions.py +++ b/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"),