Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/sentry/uptime/detectors/result_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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__)
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 2 additions & 9 deletions src/sentry/uptime/detectors/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
48 changes: 26 additions & 22 deletions src/sentry/uptime/subscriptions/subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions tests/sentry/uptime/subscriptions/test_subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand All @@ -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"),
Expand Down
Loading