fix(api): give each alert only its own history map in check-alerts

.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

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<ReturnType<typeof setupGroupedAlertBesideFiringAlert>>) => {
+        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 () => {

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<AggregatedAlertHistory, '_id'>;
+
+const buildHistory = (group?: string): TestHistory => ({
+  createdAt: new Date('2026-01-01T00:00:00Z'),
+  state: AlertState.OK,
+  group,
+});
+
+const buildPartitionTask = (
+  alerts: { id: string; previousMap: Map<string, TestHistory> }[],
+  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<AlertProvider>;
@@ -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<string, Map<string, unknown>>();
+      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