Skip to content
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
8 changes: 2 additions & 6 deletions src/sentry/incidents/utils/subscription_limits.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
# 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,
Expand All @@ -25,15 +24,12 @@ def is_metric_subscription_allowed(dataset: str, organization: Organization) ->
Returns True if allowed, False if the organization lacks the required features
(e.g. after a plan downgrade).
"""
has_incidents = features.has("organizations:incidents", organization)
if dataset == Dataset.Events.value:
return has_incidents

if dataset == Dataset.Transactions.value:
return has_incidents and features.has("organizations:performance-view", organization)
return features.has("organizations:performance-view", organization)

if dataset == Dataset.EventsAnalyticsPlatform.value:
return has_incidents and features.has("organizations:visibility-explore-view", organization)
return features.has("organizations:visibility-explore-view", organization)

if dataset == Dataset.PerformanceMetrics.value:
return features.has("organizations:on-demand-metrics-extraction", organization)
Expand Down
2 changes: 0 additions & 2 deletions tests/sentry/explore/translation/test_alerts_translation.py
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,6 @@ def test_rollback_without_snapshot(self) -> None:

@with_feature("organizations:migrate-transaction-alerts-to-spans")
@with_feature("organizations:anomaly-detection-alerts")
@with_feature("organizations:incidents")
@patch("sentry.snuba.tasks._create_rpc_in_snuba")
@patch("sentry.explore.translation.alerts_translation.handle_send_historical_data_to_seer")
@patch(
Expand Down Expand Up @@ -810,7 +809,6 @@ def test_translate_anomaly_detection_alert(

@with_feature("organizations:migrate-transaction-alerts-to-spans")
@with_feature("organizations:anomaly-detection-alerts")
@with_feature("organizations:incidents")
@patch("sentry.snuba.tasks._delete_from_snuba")
@patch("sentry.snuba.tasks._create_snql_in_snuba")
@patch("sentry.snuba.tasks._create_rpc_in_snuba")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,6 @@ def test_create_detector_trace_metrics_feature_flag_disabled(self) -> None:

@with_feature(
[
"organizations:incidents",
"organizations:performance-view",
"organizations:tracemetrics-enabled",
]
Expand All @@ -735,7 +734,6 @@ def test_create_detector_trace_metrics_invalid_aggregate(self) -> None:

@with_feature(
[
"organizations:incidents",
"organizations:performance-view",
"organizations:tracemetrics-enabled",
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,6 @@ def send_upsampled_update(
self.sub, upsampled_count=upsampled_count, time_delta=time_delta
)
with (
self.feature("organizations:incidents"),
self.feature("organizations:performance-view"),
):
SubscriptionProcessor.process(self.sub, message)
Expand Down Expand Up @@ -617,7 +616,7 @@ def send_crash_rate_detector_update(self, value, subscription, time_delta=None,
timestamp = timestamp.replace(microsecond=0)

with (
self.feature(["organizations:incidents", "organizations:performance-view"]),
self.feature("organizations:performance-view"),
self.capture_on_commit_callbacks(execute=True),
):
if value is None:
Expand Down Expand Up @@ -978,7 +977,7 @@ def test_multiple_threshold_trigger_is_reset_when_count_is_lower_than_min_thresh
def test_ensure_case_when_no_metrics_index_not_found_is_handled_gracefully(
self, helper_metrics
):
with self.feature(["organizations:incidents", "organizations:performance-view"]):
with self.feature("organizations:performance-view"):
SubscriptionProcessor.process(
self.sub,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ def send_update(self, value, time_delta=None, subscription=None):
with (
self.feature(
[
"organizations:incidents",
"organizations:performance-view",
"organizations:visibility-explore-view",
]
Expand All @@ -228,7 +227,6 @@ def test_uses_stored_last_update_value(self) -> None:
with (
self.feature(
[
"organizations:incidents",
"organizations:performance-view",
"organizations:visibility-explore-view",
]
Expand Down Expand Up @@ -264,7 +262,6 @@ def test_no_detector_returns_false_without_exception(self) -> None:
with (
self.feature(
[
"organizations:incidents",
"organizations:performance-view",
"organizations:visibility-explore-view",
]
Expand Down
46 changes: 7 additions & 39 deletions tests/sentry/incidents/utils/test_subscription_limits.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,35 +15,23 @@ def fake_features(self, enabled: set[str]):
mock_features.has.side_effect = lambda name, *a, **kw: name in enabled
yield

# -- Events: requires :incidents --
# -- Events: ungated --

def test_events_without_incidents(self) -> None:
def test_events_always_allowed(self) -> None:
with self.fake_features(set()):
assert is_metric_subscription_allowed(Dataset.Events.value, self.org) is False

def test_events_with_incidents(self) -> None:
with self.fake_features({"organizations:incidents"}):
assert is_metric_subscription_allowed(Dataset.Events.value, self.org) is True

# -- Transactions: requires :incidents + :performance-view --
# -- Transactions: requires :performance-view --

def test_transactions_without_any_features(self) -> None:
with self.fake_features(set()):
assert is_metric_subscription_allowed(Dataset.Transactions.value, self.org) is False

def test_transactions_with_only_performance_view(self) -> None:
def test_transactions_with_performance_view(self) -> None:
with self.fake_features({"organizations:performance-view"}):
assert is_metric_subscription_allowed(Dataset.Transactions.value, self.org) is False

def test_transactions_with_only_incidents(self) -> None:
with self.fake_features({"organizations:incidents"}):
assert is_metric_subscription_allowed(Dataset.Transactions.value, self.org) is False

def test_transactions_with_both_features(self) -> None:
with self.fake_features({"organizations:incidents", "organizations:performance-view"}):
assert is_metric_subscription_allowed(Dataset.Transactions.value, self.org) is True

# -- EAP: requires :incidents + :visibility-explore-view --
# -- EAP: requires :visibility-explore-view --

def test_eap_without_any_features(self) -> None:
with self.fake_features(set()):
Expand All @@ -52,24 +40,8 @@ def test_eap_without_any_features(self) -> None:
is False
)

def test_eap_with_only_explore_view(self) -> None:
def test_eap_with_explore_view(self) -> None:
with self.fake_features({"organizations:visibility-explore-view"}):
assert (
is_metric_subscription_allowed(Dataset.EventsAnalyticsPlatform.value, self.org)
is False
)

def test_eap_with_only_incidents(self) -> None:
with self.fake_features({"organizations:incidents"}):
assert (
is_metric_subscription_allowed(Dataset.EventsAnalyticsPlatform.value, self.org)
is False
)

def test_eap_with_both_features(self) -> None:
with self.fake_features(
{"organizations:incidents", "organizations:visibility-explore-view"}
):
assert (
is_metric_subscription_allowed(Dataset.EventsAnalyticsPlatform.value, self.org)
is True
Expand All @@ -91,10 +63,6 @@ def test_performance_metrics_with_on_demand(self) -> None:

# -- Unknown / other datasets: always allowed --

def test_unknown_dataset_without_incidents(self) -> None:
def test_unknown_dataset_always_allowed(self) -> None:
with self.fake_features(set()):
assert is_metric_subscription_allowed("unknown_dataset", self.org) is True

def test_unknown_dataset_with_incidents(self) -> None:
with self.fake_features({"organizations:incidents"}):
assert is_metric_subscription_allowed("unknown_dataset", self.org) is True
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,6 @@ 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)

@with_feature("organizations:workflow-engine-all-projects-detector")
def test_all_projects_detector_get_success(self) -> None:
all_projects_detector = ensure_default_all_projects_detector(self.organization.id)
Expand Down Expand Up @@ -432,19 +424,6 @@ def test_update_comparison_delta_invalid(self) -> None:
)
assert response.data["config"]["comparisonDelta"] == 300

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 @@ -1171,9 +1150,8 @@ def test_simple(self, mock_schedule_update_project_config: mock.MagicMock) -> No
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)
with outbox_runner():
self.get_success_response(self.organization.slug, self.detector.id)
Comment thread
RudraPatel2003 marked this conversation as resolved.

assert CellScheduledDeletion.objects.filter(
model_name="Detector", object_id=self.detector.id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from sentry.monitors.grouptype import MonitorIncidentType
from sentry.search.utils import _HACKY_INVALID_USER
from sentry.snuba.dataset import Dataset
from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType
from sentry.snuba.models import QuerySubscription, SnubaQuery
from sentry.snuba.subscriptions import create_snuba_query, create_snuba_subscription
from sentry.testutils.asserts import assert_org_audit_log_exists
from sentry.testutils.cases import APITestCase
Expand Down Expand Up @@ -908,14 +908,14 @@ class OrganizationDetectorIndexSubscriptionFilterTest(OrganizationDetectorIndexB
def test_list_excludes_disallowed_metric_detectors(self) -> None:
with self.tasks():
snuba_query = create_snuba_query(
query_type=SnubaQuery.Type.ERROR,
dataset=Dataset.Events,
query_type=SnubaQuery.Type.PERFORMANCE,
dataset=Dataset.Transactions,
query="test",
aggregate="count()",
time_window=timedelta(minutes=1),
resolution=timedelta(minutes=1),
environment=self.environment,
event_types=[SnubaQueryEventType.EventType.ERROR],
event_types=(),
)
query_subscription = create_snuba_subscription(
project=self.project,
Expand All @@ -930,16 +930,16 @@ def test_list_excludes_disallowed_metric_detectors(self) -> None:
)
self.create_data_source_detector(data_source=data_source, detector=metric_detector)

# With incidents feature, the metric detector appears in the list
with self.feature({"organizations:incidents": True}):
# With performance-view, the metric detector appears in the list
with self.feature({"organizations:performance-view": True}):
response = self.get_success_response(
self.organization.slug, qs_params={"project": self.project.id}
)
detector_ids = {d["id"] for d in response.data}
assert str(metric_detector.id) in detector_ids

# Without incidents feature, the metric detector is excluded
with self.feature({"organizations:incidents": False}):
# Without performance-view, the metric detector is excluded
with self.feature({"organizations:performance-view": False}):
response = self.get_success_response(
self.organization.slug, qs_params={"project": self.project.id}
)
Expand All @@ -949,7 +949,7 @@ def test_list_excludes_disallowed_metric_detectors(self) -> None:
@requires_snuba
@requires_kafka
def test_non_metric_detectors_never_excluded(self) -> None:
with self.feature({"organizations:incidents": False}):
with self.feature({"organizations:performance-view": False}):
response = self.get_success_response(
self.organization.slug, qs_params={"project": self.project.id}
)
Expand All @@ -963,7 +963,7 @@ def test_metric_detector_without_data_source_not_excluded(self) -> None:
orphan = self.create_detector(
project=self.project, name="No DataSource", type=MetricIssue.slug
)
with self.feature({"organizations:incidents": False}):
with self.feature({"organizations:performance-view": False}):
response = self.get_success_response(
self.organization.slug, qs_params={"project": self.project.id}
)
Expand All @@ -974,16 +974,16 @@ def test_metric_detector_without_data_source_not_excluded(self) -> None:
@requires_kafka
def test_allowed_metric_detector_kept_when_others_disallowed(self) -> None:
with self.tasks():
# Events dataset — requires incidents feature
# Transactions dataset — requires performance-view
disallowed_sq = create_snuba_query(
query_type=SnubaQuery.Type.ERROR,
dataset=Dataset.Events,
query_type=SnubaQuery.Type.PERFORMANCE,
dataset=Dataset.Transactions,
query="test",
aggregate="count()",
time_window=timedelta(minutes=1),
resolution=timedelta(minutes=1),
environment=self.environment,
event_types=[SnubaQueryEventType.EventType.ERROR],
event_types=(),
)
disallowed_sub = create_snuba_subscription(
project=self.project,
Expand Down Expand Up @@ -1025,11 +1025,11 @@ def test_allowed_metric_detector_kept_when_others_disallowed(self) -> None:
)
self.create_data_source_detector(data_source=allowed_ds, detector=allowed_detector)

# Disable incidents but enable on-demand-metrics-extraction:
# Events detector is excluded, PerformanceMetrics detector is kept.
# Disable performance-view but enable on-demand-metrics-extraction:
# Transactions detector is excluded, PerformanceMetrics detector is kept.
with self.feature(
{
"organizations:incidents": False,
"organizations:performance-view": False,
"organizations:on-demand-metrics-extraction": True,
}
):
Expand All @@ -1042,7 +1042,6 @@ def test_allowed_metric_detector_kept_when_others_disallowed(self) -> None:


@cell_silo_test
@with_feature("organizations:incidents")
class OrganizationDetectorIndexPutTest(OrganizationDetectorIndexBaseTest):
method = "PUT"

Expand Down
35 changes: 17 additions & 18 deletions tests/snuba/incidents/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,24 +138,23 @@ def shutdown_callback(*args, **kwargs):

subscriber_registry[INCIDENTS_SNUBA_SUBSCRIPTION_TYPE] = shutdown_callback

with self.feature("organizations:incidents"):
with self.tasks(), self.capture_on_commit_callbacks(execute=True):
# Integration test: verify taskbroker raw mode successfully processes
# subscription updates through the workflow engine without error.
_process_subscription_message(json.dumps(message).encode(), Dataset.Metrics)

# Verify the callback was invoked
assert callback_invoked, "Subscription processor callback should have been invoked"

# Verify workflow engine evaluated the detector correctly
detector_state = DetectorState.objects.filter(detector=self.detector).first()
assert detector_state is not None
assert detector_state.is_triggered

# Note: This test verifies subscription processing through the workflow engine.
# IssueOccurrences are created but not persisted to Groups in this test since
# that would require the occurrence consumer to be running, which is outside
# the scope of this taskbroker raw-mode integration test.
with self.tasks(), self.capture_on_commit_callbacks(execute=True):
# Integration test: verify taskbroker raw mode successfully processes
# subscription updates through the workflow engine without error.
_process_subscription_message(json.dumps(message).encode(), Dataset.Metrics)

# Verify the callback was invoked
assert callback_invoked, "Subscription processor callback should have been invoked"

# Verify workflow engine evaluated the detector correctly
detector_state = DetectorState.objects.filter(detector=self.detector).first()
assert detector_state is not None
assert detector_state.is_triggered

# Note: This test verifies subscription processing through the workflow engine.
# IssueOccurrences are created but not persisted to Groups in this test since
# that would require the occurrence consumer to be running, which is outside
# the scope of this taskbroker raw-mode integration test.

def test_raw_subscription_task(self) -> None:
self.run_test()
Loading