Skip to content

Commit

Permalink
fix(escalating-issues): Cleanup query for escalating forecasts (#70443)
Browse files Browse the repository at this point in the history
Cleans up some cruft left behind by feature flags and pushes the
escalation check out of the query to help with timeouts.

Fixes SENTRY-18AS
  • Loading branch information
snigdhas committed May 7, 2024
1 parent 81d3fef commit aa40406
Show file tree
Hide file tree
Showing 8 changed files with 12 additions and 28 deletions.
2 changes: 1 addition & 1 deletion src/sentry/issues/escalating.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def query_groups_past_counts(groups: Sequence[Group]) -> list[GroupsCountRespons
for g in groups:
if g.issue_category == GroupCategory.ERROR:
error_groups.append(g)
elif g.issue_type.should_detect_escalation(g.organization):
elif g.issue_type.should_detect_escalation():
other_groups.append(g)

all_results += _process_groups(error_groups, start_date, end_date, GroupCategory.ERROR)
Expand Down
4 changes: 1 addition & 3 deletions src/sentry/issues/escalating_group_forecast.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

from sentry import nodestore
from sentry.models.group import Group
from sentry.models.organization import Organization
from sentry.utils.dates import parse_timestamp

GROUP_FORECAST_TTL = 14
Expand Down Expand Up @@ -53,8 +52,7 @@ def save(self) -> None:
def _should_fetch_escalating(cls, group_id: int) -> bool:
group = Group.objects.get(id=group_id)

organization = Organization.objects.get(project__group__id=group_id)
return group.issue_type.should_detect_escalation(organization)
return group.issue_type.should_detect_escalation()

@classmethod
def fetch(cls, project_id: int, group_id: int) -> EscalatingGroupForecast | None:
Expand Down
1 change: 1 addition & 0 deletions src/sentry/issues/forecasts.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def generate_and_save_forecasts(groups: Sequence[Group]) -> None:
Generates and saves a list of forecasted values for each group.
`groups`: Sequence of groups to be forecasted
"""
groups = [group for group in groups if group.issue_type.should_detect_escalation()]
past_counts = query_groups_past_counts(groups)
group_counts = parse_groups_past_counts(past_counts)
save_forecast_per_group(groups, group_counts)
Expand Down
6 changes: 2 additions & 4 deletions src/sentry/issues/grouptype.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,9 @@ def allow_post_process_group(cls, organization: Organization) -> bool:
return features.has(cls.build_post_process_group_feature_name(), organization)

@classmethod
def should_detect_escalation(cls, organization: Organization) -> bool:
def should_detect_escalation(cls) -> bool:
"""
If the feature is enabled and enable_escalation_detection=True, then escalation detection is enabled.
When the feature flag is removed, we can remove the organization parameter from this method.
If enable_escalation_detection=True, then escalation detection is enabled.
"""
return cls.enable_escalation_detection

Expand Down
4 changes: 2 additions & 2 deletions src/sentry/tasks/post_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ def should_update_escalating_metrics(event: Event, is_transaction_event: bool) -
features.has("organizations:escalating-metrics-backend", event.project.organization)
and not is_transaction_event
and event.group is not None
and event.group.issue_type.should_detect_escalation(event.project.organization)
and event.group.issue_type.should_detect_escalation()
)


Expand Down Expand Up @@ -867,7 +867,7 @@ def process_snoozes(job: PostProcessJob) -> None:
)
return

if not group.issue_type.should_detect_escalation(group.organization):
if not group.issue_type.should_detect_escalation():
return

# groups less than a day old should use the new -> escalating logic
Expand Down
16 changes: 4 additions & 12 deletions src/sentry/tasks/weekly_escalating_forecast.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,24 +60,16 @@ def run_escalating_forecast() -> None:
)
@retry
def generate_forecasts_for_projects(project_ids: list[int]) -> None:
query_until_escalating_groups = (
group
for group in RangeQuerySetWrapper(
for until_escalating_groups in chunked(
RangeQuerySetWrapper(
Group.objects.filter(
status=GroupStatus.IGNORED,
substatus=GroupSubStatus.UNTIL_ESCALATING,
project_id__in=project_ids,
last_seen__gte=datetime.now(UTC) - timedelta(days=7),
).select_related(
"project", "project__organization"
), # TODO: Remove this once the feature flag is removed
),
step=ITERATOR_CHUNK,
)
if group.issue_type.should_detect_escalation(group.project.organization)
)

for until_escalating_groups in chunked(
query_until_escalating_groups,
),
ITERATOR_CHUNK,
):
generate_and_save_forecasts(groups=until_escalating_groups)
3 changes: 0 additions & 3 deletions tests/sentry/tasks/test_auto_resolve_issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from sentry.models.group import Group, GroupStatus
from sentry.tasks.auto_resolve_issues import schedule_auto_resolution
from sentry.testutils.cases import TestCase
from sentry.testutils.helpers.features import with_feature


class ScheduleAutoResolutionTest(TestCase):
Expand Down Expand Up @@ -83,7 +82,6 @@ def test_simple(self, mock_kick_off_status_syncs, mock_backend, mock_record):

@patch("sentry.tasks.auto_ongoing_issues.backend")
@patch("sentry.tasks.auto_resolve_issues.kick_off_status_syncs")
@with_feature("organizations:issue-platform-crons-sd")
def test_single_event_performance(self, mock_kick_off_status_syncs, mock_backend):
project = self.create_project()

Expand Down Expand Up @@ -112,7 +110,6 @@ def test_single_event_performance(self, mock_kick_off_status_syncs, mock_backend
assert project.get_option("sentry:_last_auto_resolve") > current_ts

@patch("sentry.tasks.auto_ongoing_issues.backend")
@with_feature("organizations:issue-platform-crons-sd")
def test_aggregate_performance(self, mock_backend):
project = self.create_project()

Expand Down
4 changes: 1 addition & 3 deletions tests/sentry/tasks/test_post_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -1578,9 +1578,7 @@ class SnoozeTestSkipSnoozeMixin(BasePostProgressGroupMixin):
def test_invalidates_snooze_issue_platform(self, mock_processor, mock_send_unignored_robust):
event = self.create_event(data={"message": "testing"}, project_id=self.project.id)
group = event.group
should_detect_escalation = group.issue_type.should_detect_escalation(
self.project.organization
)
should_detect_escalation = group.issue_type.should_detect_escalation()

# Check for has_reappeared=False if is_new=True
self.call_post_process_group(
Expand Down

0 comments on commit aa40406

Please sign in to comment.