From 6d83556830b1c4445808d25895a45ea2ac074a72 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Fri, 11 Apr 2025 16:36:13 -0700 Subject: [PATCH] chore(issue-platform): Clean up issue platform feature flags now that we're using flagpole We've migrated all grouptypes to either be released or using flagpole. We can remove all this transition code. Should be merged after - https://github.com/getsentry/getsentry/pull/17207 - https://github.com/getsentry/sentry/pull/89446 --- src/sentry/incidents/grouptype.py | 1 - src/sentry/issues/grouptype.py | 56 +++---------------- src/sentry/runner/commands/createflag.py | 6 +- src/sentry/uptime/grouptype.py | 6 -- .../incidents/utils/test_metric_issue_poc.py | 2 +- tests/sentry/issues/test_grouptype.py | 4 +- .../uptime/consumers/test_results_consumer.py | 2 +- .../subscriptions/test_subscriptions.py | 2 +- tests/sentry/uptime/test_issue_platform.py | 2 +- 9 files changed, 16 insertions(+), 65 deletions(-) diff --git a/src/sentry/incidents/grouptype.py b/src/sentry/incidents/grouptype.py index 3d90c419178017..619317cec334c5 100644 --- a/src/sentry/incidents/grouptype.py +++ b/src/sentry/incidents/grouptype.py @@ -72,7 +72,6 @@ def get_group_key_values( # like these when we're sending issues as alerts @dataclass(frozen=True) class MetricAlertFire(GroupType): - use_flagpole_for_all_features = True type_id = 8001 slug = "metric_alert_fire" description = "Metric alert fired" diff --git a/src/sentry/issues/grouptype.py b/src/sentry/issues/grouptype.py index f9ffc81dff92b0..f3906ae03e6b2c 100644 --- a/src/sentry/issues/grouptype.py +++ b/src/sentry/issues/grouptype.py @@ -74,13 +74,7 @@ def get_visible( with sentry_sdk.start_span(op="GroupTypeRegistry.get_visible") as span: released = [gt for gt in self.all() if gt.released] feature_to_grouptype = { - ( - gt.build_visible_feature_name() - if not gt.use_flagpole_for_all_features - else gt.build_visible_flagpole_feature_name() - ): gt - for gt in self.all() - if not gt.released + gt.build_visible_feature_name(): gt for gt in self.all() if not gt.released } batch_features = features.batch_has( list(feature_to_grouptype.keys()), actor=actor, organization=organization @@ -182,8 +176,6 @@ class GroupType: # Defaults to true to maintain the default workflow notification behavior as it exists for error group types. enable_status_change_workflow_notifications: bool = True detector_config_schema: ClassVar[dict[str, Any]] = {} - # Temporary setting so that we can slowly migrate all perf issues to use flagpole for all feature flags - use_flagpole_for_all_features = False def __init_subclass__(cls: type[GroupType], **kwargs: Any) -> None: super().__init_subclass__(**kwargs) @@ -191,17 +183,8 @@ def __init_subclass__(cls: type[GroupType], **kwargs: Any) -> None: if not cls.released: features.add(cls.build_visible_feature_name(), OrganizationFeature, True) - features.add(cls.build_ingest_feature_name(), OrganizationFeature) - features.add(cls.build_post_process_group_feature_name(), OrganizationFeature) - - # XXX: Temporary shim here. We can't use the existing feature flag names, because they're auto defined - # as being option backed. So options automator isn't able to validate them and fails. We'll instead - # move to new flag names - features.add(cls.build_visible_flagpole_feature_name(), OrganizationFeature, True) - features.add(cls.build_ingest_flagpole_feature_name(), OrganizationFeature, True) - features.add( - cls.build_post_process_group_flagpole_feature_name(), OrganizationFeature, True - ) + features.add(cls.build_ingest_feature_name(), OrganizationFeature, True) + features.add(cls.build_post_process_group_feature_name(), OrganizationFeature, True) def __post_init__(self) -> None: valid_categories = [category.value for category in GroupCategory] @@ -213,25 +196,14 @@ def allow_ingest(cls, organization: Organization) -> bool: if cls.released: return True - flag_name = ( - cls.build_ingest_feature_name() - if not cls.use_flagpole_for_all_features - else cls.build_ingest_flagpole_feature_name() - ) - return features.has(flag_name, organization) + return features.has(cls.build_ingest_feature_name(), organization) @classmethod def allow_post_process_group(cls, organization: Organization) -> bool: if cls.released: return True - flag_name = ( - cls.build_post_process_group_feature_name() - if not cls.use_flagpole_for_all_features - else cls.build_post_process_group_flagpole_feature_name() - ) - - return features.has(flag_name, organization) + return features.has(cls.build_post_process_group_feature_name(), organization) @classmethod def should_detect_escalation(cls) -> bool: @@ -245,34 +217,21 @@ def build_feature_name_slug(cls) -> str: return cls.slug.replace("_", "-") @classmethod - def build_base_feature_name(cls, prefix: str = "") -> str: - return f"organizations:{prefix}{cls.build_feature_name_slug()}" + def build_base_feature_name(cls) -> str: + return f"organizations:issue-{cls.build_feature_name_slug()}" @classmethod def build_visible_feature_name(cls) -> str: return f"{cls.build_base_feature_name()}-visible" - @classmethod - def build_visible_flagpole_feature_name(cls) -> str: - # We'll rename this too so that all the feature names are consistent - return f"{cls.build_base_feature_name("issue-")}-visible" - @classmethod def build_ingest_feature_name(cls) -> str: return f"{cls.build_base_feature_name()}-ingest" - @classmethod - def build_ingest_flagpole_feature_name(cls) -> str: - return f"{cls.build_base_feature_name("issue-")}-ingest" - @classmethod def build_post_process_group_feature_name(cls) -> str: return f"{cls.build_base_feature_name()}-post-process-group" - @classmethod - def build_post_process_group_flagpole_feature_name(cls) -> str: - return f"{cls.build_base_feature_name("issue-")}-post-process-group" - def get_all_group_type_ids() -> set[int]: # TODO: Replace uses of this with the registry @@ -594,7 +553,6 @@ class MetricIssuePOC(GroupType): enable_auto_resolve = False enable_escalation_detection = False enable_status_change_workflow_notifications = False - use_flagpole_for_all_features = True def should_create_group( diff --git a/src/sentry/runner/commands/createflag.py b/src/sentry/runner/commands/createflag.py index 3f1ea7d867c156..27c9a4ff8e37d6 100644 --- a/src/sentry/runner/commands/createflag.py +++ b/src/sentry/runner/commands/createflag.py @@ -204,9 +204,9 @@ def createissueflag( click.echo("") click.echo("=== GENERATED YAML ===\n") for feature_name in [ - group_type.build_visible_flagpole_feature_name(), - group_type.build_ingest_flagpole_feature_name(), - group_type.build_post_process_group_flagpole_feature_name(), + group_type.build_visible_feature_name(), + group_type.build_ingest_feature_name(), + group_type.build_post_process_group_feature_name(), ]: feature = Feature( diff --git a/src/sentry/uptime/grouptype.py b/src/sentry/uptime/grouptype.py index a610ccc8227bd9..294cb8f47e7b27 100644 --- a/src/sentry/uptime/grouptype.py +++ b/src/sentry/uptime/grouptype.py @@ -2,7 +2,6 @@ from dataclasses import dataclass -from sentry.issues import grouptype from sentry.issues.grouptype import GroupCategory, GroupType from sentry.ratelimits.sliding_windows import Quota from sentry.types.group import PriorityLevel @@ -33,8 +32,3 @@ class UptimeDomainCheckFailure(GroupType): }, "additionalProperties": False, } - use_flagpole_for_all_features = True - - -# XXX: Temporary hack to work around pickling issues -grouptype.UptimeDomainCheckFailure = UptimeDomainCheckFailure # type: ignore[attr-defined] diff --git a/tests/sentry/incidents/utils/test_metric_issue_poc.py b/tests/sentry/incidents/utils/test_metric_issue_poc.py index d200f690a745af..034a9ebc1e3d8f 100644 --- a/tests/sentry/incidents/utils/test_metric_issue_poc.py +++ b/tests/sentry/incidents/utils/test_metric_issue_poc.py @@ -12,7 +12,7 @@ from tests.sentry.issues.test_occurrence_consumer import IssueOccurrenceTestBase -@apply_feature_flag_on_cls(MetricIssuePOC.build_ingest_flagpole_feature_name()) +@apply_feature_flag_on_cls(MetricIssuePOC.build_ingest_feature_name()) @apply_feature_flag_on_cls("projects:metric-issue-creation") class TestMetricIssuePOC(IssueOccurrenceTestBase, APITestCase): def setUp(self): diff --git a/tests/sentry/issues/test_grouptype.py b/tests/sentry/issues/test_grouptype.py index 72c0748b6b5b94..45b74515e6091d 100644 --- a/tests/sentry/issues/test_grouptype.py +++ b/tests/sentry/issues/test_grouptype.py @@ -162,10 +162,10 @@ def test_get_visible(self) -> None: registry.add(UptimeDomainCheckFailure) registry.add(MetricIssuePOC) assert registry.get_visible(self.organization) == [] - with self.feature(UptimeDomainCheckFailure.build_visible_flagpole_feature_name()): + with self.feature(UptimeDomainCheckFailure.build_visible_feature_name()): assert registry.get_visible(self.organization) == [UptimeDomainCheckFailure] registry.add(ErrorGroupType) - with self.feature(UptimeDomainCheckFailure.build_visible_flagpole_feature_name()): + with self.feature(UptimeDomainCheckFailure.build_visible_feature_name()): assert set(registry.get_visible(self.organization)) == { UptimeDomainCheckFailure, ErrorGroupType, diff --git a/tests/sentry/uptime/consumers/test_results_consumer.py b/tests/sentry/uptime/consumers/test_results_consumer.py index 63161de5baf621..f1355c07cfa6be 100644 --- a/tests/sentry/uptime/consumers/test_results_consumer.py +++ b/tests/sentry/uptime/consumers/test_results_consumer.py @@ -79,7 +79,7 @@ def send_result( datetime.now(), ) ) - with self.feature(UptimeDomainCheckFailure.build_ingest_flagpole_feature_name()): + with self.feature(UptimeDomainCheckFailure.build_ingest_feature_name()): if consumer is None: factory = UptimeResultsStrategyFactory(mode=self.strategy_processing_mode) commit = mock.Mock() diff --git a/tests/sentry/uptime/subscriptions/test_subscriptions.py b/tests/sentry/uptime/subscriptions/test_subscriptions.py index 57f2960a0c68a8..ea15a9ee43cce5 100644 --- a/tests/sentry/uptime/subscriptions/test_subscriptions.py +++ b/tests/sentry/uptime/subscriptions/test_subscriptions.py @@ -692,7 +692,7 @@ def test(self, mock_disable_seat): def test_disable_failed(self, mock_disable_seat): with ( self.tasks(), - self.feature(UptimeDomainCheckFailure.build_ingest_flagpole_feature_name()), + self.feature(UptimeDomainCheckFailure.build_ingest_feature_name()), ): proj_sub = create_project_uptime_subscription( self.project, diff --git a/tests/sentry/uptime/test_issue_platform.py b/tests/sentry/uptime/test_issue_platform.py index 958256be5c7c32..4d39a925849c0b 100644 --- a/tests/sentry/uptime/test_issue_platform.py +++ b/tests/sentry/uptime/test_issue_platform.py @@ -119,7 +119,7 @@ def test(self): uptime_subscription=subscription ) result = self.create_uptime_result(subscription.subscription_id) - with self.feature(UptimeDomainCheckFailure.build_ingest_flagpole_feature_name()): + with self.feature(UptimeDomainCheckFailure.build_ingest_feature_name()): create_issue_platform_occurrence(result, project_subscription) hashed_fingerprint = md5(str(project_subscription.id).encode("utf-8")).hexdigest() group = Group.objects.get(grouphash__hash=hashed_fingerprint)