Skip to content

chore(issue-platform): Clean up issue platform feature flags now that we're using flagpole #89451

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
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
1 change: 0 additions & 1 deletion src/sentry/incidents/grouptype.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
56 changes: 7 additions & 49 deletions src/sentry/issues/grouptype.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -182,26 +176,15 @@ 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)
registry.add(cls)

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]
Expand All @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions src/sentry/runner/commands/createflag.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
6 changes: 0 additions & 6 deletions src/sentry/uptime/grouptype.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
2 changes: 1 addition & 1 deletion tests/sentry/incidents/utils/test_metric_issue_poc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions tests/sentry/issues/test_grouptype.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tests/sentry/uptime/consumers/test_results_consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion tests/sentry/uptime/subscriptions/test_subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tests/sentry/uptime/test_issue_platform.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading