From 98443f059d4253ed848ffcfe56a00709c818ab7d Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Fri, 13 Nov 2026 21:39:00 -0800 Subject: [PATCH 1/2] fix(api): give each alert only its own history map in check-alerts Every alert in a tick received one previousMap holding the latest history of every group of every alert, and the skip check plus three evaluation paths scanned it in full. Tick CPU grew with alerts times total groups. Split the shared map into per-alert maps once, right after loading the tasks. Keys, values, types, loaders and readers are unchanged. Refs HDX-6022 --- .changeset/alert-history-map-per-alert.md | 6 + .../__tests__/checkAlertsTask.test.ts | 183 +++++++++++++++++- packages/api/src/tasks/checkAlerts/index.ts | 46 ++++- 3 files changed, 232 insertions(+), 3 deletions(-) create mode 100644 .changeset/alert-history-map-per-alert.md diff --git a/.changeset/alert-history-map-per-alert.md b/.changeset/alert-history-map-per-alert.md new file mode 100644 index 0000000000..e6f6e424df --- /dev/null +++ b/.changeset/alert-history-map-per-alert.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/api': patch +--- + +perf: check-alerts gives each alert only its own previous history, so a tick +no longer scans every alert's group history once per alert diff --git a/packages/api/src/tasks/checkAlerts/__tests__/checkAlertsTask.test.ts b/packages/api/src/tasks/checkAlerts/__tests__/checkAlertsTask.test.ts index 5b5d348898..06f00de963 100644 --- a/packages/api/src/tasks/checkAlerts/__tests__/checkAlertsTask.test.ts +++ b/packages/api/src/tasks/checkAlerts/__tests__/checkAlertsTask.test.ts @@ -3,11 +3,19 @@ import mongoose from 'mongoose'; import * as config from '@/config'; import { ObjectId } from '@/models'; -import { AlertSource, AlertThresholdType, IAlert } from '@/models/alert'; +import { + AlertSource, + AlertState, + AlertThresholdType, + IAlert, +} from '@/models/alert'; import { ISource } from '@/models/source'; import { IWebhook } from '@/models/webhook'; import * as checkAlerts from '@/tasks/checkAlerts'; -import CheckAlertTask from '@/tasks/checkAlerts'; +import CheckAlertTask, { + AggregatedAlertHistory, + partitionPreviousMapsByAlert, +} from '@/tasks/checkAlerts'; import { AlertDetails, AlertProvider, @@ -24,6 +32,130 @@ jest.mock('@/tasks/checkAlerts/providers', () => { }; }); +type TestHistory = Omit; + +const buildHistory = (group?: string): TestHistory => ({ + createdAt: new Date('2026-01-01T00:00:00Z'), + state: AlertState.OK, + group, +}); + +const buildPartitionTask = ( + alerts: { id: string; previousMap: Map }[], + teamId = new mongoose.Types.ObjectId(), +): AlertTask => + ({ + alerts: alerts.map(({ id, previousMap }) => ({ + alert: { + id, + team: { _id: teamId }, + source: AlertSource.SAVED_SEARCH, + threshold: 10, + thresholdType: AlertThresholdType.ABOVE, + interval: '1m', + channel: { type: 'webhook', webhookId: 'webhook-123' }, + }, + taskType: AlertTaskType.SAVED_SEARCH, + previousMap, + })), + conn: { + id: 'conn-123', + _id: new mongoose.Types.ObjectId(), + host: config.CLICKHOUSE_HOST, + username: config.CLICKHOUSE_USER, + password: config.CLICKHOUSE_PASSWORD, + name: '', + team: teamId, + }, + now: new Date(), + }) as unknown as AlertTask; + +describe('partitionPreviousMapsByAlert', () => { + it('gives each alert only its own entries from a shared map, keys unchanged', () => { + const groupedA = buildHistory('service:api'); + const groupedB = buildHistory('service:web'); + const ungrouped = buildHistory(); + const otherAlert = buildHistory(); + const shared = new Map([ + ['grouped||service:api', groupedA], + ['grouped||service:web', groupedB], + ['ungrouped', ungrouped], + ['not-in-tick', otherAlert], + ]); + + const [task] = partitionPreviousMapsByAlert([ + buildPartitionTask([ + { id: 'grouped', previousMap: shared }, + { id: 'ungrouped', previousMap: shared }, + { id: 'no-history', previousMap: shared }, + ]), + ]); + + expect(task.alerts[0].previousMap).toEqual( + new Map([ + ['grouped||service:api', groupedA], + ['grouped||service:web', groupedB], + ]), + ); + expect(task.alerts[1].previousMap).toEqual( + new Map([['ungrouped', ungrouped]]), + ); + expect(task.alerts[2].previousMap.size).toBe(0); + }); + + it('keeps a group name that contains the delimiter with its alert', () => { + const history = buildHistory('path:a||b'); + const shared = new Map([['alert-1||path:a||b', history]]); + + const [task] = partitionPreviousMapsByAlert([ + buildPartitionTask([{ id: 'alert-1', previousMap: shared }]), + ]); + + expect(task.alerts[0].previousMap).toEqual( + new Map([['alert-1||path:a||b', history]]), + ); + }); + + it('splits maps shared across tasks and keeps already separate maps', () => { + const shared = new Map([ + ['alert-1', buildHistory()], + ['alert-2||g:1', buildHistory('g:1')], + ]); + const own = new Map([['alert-3', buildHistory()]]); + + const tasks = partitionPreviousMapsByAlert([ + buildPartitionTask([{ id: 'alert-1', previousMap: shared }]), + buildPartitionTask([ + { id: 'alert-2', previousMap: shared }, + { id: 'alert-3', previousMap: own }, + ]), + ]); + + expect([...tasks[0].alerts[0].previousMap.keys()]).toEqual(['alert-1']); + expect([...tasks[1].alerts[0].previousMap.keys()]).toEqual([ + 'alert-2||g:1', + ]); + expect([...tasks[1].alerts[1].previousMap.keys()]).toEqual(['alert-3']); + }); + + it('does not mutate the input tasks or maps', () => { + const shared = new Map([ + ['alert-1', buildHistory()], + ['alert-2', buildHistory()], + ]); + const input = buildPartitionTask([ + { id: 'alert-1', previousMap: shared }, + { id: 'alert-2', previousMap: shared }, + ]); + + partitionPreviousMapsByAlert([input]); + + expect(shared.size).toBe(2); + expect(input.alerts[0].previousMap).toBe(shared); + expect(input.alerts[1].previousMap).toBe(shared); + }); +}); + describe('CheckAlertTask', () => { describe('execute', () => { let mockAlertProvider: jest.Mocked; @@ -331,6 +463,53 @@ describe('CheckAlertTask', () => { ); }); + it('should pass each alert only its own previous history', async () => { + const args: CheckAlertsTaskArgs = { taskName: TaskName.CHECK_ALERTS }; + const task = new CheckAlertTask(args); + + const groupA = buildHistory('g:a'); + const groupB = buildHistory('g:b'); + const ungrouped = buildHistory(); + const shared = new Map([ + ['alert-1||g:a', groupA], + ['alert-1||g:b', groupB], + ['alert-2', ungrouped], + ]); + + mockAlertProvider.getAlertTasks.mockResolvedValue([ + buildPartitionTask([ + { id: 'alert-1', previousMap: shared }, + { id: 'alert-2', previousMap: shared }, + ]), + ]); + mockAlertProvider.getWebhooks.mockResolvedValue(new Map()); + mockAlertProvider.getClickHouseClient.mockResolvedValue( + new ClickhouseClient({}), + ); + const receivedMaps = new Map>(); + mockProcessAlert.mockImplementation( + async (_now: Date, details: AlertDetails) => { + receivedMaps.set(details.alert.id, details.previousMap); + }, + ); + + await task.execute(); + + expect(receivedMaps).toEqual( + new Map([ + [ + 'alert-1', + new Map([ + ['alert-1||g:a', groupA], + ['alert-1||g:b', groupB], + ]), + ], + ['alert-2', new Map([['alert-2', ungrouped]])], + ]), + ); + expect(shared.size).toBe(3); + }); + describe('when a connection fails', () => { // The task queue runs batches fire-and-forget, and the worker's global // unhandledRejection handler exits the process, so a single failing diff --git a/packages/api/src/tasks/checkAlerts/index.ts b/packages/api/src/tasks/checkAlerts/index.ts index d34af3dd85..18ece93b57 100644 --- a/packages/api/src/tasks/checkAlerts/index.ts +++ b/packages/api/src/tasks/checkAlerts/index.ts @@ -628,6 +628,48 @@ const extractGroupKeyFromMapKey = (mapKey: string, alertId: string): string => { : ''; }; +/** + * Give each alert a history map holding only its own entries. Providers may + * hand every alert one map shared across the tick, and every reader scans the + * map, so a shared map makes tick CPU grow with alerts times total groups. + */ +export const partitionPreviousMapsByAlert = ( + alertTasks: AlertTask[], +): AlertTask[] => { + const bucketsBySharedMap = new Map< + Map, + Map> + >(); + const getBuckets = (previousMap: Map) => { + let buckets = bucketsBySharedMap.get(previousMap); + if (buckets == null) { + buckets = new Map(); + for (const [key, history] of previousMap) { + const delimiterIndex = key.indexOf(ALERT_GROUP_DELIMITER); + const alertId = + delimiterIndex === -1 ? key : key.substring(0, delimiterIndex); + let bucket = buckets.get(alertId); + if (bucket == null) { + bucket = new Map(); + buckets.set(alertId, bucket); + } + bucket.set(key, history); + } + bucketsBySharedMap.set(previousMap, buckets); + } + return buckets; + }; + + return alertTasks.map(task => ({ + ...task, + alerts: task.alerts.map(details => ({ + ...details, + previousMap: + getBuckets(details.previousMap).get(details.alert.id) ?? new Map(), + })), + })); +}; + /** Determine if we should skip the alert check based on how recently it was last evaluated. */ const shouldSkipAlertCheck = ( details: AlertDetails, @@ -2221,7 +2263,9 @@ export default class CheckAlertTask implements HdxTask { 'finished provider initialization', ); - const alertTasks = await this.provider.getAlertTasks(); + const alertTasks = partitionPreviousMapsByAlert( + await this.provider.getAlertTasks(), + ); const taskCount = alertTasks.length; logger.debug( { From f4204fdae9662b38975fe630e644a80e2b1b34de Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Fri, 13 Nov 2026 22:28:35 -0800 Subject: [PATCH 2/2] test(api): cover per-alert history split in processAlert Run two alerts through one shared history map, split it the way CheckAlertTask.execute() does, and evaluate the grouped alert for real. Asserts that another alert's ALERT row no longer adds an empty-group history or collapses the alert's empty buckets into one. Refs HDX-6022 --- .../__tests__/checkAlerts.int.test.ts | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) 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 d3d0bbdc3b..30badac6df 100644 --- a/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts +++ b/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts @@ -43,6 +43,7 @@ import { getPreviousAlertHistories, getScheduledWindowStart, parseAlertData, + partitionPreviousMapsByAlert, processAlert, } from '@/tasks/checkAlerts'; import { @@ -6933,6 +6934,132 @@ describe('checkAlerts', () => { expect(resolutionCall).toBeDefined(); }); + describe('with a previous-history map shared by every alert in the tick', () => { + const now = new Date('2023-11-16T22:12:00.000Z'); + + const setupGroupedAlertBesideFiringAlert = async () => { + const setup = await setupSavedSearchAlertTest(); + const { team, webhook, source, savedSearch } = setup; + const alertConfig = { + source: AlertSource.SAVED_SEARCH, + channel: { type: 'webhook', webhookId: webhook._id.toString() }, + interval: '5m', + thresholdType: AlertThresholdType.ABOVE, + threshold: 1, + savedSearchId: savedSearch.id, + } as const; + const savedSearchDetails = { + taskType: AlertTaskType.SAVED_SEARCH, + savedSearch, + } as const; + + const grouped = await createAlertDetails( + team, + source, + { ...alertConfig, groupBy: 'ServiceName' }, + savedSearchDetails, + ); + const firing = await createAlertDetails( + team, + source, + alertConfig, + savedSearchDetails, + ); + await AlertHistory.create({ + alert: firing.alert.id, + createdAt: new Date('2023-11-16T22:05:00.000Z'), + state: AlertState.ALERT, + fired: true, + counts: 1, + lastValues: [], + }); + + return { ...setup, grouped, firing }; + }; + + // Mirrors CheckAlertTask.execute(): load one shared map, then split it. + const evaluateGroupedAlert = async ({ + grouped, + firing, + connection, + clickhouseClient, + teamWebhooksById, + }: Awaited>) => { + const shared = await getPreviousAlertHistories( + [grouped.alert.id, firing.alert.id], + now, + ); + expect(shared.get(firing.alert.id)?.state).toBe(AlertState.ALERT); + + const [task] = partitionPreviousMapsByAlert([ + { + alerts: [ + { ...grouped, previousMap: shared }, + { ...firing, previousMap: shared }, + ], + conn: connection, + now, + }, + ]); + await processAlert( + now, + task.alerts[0], + clickhouseClient, + connection.id, + alertProvider, + teamWebhooksById, + ); + + return AlertHistory.find({ alert: grouped.alert.id }).sort({ + createdAt: 1, + }); + }; + + it("does not write an empty-group history from another alert's ALERT state", async () => { + const setup = await setupGroupedAlertBesideFiringAlert(); + await bulkInsertLogs( + [1, 2].map(() => ({ + ServiceName: 'service-a', + Timestamp: new Date('2023-11-16T22:05:00.000Z'), + SeverityText: 'error', + Body: 'Error from service-a', + })), + ); + + const histories = await evaluateGroupedAlert(setup); + + expect(histories.map(h => [h.group, h.state])).toEqual([ + ['ServiceName:service-a', AlertState.ALERT], + ]); + }); + + it('records every empty bucket when only another alert is firing', async () => { + const setup = await setupGroupedAlertBesideFiringAlert(); + await AlertHistory.create({ + alert: setup.grouped.alert.id, + createdAt: new Date('2023-11-16T21:55:00.000Z'), + state: AlertState.OK, + group: 'ServiceName:service-a', + counts: 0, + lastValues: [], + }); + + const histories = await evaluateGroupedAlert(setup); + + const latest = histories[histories.length - 1]; + expect(histories).toHaveLength(2); + expect(latest.group).toBeUndefined(); + expect(latest.state).toBe(AlertState.OK); + expect( + latest.lastValues.map(v => [v.startTime.toISOString(), v.count]), + ).toEqual([ + ['2023-11-16T21:55:00.000Z', 0], + ['2023-11-16T22:00:00.000Z', 0], + ['2023-11-16T22:05:00.000Z', 0], + ]); + }); + }); + // The sample rows quoted in the message body carry no group predicate, so // every group's notification used to re-run the identical query. it('fetches the message body sample rows once for all groups in a window', async () => {