Skip to content

Commit

Permalink
feat(workflow): Remove 10 member fallthrough experiment (#47342)
Browse files Browse the repository at this point in the history
  • Loading branch information
scttcper authored Apr 13, 2023
1 parent 0b8eef6 commit d5e23e0
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 74 deletions.
2 changes: 0 additions & 2 deletions src/sentry/conf/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -1229,8 +1229,6 @@ def SOCIAL_AUTH_DEFAULT_USERNAME():
"organizations:invite-members": True,
# Enable rate limits for inviting members.
"organizations:invite-members-rate-limits": True,
# Test 10 member fallback vs 10 members
"organizations:issue-alert-fallback-experiment": False,
# Enable new issue alert "issue owners" fallback
"organizations:issue-alert-fallback-targeting": False,
# Enable SQL formatting for breadcrumb items
Expand Down
1 change: 0 additions & 1 deletion src/sentry/features/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@
default_manager.add("organizations:higher-ownership-limit", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
default_manager.add("organizations:invite-members", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
default_manager.add("organizations:invite-members-rate-limits", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
default_manager.add("organizations:issue-alert-fallback-experiment", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
default_manager.add("organizations:issue-alert-fallback-targeting", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
default_manager.add("organizations:issue-alert-incompatible-rules", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
default_manager.add("organizations:issue-alert-preview", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
Expand Down
12 changes: 1 addition & 11 deletions src/sentry/notifications/notifications/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,7 @@
has_alert_integration,
has_integrations,
)
from sentry.notifications.utils.participants import (
get_owner_reason,
get_send_to,
should_use_smaller_issue_alert_fallback,
)
from sentry.notifications.utils.participants import get_owner_reason, get_send_to
from sentry.plugins.base.structs import Notification
from sentry.services.hybrid_cloud.actor import ActorType, RpcActor
from sentry.types.integrations import ExternalProviders
Expand Down Expand Up @@ -113,10 +109,6 @@ def get_context(self) -> MutableMapping[str, Any]:
fallthrough_choice=self.fallthrough_choice,
)
fallback_params: MutableMapping[str, str] = {}
# Piggybacking off of notification_reason that already determines if we're using the fallback
if notification_reason and self.fallthrough_choice == FallthroughChoiceType.ACTIVE_MEMBERS:
_, fallback_experiment = should_use_smaller_issue_alert_fallback(org=self.organization)
fallback_params = {"ref_fallback": fallback_experiment}

context = {
"project_label": self.project.get_full_name(),
Expand Down Expand Up @@ -225,10 +217,8 @@ def send(self) -> None:
notify(provider, self, participants, shared_context)

def get_log_params(self, recipient: RpcActor) -> Mapping[str, Any]:
_, fallback_experiment = should_use_smaller_issue_alert_fallback(org=self.organization)
return {
"target_type": self.target_type,
"target_identifier": self.target_identifier,
"fallback_experiment": fallback_experiment,
**super().get_log_params(recipient),
}
29 changes: 1 addition & 28 deletions src/sentry/notifications/utils/participants.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from typing import TYPE_CHECKING, Any, Iterable, List, Mapping, MutableMapping, Sequence, Tuple

from sentry import features
from sentry.experiments import manager as expt_manager
from sentry.models import (
ActorTuple,
Group,
Expand Down Expand Up @@ -52,7 +51,6 @@
ExternalProviders.SLACK,
}

FALLTHROUGH_NOTIFICATION_LIMIT_EA = 10
FALLTHROUGH_NOTIFICATION_LIMIT = 20


Expand Down Expand Up @@ -390,25 +388,6 @@ def get_send_to(
return get_recipients_by_provider(project, recipients, notification_type)


def should_use_smaller_issue_alert_fallback(org: Organization) -> Tuple[bool, str]:
"""
Remove after IssueAlertFallbackExperiment experiment
Returns a tuple of (enabled, analytics_label)
"""
if not org.flags.early_adopter.is_set:
# Not early access, not in experiment
return (False, "ga")

# Disabled
if not features.has("organizations:issue-alert-fallback-experiment", org, actor=None):
return (False, "disabled")

org_exposed = expt_manager.get("IssueAlertFallbackExperiment", org=org) == 1
if org_exposed:
return (True, "expt")
return (False, "ctrl")


def get_fallthrough_recipients(
project: Project, fallthrough_choice: FallthroughChoiceType | None
) -> Iterable[RpcUser]:
Expand All @@ -432,19 +411,13 @@ def get_fallthrough_recipients(
)

elif fallthrough_choice == FallthroughChoiceType.ACTIVE_MEMBERS:
use_smaller_limit, _ = should_use_smaller_issue_alert_fallback(org=project.organization)
limit = (
FALLTHROUGH_NOTIFICATION_LIMIT_EA
if use_smaller_limit
else FALLTHROUGH_NOTIFICATION_LIMIT
)
return user_service.get_many(
filter={
"user_ids": project.member_set.order_by("-user__last_active").values_list(
"user_id", flat=True
)
}
)[:limit]
)[:FALLTHROUGH_NOTIFICATION_LIMIT]

raise NotImplementedError(f"Unknown fallthrough choice: {fallthrough_choice}")

Expand Down
32 changes: 0 additions & 32 deletions tests/sentry/notifications/utils/test_participants.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from typing import Iterable, Mapping, Optional, Sequence, Set, Union
from unittest import mock

import pytest

Expand All @@ -17,7 +16,6 @@
)
from sentry.notifications.utils.participants import (
FALLTHROUGH_NOTIFICATION_LIMIT,
FALLTHROUGH_NOTIFICATION_LIMIT_EA,
get_fallthrough_recipients,
get_owner_reason,
get_owners,
Expand Down Expand Up @@ -933,33 +931,3 @@ def test_fallthrough_admin_or_recent_over_20(self):

assert len(notified_users) == FALLTHROUGH_NOTIFICATION_LIMIT
assert notified_users.issubset(expected_notified_users)

@mock.patch("sentry.experiments.manager.get", return_value=1)
@with_feature("organizations:issue-alert-fallback-targeting")
@with_feature("organizations:issue-alert-fallback-experiment")
def test_fallthrough_ea_experiment_limit(self, mock_func):
self.organization.flags.early_adopter = True

notifiable_users = [self.user, self.user2]
for i in range(FALLTHROUGH_NOTIFICATION_LIMIT_EA + 5):
new_user = self.create_user(email=f"user_{i}@example.com", is_active=True)
self.create_member(
user=new_user, organization=self.organization, role="owner", teams=[self.team2]
)
notifiable_users.append(new_user)

for user in notifiable_users:
NotificationSetting.objects.update_settings(
ExternalProviders.SLACK,
NotificationSettingTypes.ISSUE_ALERTS,
NotificationSettingOptionValues.NEVER,
user=user,
)

event = self.store_event("admin.lol", self.project)
notified_users = self.get_send_to_fallthrough(
event, self.project, FallthroughChoiceType.ACTIVE_MEMBERS
)[ExternalProviders.EMAIL]

# Check that we notify all possible folks with the EA experiment limit.
assert len(notified_users) == FALLTHROUGH_NOTIFICATION_LIMIT_EA

0 comments on commit d5e23e0

Please sign in to comment.