Skip to content
Open
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
17 changes: 17 additions & 0 deletions src/sentry/incidents/metric_issue_detector.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from sentry.constants import ObjectStatus
from sentry.incidents.logic import enable_disable_subscriptions
from sentry.incidents.models.alert_rule import AlertRuleDetectionType
from sentry.incidents.utils.subscription_limits import is_metric_subscription_allowed
from sentry.relay.config.metric_extraction import on_demand_metrics_feature_flags
from sentry.search.eap.trace_metrics.validator import validate_trace_metrics_aggregate
from sentry.seer.anomaly_detection.delete_rule import delete_data_in_seer_for_detector
Expand Down Expand Up @@ -225,9 +226,25 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]:

if "data_sources" in attrs:
self.validate_eap_rule(attrs)
self._validate_metric_subscription_allowed(attrs)

return attrs

def _validate_metric_subscription_allowed(self, attrs: dict[str, Any]) -> None:
organization = self.context.get("organization")
if organization is None:
raise serializers.ValidationError("Missing organization context")

for data_source in attrs.get("data_sources", []):
dataset = data_source.get("dataset")
if dataset is not None and not is_metric_subscription_allowed(
dataset.value, organization
):
raise serializers.ValidationError(
f"Your organization does not have access to the {dataset.value} dataset "
"for metric alert subscriptions."
)

def _validate_transaction_dataset_deprecation(self, dataset: Dataset) -> None:
organization = self.context.get("organization")
if organization is None:
Expand Down
28 changes: 28 additions & 0 deletions src/sentry/incidents/utils/subscription_limits.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@
from sentry.models.organization import Organization
from sentry.snuba.dataset import Dataset

# Complete set of feature flags checked by is_metric_subscription_allowed.
# Enabling all of these allows every gated dataset. Used by tests to avoid
# coupling to individual dataset→feature mappings.
METRIC_SUBSCRIPTION_FEATURE_FLAGS: dict[str, bool] = {
"organizations:incidents": True,
"organizations:performance-view": True,
"organizations:visibility-explore-view": True,
"organizations:on-demand-metrics-extraction": True,
}


def is_metric_subscription_allowed(dataset: str, organization: Organization) -> bool:
"""
Expand All @@ -32,6 +42,24 @@ def is_metric_subscription_allowed(dataset: str, organization: Organization) ->
return True


def get_disallowed_metric_datasets(organization: Organization) -> list[str]:
"""
Return dataset values that are NOT allowed for metric alert subscriptions
in this organization. Used to filter out metric detectors whose subscriptions
the organization is no longer entitled to (e.g. after a plan downgrade).
"""
disallowed = []
for dataset in [
Dataset.Events,
Dataset.Transactions,
Dataset.EventsAnalyticsPlatform,
Dataset.PerformanceMetrics,
]:
if not is_metric_subscription_allowed(dataset.value, organization):
disallowed.append(dataset.value)
return disallowed


def get_max_metric_alert_subscriptions(organization: Organization) -> int:
if organization.id in options.get("metric_alerts.extended_max_subscriptions_orgs") and (
extended_max_specs := options.get("metric_alerts.extended_max_subscriptions")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from sentry.constants import ObjectStatus
from sentry.models.organization import Organization
from sentry.utils.auth import AuthenticatedHttpRequest
from sentry.workflow_engine.endpoints.utils.filters import exclude_disallowed_metric_detectors
from sentry.workflow_engine.models import Detector


Expand Down Expand Up @@ -59,7 +60,7 @@ def get(self, request: AuthenticatedHttpRequest, organization: Organization) ->
}
return self.respond(empty_response)

queryset = Detector.objects.with_type_filters().filter(
base_queryset = Detector.objects.with_type_filters().filter(
status=ObjectStatus.ACTIVE,
project__organization_id=organization.id,
project_id__in=filter_params["project_id"],
Expand All @@ -68,7 +69,9 @@ def get(self, request: AuthenticatedHttpRequest, organization: Organization) ->
# Filter by detector types if specified
detector_types = request.GET.getlist("type")
if detector_types:
queryset = queryset.filter(type__in=detector_types)
base_queryset = base_queryset.filter(type__in=detector_types)

queryset = exclude_disallowed_metric_detectors(base_queryset, organization)

counts = queryset.aggregate(
active=Count(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@
from sentry.apidocs.parameters import DetectorParams, GlobalParams
from sentry.incidents.grouptype import MetricIssue
from sentry.incidents.metric_issue_detector import schedule_update_project_config
from sentry.incidents.utils.subscription_limits import is_metric_subscription_allowed
from sentry.incidents.utils.types import DATA_SOURCE_SNUBA_QUERY_SUBSCRIPTION
from sentry.issues import grouptype
from sentry.models.organization import Organization
from sentry.models.project import Project
from sentry.snuba.models import SnubaQuery
from sentry.utils.audit import create_audit_entry
from sentry.workflow_engine.endpoints.serializers.detector_serializer import DetectorSerializer
from sentry.workflow_engine.endpoints.utils.ids import to_valid_int_id
Expand All @@ -36,7 +39,32 @@
can_edit_detector,
get_unknown_detector_type_error,
)
from sentry.workflow_engine.models import Detector
from sentry.workflow_engine.models import DataSource, Detector


def _check_metric_detector_allowed(detector: Detector, organization: Organization) -> None:
"""Raise ResourceDoesNotExist if this is a metric detector the org is not entitled to."""
if detector.type != MetricIssue.slug:
return

# Look up DataSource -> QuerySubscription -> SnubaQuery in two queries
# (source_id is a TextField, so no FK/select_related).
ds = DataSource.objects.filter(
detector=detector,
type=DATA_SOURCE_SNUBA_QUERY_SUBSCRIPTION,
).first()
if ds is None:
return

dataset = (
SnubaQuery.objects.filter(subscriptions__id=ds.source_id)
.values_list("dataset", flat=True)
.first()
)
Comment thread
kcons marked this conversation as resolved.
if dataset is None:
return
if not is_metric_subscription_allowed(dataset, organization):
raise ResourceDoesNotExist


def remove_detector(request: Request, organization: Organization, detector: Detector) -> Response:
Expand Down Expand Up @@ -148,6 +176,8 @@ def get(self, request: Request, organization: Organization, detector: Detector)

Return details on an individual monitor
"""
_check_metric_detector_allowed(detector, organization)

serialized_detector = serialize(
detector,
request.user,
Expand Down Expand Up @@ -177,6 +207,8 @@ def put(self, request: Request, organization: Organization, detector: Detector)

Update an existing monitor
"""
_check_metric_detector_allowed(detector, organization)

if not can_edit_detector(detector, request):
raise PermissionDenied

Expand Down Expand Up @@ -210,4 +242,7 @@ def delete(self, request: Request, organization: Organization, detector: Detecto

Delete a monitor
"""
# Intentionally no _check_metric_detector_allowed gate here:
# orgs should be able to delete detectors they can no longer use
# (e.g. after a plan downgrade).
return remove_detector(request, organization, detector)
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@
from sentry.workflow_engine.endpoints.serializers.detector_serializer import (
DetectorSerializerResponse,
)
from sentry.workflow_engine.endpoints.utils.filters import apply_filter
from sentry.workflow_engine.endpoints.utils.filters import (
apply_filter,
exclude_disallowed_metric_detectors,
)
from sentry.workflow_engine.endpoints.utils.ids import to_valid_int_id_list
from sentry.workflow_engine.endpoints.validators.base import BaseDetectorTypeValidator
from sentry.workflow_engine.endpoints.validators.detector_workflow_mutation import (
Expand Down Expand Up @@ -259,6 +262,7 @@ def get(self, request: Request, organization: Organization) -> Response:
return self.respond(status=status.HTTP_401_UNAUTHORIZED)

queryset = self.filter_detectors(request, organization)
queryset = exclude_disallowed_metric_detectors(queryset, organization)

sort_by = request.GET.get("sortBy", "id")
sort_by_field = sort_by.lstrip("-")
Expand Down Expand Up @@ -349,6 +353,7 @@ def put(self, request: Request, organization: Organization) -> Response:
enabled = validator.validated_data.get("enabled", True)

queryset = self.filter_detectors(request, organization)
queryset = exclude_disallowed_metric_detectors(queryset, organization)

# If explicitly filtering by IDs and some were not found, return 400
if request.GET.getlist("id") and len(queryset) != len(set(request.GET.getlist("id"))):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ def post(self, request: Request, project: Project) -> Response:
if not detector_type:
raise ValidationError({"type": ["This field is required."]})

# Restrict creating metric issue detectors by plan type
# Restrict creating metric issue detectors by plan type.
# This is a coarse pre-check; the validator enforces the dataset-specific
# is_metric_subscription_allowed check after the dataset is validated.
if detector_type == MetricIssue.slug and not features.has(
"organizations:incidents", organization, actor=request.user
):
Expand Down
38 changes: 37 additions & 1 deletion src/sentry/workflow_engine/endpoints/utils/filters.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,43 @@
from django.db.models import Model, Q, QuerySet
from django.db.models import BigIntegerField, Model, Q, QuerySet
from django.db.models.functions import Cast

from sentry.api.event_search import SearchFilter
from sentry.db.models.query import in_iexact
from sentry.incidents.grouptype import MetricIssue
from sentry.incidents.utils.subscription_limits import get_disallowed_metric_datasets
from sentry.incidents.utils.types import DATA_SOURCE_SNUBA_QUERY_SUBSCRIPTION
from sentry.models.organization import Organization
from sentry.snuba.models import QuerySubscription
from sentry.workflow_engine.models import Detector


def exclude_disallowed_metric_detectors(
queryset: QuerySet[Detector], organization: Organization
) -> QuerySet[Detector]:
"""
Exclude metric detectors whose dataset subscription is not allowed
for the given organization (e.g. after a plan downgrade).
"""
disallowed_datasets = get_disallowed_metric_datasets(organization)
if not disallowed_datasets:
return queryset

# Cast DataSource.source_id (string) to int so the subquery can use
# the index on QuerySubscription.id.
Comment on lines +25 to +26
Copy link
Copy Markdown
Member

@wedamija wedamija Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this true? I think the subquery will use the index on project_id and/or snuba_query_id

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not true. Fixed, but trying to rework this query to be more efficient. Updating coming soon(tm).

return (
queryset.annotate(
_ds_source_int=Cast("data_sources__source_id", output_field=BigIntegerField())
)
.exclude(
type=MetricIssue.slug,
data_sources__type=DATA_SOURCE_SNUBA_QUERY_SUBSCRIPTION,
_ds_source_int__in=QuerySubscription.objects.filter(
snuba_query__dataset__in=disallowed_datasets,
project__organization=organization,
).values_list("id", flat=True),
)
.distinct()
)
Comment thread
kcons marked this conversation as resolved.


def apply_filter[T: Model](
Expand Down
21 changes: 20 additions & 1 deletion tests/sentry/incidents/endpoints/validators/test_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
)
from sentry.incidents.models.alert_rule import AlertRuleDetectionType
from sentry.incidents.utils.constants import INCIDENTS_SNUBA_SUBSCRIPTION_TYPE
from sentry.incidents.utils.subscription_limits import METRIC_SUBSCRIPTION_FEATURE_FLAGS
from sentry.models.environment import Environment
from sentry.seer.anomaly_detection.store_data import seer_anomaly_detection_connection_pool
from sentry.seer.anomaly_detection.types import (
Expand Down Expand Up @@ -124,6 +125,18 @@ def test_invalid_result(self) -> None:
]


# on-demand-metrics-extraction excluded from base features: it also triggers
# the "must use generic_metrics" check in SnubaQueryValidator, which would
# break transaction-dataset deprecation tests. Tests that use the
# PerformanceMetrics dataset add it explicitly via @with_feature.
_VALIDATOR_BASE_FEATURES = {
k: v
for k, v in METRIC_SUBSCRIPTION_FEATURE_FLAGS.items()
if k != "organizations:on-demand-metrics-extraction"
}


@with_feature(_VALIDATOR_BASE_FEATURES)
class TestMetricAlertsDetectorValidator(BaseValidatorTest):
def setUp(self) -> None:
super().setUp()
Expand Down Expand Up @@ -560,7 +573,12 @@ def test_transaction_dataset_deprecation_transactions(self) -> None:
):
validator.save()

@with_feature("organizations:discover-saved-queries-deprecation")
@with_feature(
{
"organizations:discover-saved-queries-deprecation": True,
"organizations:on-demand-metrics-extraction": True,
}
)
def test_transaction_dataset_deprecation_generic_metrics(self) -> None:
data = {
**self.valid_data,
Expand Down Expand Up @@ -1386,6 +1404,7 @@ def test_update_allowed_even_with_deprecated_dataset(self) -> None:
updated_detector = update_validator.save()
assert updated_detector.name == "Updated Detector Name"

@with_feature("organizations:on-demand-metrics-extraction")
def test_transaction_dataset_deprecation_generic_metrics_update(self) -> None:
data = {
**self.valid_data,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from sentry.incidents.grouptype import MetricIssue
from sentry.incidents.models.alert_rule import AlertRuleDetectionType
from sentry.incidents.utils.constants import INCIDENTS_SNUBA_SUBSCRIPTION_TYPE
from sentry.incidents.utils.subscription_limits import METRIC_SUBSCRIPTION_FEATURE_FLAGS
from sentry.models.auditlogentry import AuditLogEntry
from sentry.silo.base import SiloMode
from sentry.snuba.dataset import Dataset
Expand Down Expand Up @@ -39,6 +40,7 @@


@pytest.mark.snuba_ci
@with_feature(METRIC_SUBSCRIPTION_FEATURE_FLAGS)
class OrganizationDetectorDetailsBaseTest(APITestCase):
endpoint = "sentry-api-0-organization-detector-details"

Expand Down Expand Up @@ -208,6 +210,14 @@ def test_without_alert_rule_mapping(self) -> None:
assert response.data["alertRuleId"] is None
assert response.data["ruleId"] is None

def test_metric_detector_not_allowed_returns_404(self) -> None:
"""
When the org lacks the incidents feature, GET for a metric detector
should return 404.
"""
with self.feature({"organizations:incidents": False}):
self.get_error_response(self.organization.slug, self.detector.id, status_code=404)


@cell_silo_test
class OrganizationDetectorDetailsPutTest(OrganizationDetectorDetailsBaseTest):
Expand Down Expand Up @@ -367,6 +377,19 @@ def test_update_eap_invalid_time_window_rejected(self) -> None:
status_code=200,
)

def test_metric_detector_not_allowed_returns_404(self) -> None:
"""
When the org lacks the incidents feature, PUT for a metric detector
should return 404.
"""
with self.feature({"organizations:incidents": False}):
self.get_error_response(
self.organization.slug,
self.detector.id,
**self.valid_data,
status_code=404,
)

def test_update_add_data_condition(self) -> None:
"""
Test that we can add an additional data condition
Expand Down Expand Up @@ -1031,6 +1054,15 @@ def test_simple(self, mock_schedule_update_project_config: mock.MagicMock) -> No
assert self.detector.status == ObjectStatus.PENDING_DELETION
mock_schedule_update_project_config.assert_called_once_with(self.detector)

def test_delete_allowed_without_metric_subscription_feature(self) -> None:
with self.feature({"organizations:incidents": False}):
with outbox_runner():
self.get_success_response(self.organization.slug, self.detector.id)

assert CellScheduledDeletion.objects.filter(
model_name="Detector", object_id=self.detector.id
).exists()

def test_error_group_type(self) -> None:
"""
Test that we do not delete the required error detector
Expand Down
Loading
Loading