feat(uptime): Add billing seat management for detector validators #102989

evanpurkhiser · 2025-11-07T21:03:03Z

Add billing seat assignment and removal to UptimeDomainCheckFailureValidator
to ensure uptime detectors created, updated, or deleted via the detector APIs
correctly handle billing seats.

Changes:

Assign seats when creating enabled detectors (gracefully disable if no seats)
Assign/remove seats when enabling/disabling detectors via updates
Remove seats immediately when deleting detectors
Raise validation errors when enabling fails due to no available seats

Also add comprehensive tests to verify all billing seat operations work
correctly across create, update, and delete operations.

Fixes NEW-527: Ensure CRUD / enable+disable detectors in new UI handles assigning / unassigning seats

linear · 2025-11-07T21:03:07Z

NEW-527 Ensure CRUD / enable+disable detectors in new UI handles assigning / unassigning seats

sentry · 2025-11-07T21:05:17Z

src/sentry/uptime/endpoints/validators.py

+        if detector.enabled:
+            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
+        else:
+            disable_uptime_detector(detector)
+
+        return detector


Bug: Detector creation ignores enabled=False in validated_data, always defaulting to True due to db_default.
_{Severity: CRITICAL | Confidence: 0.95}

🔍 Detailed Analysis

When creating a new detector via the API, the Detector model's enabled field defaults to True due to db_default=True. The BaseDetectorTypeValidator.create() method does not explicitly pass the enabled value from validated_data to the Detector constructor. Consequently, if a user requests {"enabled": False, ...}, the UptimeDomainCheckFailureValidator.create() method will incorrectly call enable_uptime_detector() because detector.enabled will always be True after creation, ignoring the user's intent.

💡 Suggested Fix

Ensure the enabled field from validated_data is explicitly passed to the Detector constructor in BaseDetectorTypeValidator.create() to override the database default when a value is provided.

🤖 Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: src/sentry/uptime/endpoints/validators.py#L469-L482 Potential issue: When creating a new detector via the API, the `Detector` model's `enabled` field defaults to `True` due to `db_default=True`. The `BaseDetectorTypeValidator.create()` method does not explicitly pass the `enabled` value from `validated_data` to the `Detector` constructor. Consequently, if a user requests `{"enabled": False, ...}`, the `UptimeDomainCheckFailureValidator.create()` method will incorrectly call `enable_uptime_detector()` because `detector.enabled` will always be `True` after creation, ignoring the user's intent.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

sentry · 2025-11-07T21:05:17Z

src/sentry/uptime/endpoints/validators.py

+
+        if was_enabled != enabled:
+            if enabled:
+                try:
+                    enable_uptime_detector(instance)
+                except UptimeMonitorNoSeatAvailable as err:
+                    raise serializers.ValidationError({"enabled": [err.result.reason]})
+            else:
+                disable_uptime_detector(instance)
+
        super().update(instance, validated_data)


Bug: Detector enabled state is toggled twice during update, once by child and once by parent class.
_{Severity: HIGH | Confidence: 0.95}

🔍 Detailed Analysis

In UptimeDomainCheckFailureValidator.update(), the detector's enabled state is toggled twice. First, the child class explicitly calls enable_uptime_detector() or disable_uptime_detector() based on the enabled state. Subsequently, super().update(instance, validated_data) is called. The parent class's update() method then re-processes the enabled field from validated_data and calls toggle_detector(), leading to redundant state changes. This can result in inconsistent states if the two toggle mechanisms have different behaviors.

💡 Suggested Fix

Remove the enabled field from validated_data before calling super().update() in UptimeDomainCheckFailureValidator.update() to prevent the parent class from re-processing it.

🤖 Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: src/sentry/uptime/endpoints/validators.py#L485-L498 Potential issue: In `UptimeDomainCheckFailureValidator.update()`, the detector's `enabled` state is toggled twice. First, the child class explicitly calls `enable_uptime_detector()` or `disable_uptime_detector()` based on the `enabled` state. Subsequently, `super().update(instance, validated_data)` is called. The parent class's `update()` method then re-processes the `enabled` field from `validated_data` and calls `toggle_detector()`, leading to redundant state changes. This can result in inconsistent states if the two toggle mechanisms have different behaviors.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

Yeah this is true, but it's not really a problem and I think it keeps this logic easier to maintain.

saponifi3d

lgtm from the workflow engine perspective, might want another review from someone with more context over the other changes.

saponifi3d · 2025-11-07T21:05:20Z

src/sentry/uptime/endpoints/validators.py

+        uptime_subscription = get_uptime_subscription(self.instance)
+        delete_uptime_subscription(uptime_subscription)
+
+        super().delete()


👀 yeah, this does feel a bit cleaner than having the hooks -- yay it's just one surface area now! 🎉

cursor · 2025-11-07T21:23:31Z

src/sentry/uptime/endpoints/validators.py

+            # assignment. The monitor will be created, but not enabled
+            pass
+
+        return detector


Bug: Disabled Features Still Activate

The create method unconditionally calls enable_uptime_detector regardless of the enabled value in validated_data. If a detector is created with enabled=False, it will still attempt to enable it and assign a seat, contradicting the user's intent and potentially consuming quota unnecessarily.

Add billing seat assignment and removal to UptimeDomainCheckFailureValidator to ensure uptime detectors created, updated, or deleted via the detector APIs correctly handle billing seats. Changes: - Assign seats when creating enabled detectors (gracefully disable if no seats) - Assign/remove seats when enabling/disabling detectors via updates - Remove seats immediately when deleting detectors - Raise validation errors when enabling fails due to no available seats Also add comprehensive tests to verify all billing seat operations work correctly across create, update, and delete operations. Fixes [NEW-527: Ensure CRUD / enable+disable detectors in new UI handles assigning / unassigning seats](https://linear.app/getsentry/issue/NEW-527/ensure-crud-enabledisable-detectors-in-new-ui-handles-assigning)

cursor · 2025-11-07T21:28:03Z

src/sentry/uptime/endpoints/validators.py

+                enable_uptime_detector(instance)
+            else:
+                disable_uptime_detector(instance)
+


Bug: Conflicting Updates Lead to Detector Instability

The update method calls enable_uptime_detector or disable_uptime_detector which update detector.enabled, then immediately calls super().update(validated_data) which also updates detector.enabled via toggle_detector. This double update creates a race condition and potential state inconsistency because toggle_detector also modifies detector.status, which may conflict with the status management in the uptime-specific enable/disable functions. The enabled field should be removed from validated_data before calling super().update() to prevent the base class from re-processing it.

brendanhsentry · 2025-11-07T21:35:05Z

src/sentry/uptime/endpoints/validators.py

+        detector = super().create(validated_data)
+
+        try:
+            enable_uptime_detector(detector, ensure_assignment=True)


Do we need to worry about mode != UptimeMonitorMode.AUTO_DETECTED_ONBOARDING and creating disabled Detectors? or are those not a possible for this?

Yeah not applicable here. These will always be created in manual mode.

brendanhsentry

nice looks good to me

evanpurkhiser requested a review from a team as a code owner November 7, 2025 21:03

evanpurkhiser requested review from a team and brendanhsentry November 7, 2025 21:03

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 7, 2025

sentry bot reviewed Nov 7, 2025

View reviewed changes

vercel bot deployed to Preview November 7, 2025 21:05 View deployment

saponifi3d approved these changes Nov 7, 2025

View reviewed changes

evanpurkhiser force-pushed the evanpurkhiser/feat-uptime-add-billing-seat-management-for-detector-validators branch from b333159 to 674fb06 Compare November 7, 2025 21:21

cursor bot reviewed Nov 7, 2025

View reviewed changes

vercel bot deployed to Preview November 7, 2025 21:24 View deployment

evanpurkhiser force-pushed the evanpurkhiser/feat-uptime-add-billing-seat-management-for-detector-validators branch from 674fb06 to 405c673 Compare November 7, 2025 21:26

cursor bot reviewed Nov 7, 2025

View reviewed changes

vercel bot deployed to Preview November 7, 2025 21:29 View deployment

brendanhsentry reviewed Nov 7, 2025

View reviewed changes

brendanhsentry approved these changes Nov 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat(uptime): Add billing seat management for detector validators #102989

feat(uptime): Add billing seat management for detector validators #102989

evanpurkhiser commented Nov 7, 2025

Uh oh!

linear bot commented Nov 7, 2025

Uh oh!

sentry bot Nov 7, 2025

Uh oh!

sentry bot Nov 7, 2025

Uh oh!

evanpurkhiser Nov 7, 2025

Uh oh!

saponifi3d left a comment

Uh oh!

saponifi3d Nov 7, 2025

Uh oh!

cursor bot Nov 7, 2025

Uh oh!

cursor bot Nov 7, 2025

Uh oh!

brendanhsentry Nov 7, 2025

Uh oh!

evanpurkhiser Nov 7, 2025

Uh oh!

brendanhsentry left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

feat(uptime): Add billing seat management for detector validators #102989

Are you sure you want to change the base?

feat(uptime): Add billing seat management for detector validators #102989

Conversation

evanpurkhiser commented Nov 7, 2025

Uh oh!

linear bot commented Nov 7, 2025

Uh oh!

sentry bot Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

sentry bot Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

evanpurkhiser Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

saponifi3d left a comment

Choose a reason for hiding this comment

Uh oh!

saponifi3d Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

cursor bot Nov 7, 2025

Choose a reason for hiding this comment

Bug: Disabled Features Still Activate

Uh oh!

cursor bot Nov 7, 2025

Choose a reason for hiding this comment

Bug: Conflicting Updates Lead to Detector Instability

Uh oh!

brendanhsentry Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

evanpurkhiser Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

brendanhsentry left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants