Skip to content

Commit 8c744ce

Browse files
cathtengcathteng
authored
fix(aci): always combine rule and workflow fire history in API (#103945)
1 parent 6cd85ab commit 8c744ce

File tree

4 files changed

+88
-105
lines changed

4 files changed

+88
-105
lines changed

‎src/sentry/api/serializers/models/rule.py‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
from django.db.models import Max, Prefetch, Q, prefetch_related_objects
1010
from rest_framework import serializers
1111

12-
from sentry import features
1312
from sentry.api.serializers import Serializer, register
1413
from sentry.constants import ObjectStatus
1514
from sentry.db.models.manager.base_query_set import BaseQuerySet
@@ -210,10 +209,7 @@ def get_attrs(self, item_list, user, **kwargs):
210209
}
211210

212211
# Update lastTriggered with WorkflowFireHistory if available
213-
if item_list and features.has(
214-
"organizations:workflow-engine-single-process-workflows",
215-
item_list[0].project.organization,
216-
):
212+
if item_list:
217213
rule_ids = [rule.id for rule in item_list]
218214
workflow_rule_lookup = dict(
219215
AlertRuleWorkflow.objects.filter(rule_id__in=rule_ids).values_list(

‎src/sentry/rules/history/backends/postgres.py‎

Lines changed: 87 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
from django.db.models import Count, Max, OuterRef, Subquery
1010
from django.db.models.functions import TruncHour
1111

12-
from sentry import features
1312
from sentry.api.paginator import GenericOffsetPaginator, OffsetPaginator
1413
from sentry.models.group import Group
1514
from sentry.models.rulefirehistory import RuleFireHistory
@@ -69,60 +68,57 @@ def fetch_rule_groups_paginated(
6968
cursor: Cursor | None = None,
7069
per_page: int = 25,
7170
) -> CursorResult[RuleGroupHistory]:
72-
if features.has(
73-
"organizations:workflow-engine-single-process-workflows", rule.project.organization
74-
):
75-
try:
76-
alert_rule_workflow = AlertRuleWorkflow.objects.get(rule_id=rule.id)
77-
workflow = alert_rule_workflow.workflow
78-
79-
# Performs the raw SQL query with pagination
80-
def data_fn(offset: int, limit: int) -> list[_Result]:
81-
query = """
82-
WITH combined_data AS (
83-
SELECT group_id, date_added, event_id
84-
FROM sentry_rulefirehistory
85-
WHERE rule_id = %s AND date_added >= %s AND date_added < %s
86-
UNION ALL
87-
SELECT group_id, date_added, event_id
88-
FROM workflow_engine_workflowfirehistory
89-
WHERE workflow_id = %s
90-
AND date_added >= %s AND date_added < %s
71+
try:
72+
alert_rule_workflow = AlertRuleWorkflow.objects.get(rule_id=rule.id)
73+
workflow = alert_rule_workflow.workflow
74+
75+
# Performs the raw SQL query with pagination
76+
def data_fn(offset: int, limit: int) -> list[_Result]:
77+
query = """
78+
WITH combined_data AS (
79+
SELECT group_id, date_added, event_id
80+
FROM sentry_rulefirehistory
81+
WHERE rule_id = %s AND date_added >= %s AND date_added < %s
82+
UNION ALL
83+
SELECT group_id, date_added, event_id
84+
FROM workflow_engine_workflowfirehistory
85+
WHERE workflow_id = %s
86+
AND date_added >= %s AND date_added < %s
87+
)
88+
SELECT
89+
group_id as group,
90+
COUNT(*) as count,
91+
MAX(date_added) as last_triggered,
92+
(ARRAY_AGG(event_id ORDER BY date_added DESC))[1] as event_id
93+
FROM combined_data
94+
GROUP BY group_id
95+
ORDER BY count DESC, last_triggered DESC
96+
LIMIT %s OFFSET %s
97+
"""
98+
99+
with connection.cursor() as cursor:
100+
cursor.execute(
101+
query, [rule.id, start, end, workflow.id, start, end, limit, offset]
102+
)
103+
return [
104+
_Result(
105+
group=row[0],
106+
count=row[1],
107+
last_triggered=row[2],
108+
event_id=row[3],
91109
)
92-
SELECT
93-
group_id as group,
94-
COUNT(*) as count,
95-
MAX(date_added) as last_triggered,
96-
(ARRAY_AGG(event_id ORDER BY date_added DESC))[1] as event_id
97-
FROM combined_data
98-
GROUP BY group_id
99-
ORDER BY count DESC, last_triggered DESC
100-
LIMIT %s OFFSET %s
101-
"""
110+
for row in cursor.fetchall()
111+
]
102112

103-
with connection.cursor() as cursor:
104-
cursor.execute(
105-
query, [rule.id, start, end, workflow.id, start, end, limit, offset]
106-
)
107-
return [
108-
_Result(
109-
group=row[0],
110-
count=row[1],
111-
last_triggered=row[2],
112-
event_id=row[3],
113-
)
114-
for row in cursor.fetchall()
115-
]
116-
117-
result = GenericOffsetPaginator(data_fn=data_fn).get_result(per_page, cursor)
118-
result.results = convert_results(result.results)
119-
120-
return result
121-
122-
except AlertRuleWorkflow.DoesNotExist:
123-
# If no workflow is associated with this rule, just use the original behavior
124-
logger.exception("No workflow associated with rule", extra={"rule_id": rule.id})
125-
pass
113+
result = GenericOffsetPaginator(data_fn=data_fn).get_result(per_page, cursor)
114+
result.results = convert_results(result.results)
115+
116+
return result
117+
118+
except AlertRuleWorkflow.DoesNotExist:
119+
# If no workflow is associated with this rule, just use the original behavior
120+
logger.exception("No workflow associated with rule", extra={"rule_id": rule.id})
121+
pass
126122

127123
rule_filtered_history = RuleFireHistory.objects.filter(
128124
rule=rule,
@@ -154,50 +150,47 @@ def fetch_rule_hourly_stats(
154150

155151
existing_data: dict[datetime, TimeSeriesValue] = {}
156152

157-
if features.has(
158-
"organizations:workflow-engine-single-process-workflows", rule.project.organization
159-
):
160-
try:
161-
alert_rule_workflow = AlertRuleWorkflow.objects.get(rule_id=rule.id)
162-
workflow = alert_rule_workflow.workflow
163-
164-
# Use raw SQL to combine data from both tables
165-
with connection.cursor() as db_cursor:
166-
db_cursor.execute(
167-
"""
168-
SELECT
169-
DATE_TRUNC('hour', date_added) as bucket,
170-
COUNT(*) as count
171-
FROM (
172-
SELECT date_added
173-
FROM sentry_rulefirehistory
174-
WHERE rule_id = %s
175-
AND date_added >= %s
176-
AND date_added < %s
177-
178-
UNION ALL
179-
180-
SELECT date_added
181-
FROM workflow_engine_workflowfirehistory
182-
WHERE workflow_id = %s
183-
AND date_added >= %s
184-
AND date_added < %s
185-
) combined_data
186-
GROUP BY DATE_TRUNC('hour', date_added)
187-
ORDER BY bucket
188-
""",
189-
[rule.id, start, end, workflow.id, start, end],
190-
)
153+
try:
154+
alert_rule_workflow = AlertRuleWorkflow.objects.get(rule_id=rule.id)
155+
workflow = alert_rule_workflow.workflow
156+
157+
# Use raw SQL to combine data from both tables
158+
with connection.cursor() as db_cursor:
159+
db_cursor.execute(
160+
"""
161+
SELECT
162+
DATE_TRUNC('hour', date_added) as bucket,
163+
COUNT(*) as count
164+
FROM (
165+
SELECT date_added
166+
FROM sentry_rulefirehistory
167+
WHERE rule_id = %s
168+
AND date_added >= %s
169+
AND date_added < %s
170+
171+
UNION ALL
172+
173+
SELECT date_added
174+
FROM workflow_engine_workflowfirehistory
175+
WHERE workflow_id = %s
176+
AND date_added >= %s
177+
AND date_added < %s
178+
) combined_data
179+
GROUP BY DATE_TRUNC('hour', date_added)
180+
ORDER BY bucket
181+
""",
182+
[rule.id, start, end, workflow.id, start, end],
183+
)
191184

192-
results = db_cursor.fetchall()
185+
results = db_cursor.fetchall()
193186

194-
# Convert raw SQL results to the expected format
195-
existing_data = {row[0]: TimeSeriesValue(row[0], row[1]) for row in results}
187+
# Convert raw SQL results to the expected format
188+
existing_data = {row[0]: TimeSeriesValue(row[0], row[1]) for row in results}
196189

197-
except AlertRuleWorkflow.DoesNotExist:
198-
# If no workflow is associated with this rule, just use the original behavior
199-
logger.exception("No workflow associated with rule", extra={"rule_id": rule.id})
200-
pass
190+
except AlertRuleWorkflow.DoesNotExist:
191+
# If no workflow is associated with this rule, just use the original behavior
192+
logger.exception("No workflow associated with rule", extra={"rule_id": rule.id})
193+
pass
201194

202195
if not existing_data:
203196
qs = (

‎tests/sentry/api/serializers/test_rule.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
from sentry.rules.filters.tagged_event import TaggedEventFilter
1515
from sentry.testutils.cases import TestCase
1616
from sentry.testutils.helpers.datetime import before_now, freeze_time
17-
from sentry.testutils.helpers.features import with_feature
1817
from sentry.users.services.user.serial import serialize_rpc_user
1918
from sentry.workflow_engine.migration_helpers.issue_alert_migration import IssueAlertMigrator
2019
from sentry.workflow_engine.models import WorkflowDataConditionGroup, WorkflowFireHistory
@@ -36,7 +35,6 @@ def test_last_triggered_rule_only(self) -> None:
3635
result = serialize(rule, self.user, RuleSerializer(expand=["lastTriggered"]))
3736
assert result["lastTriggered"] == timezone.now()
3837

39-
@with_feature("organizations:workflow-engine-single-process-workflows")
4038
def test_last_triggered_with_workflow_only(self) -> None:
4139
rule = self.create_project_rule()
4240

@@ -50,7 +48,6 @@ def test_last_triggered_with_workflow_only(self) -> None:
5048
result = serialize(rule, self.user, RuleSerializer(expand=["lastTriggered"]))
5149
assert result["lastTriggered"] == timezone.now()
5250

53-
@with_feature("organizations:workflow-engine-single-process-workflows")
5451
def test_last_triggered_with_workflow(self) -> None:
5552
rule = self.create_project_rule()
5653

‎tests/sentry/rules/history/backends/test_postgres.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
from sentry.rules.history.base import RuleGroupHistory
77
from sentry.testutils.cases import TestCase
88
from sentry.testutils.helpers.datetime import before_now, freeze_time
9-
from sentry.testutils.helpers.features import with_feature
109
from sentry.testutils.skips import requires_snuba
1110
from sentry.workflow_engine.models import AlertRuleWorkflow, WorkflowFireHistory
1211

@@ -196,7 +195,6 @@ def test_event_id(self) -> None:
196195
],
197196
)
198197

199-
@with_feature("organizations:workflow-engine-single-process-workflows")
200198
def test_combined_rule_and_workflow_history(self) -> None:
201199
"""Test combining RuleFireHistory and WorkflowFireHistory when feature flag is enabled"""
202200
rule = self.create_project_rule(project=self.event.project)
@@ -335,7 +333,6 @@ def test(self) -> None:
335333
assert len(results) == 24
336334
assert [r.count for r in results[-5:]] == [0, 0, 1, 1, 0]
337335

338-
@with_feature("organizations:workflow-engine-single-process-workflows")
339336
def test_combined_rule_and_workflow_history(self) -> None:
340337
"""Test combining RuleFireHistory and WorkflowFireHistory for hourly stats when feature flag is enabled"""
341338
rule = self.create_project_rule(project=self.event.project)

0 commit comments

Comments
 (0)