Console 2039 make metric alert notification fan out at least once with#8155
Console 2039 make metric alert notification fan out at least once with#8155jonathanawesome wants to merge 9 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a tracking mechanism for degraded metric alert channels. It adds a new database table metric_alert_channel_health to record when alert deliveries fail after exhausting retries (capped at 5 attempts). Successful deliveries clear this degraded status. On the API side, a new degradedChannels field is added to the MetricAlertRule GraphQL type, backed by a batched DataLoader. On the frontend, new UI components (DegradedChannelsBanner and DegradedChannelsIndicator) are introduced to warn users about delivery failures on the alert rules and details pages. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
🐋 This PR was built and pushed to the following Docker images: Targets: Platforms: Image Tags: |
| name: '2026.06.16T00-00-00.metric-alert-channel-health.ts', | ||
| run: ({ psql }) => psql` | ||
| -- Per-(rule, channel) record of a dropped delivery: upserted when a channel | ||
| -- exhausts its retries for an event, deleted on the next success. A sibling |
There was a problem hiding this comment.
I'm not sure I agree with deleting this record on the next success.
Are there other ways of indicating to the user that this alert was delayed due to an internal issue? I think this is valuable info if they think they should have been alerted, but weren't. It provides confidence and accountability.
There was a problem hiding this comment.
What's the alternative here? Keeping the record but marking it as non-degraded after the next successful delivery?
| AND rc."alert_channel_id" = h."alert_channel_id" | ||
| INNER JOIN "alert_channels" ac ON ac."id" = h."alert_channel_id" | ||
| WHERE h."metric_alert_rule_id" = ANY(${psql.array([...ruleIds], 'uuid')}) | ||
| AND h."degraded_at" > now() - interval '24 hours' |
There was a problem hiding this comment.
It's a backstop against rules that don't fire often. But now that I think about it...this should indicate that the channel is degraded rather than the rule + channel combination.
| outcome = 'degraded'; | ||
| try { | ||
| const lastError = (err instanceof Error ? err.message : String(err)).slice(0, 500); | ||
| await context.pg.query(psql` |
There was a problem hiding this comment.
i wonder if instead of setting the retry limit on the queue to handle limiting a requests to dead channels, if this should just handle this case here and return a success. The distinction being that if this query fails, then the job would not be retried in the current configuration. But if you keep the queue retry count at the default, and handle the logic here instead, that if this fails it will be retried up to 20 more times so ensure we've recorded the degradation.
| name | ||
| type | ||
| } | ||
| degradedChannels { |
There was a problem hiding this comment.
instead of a new type, should this be part of the AlertChannel type? e.g.
interface AlertChannel {
id: ID!
name: String!
type: AlertChannelType!
degradedAt: DateTime
lastError: String
}
2 participants
This PR limits metric alert notification retries and warns when a channel has stopped receiving alerts.
max_attempts = 5(was inheriting graphile-worker's default of 25, which retries a dead channel for days).metric_alert_channel_healthtable: the send task records a(rule, channel)pair when it exhausts all retries for an event, and clears it on the next successful delivery.MetricAlertRule.degradedChannels