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 80885513ff..e84a1651df 100644 --- a/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts +++ b/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts @@ -6690,6 +6690,285 @@ describe('checkAlerts', () => { ).toBe(true); }); + // ── Multi-series metric tiles ── + // A multi-series metric chart is one ClickHouse query with a value column + // per series. The alert compares the threshold against the LAST series' + // value, and a series with no row at a bucket yields NULL, which + // processAlert skips. + + const gaugePoint = ( + metricName: string, + value: number, + timestampMs: number, + ) => ({ + MetricName: metricName, + ServiceName: 'api', + ResourceAttributes: { host: 'host1' }, + Value: value, + TimeUnix: new Date(timestampMs), + }); + + const setupMetricTileAlert = async ({ + select, + seriesReturnType, + threshold, + }: { + select: Array>; + seriesReturnType?: 'ratio' | 'column'; + threshold: number; + }) => { + const { team, webhook, connection, teamWebhooksById, clickhouseClient } = + await setupSavedSearchAlertTest(); + + // Persistent mock: the beforeEach mockResolvedValueOnce only covers the + // first call, and resolve notifications can trigger a second one. + jest + .spyOn(slack, 'postMessageToWebhook') + .mockResolvedValue({ text: 'ok' }); + + const source = await Source.create({ + kind: 'metric', + team: team._id, + from: { + databaseName: DEFAULT_DATABASE, + tableName: '', + }, + metricTables: { + gauge: DEFAULT_METRICS_TABLE.GAUGE, + histogram: DEFAULT_METRICS_TABLE.HISTOGRAM, + sum: DEFAULT_METRICS_TABLE.SUM, + }, + timestampValueExpression: 'TimeUnix', + connection: connection.id, + name: 'Metrics', + }); + + const dashboard = await new Dashboard({ + name: 'My Dashboard', + team: team._id, + tiles: [ + { + id: 'multi1', + x: 0, + y: 0, + w: 6, + h: 4, + config: { + name: 'Errors vs Requests', + select, + ...(seriesReturnType ? { seriesReturnType } : {}), + where: '', + displayType: 'line', + source: source.id, + groupBy: '', + }, + }, + ], + }).save(); + + const tile = dashboard.tiles?.find((t: any) => t.id === 'multi1'); + if (!tile) { + throw new Error('tile not found for multi-series metric test'); + } + + const details = await createAlertDetails( + team, + source, + { + source: AlertSource.TILE, + channel: { + type: 'webhook', + webhookId: webhook._id.toString(), + }, + interval: '5m', + thresholdType: AlertThresholdType.ABOVE, + threshold, + dashboardId: dashboard.id, + tileId: 'multi1', + }, + { + taskType: AlertTaskType.TILE, + tile, + dashboard, + }, + ); + + return { details, connection, teamWebhooksById, clickhouseClient }; + }; + + it('TILE alert (metrics, multi-series) - threshold compares the last series value', async () => { + const now = new Date('2023-11-16T22:12:00.000Z'); + // Alert window is [22:05, 22:10) + const eventMs = now.getTime() - ms('7m'); + + await bulkInsertMetricsGauge([ + // Both series exceed the threshold; only the LAST one may drive the + // alert value. + gaugePoint('test.requests', 100, eventMs), + gaugePoint('test.errors', 6, eventMs), + ]); + + const { details, connection, teamWebhooksById, clickhouseClient } = + await setupMetricTileAlert({ + select: [ + { + aggFn: 'max', + valueExpression: 'Value', + metricType: 'gauge', + metricName: 'test.requests', + }, + { + aggFn: 'max', + valueExpression: 'Value', + metricType: 'gauge', + metricName: 'test.errors', + }, + ], + threshold: 5, + }); + + await processAlertAtTime( + now, + details, + clickhouseClient, + connection.id, + alertProvider, + teamWebhooksById, + ); + + expect((await Alert.findById(details.alert.id))!.state).toBe('ALERT'); + + // The evaluated value is the last series' 6, not the first series' 100. + const [history] = await AlertHistory.find({ + alert: details.alert.id, + }).sort({ createdAt: 1 }); + expect(history.state).toBe('ALERT'); + expect(history.lastValues.length).toBe(1); + expect(history.lastValues[0].count).toBe(6); + + expect(slack.postMessageToWebhook).toHaveBeenCalledTimes(1); + expect( + jest.mocked(slack.postMessageToWebhook).mock.calls[0][1].text, + ).toContain('6 meets or exceeds 5'); + + // Next window has no data -> resolves to OK. + const nextWindow = new Date('2023-11-16T22:16:00.000Z'); + await processAlertAtTime( + nextWindow, + details, + clickhouseClient, + connection.id, + alertProvider, + teamWebhooksById, + ); + expect((await Alert.findById(details.alert.id))!.state).toBe('OK'); + }); + + it('TILE alert (metrics, multi-series) - a gap in the last series is skipped, not backfilled from an earlier series', async () => { + const now = new Date('2023-11-16T22:12:00.000Z'); + // Alert window is [22:05, 22:10) + const eventMs = now.getTime() - ms('7m'); + + await bulkInsertMetricsGauge([ + // The first series exceeds the threshold inside the window. + gaugePoint('test.requests', 100, eventMs), + // The last series exists but has no data points inside the window. + gaugePoint('test.errors', 999, now.getTime() - ms('2h')), + ]); + + const { details, connection, teamWebhooksById, clickhouseClient } = + await setupMetricTileAlert({ + select: [ + { + aggFn: 'max', + valueExpression: 'Value', + metricType: 'gauge', + metricName: 'test.requests', + }, + { + aggFn: 'max', + valueExpression: 'Value', + metricType: 'gauge', + metricName: 'test.errors', + }, + ], + threshold: 5, + }); + + await processAlertAtTime( + now, + details, + clickhouseClient, + connection.id, + alertProvider, + teamWebhooksById, + ); + + // The last series is NULL for the bucket, so the row is skipped: no + // alert fires from the first series' 100, and the run records a single + // default OK history with no values. + expect((await Alert.findById(details.alert.id))!.state).toBe('OK'); + + const histories = await AlertHistory.find({ alert: details.alert.id }); + expect(histories.length).toBe(1); + expect(histories[0].state).toBe('OK'); + expect(histories[0].counts).toBe(0); + expect(histories[0].lastValues.length).toBe(0); + + expect(slack.postMessageToWebhook).not.toHaveBeenCalled(); + }); + + it('TILE alert (metrics, ratio) - a zero denominator yields NULL and is skipped without NaN history', async () => { + const now = new Date('2023-11-16T22:12:00.000Z'); + // Alert window is [22:05, 22:10) + const eventMs = now.getTime() - ms('7m'); + + await bulkInsertMetricsGauge([ + // Numerator present, denominator zero: the ratio is NULL. + gaugePoint('test.errors', 5, eventMs), + gaugePoint('test.requests', 0, eventMs), + ]); + + const { details, connection, teamWebhooksById, clickhouseClient } = + await setupMetricTileAlert({ + select: [ + { + aggFn: 'max', + valueExpression: 'Value', + metricType: 'gauge', + metricName: 'test.errors', + }, + { + aggFn: 'max', + valueExpression: 'Value', + metricType: 'gauge', + metricName: 'test.requests', + }, + ], + seriesReturnType: 'ratio', + threshold: 0.1, + }); + + await processAlertAtTime( + now, + details, + clickhouseClient, + connection.id, + alertProvider, + teamWebhooksById, + ); + + expect((await Alert.findById(details.alert.id))!.state).toBe('OK'); + + const histories = await AlertHistory.find({ alert: details.alert.id }); + expect(histories.length).toBe(1); + expect(histories[0].state).toBe('OK'); + expect(histories[0].counts).toBe(0); + expect(histories[0].lastValues.length).toBe(0); + + expect(slack.postMessageToWebhook).not.toHaveBeenCalled(); + }); + // The auto-resolve logic ensures that if a subsequent bucket within the same tick drops below the threshold, // the alert state resets to OK, even though an alert notification might have already fired for the earlier bucket. it('should check 3 time buckets [1 error, 3 errors, 1 error] with threshold 2 and auto-resolve to OK state with 3 lastValues entries', async () => { diff --git a/packages/api/src/tasks/checkAlerts/index.ts b/packages/api/src/tasks/checkAlerts/index.ts index 3c0a1f5fc0..1bb8fce290 100644 --- a/packages/api/src/tasks/checkAlerts/index.ts +++ b/packages/api/src/tasks/checkAlerts/index.ts @@ -1366,8 +1366,9 @@ export const processAlert = async ( for (const checkData of dataForBucket) { const { value, extraFields } = parseAlertData(checkData, meta); - // TODO: we might want to fix the null value from the upstream (check if this is still needed) - // this happens when the ratio is 0/0 + // NULL means no data: a metric series with no row at this bucket, or + // a ratio with a missing/zero denominator. Skip the row instead of + // fabricating a state from a gap. if (value == null) { continue; }