From 05d8803cd5364aa9a5e19875aeaead9c7a2802db Mon Sep 17 00:00:00 2001 From: Andrew Dunn Date: Sat, 24 Oct 2026 22:09:18 -0400 Subject: [PATCH 1/2] fix(api): resolve a vanished alert group with its values Store a group-by alert group's values on its history row and fall back to them when the group has no rows left at resolve time, so {{attributes.x}} and {{var.x}} render instead of coming out empty. The joined group string cannot be parsed back into values. --- .../alert-resolve-vanished-group-values.md | 5 + docs/alert-webhook-template-variables.md | 2 +- packages/api/src/models/alertHistory.ts | 18 ++ .../__tests__/checkAlerts.int.test.ts | 204 +++++++++++++++++- packages/api/src/tasks/checkAlerts/index.ts | 20 +- 5 files changed, 245 insertions(+), 4 deletions(-) create mode 100644 .changeset/alert-resolve-vanished-group-values.md diff --git a/.changeset/alert-resolve-vanished-group-values.md b/.changeset/alert-resolve-vanished-group-values.md new file mode 100644 index 0000000000..fd926ea135 --- /dev/null +++ b/.changeset/alert-resolve-vanished-group-values.md @@ -0,0 +1,5 @@ +--- +'@hyperdx/api': patch +--- + +fix: A grouped alert whose group vanishes from the data now resolves with the group's values in `{{attributes.*}}` (alert title and message) and `{{var.*}}` (webhook body) diff --git a/docs/alert-webhook-template-variables.md b/docs/alert-webhook-template-variables.md index 83fb0ee7b2..b7d826f8eb 100644 --- a/docs/alert-webhook-template-variables.md +++ b/docs/alert-webhook-template-variables.md @@ -21,7 +21,7 @@ a HyperDX alert without parsing the human-readable message body. | `{{thresholdMax}}` | number | Upper bound of a `between`/`outside` condition. Always unset for every other comparator, even if the alert was once a range, so guard it (see below). | | `{{value}}` | number | The value that triggered or resolved the alert. | | `{{groupKey}}` | string | The breaching group, for a grouped alert. | -| `{{var}}` | object | The breaching group's values by group-by column or SQL alias. Use one value at a time, e.g. `{{var.customer_id}}`, or `{{var.[service.name]}}` for a name with dots. Empty when not grouped, and on the resolve of a group that has no rows left. | +| `{{var}}` | object | The breaching group's values by group-by column or SQL alias. Use one value at a time, e.g. `{{var.customer_id}}`, or `{{var.[service.name]}}` for a name with dots. Empty when not grouped. | | `{{sourceQuery}}` | string | The condition behind the alert — search expression or SQL (see below). | | `{{teamId}}` | string | Team the alert belongs to. | | `{{note}}` | string | The alert's freeform note — commonly a runbook link. | diff --git a/packages/api/src/models/alertHistory.ts b/packages/api/src/models/alertHistory.ts index a0b9fbb3b3..2a70b43c4c 100644 --- a/packages/api/src/models/alertHistory.ts +++ b/packages/api/src/models/alertHistory.ts @@ -40,6 +40,13 @@ export interface IAlertHistory { lastValues: { startTime: Date; count: number }[]; group?: string; // For group-by alerts, stores the group identifier fired?: boolean; + /** + * Group-by values of a group-by alert's group. Kept so a group that later + * vanishes from the data still resolves with its values, which the joined + * `group` string cannot be parsed back into. Stored as pairs because column + * names may contain dots, which Mongo would read as nested paths in a key. + */ + groupValues?: { key: string; value: string }[]; /** * Errors recorded for this evaluation window. Present on ERROR-state rows * (query/processing failures where no normal history is written) and on @@ -85,6 +92,17 @@ const AlertHistorySchema = new Schema({ type: Boolean, required: false, }, + groupValues: { + type: [ + { + _id: false, + key: { type: String, required: true }, + value: { type: String }, + }, + ], + required: false, + default: undefined, + }, errors: { type: [ { diff --git a/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts b/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts index 8ef7412f5c..feba360790 100644 --- a/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts +++ b/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts @@ -2070,7 +2070,7 @@ describe('checkAlerts', () => { const setupSavedSearchAlertTest = async ({ webhookSettings, }: Partial<{ - webhookSettings: IWebhook; + webhookSettings: Partial; }> = {}) => { const team = await createTeam({ name: 'My Team' }); @@ -5744,6 +5744,208 @@ describe('checkAlerts', () => { expect(renderedBody).toContain('Group: "ServiceName:web"'); }); + // Fires a grouped raw SQL alert for one service, then evaluates again once + // the only event has left the window so the group has no rows. + const fireThenVanish = async ({ + alias, + serviceName = 'web', + setup, + name, + message, + }: { + alias: string; + serviceName?: string; + setup?: Parameters[0]; + name?: string; + message?: string; + }) => { + const { team, webhook, connection, teamWebhooksById, clickhouseClient } = + await setupSavedSearchAlertTest(setup); + + const firedAt = new Date('2023-11-16T22:12:00.000Z'); + const eventMs = firedAt.getTime() - ms('5m'); + + await bulkInsertLogs( + [1, 2].map(i => ({ + ServiceName: serviceName, + Timestamp: new Date(eventMs), + SeverityText: 'error', + Body: `error ${i}`, + })), + ); + + const dashboard = await new Dashboard({ + name: 'Raw SQL Vanished Group Dashboard', + team: team._id, + tiles: [ + { + id: 'rawsql-vanished', + x: 0, + y: 0, + w: 6, + h: 4, + config: { + configType: 'sql', + displayType: 'line', + sqlTemplate: ` + SELECT + toStartOfInterval(Timestamp, INTERVAL {intervalSeconds:Int64} second) AS ts, + ServiceName AS "${alias}", + count() AS cnt + FROM default.otel_logs + WHERE Timestamp >= fromUnixTimestamp64Milli({startDateMilliseconds:Int64}) + AND Timestamp < fromUnixTimestamp64Milli({endDateMilliseconds:Int64}) + GROUP BY ts, "${alias}" + ORDER BY ts`, + connection: connection.id, + }, + }, + ], + }).save(); + + const tile = dashboard.tiles?.find(t => t.id === 'rawsql-vanished'); + if (!tile) throw new Error('tile not found'); + + const details = await createAlertDetails( + team, + undefined, + { + source: AlertSource.TILE, + channel: { + type: 'webhook', + webhookId: webhook._id.toString(), + }, + interval: '5m', + thresholdType: AlertThresholdType.ABOVE, + threshold: 2, + dashboardId: dashboard.id, + tileId: 'rawsql-vanished', + name, + message, + }, + { + taskType: AlertTaskType.TILE, + tile, + dashboard, + }, + ); + + const evaluateAt = (at: Date) => + processAlertAtTime( + at, + details, + clickhouseClient, + connection, + alertProvider, + teamWebhooksById, + ); + + await evaluateAt(firedAt); + expect((await Alert.findById(details.alert.id))!.state).toBe('ALERT'); + + const firedRow = await AlertHistory.findOne({ + alert: details.alert.id, + group: `${alias}:${serviceName}`, + }).lean(); + + return { + details, + firedRow, + resolve: async () => { + await evaluateAt(new Date(firedAt.getTime() + ms('10m'))); + expect((await Alert.findById(details.alert.id))!.state).toBe('OK'); + }, + }; + }; + + // Alert titles and messages read the nested `attributes` view, so a dotted + // alias is addressed by its path; `var` exists only in webhook bodies. + it.each([ + ['ServiceName', 'attributes.ServiceName'], + ['service.name', 'attributes.service.name'], + ])( + 'TILE alert (raw SQL) - a vanished group keyed by %s persists and resolves with its values', + async (alias, ref) => { + const { firedRow, resolve } = await fireThenVanish({ + alias, + name: `Errors for {{${ref}}}`, + message: `Affected service: {{${ref}}}`, + }); + expect(firedRow?.groupValues).toEqual([{ key: alias, value: 'web' }]); + + await resolve(); + + expect(slack.postMessageToWebhook).toHaveBeenCalledTimes(2); + const [, resolvePayload] = jest.mocked(slack.postMessageToWebhook).mock + .calls[1]; + expect(resolvePayload.text).toContain('Errors for web'); + expect(JSON.stringify(resolvePayload.blocks)).toContain( + 'Affected service: web', + ); + }, + ); + + it('TILE alert (raw SQL) - a vanished group resolves with its values in a generic webhook body', async () => { + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + text: jest.fn().mockResolvedValue(''), + }); + global.fetch = fetchMock; + + const { resolve } = await fireThenVanish({ + alias: 'service.name', + setup: { + webhookSettings: { + service: WebhookService.Generic, + url: 'https://webhook.site/123', + name: 'Generic Webhook', + body: JSON.stringify({ + state: '{{state}}', + service: '{{var.[service.name]}}', + }), + }, + }, + }); + await resolve(); + + expect(fetchMock).toHaveBeenCalledTimes(2); + const resolveBody = JSON.parse(fetchMock.mock.calls[1][1].body); + expect(resolveBody).toEqual({ state: 'OK', service: 'web' }); + }); + + it('TILE alert (raw SQL) - a vanished group with an empty value persists and resolves', async () => { + const { firedRow, resolve } = await fireThenVanish({ + alias: 'ServiceName', + serviceName: '', + }); + expect(firedRow?.groupValues).toEqual([ + { key: 'ServiceName', value: '' }, + ]); + + await resolve(); + + expect(slack.postMessageToWebhook).toHaveBeenCalledTimes(2); + }); + + it('TILE alert (raw SQL) - a previous ALERT row without groupValues still resolves with empty attributes', async () => { + const { details, resolve } = await fireThenVanish({ + alias: 'ServiceName', + name: 'Errors for {{attributes.ServiceName}}', + }); + await AlertHistory.updateMany( + { alert: details.alert.id }, + { $unset: { groupValues: 1 } }, + ); + + await resolve(); + + expect(slack.postMessageToWebhook).toHaveBeenCalledTimes(2); + const [, resolvePayload] = jest.mocked(slack.postMessageToWebhook).mock + .calls[1]; + expect(resolvePayload.text).toContain('Errors for'); + expect(resolvePayload.text).not.toContain('web'); + }); + it('TILE alert (raw SQL) - alert is evaluated using the last numeric column', async () => { const { team, webhook, connection, teamWebhooksById, clickhouseClient } = await setupSavedSearchAlertTest(); diff --git a/packages/api/src/tasks/checkAlerts/index.ts b/packages/api/src/tasks/checkAlerts/index.ts index 17a6b66c35..4a103fddfc 100644 --- a/packages/api/src/tasks/checkAlerts/index.ts +++ b/packages/api/src/tasks/checkAlerts/index.ts @@ -1287,7 +1287,6 @@ export const processAlert = async ( string, { value: number; attributes: Record; startTime: Date } >(); - // ponytail: a group that vanished from the data resolves without its values. const groupAttributes = new Map>(); // Helper to get or create history for a group @@ -1474,7 +1473,16 @@ export const processAlert = async ( group: groupKey, totalCount: lastValue?.count || 0, startTime: lastValue?.startTime || nowInMinsRoundDown, - attributes: groupAttributes.get(groupKey), + // A group with no rows left has no current values; use the ones + // recorded when it last reported. The aggregation yields null for + // rows that never stored any, which would skip the `{}` default. + attributes: + groupAttributes.get(groupKey) ?? + (previousHistory?.groupValues + ? Object.fromEntries( + previousHistory.groupValues.map(v => [v.key, v.value]), + ) + : undefined), }); } }; @@ -1651,6 +1659,11 @@ export const processAlert = async ( for (const [groupKey, evaluation] of bucketEvaluations.entries()) { const history = getOrCreateHistory(groupKey); + if (Object.keys(evaluation.attributes).length > 0) { + history.groupValues = Object.entries(evaluation.attributes).map( + ([key, value]) => ({ key, value }), + ); + } if (evaluation.exceeds) { history.counts += 1; @@ -1822,6 +1835,7 @@ export interface AggregatedAlertHistory { state: AlertState; group?: string; fired?: boolean; + groupValues?: { key: string; value: string }[]; } /** @@ -1883,6 +1897,7 @@ export const getPreviousAlertHistories = async ( createdAt: { $first: '$createdAt' }, state: { $first: '$state' }, fired: { $first: '$fired' }, + groupValues: { $first: '$groupValues' }, }, }, { @@ -1892,6 +1907,7 @@ export const getPreviousAlertHistories = async ( state: 1, group: '$_id.group', fired: 1, + groupValues: 1, }, }, ]); From 3b2a6d91d3ec370f1452c3d48cf6cd10358e70cb Mon Sep 17 00:00:00 2001 From: Andrew Dunn Date: Fri, 30 Oct 2026 19:11:16 -0400 Subject: [PATCH 2/2] refactor: store group values only on rows a resolve reads, behind shared helpers --- packages/api/src/models/alertHistory.ts | 21 ++++++++++++++- packages/api/src/tasks/checkAlerts/index.ts | 29 ++++++++++++--------- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/packages/api/src/models/alertHistory.ts b/packages/api/src/models/alertHistory.ts index 2a70b43c4c..53712c30a0 100644 --- a/packages/api/src/models/alertHistory.ts +++ b/packages/api/src/models/alertHistory.ts @@ -32,6 +32,25 @@ export interface IAlertHistoryAnalytics { notificationTargets?: AlertNotificationTargetTiming[]; } +/** One group-by column and its value, stored as a pair. */ +export type GroupValue = { key: string; value: string }; + +export const attributesToGroupValues = ( + attributes: Record, +): GroupValue[] | undefined => { + const entries = Object.entries(attributes); + return entries.length > 0 + ? entries.map(([key, value]) => ({ key, value })) + : undefined; +}; + +export const groupValuesToAttributes = ( + groupValues: GroupValue[] | null | undefined, +): Record | undefined => + groupValues?.length + ? Object.fromEntries(groupValues.map(v => [v.key, v.value])) + : undefined; + export interface IAlertHistory { alert: ObjectId; counts: number; @@ -46,7 +65,7 @@ export interface IAlertHistory { * `group` string cannot be parsed back into. Stored as pairs because column * names may contain dots, which Mongo would read as nested paths in a key. */ - groupValues?: { key: string; value: string }[]; + groupValues?: GroupValue[]; /** * Errors recorded for this evaluation window. Present on ERROR-state rows * (query/processing failures where no normal history is written) and on diff --git a/packages/api/src/tasks/checkAlerts/index.ts b/packages/api/src/tasks/checkAlerts/index.ts index 4a103fddfc..ca5e42ce01 100644 --- a/packages/api/src/tasks/checkAlerts/index.ts +++ b/packages/api/src/tasks/checkAlerts/index.ts @@ -60,6 +60,9 @@ import { serializeError } from 'serialize-error'; import { ALERT_HISTORY_QUERY_CONCURRENCY } from '@/controllers/alertHistory'; import { AlertState, IAlert, IAlertError } from '@/models/alert'; import AlertHistory, { + attributesToGroupValues, + GroupValue, + groupValuesToAttributes, IAlertHistory, IAlertHistoryAnalytics, } from '@/models/alertHistory'; @@ -1478,11 +1481,7 @@ export const processAlert = async ( // rows that never stored any, which would skip the `{}` default. attributes: groupAttributes.get(groupKey) ?? - (previousHistory?.groupValues - ? Object.fromEntries( - previousHistory.groupValues.map(v => [v.key, v.value]), - ) - : undefined), + groupValuesToAttributes(previousHistory?.groupValues), }); } }; @@ -1659,12 +1658,6 @@ export const processAlert = async ( for (const [groupKey, evaluation] of bucketEvaluations.entries()) { const history = getOrCreateHistory(groupKey); - if (Object.keys(evaluation.attributes).length > 0) { - history.groupValues = Object.entries(evaluation.attributes).map( - ([key, value]) => ({ key, value }), - ); - } - if (evaluation.exceeds) { history.counts += 1; if (shouldFireBasedOnConsecutiveWindows(groupKey)) { @@ -1728,6 +1721,18 @@ export const processAlert = async ( getOrCreateHistory(''); } + // A resolve reads values only from a previous ALERT or PENDING row. + for (const [groupKey, history] of histories.entries()) { + if ( + history.state === AlertState.ALERT || + history.state === AlertState.PENDING + ) { + history.groupValues = attributesToGroupValues( + groupAttributes.get(groupKey) ?? {}, + ); + } + } + // Check for state transitions and send notifications for (const [groupKey, history] of histories.entries()) { const previousKey = computeHistoryMapKey(alert.id, groupKey); @@ -1835,7 +1840,7 @@ export interface AggregatedAlertHistory { state: AlertState; group?: string; fired?: boolean; - groupValues?: { key: string; value: string }[]; + groupValues?: GroupValue[] | null; } /**