diff --git a/.changeset/alert-evaluator-extension-point.md b/.changeset/alert-evaluator-extension-point.md new file mode 100644 index 0000000000..ed39fc7334 --- /dev/null +++ b/.changeset/alert-evaluator-extension-point.md @@ -0,0 +1,5 @@ +--- +'@hyperdx/api': patch +--- + +feat: add an internal extension point for alert detection strategies; threshold alerts are evaluated as before, and the `alerts.evaluate` metric now carries an `alert_evaluator` attribute diff --git a/packages/api/src/extensions/index.ts b/packages/api/src/extensions/index.ts new file mode 100644 index 0000000000..25cfbc9437 --- /dev/null +++ b/packages/api/src/extensions/index.ts @@ -0,0 +1,5 @@ +// Registers optional extensions with the API's extension points, such as alert +// evaluators (see tasks/checkAlerts/evaluators.ts). Extension points call this +// once, on first use, so it runs after every module has loaded. This build +// registers none. +export const registerExtensions = (): void => {}; diff --git a/packages/api/src/tasks/checkAlerts/__tests__/evaluators.test.ts b/packages/api/src/tasks/checkAlerts/__tests__/evaluators.test.ts new file mode 100644 index 0000000000..77b317248b --- /dev/null +++ b/packages/api/src/tasks/checkAlerts/__tests__/evaluators.test.ts @@ -0,0 +1,413 @@ +import { AlertErrorType, DisplayType } from '@hyperdx/common-utils/dist/types'; +import { Types } from 'mongoose'; + +import { AlertState, IAlertError } from '@/models/alert'; +import AlertHistory from '@/models/alertHistory'; +import { + getAlertWindowStart, + getConsecutiveWindowHistories, + processAlert, +} from '@/tasks/checkAlerts'; +import { + AlertEvaluationContext, + AlertEvaluator, + clearAlertEvaluators, + findAlertEvaluator, + registerAlertEvaluator, +} from '@/tasks/checkAlerts/evaluators'; +import { AlertTaskType } from '@/tasks/checkAlerts/providers'; + +const makeAlert = (overrides: Record = {}): any => ({ + id: new Types.ObjectId().toString(), + interval: '5m', + threshold: 1, + thresholdType: 'above', + channel: { type: 'webhook' }, + source: 'tile', + ...overrides, +}); + +// A raw SQL inline alert needs no source and skips materialized-view +// optimization, so processAlert reaches evaluation without ClickHouse. +const makeDetails = (alert: any): any => ({ + alert, + previousMap: new Map(), + taskType: AlertTaskType.INLINE, + chartConfig: { + configType: 'sql', + sqlTemplate: 'SELECT 1', + displayType: DisplayType.Line, + }, +}); + +const makeFiredHistory = (): any => ({ + _id: new Types.ObjectId(), + createdAt: new Date(), + state: AlertState.ALERT, + fired: true, +}); + +const makeProvider = (): any => ({ + updateAlertState: jest.fn().mockResolvedValue(undefined), + recordAlertErrors: jest.fn().mockResolvedValue(undefined), +}); + +const makeClient = (): any => ({ + queryChartConfig: jest.fn().mockRejectedValue(new Error('not expected')), + requestTimeoutMs: 1000, +}); + +const run = (alert: any, now: Date, provider = makeProvider()) => { + const client = makeClient(); + return processAlert( + now, + makeDetails(alert), + client, + 'connection-1', + provider, + new Map(), + ).then(() => ({ client, provider })); +}; + +/** Registers an evaluator that runs `body` and returns its context. */ +const evaluateWith = async ( + body: (ctx: AlertEvaluationContext) => Promise, + alert = makeAlert(), +) => { + let ctx: AlertEvaluationContext | undefined; + registerAlertEvaluator({ + name: 'custom', + appliesTo: () => true, + evaluate: async c => { + ctx = c; + await body(c); + }, + }); + const result = await run(alert, new Date('2025-01-01T00:07:00Z')); + return { ...result, ctx: ctx! }; +}; + +describe('alert evaluators', () => { + afterEach(() => { + clearAlertEvaluators(); + }); + + it('finds the first evaluator that applies to the alert', () => { + const custom: AlertEvaluator = { + name: 'custom', + appliesTo: alert => alert.threshold === 42, + evaluate: jest.fn(), + }; + registerAlertEvaluator(custom); + + expect(findAlertEvaluator(makeAlert({ threshold: 42 }))).toBe(custom); + expect(findAlertEvaluator(makeAlert())).toBeUndefined(); + }); + + it('rejects a second evaluator with the same name', () => { + const evaluator: AlertEvaluator = { + name: 'custom', + appliesTo: () => true, + evaluate: jest.fn(), + }; + registerAlertEvaluator(evaluator); + expect(() => registerAlertEvaluator(evaluator)).toThrow( + 'already registered', + ); + }); + + it('evaluates with the registered evaluator instead of the threshold query', async () => { + let ctx: AlertEvaluationContext | undefined; + registerAlertEvaluator({ + name: 'custom', + appliesTo: () => true, + evaluate: async c => { + ctx = c; + const history = c.getOrCreateHistory(''); + history.state = AlertState.OK; + await c.persistHistories([history]); + }, + }); + + const alert = makeAlert(); + const { client, provider } = await run( + alert, + new Date('2025-01-01T00:07:00Z'), + ); + + expect(client.queryChartConfig).not.toHaveBeenCalled(); + expect(ctx?.alert).toBe(alert); + expect(ctx?.nowInMinsRoundDown).toEqual(new Date('2025-01-01T00:05:00Z')); + expect(provider.updateAlertState).toHaveBeenCalledTimes(1); + const [alertId, records, errors] = provider.updateAlertState.mock.calls[0]; + expect(alertId).toBe(alert.id); + expect(records).toEqual([ + expect.objectContaining({ state: AlertState.OK }), + ]); + expect(errors).toEqual([]); + }); + + it('schedules the window from the evaluator clock', async () => { + let ctx: AlertEvaluationContext | undefined; + registerAlertEvaluator({ + name: 'delayed', + appliesTo: () => true, + // Two minutes behind: 00:06 on the wall clock evaluates as 00:04. + evaluationNow: (_alert, now) => new Date(now.getTime() - 2 * 60_000), + evaluate: async c => { + ctx = c; + }, + }); + + await run(makeAlert(), new Date('2025-01-01T00:06:00Z')); + + expect(ctx?.nowInMinsRoundDown).toEqual(new Date('2025-01-01T00:00:00Z')); + }); + + it('records an error thrown by the evaluator on the alert', async () => { + registerAlertEvaluator({ + name: 'failing', + appliesTo: () => true, + evaluate: async () => { + throw new Error('boom'); + }, + }); + + const { provider } = await run( + makeAlert(), + new Date('2025-01-01T00:07:00Z'), + ); + + expect(provider.updateAlertState).not.toHaveBeenCalled(); + expect(provider.recordAlertErrors).toHaveBeenCalledTimes(1); + const [, errors, windowStart] = provider.recordAlertErrors.mock.calls[0]; + expect(errors).toEqual([expect.objectContaining({ type: 'UNKNOWN' })]); + expect(windowStart).toEqual(new Date('2025-01-01T00:05:00Z')); + }); + + it('uses the threshold query when no evaluator applies', async () => { + registerAlertEvaluator({ + name: 'other', + appliesTo: () => false, + evaluate: jest.fn(), + }); + + const { client } = await run(makeAlert(), new Date('2025-01-01T00:07:00Z')); + + expect(client.queryChartConfig).toHaveBeenCalledTimes(1); + }); + + it('reports a silenced send as suppressed', async () => { + let sent: boolean | undefined; + const { ctx } = await evaluateWith( + async c => { + sent = await c.trySendNotification({ + state: AlertState.ALERT, + group: '', + totalCount: 5, + }); + }, + makeAlert({ silenced: { until: new Date(Date.now() + 60_000) } }), + ); + + expect(sent).toBe(false); + expect(ctx.executionErrors).toEqual([]); + }); + + it('reports an attempted send even when delivery fails', async () => { + let sent: boolean | undefined; + // The test alert has no team, so the attempt records a webhook error. + const { ctx } = await evaluateWith(async c => { + sent = await c.trySendNotification({ + state: AlertState.ALERT, + group: '', + totalCount: 5, + }); + }); + + expect(sent).toBe(true); + expect(ctx.executionErrors).toHaveLength(1); + }); + + it('sends a resolve notification through the context', async () => { + const { ctx } = await evaluateWith(async c => { + const history = c.getOrCreateHistory(''); + history.state = AlertState.OK; + await c.sendNotificationIfResolved(makeFiredHistory(), history, ''); + }); + + // One dispatch attempt, failing on the missing team. + expect(ctx.executionErrors).toHaveLength(1); + }); + + it('persists extra errors with the collected ones', async () => { + const extra: IAlertError = { + timestamp: new Date(), + type: AlertErrorType.INVALID_ALERT, + message: 'extra', + }; + const { provider } = await evaluateWith(async c => { + c.executionErrors.push({ + timestamp: new Date(), + type: AlertErrorType.WEBHOOK_ERROR, + message: 'a', + }); + await c.persistHistories([c.getOrCreateHistory('')], [extra]); + }); + + const [, , errors] = provider.updateAlertState.mock.calls[0]; + expect(errors).toEqual([expect.objectContaining({ message: 'a' }), extra]); + }); + + it('attaches analytics only when something was measured', async () => { + const measured = await evaluateWith(async c => { + c.evaluationAnalytics.queryDurationMs = 12; + await c.persistHistories([c.getOrCreateHistory('')]); + }); + clearAlertEvaluators(); + const unmeasured = await evaluateWith(async c => { + await c.persistHistories([c.getOrCreateHistory('')]); + }); + + const [[, [measuredRecord]]] = + measured.provider.updateAlertState.mock.calls; + const [[, [unmeasuredRecord]]] = + unmeasured.provider.updateAlertState.mock.calls; + expect(measuredRecord.analytics).toEqual({ queryDurationMs: 12 }); + expect(unmeasuredRecord.analytics).toBeUndefined(); + }); + + it('aligns the up-front history window with the evaluator clock', () => { + registerAlertEvaluator({ + name: 'delayed', + appliesTo: () => true, + evaluationNow: (_alert, now) => new Date(now.getTime() - 2 * 60_000), + evaluate: jest.fn(), + }); + + expect( + getAlertWindowStart(makeAlert(), new Date('2025-01-01T00:06:00Z')), + ).toEqual(new Date('2025-01-01T00:00:00Z')); + }); + + it('ignores an evaluator clock that is ahead of the wall clock', () => { + registerAlertEvaluator({ + name: 'ahead', + appliesTo: () => true, + evaluationNow: (_alert, now) => new Date(now.getTime() + 10 * 60_000), + evaluate: jest.fn(), + }); + + expect( + getAlertWindowStart(makeAlert(), new Date('2025-01-01T00:06:00Z')), + ).toEqual(new Date('2025-01-01T00:05:00Z')); + }); + + it('isolates a failing evaluator lookup during the history prefetch', async () => { + registerAlertEvaluator({ + name: 'broken', + appliesTo: alert => { + if (alert.threshold === 42) { + throw new Error('lookup failed'); + } + return false; + }, + evaluate: jest.fn(), + }); + const healthy = makeAlert({ numConsecutiveWindows: 2 }); + const broken = makeAlert({ numConsecutiveWindows: 2, threshold: 42 }); + const aggregate = jest + .spyOn(AlertHistory, 'aggregate') + .mockResolvedValue([ + { createdAt: new Date(), state: AlertState.ALERT, group: '' }, + ]); + + try { + const map = await getConsecutiveWindowHistories( + [broken, healthy], + new Date('2025-01-01T00:07:00Z'), + ); + + expect(aggregate).toHaveBeenCalledTimes(1); + expect(map.has(healthy.id)).toBe(true); + expect(map.has(broken.id)).toBe(false); + } finally { + aggregate.mockRestore(); + } + }); + + it('writes collected errors when the evaluator does not persist', async () => { + const { provider } = await evaluateWith(async c => { + await c.trySendNotification({ + state: AlertState.ALERT, + group: '', + totalCount: 5, + }); + }); + + expect(provider.updateAlertState).not.toHaveBeenCalled(); + expect(provider.recordAlertErrors).toHaveBeenCalledTimes(1); + const [, errors, windowStart] = provider.recordAlertErrors.mock.calls[0]; + expect(errors).toEqual([ + expect.objectContaining({ type: AlertErrorType.WEBHOOK_ERROR }), + ]); + // No history row: the evaluator decided not to record this window. + expect(windowStart).toBeUndefined(); + }); + + it('records a failing evaluator lookup on the alert', async () => { + registerAlertEvaluator({ + name: 'broken', + appliesTo: () => { + throw new Error('lookup failed'); + }, + evaluate: jest.fn(), + }); + + const { provider } = await run( + makeAlert(), + new Date('2025-01-01T00:07:00Z'), + ); + + expect(provider.recordAlertErrors).toHaveBeenCalledTimes(1); + const [, errors] = provider.recordAlertErrors.mock.calls[0]; + expect(errors).toEqual([expect.objectContaining({ type: 'UNKNOWN' })]); + }); +}); + +describe('extension registration', () => { + it('rolls back and retries a failed registration', () => { + jest.isolateModules(() => { + const registerExtensions = jest + .fn() + .mockImplementationOnce(() => { + evaluatorsModule.registerAlertEvaluator({ + name: 'partial', + appliesTo: () => true, + evaluate: jest.fn(), + }); + throw new Error('registration failed'); + }) + .mockImplementationOnce(() => { + evaluatorsModule.registerAlertEvaluator({ + name: 'partial', + appliesTo: () => true, + evaluate: jest.fn(), + }); + }); + jest.doMock('@/extensions', () => ({ registerExtensions })); + // eslint-disable-next-line @typescript-eslint/no-require-imports, n/no-missing-require + const evaluatorsModule = require('@/tasks/checkAlerts/evaluators'); + + expect(() => evaluatorsModule.findAlertEvaluator(makeAlert())).toThrow( + 'registration failed', + ); + // The partial registration was rolled back, so the retry can register + // the same name again. + expect(evaluatorsModule.findAlertEvaluator(makeAlert())?.name).toBe( + 'partial', + ); + expect(registerExtensions).toHaveBeenCalledTimes(2); + }); + }); +}); diff --git a/packages/api/src/tasks/checkAlerts/evaluators.ts b/packages/api/src/tasks/checkAlerts/evaluators.ts new file mode 100644 index 0000000000..c2504fffdb --- /dev/null +++ b/packages/api/src/tasks/checkAlerts/evaluators.ts @@ -0,0 +1,141 @@ +import type { ClickhouseClient } from '@hyperdx/common-utils/dist/clickhouse/node'; +import type { Metadata } from '@hyperdx/common-utils/dist/core/metadata'; +import type { ChartConfigWithOptDateRange } from '@hyperdx/common-utils/dist/types'; + +import { registerExtensions } from '@/extensions'; +import type { AlertState, IAlert, IAlertError } from '@/models/alert'; +import type { + IAlertHistory, + IAlertHistoryAnalytics, +} from '@/models/alertHistory'; +import type { ISource } from '@/models/source'; +import type { AggregatedAlertHistory } from '@/tasks/checkAlerts'; +import type { + AlertDetails, + AlertProvider, +} from '@/tasks/checkAlerts/providers'; + +/** + * Internal extension point, not a stable API: members follow processAlert and + * can change in any release. + * + * What processAlert has resolved by the time it evaluates an alert: the + * scheduled window, the chart config to query, and the helpers that record + * state and send notifications. The threshold path in processAlert uses the + * same helpers, so an evaluator gets identical silencing, resolve, and + * persistence behavior. + */ +export interface AlertEvaluationContext { + alert: IAlert; + details: AlertDetails; + source: ISource | undefined; + /** Unoptimized config: evaluators that change the date range or + * granularity must re-run materialized-view selection themselves. */ + chartConfig: ChartConfigWithOptDateRange; + clickhouseClient: ClickhouseClient; + metadata: Metadata; + dateRange: [Date, Date]; + windowSizeInMins: number; + /** Scheduled start of the window being evaluated. */ + nowInMinsRoundDown: Date; + hasGroupBy: boolean; + previousMap: Map; + alertProvider: AlertProvider; + /** Persisted on every history record this evaluation writes. */ + evaluationAnalytics: IAlertHistoryAnalytics; + /** Errors collected so far (e.g. failed notification targets). */ + executionErrors: IAlertError[]; + getOrCreateHistory: (groupKey: string) => IAlertHistory; + /** Resolves false when the send was suppressed (the alert is silenced). */ + trySendNotification: (args: { + state: AlertState; + totalCount: number; + group: string; + startTime?: Date; + attributes?: Record; + }) => Promise; + sendNotificationIfResolved: ( + previousHistory: AggregatedAlertHistory | undefined, + currentHistory: IAlertHistory, + groupKey: string, + ) => Promise; + /** Writes the records and alert state the way the threshold path does, + * with `extraErrors` added to the collected execution errors. Attaches + * `evaluationAnalytics` to each record when anything was measured. */ + persistHistories: ( + records: IAlertHistory[], + extraErrors?: IAlertError[], + ) => Promise; +} + +/** + * A detection strategy for alerts. processAlert evaluates an alert with the + * first registered evaluator whose `appliesTo` returns true, and falls back to + * the built-in threshold evaluation when none does. + */ +export interface AlertEvaluator { + /** Low-cardinality name, recorded on the alerts.evaluate metric. */ + name: string; + appliesTo: (alert: IAlert) => boolean; + /** + * The clock to evaluate against, e.g. shifted back to wait for late data. + * Defaults to the wall clock, and a result later than the wall clock is + * ignored. Applied both in processAlert and when fetching history up front, + * so the scheduled window and the history it is compared to stay aligned. + */ + evaluationNow?: (alert: IAlert, wallClockNow: Date) => Date; + /** Evaluates the window and persists state through `ctx`. Throwing records + * the error on the alert, as for the threshold path. Returning without + * calling `persistHistories` leaves state and history unchanged, but still + * writes `executionErrors` to the alert. */ + evaluate: (ctx: AlertEvaluationContext) => Promise; +} + +export const THRESHOLD_EVALUATOR_NAME = 'threshold'; + +const evaluators: AlertEvaluator[] = []; +let extensionsRegistered = false; + +export const registerAlertEvaluator = (evaluator: AlertEvaluator) => { + if (evaluators.some(e => e.name === evaluator.name)) { + throw new Error( + `Alert evaluator "${evaluator.name}" is already registered`, + ); + } + evaluators.push(evaluator); +}; + +/** + * Runs registerExtensions once. A failed registration is rolled back and + * rethrown, and retried on the next lookup: routing alerts to the threshold + * path because their evaluator failed to register would evaluate them wrongly + * without any error. + */ +const ensureExtensionsRegistered = () => { + if (extensionsRegistered) { + return; + } + const registeredBefore = evaluators.length; + try { + registerExtensions(); + } catch (e) { + evaluators.length = registeredBefore; + throw e; + } + extensionsRegistered = true; +}; + +/** The registered evaluator for this alert, or undefined for the threshold + * path. */ +export const findAlertEvaluator = ( + alert: IAlert, +): AlertEvaluator | undefined => { + ensureExtensionsRegistered(); + return evaluators.find(e => e.appliesTo(alert)); +}; + +/** @internal Test helper. Extensions are not registered again afterwards. */ +export const clearAlertEvaluators = () => { + extensionsRegistered = true; + evaluators.length = 0; +}; diff --git a/packages/api/src/tasks/checkAlerts/index.ts b/packages/api/src/tasks/checkAlerts/index.ts index 6381d90507..3ad18042f1 100644 --- a/packages/api/src/tasks/checkAlerts/index.ts +++ b/packages/api/src/tasks/checkAlerts/index.ts @@ -79,6 +79,12 @@ import { WebhookNotFoundError, WebhookRedirectError, } from '@/tasks/checkAlerts/errors'; +import { + AlertEvaluationContext, + AlertEvaluator, + findAlertEvaluator, + THRESHOLD_EVALUATOR_NAME, +} from '@/tasks/checkAlerts/evaluators'; import { AlertDetails, AlertProvider, @@ -436,13 +442,32 @@ export const getScheduledWindowStart = ( return fns.addMinutes(roundedShiftedNow, scheduleOffsetMinutes); }; +/** + * The clock an alert is evaluated against: the wall clock, unless the alert's + * evaluator shifts it back (see AlertEvaluator.evaluationNow). Never later + * than the wall clock: previous-state lookups bound history by the wall clock, + * which only covers every window when the evaluation clock is not ahead of it. + */ +const getAlertEvaluationNow = ( + alert: IAlert, + now: Date, + evaluator = findAlertEvaluator(alert), +): Date => { + const shifted = evaluator?.evaluationNow?.(alert, now); + return shifted != null && shifted < now ? shifted : now; +}; + /** * Compute the scheduled window start ("now rounded down to the window") for an * alert at the given time. This mirrors the computation inside processAlert so * that history fetched up-front (see getConsecutiveWindowHistories) lines up * exactly with the window processAlert evaluates. */ -const getAlertWindowStart = (alert: IAlert, now: Date): Date => { +export const getAlertWindowStart = ( + alert: IAlert, + wallClockNow: Date, +): Date => { + const now = getAlertEvaluationNow(alert, wallClockNow); const windowSizeInMins = ms(alert.interval) / 60000; const scheduleStartAt = normalizeScheduleStartAt({ alertId: alert.id, @@ -976,7 +1001,7 @@ export const alertQueryAttribution = ( }); export const processAlert = async ( - now: Date, + wallClockNow: Date, details: AlertDetails, clickhouseClient: ClickhouseClient, connectionId: string, @@ -984,6 +1009,9 @@ export const processAlert = async ( teamWebhooksById: Map, ) => { const { alert, previousMap, recentHistoryMap } = details; + // Resolved inside the try so a failing evaluator lookup is recorded on the + // alert; hoisted so the finally can label the evaluation. + let evaluator: AlertEvaluator | undefined; const source = 'source' in details ? details.source : undefined; // Errors collected during this execution. Webhook errors accumulate here; query // and validation errors are recorded via recordAlertErrors before returning. @@ -1041,6 +1069,8 @@ export const processAlert = async ( .slice(0, ALERT_NOTIFICATION_TARGETS_LIMIT); }; try { + evaluator = findAlertEvaluator(alert); + const now = getAlertEvaluationNow(alert, wallClockNow, evaluator); const windowSizeInMins = ms(alert.interval) / 60000; const scheduleStartAt = normalizeScheduleStartAt({ alertId: alert.id, @@ -1206,81 +1236,6 @@ export const processAlert = async ( ? { readonly: '2' } : {}; - // Query for alert data. If the query fails, record the error and exit - // without touching alert state or creating an AlertHistory. - let checksData; - const queryStartedAt = performance.now(); - try { - checksData = await clickhouseClient.queryChartConfig({ - config: optimizedChartConfig, - metadata, - opts: { clickhouse_settings: clickHouseSettings }, - querySettings: source?.querySettings, - }); - // SLO signal for alert data fetching (distinct from the end-to-end - // evaluation SLI): did ClickHouse serve the alert query, and how fast. - const queryDurationMs = performance.now() - queryStartedAt; - evaluationAnalytics.queryDurationMs = Math.round(queryDurationMs); - recordOperationOutcome({ - operation: 'alerts.query', - outcome: 'success', - durationMs: queryDurationMs, - attributes: { alert_source: alert.source ?? 'unknown' }, - }); - } catch (e) { - const queryDurationMs = performance.now() - queryStartedAt; - // Time-to-failure — for QUERY_TIMEOUT this is roughly the configured - // evaluation timeout. - evaluationAnalytics.queryDurationMs = Math.round(queryDurationMs); - recordOperationOutcome({ - operation: 'alerts.query', - outcome: 'error', - durationMs: queryDurationMs, - attributes: { alert_source: alert.source ?? 'unknown' }, - }); - evalOutcome = 'error'; - const alertError = makeQueryAlertError( - e, - clickhouseClient.requestTimeoutMs, - ); - alertQueryFailuresCounter.add(1, { - error_type: - alertError.type === AlertErrorType.QUERY_TIMEOUT - ? 'timeout' - : 'error', - }); - logger.error( - { - alertId: alert.id, - errorType: alertError.type, - error: serializeError(e), - }, - 'Alert query failed, skipping state/history update', - ); - // Record the error on the alert and as an ERROR history row for this - // window. ERROR rows are excluded from the due-ness gate and date-range - // computation, so the failed window is still retried/backfilled. - await alertProvider.recordAlertErrors( - alert.id, - [alertError], - nowInMinsRoundDown, - evaluationAnalytics, - ); - return; - } - - logger.info( - { - alertId: alert.id, - chartConfig, - optimizedChartConfig, - checksData, - checkStartTime: dateRange[0], - checkEndTime: dateRange[1], - }, - `Received alert metric [${alert.source} source]`, - ); - // Track state per group (or one history if no groupBy) const histories = new Map(); const latestAlertContext = new Map< @@ -1331,6 +1286,10 @@ export const processAlert = async ( : undefined; // Helper to send a notification, catching and logging any errors. + // Resolves true when a dispatch was attempted (even if delivery later + // fails — failures surface via executionErrors, mirroring the threshold + // path which marks history.fired before dispatching), and false when the + // send was suppressed entirely (silenced). const trySendNotification = async ({ group, totalCount, @@ -1343,7 +1302,7 @@ export const processAlert = async ( group: string; startTime?: Date; attributes?: Record; - }) => { + }): Promise => { // KNOWN LIMITATION: Alert data (including silenced state) is fetched when // the task is queued via AlertProvider, not when it processes. If a user // silences an alert after it's queued but before it processes, this @@ -1359,7 +1318,7 @@ export const processAlert = async ( }, 'Skipped firing alert due to silence', ); - return; + return false; } alertEvaluationsCounter.add(1, { @@ -1430,6 +1389,7 @@ export const processAlert = async ( ); executionErrors.push(makeWebhookAlertError(e)); } + return true; }; const numWindowsToLookBack = alert.numConsecutiveWindows ?? 1; @@ -1483,6 +1443,133 @@ export const processAlert = async ( } }; + let persisted = false; + const persistHistories = async ( + records: IAlertHistory[], + extraErrors: IAlertError[] = [], + ) => { + persisted = true; + flushNotificationTimings(); + // An evaluation that measured nothing persists no analytics object. + if (Object.keys(evaluationAnalytics).length > 0) { + for (const record of records) { + record.analytics = evaluationAnalytics; + } + } + await alertProvider.updateAlertState( + alert.id, + records, + [...executionErrors, ...extraErrors], + dateRange, + ); + }; + + if (evaluator) { + const ctx: AlertEvaluationContext = { + alert, + details, + source, + chartConfig, + clickhouseClient, + metadata, + dateRange, + windowSizeInMins, + nowInMinsRoundDown, + hasGroupBy, + previousMap, + alertProvider, + evaluationAnalytics, + executionErrors, + getOrCreateHistory, + trySendNotification, + sendNotificationIfResolved, + persistHistories, + }; + await evaluator.evaluate(ctx); + if (!persisted) { + // Every completed evaluation overwrites the alert's execution errors, + // as the threshold path does through persistHistories. Without this, + // failed notifications would be dropped and stale errors would stay. + await alertProvider.recordAlertErrors(alert.id, executionErrors); + } + return; + } + + // Query for threshold alert data. If the query fails, record the error and exit + // without touching alert state or creating an AlertHistory. + let checksData; + const queryStartedAt = performance.now(); + try { + checksData = await clickhouseClient.queryChartConfig({ + config: optimizedChartConfig, + metadata, + opts: { clickhouse_settings: clickHouseSettings }, + querySettings: source?.querySettings, + }); + // SLO signal for alert data fetching (distinct from the end-to-end + // evaluation SLI): did ClickHouse serve the alert query, and how fast. + const queryDurationMs = performance.now() - queryStartedAt; + evaluationAnalytics.queryDurationMs = Math.round(queryDurationMs); + recordOperationOutcome({ + operation: 'alerts.query', + outcome: 'success', + durationMs: queryDurationMs, + attributes: { alert_source: alert.source ?? 'unknown' }, + }); + } catch (e) { + const queryDurationMs = performance.now() - queryStartedAt; + // Time-to-failure — for QUERY_TIMEOUT this is roughly the configured + // evaluation timeout. + evaluationAnalytics.queryDurationMs = Math.round(queryDurationMs); + recordOperationOutcome({ + operation: 'alerts.query', + outcome: 'error', + durationMs: queryDurationMs, + attributes: { alert_source: alert.source ?? 'unknown' }, + }); + evalOutcome = 'error'; + const alertError = makeQueryAlertError( + e, + clickhouseClient.requestTimeoutMs, + ); + alertQueryFailuresCounter.add(1, { + error_type: + alertError.type === AlertErrorType.QUERY_TIMEOUT + ? 'timeout' + : 'error', + }); + logger.error( + { + alertId: alert.id, + errorType: alertError.type, + error: serializeError(e), + }, + 'Alert query failed, skipping state/history update', + ); + // Record the error on the alert and as an ERROR history row for this + // window. ERROR rows are excluded from the due-ness gate and date-range + // computation, so the failed window is still retried/backfilled. + await alertProvider.recordAlertErrors( + alert.id, + [alertError], + nowInMinsRoundDown, + evaluationAnalytics, + ); + return; + } + + logger.info( + { + alertId: alert.id, + chartConfig, + optimizedChartConfig, + checksData, + checkStartTime: dateRange[0], + checkEndTime: dateRange[1], + }, + `Received alert metric [${alert.source} source]`, + ); + const meta = getResponseMetadata(chartConfig, checksData); if (!meta) { evalOutcome = 'error'; @@ -1529,17 +1616,7 @@ export const processAlert = async ( // Single-value evaluations always cover exactly the current window. evaluationAnalytics.backfilledBuckets = 0; - flushNotificationTimings(); - const historyRecords = Array.from(histories.values()); - for (const record of historyRecords) { - record.analytics = evaluationAnalytics; - } - await alertProvider.updateAlertState( - alert.id, - historyRecords, - executionErrors, - dateRange, - ); + await persistHistories(Array.from(histories.values())); return; } @@ -1763,17 +1840,7 @@ export const processAlert = async ( } // Save all history records and update alert state - flushNotificationTimings(); - const historyRecords = Array.from(histories.values()); - for (const record of historyRecords) { - record.analytics = evaluationAnalytics; - } - await alertProvider.updateAlertState( - alert.id, - historyRecords, - executionErrors, - dateRange, - ); + await persistHistories(Array.from(histories.values())); } catch (e) { // Uncomment this for better error messages locally // console.error(e); @@ -1821,7 +1888,10 @@ export const processAlert = async ( operation: 'alerts.evaluate', outcome: evalOutcome, durationMs: performance.now() - evalStartedAt, - attributes: { alert_source: alert.source ?? 'unknown' }, + attributes: { + alert_source: alert.source ?? 'unknown', + alert_evaluator: evaluator?.name ?? THRESHOLD_EVALUATOR_NAME, + }, }); } } @@ -1969,7 +2039,20 @@ export const getConsecutiveWindowHistories = async ( queue.add(async () => { const numWindowsToLookBack = alert.numConsecutiveWindows ?? 1; const windowSizeInMins = ms(alert.interval) / 60000; - const windowStart = getAlertWindowStart(alert, now); + let windowStart: Date; + try { + windowStart = getAlertWindowStart(alert, now); + } catch (e) { + // The evaluator lookup runs extension code. A failure must not + // reject this batch, which would stop every alert loading this + // tick. processAlert repeats the lookup and records the error on + // this alert. + logger.error( + { alertId: alert.id, error: serializeError(e) }, + 'Failed to compute alert window start for history prefetch', + ); + return undefined; + } const earliestAllowedTime = new Date( windowStart.getTime() - (numWindowsToLookBack - 1) * windowSizeInMins * 60_000,