Remove references to RuleFireHistory#115036
Conversation
github-actions
Bot
added
the
Scope: Backend
ceorourke
changed the title
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c9938f9. Configure here.
| FROM workflow_engine_workflowfirehistory | ||
| WHERE workflow_id = %s AND date_added >= %s AND date_added < %s |
There was a problem hiding this comment.
I assume we backfilled this data and it's all in the workflowfirehistory table now?
There was a problem hiding this comment.
I'm not sure if we backfilled it but rather started writing to it instead and we only need so much history. I forget the exact retention of rulefirehistory but it's totally empty because of the cleanup tasks
There was a problem hiding this comment.
Oh, if it's totally empty you can ignore my comments about the deletion code then
| def record( | ||
| self, | ||
| rule: Rule, | ||
| group: Group, | ||
| event_id: str | None = None, | ||
| notification_uuid: str | None = None, | ||
| ) -> RuleFireHistory | None: |
There was a problem hiding this comment.
Bug: When the organizations:workflow-engine-issue-alert-endpoints-get flag is off, fetch_rule_hourly_stats and fetch_rule_groups_paginated receive a Rule object but expect a Workflow, causing them to silently return empty data.
Severity: HIGH
Suggested Fix
Reintroduce logic to handle both Rule and Workflow objects passed as the target argument. This could involve checking the type of target and using a helper like the removed get_rule_workflow_ids to correctly retrieve the workflow_id when a Rule object is provided, ensuring backward compatibility when the feature flag is disabled.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/rules/history/backends/postgres.py#L88-L94
Potential issue: The functions `fetch_rule_hourly_stats` and
`fetch_rule_groups_paginated` were updated to assume their `target` argument is always a
`Workflow` object. However, when the
`organizations:workflow-engine-issue-alert-endpoints-get` feature flag is disabled,
legacy API endpoints pass a `Rule` object instead. The functions then use `target.id`
(which is a `Rule.id`) to query the `workflow_engine_workflowfirehistory` table, which
expects a `Workflow.id`. Since `Rule.id` and `Workflow.id` are from different tables,
the query finds no match and silently returns empty history and stats for legacy rules,
breaking functionality for users without the feature flag enabled.
2 participants
Remove a lot (but not all) of references to
RuleFireHistory. This table is completely empty as we don't write to it anymore and it's passed the retention period so the records have been cleaned up. I'm working on removing the remaining references #115035 which are tied to notifications but I need to do a migration to remove the FK onNotificationMessagefirst and this is still a large diff so I chose to keep it separate.