From a6bb0405bad35b783c4c805e7d1ed572dabcfc0d Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Wed, 26 Aug 2026 16:16:02 -0700 Subject: [PATCH 1/6] feat(api): support alerts with inline chart configs (HDX-5090) Add a 'chart' alert source that persists its own chart config directly on the alert document, so alerts no longer require a saved search (logs) or a dashboard tile (metrics). Builder configs on log/trace/metric sources plus raw SQL (Line/StackedBar/Number) are supported; PromQL is rejected. - common-utils: AlertSource.CHART, AlertChartConfigSchema, zChartAlert, AlertSchema union member, chartConfig on AlertsPageItemSchema - model: chartConfig (Mixed) on Alert; makeAlert persists/clears it like the other source references - internal API: internalAlertSchema accepts the new source (external v2 keeps the narrower alertSchema until its contract is extended); validateAlertInput checks display type, raw SQL template, and team-scoped source/connection ownership; responses include chartConfig - check-alerts: new CHART task type evaluated through the same code path as tile alerts (shared buildAlertChartConfigFromSavedConfig), including group-by and multi-window behavior; notifications link to the chart explorer seeded with the alert's config and default their title to the config's name Backend only; the creation/edit UI and external API v2 support land separately. --- .changeset/detached-chart-alerts-backend.md | 6 + .../src/controllers/__tests__/alerts.test.ts | 57 +++- packages/api/src/controllers/alerts.ts | 74 ++++- packages/api/src/fixtures.ts | 46 +++ packages/api/src/models/alert.ts | 13 + .../routers/api/__tests__/alerts.int.test.ts | 255 +++++++++++++++ packages/api/src/routers/api/alerts.ts | 9 +- .../external-api/__tests__/alerts.int.test.ts | 35 ++ .../__tests__/checkAlerts.int.test.ts | 300 ++++++++++++++++++ .../__tests__/checkAlertsTask.test.ts | 1 + packages/api/src/tasks/checkAlerts/index.ts | 272 +++++++++------- .../tasks/checkAlerts/providers/default.ts | 131 +++++++- .../src/tasks/checkAlerts/providers/index.ts | 26 +- .../api/src/tasks/checkAlerts/template.ts | 32 +- packages/api/src/utils/zod.ts | 50 ++- packages/common-utils/src/types.ts | 33 ++ 16 files changed, 1206 insertions(+), 134 deletions(-) create mode 100644 .changeset/detached-chart-alerts-backend.md diff --git a/.changeset/detached-chart-alerts-backend.md b/.changeset/detached-chart-alerts-backend.md new file mode 100644 index 0000000000..0a3dd2ce0a --- /dev/null +++ b/.changeset/detached-chart-alerts-backend.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/common-utils': minor +'@hyperdx/api': minor +--- + +Add a new `chart` alert source that persists its own chart config directly on the alert, so alerts no longer require a saved search (logs) or a dashboard tile (metrics). The config is the same shape a dashboard tile stores — builder configs on log/trace/metric sources plus raw SQL (Line/Stacked Bar/Number display types); PromQL is rejected. The internal alerts API accepts and returns the new source, and the check-alerts task evaluates chart alerts through the same code path as tile alerts (including group-by and multi-window behavior). Notifications for chart alerts link to the chart explorer seeded with the alert's config over the alerting window, and default their title to the config's name. Backend only — the creation/edit UI and external API v2 support land separately. diff --git a/packages/api/src/controllers/__tests__/alerts.test.ts b/packages/api/src/controllers/__tests__/alerts.test.ts index aab7cfc15e..b5dbbb028d 100644 --- a/packages/api/src/controllers/__tests__/alerts.test.ts +++ b/packages/api/src/controllers/__tests__/alerts.test.ts @@ -1,5 +1,10 @@ +import { + AlertChartConfig, + DisplayType, +} from '@hyperdx/common-utils/dist/types'; + import { makeAlert } from '@/controllers/alerts'; -import { AlertChannel, AlertThresholdType } from '@/models/alert'; +import { AlertChannel, AlertSource, AlertThresholdType } from '@/models/alert'; // A channel type this repo doesn't define -- see // models/__tests__/alert.test.ts for why. `value: any` (rather than an `as` @@ -28,4 +33,54 @@ describe('makeAlert', () => { expect(result.channel).toEqual(exotic); expect(result.channels).toEqual([exotic]); }); + + const chartConfig: AlertChartConfig = { + name: 'Errors', + source: 'source-id', + displayType: DisplayType.Line, + select: [ + { + aggFn: 'count', + aggCondition: '', + aggConditionLanguage: 'lucene', + valueExpression: '', + }, + ], + where: '', + whereLanguage: 'lucene', + }; + + it('persists chartConfig for chart alerts and clears the other source references', () => { + const result = makeAlert({ + interval: '5m', + threshold: 1, + thresholdType: AlertThresholdType.ABOVE, + channels: [{ type: 'webhook', webhookId: 'webhook-id' }], + source: AlertSource.CHART, + chartConfig, + }); + + expect(result.chartConfig).toEqual(chartConfig); + expect(result.savedSearch).toBeNull(); + expect(result.groupBy).toBeNull(); + expect(result.dashboard).toBeNull(); + expect(result.tileId).toBeNull(); + }); + + it('clears chartConfig when the alert source is not chart', () => { + // Converting a chart alert to another source must not leave the old + // config behind (mirrors how savedSearch/dashboard references clear). + const result = makeAlert({ + interval: '5m', + threshold: 1, + thresholdType: AlertThresholdType.ABOVE, + channels: [{ type: 'webhook', webhookId: 'webhook-id' }], + source: AlertSource.SAVED_SEARCH, + savedSearchId: 'saved-search-id', + chartConfig, + }); + + expect(result.chartConfig).toBeNull(); + expect(result.savedSearch).toBe('saved-search-id'); + }); }); diff --git a/packages/api/src/controllers/alerts.ts b/packages/api/src/controllers/alerts.ts index b5fc31e426..d0a446a0e5 100644 --- a/packages/api/src/controllers/alerts.ts +++ b/packages/api/src/controllers/alerts.ts @@ -1,4 +1,5 @@ import { + displayTypeSupportsBuilderAlerts, displayTypeSupportsRawSqlAlerts, validateRawSqlForAlert, } from '@hyperdx/common-utils/dist/core/utils'; @@ -13,12 +14,14 @@ import Alert, { getAlertChannels, IAlert, } from '@/models/alert'; +import Connection from '@/models/connection'; import Dashboard, { IDashboard } from '@/models/dashboard'; import { ISavedSearch, SavedSearch } from '@/models/savedSearch'; +import { Source } from '@/models/source'; import { IUser } from '@/models/user'; import Webhook from '@/models/webhook'; import { Api400Error } from '@/utils/errors'; -import { alertSchema, objectIdSchema } from '@/utils/zod'; +import { internalAlertSchema, objectIdSchema } from '@/utils/zod'; export type AlertInput = Omit< IAlert, @@ -57,6 +60,7 @@ export const validateAlertInput = async ( | 'dashboardId' | 'tileId' | 'savedSearchId' + | 'chartConfig' | 'channel' | 'channels' >, @@ -108,6 +112,66 @@ export const validateAlertInput = async ( } } + if (alertInput.source === AlertSource.CHART) { + const chartConfig = alertInput.chartConfig; + if (chartConfig == null) { + throw new Api400Error('Chart config is required'); + } + + if (isRawSqlSavedChartConfig(chartConfig)) { + if (!displayTypeSupportsRawSqlAlerts(chartConfig.displayType)) { + throw new Api400Error( + 'Alerts on Raw SQL charts are only supported for Line, Stacked Bar, or Number display types', + ); + } + + const { errors } = validateRawSqlForAlert(chartConfig); + if (errors.length > 0) { + throw new Api400Error( + `Raw SQL alert query is invalid: ${errors.join(', ')}`, + ); + } + + // Raw SQL configs carry the connection directly; the source reference + // is optional metadata for macro expansion ($__sourceTable). + validateObjectId(chartConfig.connection, 'Invalid connection ID'); + const connection = await Connection.findOne({ + _id: chartConfig.connection, + team: teamId, + }); + if (connection == null) { + throw new Api400Error('Connection not found'); + } + if (chartConfig.source) { + validateObjectId(chartConfig.source, 'Invalid source ID'); + const source = await Source.findOne({ + _id: chartConfig.source, + team: teamId, + }); + if (source == null) { + throw new Api400Error('Source not found'); + } + } + } else { + // Builder configs: same display types the alert task can evaluate as a + // time series (mirrors the tile-alert rules). + if (!displayTypeSupportsBuilderAlerts(chartConfig.displayType)) { + throw new Api400Error( + 'Chart alerts are only supported for Line, Stacked Bar, or Number display types', + ); + } + + validateObjectId(chartConfig.source, 'Invalid source ID'); + const source = await Source.findOne({ + _id: chartConfig.source, + team: teamId, + }); + if (source == null) { + throw new Api400Error('Source not found'); + } + } + } + const channels = getAlertChannels(alertInput); if (channels.length === 0) { throw new Api400Error('At least one notification channel is required'); @@ -150,6 +214,7 @@ export const makeAlert = ( : alert.scheduleOffsetMinutes; const isSavedSearch = alert.source === AlertSource.SAVED_SEARCH; const isTile = alert.source === AlertSource.TILE; + const isChart = alert.source === AlertSource.CHART; const channels = getAlertChannels(alert); return { @@ -184,12 +249,15 @@ export const makeAlert = ( ? ((alert.savedSearchId ?? null) as unknown as ObjectId) : null, groupBy: isSavedSearch ? (alert.groupBy ?? null) : null, - // Chart alerts + // Tile alerts dashboard: isTile ? ((alert.dashboardId ?? null) as unknown as ObjectId) : null, tileId: isTile ? (alert.tileId ?? null) : null, + // Chart alerts + chartConfig: isChart ? (alert.chartConfig ?? null) : null, + // Multi-window alerting numConsecutiveWindows: alert.numConsecutiveWindows ?? null, }; @@ -197,7 +265,7 @@ export const makeAlert = ( export const createAlert = async ( teamId: ObjectId, - alertInput: z.infer, + alertInput: z.infer, userId: ObjectId, ) => { return new Alert({ diff --git a/packages/api/src/fixtures.ts b/packages/api/src/fixtures.ts index 677a90465a..aa7a5b6d34 100644 --- a/packages/api/src/fixtures.ts +++ b/packages/api/src/fixtures.ts @@ -1,5 +1,6 @@ import { createNativeClient } from '@hyperdx/common-utils/dist/clickhouse/node'; import { + AlertChartConfig, AlertThresholdType, BuilderSavedChartConfig, DisplayType, @@ -780,3 +781,48 @@ export const makeSavedSearchAlertInput = ({ source: AlertSource.SAVED_SEARCH, savedSearchId, }); + +export const makeChartAlertConfig = (opts: { + sourceId: string; + name?: string; + displayType?: DisplayType; + aggCondition?: string; + groupBy?: string; +}): AlertChartConfig => ({ + name: opts.name ?? 'Chart Alert Query', + source: opts.sourceId, + displayType: opts.displayType ?? DisplayType.Line, + select: [ + { + aggFn: 'count', + aggCondition: opts.aggCondition ?? '', + aggConditionLanguage: 'lucene', + valueExpression: '', + }, + ], + where: '', + whereLanguage: 'lucene', + ...(opts.groupBy != null && { groupBy: opts.groupBy }), +}); + +export const makeChartAlertInput = ({ + chartConfig, + interval = '15m', + threshold = 8, + webhookId = 'test-webhook-id', +}: { + chartConfig: AlertChartConfig; + interval?: AlertInterval; + threshold?: number; + webhookId?: string; +}): Partial => ({ + channel: { + type: 'webhook', + webhookId, + }, + interval, + threshold, + thresholdType: AlertThresholdType.ABOVE, + source: AlertSource.CHART, + chartConfig, +}); diff --git a/packages/api/src/models/alert.ts b/packages/api/src/models/alert.ts index 3937f290e3..529a75bbdb 100644 --- a/packages/api/src/models/alert.ts +++ b/packages/api/src/models/alert.ts @@ -1,5 +1,6 @@ import { ALERT_INTERVAL_TO_MINUTES, + AlertChartConfig, AlertErrorType, AlertThresholdType, } from '@hyperdx/common-utils/dist/types'; @@ -71,6 +72,8 @@ export const getAlertChannels = (alert: { export enum AlertSource { SAVED_SEARCH = 'saved_search', TILE = 'tile', + /** Detached alert that persists its own chart config (no saved search/tile). */ + CHART = 'chart', } export interface IAlert { @@ -104,6 +107,10 @@ export interface IAlert { dashboard?: ObjectId | null; tileId?: string | null; + // Chart alerts: the persisted chart config (same shape as a dashboard + // tile's config, minus the embedded alert field) + chartConfig?: AlertChartConfig | null; + // Silenced silenced?: { by?: ObjectId; @@ -224,6 +231,12 @@ const AlertSchema = new Schema( type: String, required: false, }, + + // Chart alerts + chartConfig: { + type: Schema.Types.Mixed, + required: false, + }, numConsecutiveWindows: { type: Number, required: false, diff --git a/packages/api/src/routers/api/__tests__/alerts.int.test.ts b/packages/api/src/routers/api/__tests__/alerts.int.test.ts index a6d4f7e564..e8b42912cd 100644 --- a/packages/api/src/routers/api/__tests__/alerts.int.test.ts +++ b/packages/api/src/routers/api/__tests__/alerts.int.test.ts @@ -2,6 +2,7 @@ import { AlertErrorType, AlertThresholdType, DisplayType, + SourceKind, } from '@hyperdx/common-utils/dist/types'; import mongoose from 'mongoose'; @@ -9,6 +10,8 @@ import { getLoggedInAgent, getServer, makeAlertInput, + makeChartAlertConfig, + makeChartAlertInput, makeRawSqlAlertTile, makeRawSqlNumberAlertTile, makeRawSqlTile, @@ -19,7 +22,9 @@ import { } from '@/fixtures'; import Alert, { AlertSource, AlertState } from '@/models/alert'; import AlertHistory from '@/models/alertHistory'; +import Connection from '@/models/connection'; import { SavedSearch } from '@/models/savedSearch'; +import { Source } from '@/models/source'; import Webhook, { WebhookDocument, WebhookService } from '@/models/webhook'; const MOCK_TILES = [makeTile(), makeTile(), makeTile(), makeTile(), makeTile()]; @@ -1649,4 +1654,254 @@ describe('alerts router', () => { .expect(400); }); }); + + describe('chart alerts', () => { + const makeSource = async () => { + const connection = await Connection.create({ + team: team._id, + name: 'Default', + host: 'http://localhost:8123', + username: 'default', + password: '', + }); + const source = await Source.create({ + kind: SourceKind.Log, + team: team._id, + from: { databaseName: 'default', tableName: 'otel_logs' }, + timestampValueExpression: 'Timestamp', + connection: connection._id, + name: 'Logs', + }); + return { connection, source }; + }; + + it('creates a chart alert and round-trips chartConfig through GET', async () => { + const { source } = await makeSource(); + const chartConfig = makeChartAlertConfig({ + sourceId: source._id.toString(), + }); + + const created = await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig, + webhookId: webhook._id.toString(), + }), + ) + .expect(200); + + expect(created.body.data.source).toBe(AlertSource.CHART); + expect(created.body.data.chartConfig).toMatchObject({ + name: 'Chart Alert Query', + source: source._id.toString(), + }); + + const single = await agent + .get(`/alerts/${created.body.data._id}`) + .expect(200); + expect(single.body.data.chartConfig).toMatchObject({ + name: 'Chart Alert Query', + source: source._id.toString(), + }); + expect(single.body.data.savedSearchId).toBeUndefined(); + expect(single.body.data.dashboardId).toBeUndefined(); + + const list = await agent.get('/alerts').expect(200); + expect(list.body.data).toHaveLength(1); + expect(list.body.data[0].chartConfig).toMatchObject({ + source: source._id.toString(), + }); + }); + + it('updates a chart alert config', async () => { + const { source } = await makeSource(); + const created = await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig: makeChartAlertConfig({ + sourceId: source._id.toString(), + }), + webhookId: webhook._id.toString(), + }), + ) + .expect(200); + + const updatedConfig = makeChartAlertConfig({ + sourceId: source._id.toString(), + groupBy: 'ServiceName', + }); + await agent + .put(`/alerts/${created.body.data._id}`) + .send( + makeChartAlertInput({ + chartConfig: updatedConfig, + threshold: 42, + webhookId: webhook._id.toString(), + }), + ) + .expect(200); + + const stored = await Alert.findById(created.body.data._id); + expect(stored!.threshold).toBe(42); + expect(stored!.chartConfig).toMatchObject({ groupBy: 'ServiceName' }); + }); + + it('clears source-specific references when switching between chart and tile sources', async () => { + const { source } = await makeSource(); + const dashboard = await agent + .post('/dashboards') + .send(MOCK_DASHBOARD) + .expect(200); + const tileId = dashboard.body.tiles[0].id; + + const created = await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig: makeChartAlertConfig({ + sourceId: source._id.toString(), + }), + webhookId: webhook._id.toString(), + }), + ) + .expect(200); + + await agent + .put(`/alerts/${created.body.data._id}`) + .send( + makeAlertInput({ + dashboardId: dashboard.body.id, + tileId, + webhookId: webhook._id.toString(), + }), + ) + .expect(200); + + let stored = await Alert.findById(created.body.data._id); + expect(stored!.chartConfig).toBeNull(); + expect(stored!.dashboard?.toString()).toBe(dashboard.body.id); + expect(stored!.tileId).toBe(tileId); + + await agent + .put(`/alerts/${created.body.data._id}`) + .send( + makeChartAlertInput({ + chartConfig: makeChartAlertConfig({ + sourceId: source._id.toString(), + }), + webhookId: webhook._id.toString(), + }), + ) + .expect(200); + + stored = await Alert.findById(created.body.data._id); + expect(stored!.chartConfig).toMatchObject({ + source: source._id.toString(), + }); + expect(stored!.dashboard).toBeNull(); + expect(stored!.tileId).toBeNull(); + }); + + it('rejects a chart alert whose source does not exist in the team', async () => { + await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig: makeChartAlertConfig({ sourceId: randomMongoId() }), + webhookId: webhook._id.toString(), + }), + ) + .expect(400); + }); + + it('rejects a chart alert with an unsupported display type', async () => { + const { source } = await makeSource(); + await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig: makeChartAlertConfig({ + sourceId: source._id.toString(), + displayType: DisplayType.Table, + }), + webhookId: webhook._id.toString(), + }), + ) + .expect(400); + }); + + it('rejects a chart alert with a PromQL config', async () => { + await agent + .post('/alerts') + .send({ + ...makeChartAlertInput({ + chartConfig: makeChartAlertConfig({ sourceId: randomMongoId() }), + webhookId: webhook._id.toString(), + }), + chartConfig: { + configType: 'promql', + promqlQuery: 'up', + source: randomMongoId(), + }, + }) + .expect(400); + }); + + it('accepts a raw SQL chart alert and validates its template', async () => { + const { connection } = await makeSource(); + + // Missing the required time-filter/interval parameters + await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig: { + configType: 'sql', + displayType: DisplayType.Line, + sqlTemplate: 'SELECT 1', + connection: connection._id.toString(), + }, + webhookId: webhook._id.toString(), + }), + ) + .expect(400); + + // A connection outside the team is rejected + await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig: { + configType: 'sql', + displayType: DisplayType.Line, + sqlTemplate: RAW_SQL_ALERT_TEMPLATE, + connection: randomMongoId(), + }, + webhookId: webhook._id.toString(), + }), + ) + .expect(400); + + const created = await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig: { + configType: 'sql', + displayType: DisplayType.Line, + sqlTemplate: RAW_SQL_ALERT_TEMPLATE, + connection: connection._id.toString(), + }, + webhookId: webhook._id.toString(), + }), + ) + .expect(200); + expect(created.body.data.chartConfig).toMatchObject({ + configType: 'sql', + connection: connection._id.toString(), + }); + }); + }); }); diff --git a/packages/api/src/routers/api/alerts.ts b/packages/api/src/routers/api/alerts.ts index 8fd8c551a1..bebe5d490a 100644 --- a/packages/api/src/routers/api/alerts.ts +++ b/packages/api/src/routers/api/alerts.ts @@ -29,7 +29,7 @@ import { import { getAlertChannels } from '@/models/alert'; import { IAlertHistory } from '@/models/alertHistory'; import { PreSerialized, sendJson } from '@/utils/serialization'; -import { alertSchema, objectIdSchema } from '@/utils/zod'; +import { internalAlertSchema, objectIdSchema } from '@/utils/zod'; const router = express.Router(); @@ -78,6 +78,9 @@ const formatAlertResponse = ( 'tags', ]), }), + // Chart alerts carry their persisted config so edit surfaces can seed the + // chart editor and the detail page can render the query. + ...(alert.chartConfig && { chartConfig: alert.chartConfig }), ...pick(alert, [ '_id', 'interval', @@ -303,7 +306,7 @@ router.get( router.post( '/', - processRequest({ body: alertSchema }), + processRequest({ body: internalAlertSchema }), async (req, res, next) => { const teamId = req.user?.team; const userId = req.user?._id; @@ -325,7 +328,7 @@ router.post( router.put( '/:id', processRequest({ - body: alertSchema, + body: internalAlertSchema, params: z.object({ id: objectIdSchema, }), diff --git a/packages/api/src/routers/external-api/__tests__/alerts.int.test.ts b/packages/api/src/routers/external-api/__tests__/alerts.int.test.ts index 9cc7617f53..d2e613fded 100644 --- a/packages/api/src/routers/external-api/__tests__/alerts.int.test.ts +++ b/packages/api/src/routers/external-api/__tests__/alerts.int.test.ts @@ -658,6 +658,41 @@ describe('External API Alerts', () => { }); describe('Input validation', () => { + it('rejects the internal-only chart alert source', async () => { + // Chart alerts (source: 'chart') are creatable through the internal API + // only; the external v2 contract (OpenAPI docs, Terraform provider) has + // not been extended to cover them yet. + const webhook = await createTestWebhook(); + + const alertInput = { + threshold: 100, + interval: '1h', + source: 'chart', + chartConfig: { + name: 'Chart Alert Query', + source: new ObjectId().toString(), + displayType: 'line', + select: [ + { + aggFn: 'count', + aggCondition: '', + aggConditionLanguage: 'lucene', + valueExpression: '', + }, + ], + where: '', + whereLanguage: 'lucene', + }, + thresholdType: AlertThresholdType.ABOVE, + channel: { + type: 'webhook', + webhookId: webhook._id.toString(), + }, + }; + + await authRequest('post', ALERTS_BASE_URL).send(alertInput).expect(400); + }); + describe('webhook validation', () => { it('should reject a non-existent webhook', async () => { const dashboard = await createTestDashboard(); 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 de95dc1fd8..4aba634291 100644 --- a/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts +++ b/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts @@ -1,5 +1,6 @@ import { ClickhouseClient } from '@hyperdx/common-utils/dist/clickhouse/node'; import { + AlertChartConfig, AlertErrorType, AlertState, AlertThresholdType, @@ -21,6 +22,7 @@ import { DEFAULT_METRICS_TABLE, getServer, getTestFixtureClickHouseClient, + makeChartAlertConfig, makeTile, RAW_SQL_ALERT_TEMPLATE, RAW_SQL_NUMBER_ALERT_TEMPLATE, @@ -1123,6 +1125,17 @@ describe('checkAlerts', () => { }; } + if (overrides.taskType === AlertTaskType.CHART) { + return { + ...base, + taskType: AlertTaskType.CHART, + chartConfig: makeChartAlertConfig({ + sourceId: 'fake-source-id', + groupBy: overrides.tileGroupBy ?? '', + }), + }; + } + return { ...base, taskType: AlertTaskType.SAVED_SEARCH, @@ -1180,6 +1193,25 @@ describe('checkAlerts', () => { ), ).toBe(true); }); + + it('should return false for chart alert with empty config groupBy', () => { + expect( + alertHasGroupBy( + makeDetails({ taskType: AlertTaskType.CHART, tileGroupBy: '' }), + ), + ).toBe(false); + }); + + it('should return true for chart alert with config groupBy', () => { + expect( + alertHasGroupBy( + makeDetails({ + taskType: AlertTaskType.CHART, + tileGroupBy: 'ServiceName', + }), + ), + ).toBe(true); + }); }); describe('Alert Templates', () => { @@ -1294,6 +1326,29 @@ describe('checkAlerts', () => { value: 5, }; + const chartAlertConfig = makeChartAlertConfig({ + sourceId: 'fake-source-id', + }); + const defaultChartAlertView: AlertMessageTemplateDefaultView = { + alert: { + thresholdType: AlertThresholdType.ABOVE, + threshold: 1, + source: AlertSource.CHART, + channel: { + type: 'webhook', + webhookId: 'fake-webhook-id', + }, + interval: '1m', + chartConfig: chartAlertConfig, + }, + startTime: new Date('2023-03-17T22:13:03.103Z'), + endTime: new Date('2023-03-17T22:13:59.103Z'), + attributes: {}, + granularity: '5 minute', + isGroupedAlert: false, + value: 5, + }; + const server = getServer(); beforeAll(async () => { @@ -1326,6 +1381,19 @@ describe('checkAlerts', () => { ).toMatchInlineSnapshot( `"http://app:8080/dashboards/id-123?from=1679089083103&granularity=5+minute&to=1679093339103&highlightedTileId=test-tile-id"`, ); + + // Chart alerts link to the chart explorer seeded with the persisted + // config. Built programmatically — the URL-encoded config JSON makes an + // inline snapshot unreadable. + const expectedChartAlertUrl = new URL('http://app:8080/chart'); + expectedChartAlertUrl.search = new URLSearchParams({ + config: JSON.stringify(chartAlertConfig), + from: '1679089083103', + to: '1679093339103', + }).toString(); + expect( + buildAlertMessageTemplateHdxLink(alertProvider, defaultChartAlertView), + ).toBe(expectedChartAlertUrl.toString()); }); it('formatValueToMatchThreshold', () => { @@ -1428,6 +1496,14 @@ describe('checkAlerts', () => { ).toMatchInlineSnapshot( `"🚨 Alert for "Test Chart" in "My Dashboard" - 5 meets or exceeds 1"`, ); + // Chart alerts default to the chart config's name + expect( + buildAlertMessageTemplateTitle({ + view: defaultChartAlertView, + }), + ).toMatchInlineSnapshot( + `"🚨 Alert for "Chart Alert Query" - 5 meets or exceeds 1"`, + ); }); it('buildAlertMessageTemplateTitle with state parameter', () => { @@ -2071,6 +2147,10 @@ describe('checkAlerts', () => { taskType: AlertTaskType.TILE; tile: Tile; dashboard: IDashboard; + } + | { + taskType: AlertTaskType.CHART; + chartConfig: AlertChartConfig; }, ): Promise => { const mockUserId = new mongoose.Types.ObjectId(); @@ -2783,6 +2863,226 @@ describe('checkAlerts', () => { ); }); + it('CHART alert (events) - slack webhook', async () => { + const { + team, + webhook, + connection, + source, + teamWebhooksById, + clickhouseClient, + } = await setupSavedSearchAlertTest(); + + const now = new Date('2023-11-16T22:12:00.000Z'); + // Send events in the last alert window 22:05 - 22:10 + const eventMs = now.getTime() - ms('5m'); + + await bulkInsertLogs( + Array.from({ length: 3 }, () => ({ + ServiceName: 'api', + Timestamp: new Date(eventMs), + SeverityText: 'error', + Body: 'Oh no! Something went wrong!', + })), + ); + + // The config lives on the alert itself — no saved search or dashboard. + const chartConfig = makeChartAlertConfig({ + sourceId: source.id, + name: 'Error Count', + aggCondition: 'ServiceName:api', + }); + + const details = await createAlertDetails( + team, + source, + { + source: AlertSource.CHART, + channel: { + type: 'webhook', + webhookId: webhook._id.toString(), + }, + interval: '5m', + thresholdType: AlertThresholdType.ABOVE, + threshold: 1, + chartConfig, + }, + { + taskType: AlertTaskType.CHART, + chartConfig, + }, + ); + + // should fetch 5m of logs + await processAlertAtTime( + now, + details, + clickhouseClient, + connection.id, + alertProvider, + teamWebhooksById, + ); + expect((await Alert.findById(details.alert.id))!.state).toBe('ALERT'); + + // skip since time diff is less than 1 window size + const later = new Date('2023-11-16T22:14:00.000Z'); + await processAlertAtTime( + later, + details, + clickhouseClient, + connection.id, + alertProvider, + teamWebhooksById, + ); + expect((await Alert.findById(details.alert.id))!.state).toBe('ALERT'); + + const nextWindow = new Date('2023-11-16T22:16:00.000Z'); + await processAlertAtTime( + nextWindow, + details, + clickhouseClient, + connection.id, + alertProvider, + teamWebhooksById, + ); + // alert should be in ok state + expect((await Alert.findById(details.alert.id))!.state).toBe('OK'); + + // check alert history + const alertHistories = await AlertHistory.find({ + alert: details.alert.id, + }).sort({ + createdAt: 1, + }); + + expect(alertHistories.length).toBe(2); + const [history1, history2] = alertHistories; + expect(history1.state).toBe('ALERT'); + expect(history1.counts).toBe(1); + expect(history1.createdAt).toEqual(new Date('2023-11-16T22:10:00.000Z')); + expect(history2.state).toBe('OK'); + expect(history2.createdAt).toEqual(new Date('2023-11-16T22:15:00.000Z')); + + // Notification links to the chart explorer seeded with the persisted + // config, padded by 7x granularity on both sides of the window. + const expectedUrl = new URL('http://app:8080/chart'); + expectedUrl.search = new URLSearchParams({ + config: JSON.stringify( + (await Alert.findById(details.alert.id))!.chartConfig, + ), + from: String( + new Date('2023-11-16T22:05:00.000Z').getTime() - ms('5m') * 7, + ), + to: String( + new Date('2023-11-16T22:10:00.000Z').getTime() + ms('5m') * 7, + ), + }).toString(); + + expect(slack.postMessageToWebhook).toHaveBeenNthCalledWith( + 1, + 'https://hooks.slack.com/services/123', + { + text: '🚨 Alert for "Error Count" - 3 meets or exceeds 1', + blocks: [ + { + text: { + text: [ + `*<${expectedUrl.toString()} | 🚨 Alert for "Error Count" - 3 meets or exceeds 1>*`, + '', + '3 meets or exceeds 1', + 'Time Range (UTC): [Nov 16 10:05:00 PM - Nov 16 10:10:00 PM)', + '', + ].join('\n'), + type: 'mrkdwn', + }, + type: 'section', + }, + ], + }, + ); + }); + + it('CHART alert (group by) - notifies per group', async () => { + // Two groups fire, so two notifications go out (the shared beforeEach + // only mocks a single call). + jest.spyOn(slack, 'postMessageToWebhook').mockResolvedValue(null as any); + + const { + team, + webhook, + connection, + source, + teamWebhooksById, + clickhouseClient, + } = await setupSavedSearchAlertTest(); + + const now = new Date('2023-11-16T22:12:00.000Z'); + const eventMs = now.getTime() - ms('5m'); + + await bulkInsertLogs([ + ...Array.from({ length: 3 }, () => ({ + ServiceName: 'api', + Timestamp: new Date(eventMs), + SeverityText: 'error', + Body: 'Oh no! Something went wrong!', + })), + ...Array.from({ length: 2 }, () => ({ + ServiceName: 'worker', + Timestamp: new Date(eventMs), + SeverityText: 'error', + Body: 'Oh no! Something went wrong!', + })), + ]); + + const chartConfig = makeChartAlertConfig({ + sourceId: source.id, + name: 'Errors by service', + groupBy: 'ServiceName', + }); + + const details = await createAlertDetails( + team, + source, + { + source: AlertSource.CHART, + channel: { + type: 'webhook', + webhookId: webhook._id.toString(), + }, + interval: '5m', + thresholdType: AlertThresholdType.ABOVE, + threshold: 1, + chartConfig, + }, + { + taskType: AlertTaskType.CHART, + chartConfig, + }, + ); + + await processAlertAtTime( + now, + details, + clickhouseClient, + connection.id, + alertProvider, + teamWebhooksById, + ); + expect((await Alert.findById(details.alert.id))!.state).toBe('ALERT'); + + // One firing history per group (the config's groupBy drives grouping) + const alertHistories = await AlertHistory.find({ + alert: details.alert.id, + }).sort({ group: 1 }); + expect(alertHistories.length).toBe(2); + expect(alertHistories.map(h => h.group)).toEqual([ + 'ServiceName:api', + 'ServiceName:worker', + ]); + expect(alertHistories.every(h => h.state === 'ALERT')).toBe(true); + expect(slack.postMessageToWebhook).toHaveBeenCalledTimes(2); + }); + it.each([AlertThresholdType.BETWEEN, AlertThresholdType.NOT_BETWEEN])( 'should not fire or record history when thresholdMax is missing for %s', async thresholdType => { diff --git a/packages/api/src/tasks/checkAlerts/__tests__/checkAlertsTask.test.ts b/packages/api/src/tasks/checkAlerts/__tests__/checkAlertsTask.test.ts index e6a452240a..5b5d348898 100644 --- a/packages/api/src/tasks/checkAlerts/__tests__/checkAlertsTask.test.ts +++ b/packages/api/src/tasks/checkAlerts/__tests__/checkAlertsTask.test.ts @@ -40,6 +40,7 @@ describe('CheckAlertTask', () => { recordAlertErrors: jest.fn(), asyncDispose: jest.fn(), buildChartLink: jest.fn(), + buildChartExplorerLink: jest.fn(), buildLogSearchLink: jest.fn(), getClickHouseClient: jest .fn() diff --git a/packages/api/src/tasks/checkAlerts/index.ts b/packages/api/src/tasks/checkAlerts/index.ts index b5ee953242..1d206f9685 100644 --- a/packages/api/src/tasks/checkAlerts/index.ts +++ b/packages/api/src/tasks/checkAlerts/index.ts @@ -40,6 +40,7 @@ import { DisplayType, getSampleWeightExpression, pickSampleWeightExpressionProps, + SavedChartConfig, SourceKind, } from '@hyperdx/common-utils/dist/types'; import * as fns from 'date-fns'; @@ -125,18 +126,31 @@ const alertBatchFailuresCounter = getCounter('hyperdx.alerts.batch_failures', { /** * Determine if an alert has group-by behavior. * For saved search alerts, groupBy is on alert.groupBy. - * For tile alerts, groupBy is on tile.config.groupBy. + * For tile and chart alerts, groupBy is on the chart config. */ export const alertHasGroupBy = (details: AlertDetails): boolean => { const { alert } = details; if (alert.groupBy && alert.groupBy.length > 0) { return true; } + + // AlertChartConfig members are the tile config types minus the embedded + // alert field, so they're assignable to SavedChartConfig (which the + // chart-config guards narrow on). + const savedConfig: SavedChartConfig | undefined = + details.taskType === AlertTaskType.TILE + ? details.tile.config + : details.taskType === AlertTaskType.CHART + ? details.chartConfig + : undefined; + if (savedConfig == null) { + return false; + } + if ( - details.taskType === AlertTaskType.TILE && - isBuilderSavedChartConfig(details.tile.config) && - details.tile.config.groupBy && - details.tile.config.groupBy.length > 0 + isBuilderSavedChartConfig(savedConfig) && + savedConfig.groupBy && + savedConfig.groupBy.length > 0 ) { return true; } @@ -145,11 +159,8 @@ export const alertHasGroupBy = (details: AlertDetails): boolean => { // group by (besides the group by on the interval), so we'll assume it might // in the case of time series charts, and assume it will not in the case of number charts. // Group name will just be blank if there are no group by values. - if ( - details.taskType === AlertTaskType.TILE && - isRawSqlSavedChartConfig(details.tile.config) - ) { - return details.tile.config.displayType !== DisplayType.Number; + if (isRawSqlSavedChartConfig(savedConfig)) { + return savedConfig.displayType !== DisplayType.Number; } return false; }; @@ -533,6 +544,9 @@ const fireChannelEvent = async ({ thresholdMax: alert.thresholdMax, thresholdType: alert.thresholdType, tileId: alert.tileId, + // Chart alerts: the persisted config, used to build the explorer link + // and the default notification title. + chartConfig: alert.chartConfig, }, attributes: attributesNested, dashboard, @@ -667,6 +681,124 @@ const getAlertEvaluationDateRange = ( ); }; +/** + * Assemble the queryable chart config for an alert backed by a saved chart + * config — a dashboard tile's config or a chart alert's persisted config. + * Shared so tile and chart alerts evaluate identically; only the dashboard + * variables differ (chart alerts have no dashboard, so none are declared). + */ +const buildAlertChartConfigFromSavedConfig = ({ + alertId, + savedConfig, + source, + connection, + dateRange, + windowSizeInMins, + variables, +}: { + alertId: string; + // AlertChartConfig (a chart alert's persisted config) is assignable here: + // its members are the tile config types minus the embedded alert field. + savedConfig: SavedChartConfig; + source?: ISource; + connection: string; + dateRange: [Date, Date]; + windowSizeInMins: number; + variables: NonNullable; +}): ChartConfigWithOptDateRange | undefined => { + // Raw SQL configs: build a RawSqlChartConfig + if (isRawSqlSavedChartConfig(savedConfig)) { + if (displayTypeSupportsRawSqlAlerts(savedConfig.displayType)) { + return { + ...pick(savedConfig, [ + 'configType', + 'sqlTemplate', + 'displayType', + 'source', + ]), + connection, + dateRange, + variables, + // Only time-series charts use interval bucketing + ...(isTimeSeriesDisplayType(savedConfig.displayType) && { + granularity: `${windowSizeInMins} minute`, + }), + // Include source metadata for macro expansion ($__sourceTable) + ...(source && { + from: source.from, + metricTables: + source.kind === SourceKind.Metric ? source.metricTables : undefined, + }), + }; + } + return undefined; + } + + // PromQL charts don't support alerts yet + if (isPromqlSavedChartConfig(savedConfig)) { + return undefined; + } + + if (!source) { + logger.error({ alertId }, 'Source not found for builder chart alert'); + return undefined; + } + + if ( + savedConfig.displayType === DisplayType.Line || + savedConfig.displayType === DisplayType.StackedBar || + savedConfig.displayType === DisplayType.Number + ) { + // Alerts can use Log, Trace, or Metric sources. + // implicitColumnExpression+useTextIndexForImplicitColumn exist on Log and Trace sources; + // metricTables exists on Metric sources. + const implicitColumnExpression = + source.kind === SourceKind.Log || source.kind === SourceKind.Trace + ? source.implicitColumnExpression + : undefined; + const useTextIndexForImplicitColumn = + source.kind === SourceKind.Log || source.kind === SourceKind.Trace + ? source.useTextIndexForImplicitColumn + : undefined; + const sampleWeightExpression = getSampleWeightExpression(source); + const metricTables = + source.kind === SourceKind.Metric ? source.metricTables : undefined; + return { + connection, + dateRange, + dateRangeStartInclusive: true, + dateRangeEndInclusive: false, + displayType: savedConfig.displayType, + from: source.from, + granularity: `${windowSizeInMins} minute`, + groupBy: savedConfig.groupBy, + implicitColumnExpression, + useTextIndexForImplicitColumn, + sampleWeightExpression, + metricTables, + select: savedConfig.select, + timestampValueExpression: source.timestampValueExpression, + where: savedConfig.where, + whereLanguage: savedConfig.whereLanguage, + seriesReturnType: savedConfig.seriesReturnType, + // Grouped ratios can divide per-group or share-of-total; without this + // the alert would silently evaluate the default (per-group) mode. + ratioMode: savedConfig.ratioMode, + // Metric formulas (HDX-5080): the alert must evaluate the derived + // formula column, not a raw operand series. Operand columns are + // always dropped from the alert query — regardless of the tile's + // "Show input series" display toggle — so the formula is the value + // column parseAlertData picks (the last one wins, consistent with + // the multi-series "last series drives the alert" semantics). + formulas: savedConfig.formulas, + ...(savedConfig.formulas?.length ? { showOperandSeries: false } : {}), + variables, + }; + } + + return undefined; +}; + const getChartConfigFromAlert = ( details: AlertDetails, connection: string, @@ -695,8 +827,6 @@ const getChartConfigFromAlert = ( granularity: `${windowSizeInMins} minute`, }); } else if (details.taskType === AlertTaskType.TILE) { - const tile = details.tile; - // Substitute empty selections for each variable the dashboard defines const variables = getDashboardVariableDeclarations( details.dashboard.filters, @@ -705,101 +835,31 @@ const getChartConfigFromAlert = ( values: [], })); - // Raw SQL tiles: build a RawSqlChartConfig - if (isRawSqlSavedChartConfig(tile.config)) { - if (displayTypeSupportsRawSqlAlerts(tile.config.displayType)) { - return { - ...pick(tile.config, [ - 'configType', - 'sqlTemplate', - 'displayType', - 'source', - ]), - connection, - dateRange, - variables, - // Only time-series charts use interval bucketing - ...(isTimeSeriesDisplayType(tile.config.displayType) && { - granularity: `${windowSizeInMins} minute`, - }), - // Include source metadata for macro expansion ($__sourceTable) - ...(details.source && { - from: details.source.from, - metricTables: - details.source.kind === SourceKind.Metric - ? details.source.metricTables - : undefined, - }), - }; - } - return undefined; - } - - // PromQL tiles don't support alerts yet - if (isPromqlSavedChartConfig(tile.config)) { - return undefined; - } - - const { source } = details; - if (!source) { - logger.error( - { alertId: alert.id }, - 'Source not found for builder tile alert', - ); - return undefined; + const config = buildAlertChartConfigFromSavedConfig({ + alertId: alert.id, + savedConfig: details.tile.config, + source: details.source, + connection, + dateRange, + windowSizeInMins, + variables, + }); + if (config != null) { + return config; } - - // Doesn't work for metric alerts yet - if ( - tile.config.displayType === DisplayType.Line || - tile.config.displayType === DisplayType.StackedBar || - tile.config.displayType === DisplayType.Number - ) { - // Tile alerts can use Log, Trace, or Metric sources. - // implicitColumnExpression+useTextIndexForImplicitColumn exist on Log and Trace sources; - // metricTables exists on Metric sources. - const implicitColumnExpression = - source.kind === SourceKind.Log || source.kind === SourceKind.Trace - ? source.implicitColumnExpression - : undefined; - const useTextIndexForImplicitColumn = - source.kind === SourceKind.Log || source.kind === SourceKind.Trace - ? source.useTextIndexForImplicitColumn - : undefined; - const sampleWeightExpression = getSampleWeightExpression(source); - const metricTables = - source.kind === SourceKind.Metric ? source.metricTables : undefined; - return { - connection, - dateRange, - dateRangeStartInclusive: true, - dateRangeEndInclusive: false, - displayType: tile.config.displayType, - from: source.from, - granularity: `${windowSizeInMins} minute`, - groupBy: tile.config.groupBy, - implicitColumnExpression, - useTextIndexForImplicitColumn, - sampleWeightExpression, - metricTables, - select: tile.config.select, - timestampValueExpression: source.timestampValueExpression, - where: tile.config.where, - whereLanguage: tile.config.whereLanguage, - seriesReturnType: tile.config.seriesReturnType, - // Grouped ratios can divide per-group or share-of-total; without this - // the alert would silently evaluate the default (per-group) mode. - ratioMode: tile.config.ratioMode, - // Metric formulas (HDX-5080): the alert must evaluate the derived - // formula column, not a raw operand series. Operand columns are - // always dropped from the alert query — regardless of the tile's - // "Show input series" display toggle — so the formula is the value - // column parseAlertData picks (the last one wins, consistent with - // the multi-series "last series drives the alert" semantics). - formulas: tile.config.formulas, - ...(tile.config.formulas?.length ? { showOperandSeries: false } : {}), - variables, - }; + } else if (details.taskType === AlertTaskType.CHART) { + // Chart alerts have no dashboard, so no variables are declared. + const config = buildAlertChartConfigFromSavedConfig({ + alertId: alert.id, + savedConfig: details.chartConfig, + source: details.source, + connection, + dateRange, + windowSizeInMins, + variables: [], + }); + if (config != null) { + return config; } } diff --git a/packages/api/src/tasks/checkAlerts/providers/default.ts b/packages/api/src/tasks/checkAlerts/providers/default.ts index 1d4e6d6c94..7a0c636306 100644 --- a/packages/api/src/tasks/checkAlerts/providers/default.ts +++ b/packages/api/src/tasks/checkAlerts/providers/default.ts @@ -1,7 +1,7 @@ import PQueue from '@esm2cjs/p-queue'; import { displayTypeSupportsRawSqlAlerts } from '@hyperdx/common-utils/dist/core/utils'; import { isRawSqlSavedChartConfig } from '@hyperdx/common-utils/dist/guards'; -import { Tile } from '@hyperdx/common-utils/dist/types'; +import { AlertChartConfig, Tile } from '@hyperdx/common-utils/dist/types'; import mongoose from 'mongoose'; import ms from 'ms'; import { URLSearchParams } from 'url'; @@ -218,6 +218,105 @@ async function getTileDetails( ]; } +async function getChartDetails( + alert: IAlert, +): Promise<[IConnection, PartialAlertDetails] | []> { + const chartConfig = alert.chartConfig; + if (chartConfig == null) { + logger.error({ + message: 'chart alert has no chartConfig', + alertId: alert.id, + }); + return []; + } + + if (isRawSqlSavedChartConfig(chartConfig)) { + if (!displayTypeSupportsRawSqlAlerts(chartConfig.displayType)) { + logger.warn({ + alertId: alert.id, + message: + 'skipping chart alert with raw sql chart config, only line/bar/number display types are supported', + }); + return []; + } + + // Raw SQL configs store the connection ID directly + const connection = await Connection.findOne({ + _id: chartConfig.connection, + team: alert.team, + }).select('+password'); + + if (!connection) { + logger.error({ + message: 'connection not found for raw sql chart alert', + connectionId: chartConfig.connection, + alertId: alert.id, + }); + return []; + } + + // Optionally look up source for filter/macro metadata + let source: ISource | undefined; + if (chartConfig.source) { + const sourceDoc = await Source.findOne({ + _id: chartConfig.source, + team: alert.team, + }); + if (sourceDoc) { + source = sourceDoc.toObject(); + } + } + + return [ + connection, + { + alert, + source, + taskType: AlertTaskType.CHART, + chartConfig, + }, + ]; + } + + const source = await Source.findOne({ + _id: chartConfig.source, + team: alert.team, + }).populate & { connection: IConnection }>({ + path: 'connection', + match: { team: alert.team }, + select: '+password', + }); + if (!source) { + logger.error({ + message: 'source not found', + sourceId: chartConfig.source, + alertId: alert.id, + }); + return []; + } + + if (!source.connection) { + logger.error({ + message: 'connection not found', + alertId: alert.id, + sourceId: source.id, + }); + return []; + } + + const connection = source.connection; + const sourceProps = source.toObject(); + return [ + connection, + { + alert, + source: { ...sourceProps, connection: connection.id }, + taskType: AlertTaskType.CHART, + chartConfig, + }, + ]; +} + async function loadAlert( alert: IAlert, groupedTasks: Map, @@ -246,6 +345,10 @@ async function loadAlert( [conn, details] = await getTileDetails(alert); break; + case AlertSource.CHART: + [conn, details] = await getChartDetails(alert); + break; + default: throw new Error(`unsupported source: ${alert.source}`); } @@ -374,6 +477,32 @@ export default class DefaultAlertProvider implements AlertProvider { return url.toString(); } + buildChartExplorerLink({ + chartConfig, + endTime, + granularity, + startTime, + }: { + chartConfig: AlertChartConfig; + endTime: Date; + granularity: string; + startTime: Date; + }): string { + const url = new URL(`${config.FRONTEND_URL}/chart`); + // Extend both start and end time by 7x granularity, matching the + // dashboard link so the alerting window has surrounding context. The + // chart explorer reads `config` (JSON) and `from`/`to` (epoch ms). + const from = (startTime.getTime() - ms(granularity) * 7).toString(); + const to = (endTime.getTime() + ms(granularity) * 7).toString(); + const queryParams = new URLSearchParams({ + config: JSON.stringify(chartConfig), + from, + to, + }); + url.search = queryParams.toString(); + return url.toString(); + } + async updateAlertState( alertId: string, histories: IAlertHistory[], diff --git a/packages/api/src/tasks/checkAlerts/providers/index.ts b/packages/api/src/tasks/checkAlerts/providers/index.ts index 4f010b7032..e59c23e6e4 100644 --- a/packages/api/src/tasks/checkAlerts/providers/index.ts +++ b/packages/api/src/tasks/checkAlerts/providers/index.ts @@ -1,5 +1,5 @@ import { ClickhouseClient } from '@hyperdx/common-utils/dist/clickhouse/node'; -import { Tile } from '@hyperdx/common-utils/dist/types'; +import { AlertChartConfig, Tile } from '@hyperdx/common-utils/dist/types'; import _ from 'lodash'; import { ObjectId } from '@/models'; @@ -17,6 +17,7 @@ import logger from '@/utils/logger'; export enum AlertTaskType { SAVED_SEARCH, TILE, + CHART, } // Discriminated union of possible alert channel types with populated channel data @@ -25,7 +26,8 @@ export type PopulatedAlertChannel = { type: 'webhook' } & { channel: IWebhook }; // Details about the alert and the source for the alert. Depending on // the taskType either: // 1. the savedSearch field is required or -// 2. the tile and dashboard field are required +// 2. the tile and dashboard field are required or +// 3. the chartConfig field is required (persisted on the alert itself) // // The dependent typing means less null checks when using these values as // the are required when the type is set accordingly. @@ -47,6 +49,14 @@ export type AlertDetails = { tile: Tile; dashboard: IDashboard; } + | { + // Chart alerts: the config lives on the alert document. `source` is + // present for builder configs and optional for raw SQL configs (which + // carry the connection directly). + taskType: AlertTaskType.CHART; + source?: ISource; + chartConfig: AlertChartConfig; + } ); // AlertTask instances can carry metadata, of type T, for the provider that created @@ -78,6 +88,18 @@ export interface AlertProvider { tileId?: string; }): string; + /** + * Link for chart alerts (which have no saved search or dashboard to open): + * the chart explorer with the alert's persisted config over the alerting + * window. Works regardless of whether the alert-details page is enabled. + */ + buildChartExplorerLink(params: { + chartConfig: AlertChartConfig; + endTime: Date; + granularity: string; + startTime: Date; + }): string; + /** * Save the given AlertHistory records and update the associated alert's state. * Uses Promise.allSettled to handle partial failures gracefully. diff --git a/packages/api/src/tasks/checkAlerts/template.ts b/packages/api/src/tasks/checkAlerts/template.ts index 502d1a19fc..bca6ac08f1 100644 --- a/packages/api/src/tasks/checkAlerts/template.ts +++ b/packages/api/src/tasks/checkAlerts/template.ts @@ -210,6 +210,18 @@ export const buildAlertMessageTemplateHdxLink = ( startTime, tileId: alert.tileId ?? undefined, }); + } else if (alert.source === AlertSource.CHART) { + if (alert.chartConfig == null) { + throw new Error(`Source is ${alert.source} but chartConfig is null`); + } + // Chart alerts have no saved search or dashboard to open — link to the + // chart explorer seeded with the alert's persisted config. + return alertProvider.buildChartExplorerLink({ + chartConfig: alert.chartConfig, + endTime, + granularity, + startTime, + }); } throw new Error(`Unsupported alert source: ${alert.source}`); @@ -258,6 +270,19 @@ export const buildAlertMessageTemplateTitle = ({ : describeThresholdResolution(alert.thresholdType) } ${describeThreshold(alert)}`; return `${emoji}${baseTitle}`; + } else if (alert.source === AlertSource.CHART) { + const formattedValue = formatValueToMatchThreshold(value, alert.threshold); + // Chart alerts have no saved search/tile to name them; the alert's `name` + // doubles as the title template, so the default falls back to the chart + // config's name. + const baseTitle = template + ? handlebars.compile(template)(view) + : `Alert for "${alert.chartConfig?.name ?? 'chart'}" - ${formattedValue} ${ + doesExceedThreshold(alert, value) + ? describeThresholdViolation(alert.thresholdType) + : describeThresholdResolution(alert.thresholdType) + } ${describeThreshold(alert)}`; + return `${emoji}${baseTitle}`; } throw new Error(`Unsupported alert source: ${alert.source}`); @@ -692,8 +717,11 @@ ${targetTemplate} \`\`\` {{{__hdx_query_results__}}} \`\`\``; - } else if (alert.source === AlertSource.TILE) { - if (dashboard == null) { + } else if ( + alert.source === AlertSource.TILE || + alert.source === AlertSource.CHART + ) { + if (alert.source === AlertSource.TILE && dashboard == null) { throw new Error(`Source is ${alert.source} but dashboard is null`); } const formattedValue = formatValueToMatchThreshold(value, alert.threshold); diff --git a/packages/api/src/utils/zod.ts b/packages/api/src/utils/zod.ts index 3dd6acff01..61f3b187f8 100644 --- a/packages/api/src/utils/zod.ts +++ b/packages/api/src/utils/zod.ts @@ -2,6 +2,7 @@ import { validateChartConfigFormulas } from '@hyperdx/common-utils/dist/dashboar import { addDuplicateTileIdIssues, AggregateFunctionSchema, + AlertChartConfigSchema, alertNoteSchema, AlertThresholdType, BackgroundChartSchema, @@ -729,27 +730,44 @@ const zTileAlert = z.object({ dashboardId: z.string().min(1), }); -export const alertSchema = z - .object({ - channel: zAlertChannel.optional(), - channels: zAlertChannels.optional(), - interval: z.enum(['1m', '5m', '15m', '30m', '1h', '6h', '12h', '1d']), - scheduleOffsetMinutes: z.number().int().min(0).max(1439).optional(), - scheduleStartAt: scheduleStartAtSchema, - threshold: z.number(), - thresholdType: z.nativeEnum(AlertThresholdType), - thresholdMax: z.number().optional(), - source: z.nativeEnum(AlertSource).default(AlertSource.SAVED_SEARCH), - name: z.string().min(1).max(512).nullish(), - message: z.string().min(1).max(4096).nullish(), - note: alertNoteSchema, - numConsecutiveWindows: z.number().int().min(1).nullish(), - }) +const zChartAlert = z.object({ + source: z.literal(AlertSource.CHART), + // Builder + raw SQL configs only; the schema has no PromQL variant. + chartConfig: AlertChartConfigSchema, +}); + +const alertBaseSchema = z.object({ + channel: zAlertChannel.optional(), + channels: zAlertChannels.optional(), + interval: z.enum(['1m', '5m', '15m', '30m', '1h', '6h', '12h', '1d']), + scheduleOffsetMinutes: z.number().int().min(0).max(1439).optional(), + scheduleStartAt: scheduleStartAtSchema, + threshold: z.number(), + thresholdType: z.nativeEnum(AlertThresholdType), + thresholdMax: z.number().optional(), + source: z.nativeEnum(AlertSource).default(AlertSource.SAVED_SEARCH), + name: z.string().min(1).max(512).nullish(), + message: z.string().min(1).max(4096).nullish(), + note: alertNoteSchema, + numConsecutiveWindows: z.number().int().min(1).nullish(), +}); + +export const alertSchema = alertBaseSchema .and(zSavedSearchAlert.or(zTileAlert)) .superRefine(validateAlertChannelSelection) .superRefine(validateAlertScheduleOffsetMinutes) .superRefine(validateAlertThresholdMax); +// Superset of alertSchema for the internal API only: also accepts chart +// alerts (source: 'chart'), which persist their own chart config. The +// external v2 API keeps the narrower alertSchema until its contract (OpenAPI +// docs, Terraform provider) is extended to cover chart alerts. +export const internalAlertSchema = alertBaseSchema + .and(zSavedSearchAlert.or(zTileAlert).or(zChartAlert)) + .superRefine(validateAlertChannelSelection) + .superRefine(validateAlertScheduleOffsetMinutes) + .superRefine(validateAlertThresholdMax); + // ============================== // Webhooks // ============================== diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 506222f3bd..e0565f25d2 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -666,6 +666,11 @@ export type AlertError = z.infer; export enum AlertSource { SAVED_SEARCH = 'saved_search', TILE = 'tile', + /** + * A "detached" alert that persists its own chart config (the same shape a + * dashboard tile stores) instead of referencing a saved search or tile. + */ + CHART = 'chart', } export const AlertIntervalSchema = z.union([ @@ -821,6 +826,18 @@ export const zTileAlert = z.object({ dashboardId: z.string().min(1), }); +/** + * Chart alerts persist their query/chart definition directly on the alert — + * the same shape a dashboard tile stores (minus the embedded `alert` field). + * Builder and raw SQL configs only; PromQL charts cannot be alerted on. + * `z.lazy` defers resolution because the chart-config schemas are declared + * later in this module. + */ +export const zChartAlert = z.object({ + source: z.literal(AlertSource.CHART), + chartConfig: z.lazy(() => AlertChartConfigSchema), +}); + export const validateAlertScheduleOffsetMinutes = ( alert: { interval: AlertInterval; @@ -968,6 +985,7 @@ const ChartAlertBaseValidatedSchema = ChartAlertBaseSchema.superRefine( export const AlertSchema = z.union([ z.intersection(AlertBaseValidatedSchema, zSavedSearchAlert), z.intersection(ChartAlertBaseValidatedSchema, zTileAlert), + z.intersection(ChartAlertBaseValidatedSchema, zChartAlert), ]); export type Alert = z.infer; @@ -1738,6 +1756,19 @@ export const SavedChartConfigSchema = z.union([ PromqlSavedChartConfigSchema, ]); +/** + * The chart config a chart-source alert persists (see `zChartAlert`). Same + * shape as a dashboard tile's config, but without the embedded `alert` field + * (the alert's own document carries those fields) and without the PromQL + * variant (PromQL charts cannot be alerted on). + */ +export const AlertChartConfigSchema = z.union([ + BuilderSavedChartConfigWithoutAlertSchema, + RawSqlSavedChartConfigWithoutAlertSchema, +]); + +export type AlertChartConfig = z.infer; + export type RawSqlSavedChartConfig = z.infer< typeof RawSqlSavedChartConfigSchema >; @@ -2417,6 +2448,8 @@ export const AlertsPageItemSchema = z.object({ dashboardId: z.string().optional(), savedSearchId: z.string().optional(), tileId: z.string().optional(), + // Chart alerts: the persisted chart config (absent on other sources). + chartConfig: AlertChartConfigSchema.optional(), groupBy: z.string().optional(), name: z.string().nullish(), message: z.string().nullish(), From 5eb547baf3b9af04ef725236ce1a7f10a8684b9a Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:50:27 -0700 Subject: [PATCH 2/6] fix(api): validate chart alert formulas and raw-SQL source/connection consistency (HDX-5090) Two write-path gaps in the new chart alert source, both of which would otherwise persist configs that fail on every evaluation tick: - Builder configs skipped validateChartConfigFormulas (dashboards get it from the editor and the external tile refinement, but chart alerts are authored through this API directly). internalAlertSchema now rejects malformed formulas, references to nonexistent series, and formulas combined with seriesReturnType: 'ratio' (mapped onto the helper's external-shape asRatio). - Raw-SQL configs accepted a team-owned source on a different team-owned connection. The worker executes through chartConfig.connection while expanding $__sourceTable/metricTables from the source, so a divergent pair yields wrong values or repeated query failures. validateAlertInput now requires the source to belong to the configured connection. --- packages/api/src/controllers/alerts.ts | 10 ++ .../routers/api/__tests__/alerts.int.test.ts | 102 ++++++++++++++++++ packages/api/src/utils/zod.ts | 34 +++++- 3 files changed, 145 insertions(+), 1 deletion(-) diff --git a/packages/api/src/controllers/alerts.ts b/packages/api/src/controllers/alerts.ts index d0a446a0e5..07075719d6 100644 --- a/packages/api/src/controllers/alerts.ts +++ b/packages/api/src/controllers/alerts.ts @@ -151,6 +151,16 @@ export const validateAlertInput = async ( if (source == null) { throw new Api400Error('Source not found'); } + // The worker executes the query through chartConfig.connection while + // expanding $__sourceTable/metricTables from this source. Accepting a + // source on a different (even team-owned) connection would query the + // wrong database — silently wrong values when the table also exists + // there, repeated query failures when it does not. + if (source.connection.toString() !== chartConfig.connection) { + throw new Api400Error( + 'Source does not belong to the specified connection', + ); + } } } else { // Builder configs: same display types the alert task can evaluate as a diff --git a/packages/api/src/routers/api/__tests__/alerts.int.test.ts b/packages/api/src/routers/api/__tests__/alerts.int.test.ts index e8b42912cd..cebcbc1c6a 100644 --- a/packages/api/src/routers/api/__tests__/alerts.int.test.ts +++ b/packages/api/src/routers/api/__tests__/alerts.int.test.ts @@ -1832,6 +1832,108 @@ describe('alerts router', () => { .expect(400); }); + it('validates metric formulas on builder chart configs', async () => { + const { source } = await makeSource(); + const base = makeChartAlertConfig({ sourceId: source._id.toString() }); + + // Formula referencing a nonexistent series (only A exists) + await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig: { ...base, formulas: [{ expression: 'B * 2' }] }, + webhookId: webhook._id.toString(), + }), + ) + .expect(400); + + // Malformed expression + await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig: { ...base, formulas: [{ expression: 'A +' }] }, + webhookId: webhook._id.toString(), + }), + ) + .expect(400); + + // Formulas are mutually exclusive with the ratio toggle (internal + // configs spell it seriesReturnType: 'ratio') + await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig: { + ...base, + seriesReturnType: 'ratio', + formulas: [{ expression: 'A * 2' }], + }, + webhookId: webhook._id.toString(), + }), + ) + .expect(400); + + const created = await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig: { ...base, formulas: [{ expression: 'A * 2' }] }, + webhookId: webhook._id.toString(), + }), + ) + .expect(200); + expect(created.body.data.chartConfig).toMatchObject({ + formulas: [{ expression: 'A * 2' }], + }); + }); + + it('rejects a raw SQL chart alert whose source is on a different connection', async () => { + const { connection, source } = await makeSource(); + const otherConnection = await Connection.create({ + team: team._id, + name: 'Other', + host: 'http://localhost:8124', + username: 'default', + password: '', + }); + + const rawSqlConfig = { + configType: 'sql' as const, + displayType: DisplayType.Line, + sqlTemplate: RAW_SQL_ALERT_TEMPLATE, + source: source._id.toString(), + }; + + // The source belongs to `connection`, not `otherConnection` — the + // worker would execute on one and expand $__sourceTable from the other. + await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig: { + ...rawSqlConfig, + connection: otherConnection._id.toString(), + }, + webhookId: webhook._id.toString(), + }), + ) + .expect(400); + + await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig: { + ...rawSqlConfig, + connection: connection._id.toString(), + }, + webhookId: webhook._id.toString(), + }), + ) + .expect(200); + }); + it('rejects a chart alert with a PromQL config', async () => { await agent .post('/alerts') diff --git a/packages/api/src/utils/zod.ts b/packages/api/src/utils/zod.ts index 61f3b187f8..46ab345aed 100644 --- a/packages/api/src/utils/zod.ts +++ b/packages/api/src/utils/zod.ts @@ -736,6 +736,37 @@ const zChartAlert = z.object({ chartConfig: AlertChartConfigSchema, }); +/** + * Metric-formula validation for chart alerts (builder configs only — raw SQL + * configs have no formulas). Dashboard tiles get this through the editor's + * save-time rules and the external tile-config refinement above; chart alerts + * are authored through this API directly, so without it a malformed formula + * (or one referencing a nonexistent series) persists and then throws on every + * evaluation tick instead of being rejected at write time. The helper checks + * the external-shape `asRatio`, so map the internal `seriesReturnType: 'ratio'` + * onto it. + */ +const validateChartAlertFormulas = ( + alert: { + source: AlertSource; + chartConfig?: z.infer; + }, + ctx: z.RefinementCtx, +) => { + if (alert.source !== AlertSource.CHART || alert.chartConfig == null) { + return; + } + const chartConfig = alert.chartConfig; + if ('configType' in chartConfig) { + return; + } + validateChartConfigFormulas( + { ...chartConfig, asRatio: chartConfig.seriesReturnType === 'ratio' }, + ctx, + { configPath: ['chartConfig'] }, + ); +}; + const alertBaseSchema = z.object({ channel: zAlertChannel.optional(), channels: zAlertChannels.optional(), @@ -766,7 +797,8 @@ export const internalAlertSchema = alertBaseSchema .and(zSavedSearchAlert.or(zTileAlert).or(zChartAlert)) .superRefine(validateAlertChannelSelection) .superRefine(validateAlertScheduleOffsetMinutes) - .superRefine(validateAlertThresholdMax); + .superRefine(validateAlertThresholdMax) + .superRefine(validateChartAlertFormulas); // ============================== // Webhooks From 34eacd1eb578d3b976ecbad5b9301dccefef420c Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Wed, 26 Aug 2026 23:18:46 -0700 Subject: [PATCH 3/6] fix(api): compare chart alert connection IDs as ObjectIds (HDX-5090) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit objectIdSchema admits every representation ObjectId.isValid accepts (uppercase hex, 12-byte strings) and the Mongo lookups cast them, but the raw-SQL source/connection consistency check compared strings — so an equivalent non-canonical connection ID resolved both documents and was then rejected as a mismatch. Compare via ObjectId.equals instead. --- packages/api/src/controllers/alerts.ts | 12 +++++++++++- .../src/routers/api/__tests__/alerts.int.test.ts | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/api/src/controllers/alerts.ts b/packages/api/src/controllers/alerts.ts index 07075719d6..3c95bf4d0d 100644 --- a/packages/api/src/controllers/alerts.ts +++ b/packages/api/src/controllers/alerts.ts @@ -5,6 +5,7 @@ import { } from '@hyperdx/common-utils/dist/core/utils'; import { isRawSqlSavedChartConfig } from '@hyperdx/common-utils/dist/guards'; import { groupBy } from 'lodash'; +import { Types } from 'mongoose'; import { z } from 'zod'; import type { ObjectId } from '@/models'; @@ -156,7 +157,16 @@ export const validateAlertInput = async ( // source on a different (even team-owned) connection would query the // wrong database — silently wrong values when the table also exists // there, repeated query failures when it does not. - if (source.connection.toString() !== chartConfig.connection) { + // + // Compare as ObjectIds, not strings: objectIdSchema admits every + // representation ObjectId.isValid does (uppercase hex, 12-byte + // strings), and the Mongo lookups above cast them — a lexical + // comparison would reject an equivalent non-canonical ID. + if ( + !new Types.ObjectId(String(source.connection)).equals( + chartConfig.connection, + ) + ) { throw new Api400Error( 'Source does not belong to the specified connection', ); diff --git a/packages/api/src/routers/api/__tests__/alerts.int.test.ts b/packages/api/src/routers/api/__tests__/alerts.int.test.ts index cebcbc1c6a..cc878b0e47 100644 --- a/packages/api/src/routers/api/__tests__/alerts.int.test.ts +++ b/packages/api/src/routers/api/__tests__/alerts.int.test.ts @@ -1932,6 +1932,22 @@ describe('alerts router', () => { }), ) .expect(200); + + // Equivalent non-canonical representations (uppercase hex) of the same + // connection ID must be accepted — the consistency check compares + // ObjectIds, not strings. + await agent + .post('/alerts') + .send( + makeChartAlertInput({ + chartConfig: { + ...rawSqlConfig, + connection: connection._id.toString().toUpperCase(), + }, + webhookId: webhook._id.toString(), + }), + ) + .expect(200); }); it('rejects a chart alert with a PromQL config', async () => { From b464cae02d62246eb8a6497aeec5e98b14defa3e Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Wed, 26 Aug 2026 23:30:57 -0700 Subject: [PATCH 4/6] fix(api): ignore stale source metadata when a raw-SQL alert source moves connections (HDX-5090) Write-path validation ties a raw-SQL chart alert's source to its connection at save time, but a source can be moved to a different connection afterwards. The worker executes through the config's pinned connection while expanding $__sourceTable/metricTables from the source, so a moved source silently targeted the wrong database (when the same table exists there) or failed confusingly. Extract the optional source lookup shared by the tile and chart raw-SQL branches into getRawSqlSourceMetadata, which now drops (with a warning) a source whose connection no longer matches the config's. Templates that don't use source macros keep evaluating correctly; templates that do fail with a visible query error recorded on the alert instead of returning wrong values. This intentionally also covers raw-SQL tile alerts, which had the identical pre-existing exposure. --- .../providers/__tests__/default.int.test.ts | 212 +++++++++++++++++- .../tasks/checkAlerts/providers/default.ts | 74 ++++-- 2 files changed, 264 insertions(+), 22 deletions(-) diff --git a/packages/api/src/tasks/checkAlerts/providers/__tests__/default.int.test.ts b/packages/api/src/tasks/checkAlerts/providers/__tests__/default.int.test.ts index fad903b650..8ae40af6d1 100644 --- a/packages/api/src/tasks/checkAlerts/providers/__tests__/default.int.test.ts +++ b/packages/api/src/tasks/checkAlerts/providers/__tests__/default.int.test.ts @@ -1,8 +1,14 @@ +import { DisplayType } from '@hyperdx/common-utils/dist/types'; import mongoose from 'mongoose'; import { createAlert } from '@/controllers/alerts'; import { createTeam } from '@/controllers/team'; -import { getServer, makeTile } from '@/fixtures'; +import { + getServer, + makeChartAlertConfig, + makeTile, + RAW_SQL_ALERT_TEMPLATE, +} from '@/fixtures'; import Alert, { AlertSource, AlertThresholdType } from '@/models/alert'; import Connection from '@/models/connection'; import Dashboard from '@/models/dashboard'; @@ -728,6 +734,210 @@ describe('DefaultAlertProvider', () => { expect(result[0].conn.username).toBe('tile-user'); expect(result[0].conn.host).toBe('http://localhost:8124'); }); + + it('should process a single chart alert', async () => { + const team = await createTeam({ name: 'Test Team' }); + + const connection = await Connection.create({ + team: team._id, + name: 'Test Connection', + host: 'http://localhost:8123', + username: 'test', + password: 'test', + }); + + const source = await Source.create({ + team: team._id, + name: 'Test Source', + kind: 'log', + from: { + databaseName: 'default', + tableName: 'logs', + }, + timestampValueExpression: 'timestamp', + connection: connection._id, + }); + + const chartConfig = makeChartAlertConfig({ + sourceId: source._id.toString(), + }); + + const alert = await createAlert( + team._id, + { + source: AlertSource.CHART, + chartConfig, + threshold: 10, + thresholdType: AlertThresholdType.ABOVE, + interval: '5m', + channel: { + type: 'webhook', + webhookId: new mongoose.Types.ObjectId().toString(), + }, + }, + new mongoose.Types.ObjectId(), + ); + + const result = await provider.getAlertTasks(); + + expect(result).toHaveLength(1); + // Builder chart alerts derive their connection from the source + expect(result[0].conn.id).toBe(connection.id); + expect(result[0].alerts).toHaveLength(1); + expect(result[0].alerts[0].taskType).toBe(AlertTaskType.CHART); + expect(result[0].alerts[0].alert.id).toBe(alert.id); + + if (result[0].alerts[0].taskType === AlertTaskType.CHART) { + expect(result[0].alerts[0].chartConfig).toMatchObject({ + source: source._id.toString(), + }); + expect(result[0].alerts[0].source?.name).toBe('Test Source'); + } + }); + + // The stale-pairing scenario: write-path validation ties a raw-SQL + // config's source to its connection, but the source can be moved to a + // different connection afterwards. The worker must not expand + // $__sourceTable/metricTables from the moved source while executing + // through the pinned connection. + const setupRawSqlSourceMove = async () => { + const team = await createTeam({ name: 'Test Team' }); + const connection = await Connection.create({ + team: team._id, + name: 'Original Connection', + host: 'http://localhost:8123', + username: 'test', + password: 'test', + }); + const otherConnection = await Connection.create({ + team: team._id, + name: 'Other Connection', + host: 'http://localhost:8124', + username: 'test', + password: 'test', + }); + const source = await Source.create({ + team: team._id, + name: 'Test Source', + kind: 'log', + from: { + databaseName: 'default', + tableName: 'logs', + }, + timestampValueExpression: 'timestamp', + connection: connection._id, + }); + return { team, connection, otherConnection, source }; + }; + + it('drops raw-SQL chart alert source metadata when the source moved to a different connection', async () => { + const { team, connection, otherConnection, source } = + await setupRawSqlSourceMove(); + + await createAlert( + team._id, + { + source: AlertSource.CHART, + chartConfig: { + configType: 'sql', + displayType: DisplayType.Line, + sqlTemplate: RAW_SQL_ALERT_TEMPLATE, + connection: connection._id.toString(), + source: source._id.toString(), + }, + threshold: 10, + thresholdType: AlertThresholdType.ABOVE, + interval: '5m', + channel: { + type: 'webhook', + webhookId: new mongoose.Types.ObjectId().toString(), + }, + }, + new mongoose.Types.ObjectId(), + ); + + let result = await provider.getAlertTasks(); + expect(result).toHaveLength(1); + expect(result[0].alerts[0].taskType).toBe(AlertTaskType.CHART); + if (result[0].alerts[0].taskType === AlertTaskType.CHART) { + expect(result[0].alerts[0].source?.name).toBe('Test Source'); + } + + await Source.updateOne( + { _id: source._id }, + { $set: { connection: otherConnection._id } }, + ); + + result = await provider.getAlertTasks(); + expect(result).toHaveLength(1); + // Still executes through the alert's pinned connection... + expect(result[0].conn.id).toBe(connection.id); + // ...but without the moved source's metadata. + if (result[0].alerts[0].taskType === AlertTaskType.CHART) { + expect(result[0].alerts[0].source).toBeUndefined(); + } + }); + + it('drops raw-SQL tile alert source metadata when the source moved to a different connection', async () => { + const { team, connection, otherConnection, source } = + await setupRawSqlSourceMove(); + + const tile = { + id: 'raw-sql-tile-123', + x: 1, + y: 1, + w: 1, + h: 1, + config: { + configType: 'sql' as const, + displayType: DisplayType.Line, + sqlTemplate: RAW_SQL_ALERT_TEMPLATE, + connection: connection._id.toString(), + source: source._id.toString(), + }, + }; + const dashboard = await Dashboard.create({ + team: team._id, + name: 'Test Dashboard', + tiles: [tile], + }); + + await createAlert( + team._id, + { + source: AlertSource.TILE, + dashboardId: dashboard._id.toString(), + tileId: tile.id, + threshold: 10, + thresholdType: AlertThresholdType.ABOVE, + interval: '5m', + channel: { + type: 'webhook', + webhookId: new mongoose.Types.ObjectId().toString(), + }, + }, + new mongoose.Types.ObjectId(), + ); + + let result = await provider.getAlertTasks(); + expect(result).toHaveLength(1); + expect(result[0].alerts[0].taskType).toBe(AlertTaskType.TILE); + if (result[0].alerts[0].taskType === AlertTaskType.TILE) { + expect(result[0].alerts[0].source?.name).toBe('Test Source'); + } + + await Source.updateOne( + { _id: source._id }, + { $set: { connection: otherConnection._id } }, + ); + + result = await provider.getAlertTasks(); + expect(result).toHaveLength(1); + expect(result[0].conn.id).toBe(connection.id); + if (result[0].alerts[0].taskType === AlertTaskType.TILE) { + expect(result[0].alerts[0].source).toBeUndefined(); + } + }); }); describe('buildLogSearchLink', () => { diff --git a/packages/api/src/tasks/checkAlerts/providers/default.ts b/packages/api/src/tasks/checkAlerts/providers/default.ts index 7a0c636306..12007dcdea 100644 --- a/packages/api/src/tasks/checkAlerts/providers/default.ts +++ b/packages/api/src/tasks/checkAlerts/providers/default.ts @@ -1,7 +1,11 @@ import PQueue from '@esm2cjs/p-queue'; import { displayTypeSupportsRawSqlAlerts } from '@hyperdx/common-utils/dist/core/utils'; import { isRawSqlSavedChartConfig } from '@hyperdx/common-utils/dist/guards'; -import { AlertChartConfig, Tile } from '@hyperdx/common-utils/dist/types'; +import { + AlertChartConfig, + RawSqlSavedChartConfig, + Tile, +} from '@hyperdx/common-utils/dist/types'; import mongoose from 'mongoose'; import ms from 'ms'; import { URLSearchParams } from 'url'; @@ -91,6 +95,52 @@ async function getSavedSearchDetails( ]; } +/** + * Load the optional source a raw-SQL config references for macro metadata + * ($__sourceTable, metricTables). Write-path validation guarantees the source + * belongs to the config's connection at save time, but a source can be moved + * to a different connection afterwards. The query always executes through the + * config's pinned connection, so metadata from a moved source would silently + * target the wrong database (when the same table exists there) or fail + * confusingly. Drop it instead, with a warning: templates that don't use + * source macros keep evaluating correctly, and templates that do fail with a + * visible query error recorded on the alert. + */ +async function getRawSqlSourceMetadata( + alert: IAlert, + chartConfig: RawSqlSavedChartConfig, +): Promise { + if (!chartConfig.source) { + return undefined; + } + const sourceDoc = await Source.findOne({ + _id: chartConfig.source, + team: alert.team, + }); + if (!sourceDoc) { + return undefined; + } + // Compare as ObjectIds: stored configs may hold non-canonical but valid + // representations (e.g. uppercase hex), which a lexical compare would + // misjudge as a mismatch. + if ( + !new mongoose.Types.ObjectId(String(sourceDoc.connection)).equals( + chartConfig.connection, + ) + ) { + logger.warn({ + message: + 'raw sql alert source has moved to a different connection; ignoring its metadata', + alertId: alert.id, + sourceId: chartConfig.source, + sourceConnectionId: String(sourceDoc.connection), + configConnectionId: chartConfig.connection, + }); + return undefined; + } + return sourceDoc.toObject(); +} + async function getTileDetails( alert: IAlert, ): Promise<[IConnection, PartialAlertDetails] | []> { @@ -151,16 +201,7 @@ async function getTileDetails( } // Optionally look up source for filter/macro metadata - let source: ISource | undefined; - if (tile.config.source) { - const sourceDoc = await Source.findOne({ - _id: tile.config.source, - team: alert.team, - }); - if (sourceDoc) { - source = sourceDoc.toObject(); - } - } + const source = await getRawSqlSourceMetadata(alert, tile.config); return [ connection, @@ -256,16 +297,7 @@ async function getChartDetails( } // Optionally look up source for filter/macro metadata - let source: ISource | undefined; - if (chartConfig.source) { - const sourceDoc = await Source.findOne({ - _id: chartConfig.source, - team: alert.team, - }); - if (sourceDoc) { - source = sourceDoc.toObject(); - } - } + const source = await getRawSqlSourceMetadata(alert, chartConfig); return [ connection, From 14175ec2832c5d644c9b1c40aeeb981680d36a64 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Thu, 27 Aug 2026 09:04:22 -0700 Subject: [PATCH 5/6] refactor(api): rename chart alert source to inline (HDX-5090) 'chart' collided with existing vocabulary: ChartAlertBaseSchema and the alerts-page copy ("created from dashboard charts") both already use "chart" to mean tile alerts, and the enum's other values name the referenced entity while the new kind's defining property is that the definition lives inline on the alert with no referenced entity. Wire value 'chart' -> 'inline' (unreleased, so no migration), plus the matching identifiers: AlertSource.INLINE, AlertTaskType.INLINE, zInlineAlert, getInlineAlertDetails, validateInlineAlertFormulas, and the test fixtures. Shape-named identifiers keep their names (chartConfig, AlertChartConfigSchema, buildChartExplorerLink) since they describe the config's content or link target, not the source kind. --- .changeset/detached-chart-alerts-backend.md | 6 -- .changeset/detached-inline-alerts-backend.md | 6 ++ .../src/controllers/__tests__/alerts.test.ts | 6 +- packages/api/src/controllers/alerts.ts | 10 +-- packages/api/src/fixtures.ts | 6 +- packages/api/src/models/alert.ts | 8 +- .../routers/api/__tests__/alerts.int.test.ts | 76 +++++++++---------- .../external-api/__tests__/alerts.int.test.ts | 6 +- .../__tests__/checkAlerts.int.test.ts | 52 ++++++------- packages/api/src/tasks/checkAlerts/index.ts | 20 ++--- .../providers/__tests__/default.int.test.ts | 24 +++--- .../tasks/checkAlerts/providers/default.ts | 16 ++-- .../src/tasks/checkAlerts/providers/index.ts | 8 +- .../api/src/tasks/checkAlerts/template.ts | 10 +-- packages/api/src/utils/zod.ts | 22 +++--- packages/common-utils/src/types.ts | 19 ++--- 16 files changed, 148 insertions(+), 147 deletions(-) delete mode 100644 .changeset/detached-chart-alerts-backend.md create mode 100644 .changeset/detached-inline-alerts-backend.md diff --git a/.changeset/detached-chart-alerts-backend.md b/.changeset/detached-chart-alerts-backend.md deleted file mode 100644 index 0a3dd2ce0a..0000000000 --- a/.changeset/detached-chart-alerts-backend.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -'@hyperdx/common-utils': minor -'@hyperdx/api': minor ---- - -Add a new `chart` alert source that persists its own chart config directly on the alert, so alerts no longer require a saved search (logs) or a dashboard tile (metrics). The config is the same shape a dashboard tile stores — builder configs on log/trace/metric sources plus raw SQL (Line/Stacked Bar/Number display types); PromQL is rejected. The internal alerts API accepts and returns the new source, and the check-alerts task evaluates chart alerts through the same code path as tile alerts (including group-by and multi-window behavior). Notifications for chart alerts link to the chart explorer seeded with the alert's config over the alerting window, and default their title to the config's name. Backend only — the creation/edit UI and external API v2 support land separately. diff --git a/.changeset/detached-inline-alerts-backend.md b/.changeset/detached-inline-alerts-backend.md new file mode 100644 index 0000000000..bf53dd7a04 --- /dev/null +++ b/.changeset/detached-inline-alerts-backend.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/common-utils': minor +'@hyperdx/api': minor +--- + +Add a new `inline` alert source that persists its own chart config directly on the alert, so alerts no longer require a saved search (logs) or a dashboard tile (metrics). The config is the same shape a dashboard tile stores — builder configs on log/trace/metric sources plus raw SQL (Line/Stacked Bar/Number display types); PromQL is rejected. The internal alerts API accepts and returns the new source, and the check-alerts task evaluates inline alerts through the same code path as tile alerts (including group-by and multi-window behavior). Notifications for inline alerts link to the chart explorer seeded with the alert's config over the alerting window, and default their title to the config's name. Backend only — the creation/edit UI and external API v2 support land separately. diff --git a/packages/api/src/controllers/__tests__/alerts.test.ts b/packages/api/src/controllers/__tests__/alerts.test.ts index b5dbbb028d..e0be4bae53 100644 --- a/packages/api/src/controllers/__tests__/alerts.test.ts +++ b/packages/api/src/controllers/__tests__/alerts.test.ts @@ -50,13 +50,13 @@ describe('makeAlert', () => { whereLanguage: 'lucene', }; - it('persists chartConfig for chart alerts and clears the other source references', () => { + it('persists chartConfig for inline alerts and clears the other source references', () => { const result = makeAlert({ interval: '5m', threshold: 1, thresholdType: AlertThresholdType.ABOVE, channels: [{ type: 'webhook', webhookId: 'webhook-id' }], - source: AlertSource.CHART, + source: AlertSource.INLINE, chartConfig, }); @@ -68,7 +68,7 @@ describe('makeAlert', () => { }); it('clears chartConfig when the alert source is not chart', () => { - // Converting a chart alert to another source must not leave the old + // Converting an inline alert to another source must not leave the old // config behind (mirrors how savedSearch/dashboard references clear). const result = makeAlert({ interval: '5m', diff --git a/packages/api/src/controllers/alerts.ts b/packages/api/src/controllers/alerts.ts index 3c95bf4d0d..feba0444f9 100644 --- a/packages/api/src/controllers/alerts.ts +++ b/packages/api/src/controllers/alerts.ts @@ -113,7 +113,7 @@ export const validateAlertInput = async ( } } - if (alertInput.source === AlertSource.CHART) { + if (alertInput.source === AlertSource.INLINE) { const chartConfig = alertInput.chartConfig; if (chartConfig == null) { throw new Api400Error('Chart config is required'); @@ -177,7 +177,7 @@ export const validateAlertInput = async ( // time series (mirrors the tile-alert rules). if (!displayTypeSupportsBuilderAlerts(chartConfig.displayType)) { throw new Api400Error( - 'Chart alerts are only supported for Line, Stacked Bar, or Number display types', + 'Inline chart alerts are only supported for Line, Stacked Bar, or Number display types', ); } @@ -234,7 +234,7 @@ export const makeAlert = ( : alert.scheduleOffsetMinutes; const isSavedSearch = alert.source === AlertSource.SAVED_SEARCH; const isTile = alert.source === AlertSource.TILE; - const isChart = alert.source === AlertSource.CHART; + const isInline = alert.source === AlertSource.INLINE; const channels = getAlertChannels(alert); return { @@ -275,8 +275,8 @@ export const makeAlert = ( : null, tileId: isTile ? (alert.tileId ?? null) : null, - // Chart alerts - chartConfig: isChart ? (alert.chartConfig ?? null) : null, + // Inline alerts + chartConfig: isInline ? (alert.chartConfig ?? null) : null, // Multi-window alerting numConsecutiveWindows: alert.numConsecutiveWindows ?? null, diff --git a/packages/api/src/fixtures.ts b/packages/api/src/fixtures.ts index aa7a5b6d34..0cc0d35e33 100644 --- a/packages/api/src/fixtures.ts +++ b/packages/api/src/fixtures.ts @@ -782,7 +782,7 @@ export const makeSavedSearchAlertInput = ({ savedSearchId, }); -export const makeChartAlertConfig = (opts: { +export const makeAlertChartConfig = (opts: { sourceId: string; name?: string; displayType?: DisplayType; @@ -805,7 +805,7 @@ export const makeChartAlertConfig = (opts: { ...(opts.groupBy != null && { groupBy: opts.groupBy }), }); -export const makeChartAlertInput = ({ +export const makeInlineAlertInput = ({ chartConfig, interval = '15m', threshold = 8, @@ -823,6 +823,6 @@ export const makeChartAlertInput = ({ interval, threshold, thresholdType: AlertThresholdType.ABOVE, - source: AlertSource.CHART, + source: AlertSource.INLINE, chartConfig, }); diff --git a/packages/api/src/models/alert.ts b/packages/api/src/models/alert.ts index 529a75bbdb..851ff644ef 100644 --- a/packages/api/src/models/alert.ts +++ b/packages/api/src/models/alert.ts @@ -72,8 +72,8 @@ export const getAlertChannels = (alert: { export enum AlertSource { SAVED_SEARCH = 'saved_search', TILE = 'tile', - /** Detached alert that persists its own chart config (no saved search/tile). */ - CHART = 'chart', + /** Detached alert whose chart config lives inline on the alert (no saved search/tile). */ + INLINE = 'inline', } export interface IAlert { @@ -107,7 +107,7 @@ export interface IAlert { dashboard?: ObjectId | null; tileId?: string | null; - // Chart alerts: the persisted chart config (same shape as a dashboard + // Inline alerts: the persisted chart config (same shape as a dashboard // tile's config, minus the embedded alert field) chartConfig?: AlertChartConfig | null; @@ -232,7 +232,7 @@ const AlertSchema = new Schema( required: false, }, - // Chart alerts + // Inline alerts chartConfig: { type: Schema.Types.Mixed, required: false, diff --git a/packages/api/src/routers/api/__tests__/alerts.int.test.ts b/packages/api/src/routers/api/__tests__/alerts.int.test.ts index cc878b0e47..c2dd2bc68d 100644 --- a/packages/api/src/routers/api/__tests__/alerts.int.test.ts +++ b/packages/api/src/routers/api/__tests__/alerts.int.test.ts @@ -9,9 +9,9 @@ import mongoose from 'mongoose'; import { getLoggedInAgent, getServer, + makeAlertChartConfig, makeAlertInput, - makeChartAlertConfig, - makeChartAlertInput, + makeInlineAlertInput, makeRawSqlAlertTile, makeRawSqlNumberAlertTile, makeRawSqlTile, @@ -1655,7 +1655,7 @@ describe('alerts router', () => { }); }); - describe('chart alerts', () => { + describe('inline alerts', () => { const makeSource = async () => { const connection = await Connection.create({ team: team._id, @@ -1675,23 +1675,23 @@ describe('alerts router', () => { return { connection, source }; }; - it('creates a chart alert and round-trips chartConfig through GET', async () => { + it('creates an inline alert and round-trips chartConfig through GET', async () => { const { source } = await makeSource(); - const chartConfig = makeChartAlertConfig({ + const chartConfig = makeAlertChartConfig({ sourceId: source._id.toString(), }); const created = await agent .post('/alerts') .send( - makeChartAlertInput({ + makeInlineAlertInput({ chartConfig, webhookId: webhook._id.toString(), }), ) .expect(200); - expect(created.body.data.source).toBe(AlertSource.CHART); + expect(created.body.data.source).toBe(AlertSource.INLINE); expect(created.body.data.chartConfig).toMatchObject({ name: 'Chart Alert Query', source: source._id.toString(), @@ -1714,13 +1714,13 @@ describe('alerts router', () => { }); }); - it('updates a chart alert config', async () => { + it('updates an inline alert config', async () => { const { source } = await makeSource(); const created = await agent .post('/alerts') .send( - makeChartAlertInput({ - chartConfig: makeChartAlertConfig({ + makeInlineAlertInput({ + chartConfig: makeAlertChartConfig({ sourceId: source._id.toString(), }), webhookId: webhook._id.toString(), @@ -1728,14 +1728,14 @@ describe('alerts router', () => { ) .expect(200); - const updatedConfig = makeChartAlertConfig({ + const updatedConfig = makeAlertChartConfig({ sourceId: source._id.toString(), groupBy: 'ServiceName', }); await agent .put(`/alerts/${created.body.data._id}`) .send( - makeChartAlertInput({ + makeInlineAlertInput({ chartConfig: updatedConfig, threshold: 42, webhookId: webhook._id.toString(), @@ -1759,8 +1759,8 @@ describe('alerts router', () => { const created = await agent .post('/alerts') .send( - makeChartAlertInput({ - chartConfig: makeChartAlertConfig({ + makeInlineAlertInput({ + chartConfig: makeAlertChartConfig({ sourceId: source._id.toString(), }), webhookId: webhook._id.toString(), @@ -1787,8 +1787,8 @@ describe('alerts router', () => { await agent .put(`/alerts/${created.body.data._id}`) .send( - makeChartAlertInput({ - chartConfig: makeChartAlertConfig({ + makeInlineAlertInput({ + chartConfig: makeAlertChartConfig({ sourceId: source._id.toString(), }), webhookId: webhook._id.toString(), @@ -1804,25 +1804,25 @@ describe('alerts router', () => { expect(stored!.tileId).toBeNull(); }); - it('rejects a chart alert whose source does not exist in the team', async () => { + it('rejects an inline alert whose source does not exist in the team', async () => { await agent .post('/alerts') .send( - makeChartAlertInput({ - chartConfig: makeChartAlertConfig({ sourceId: randomMongoId() }), + makeInlineAlertInput({ + chartConfig: makeAlertChartConfig({ sourceId: randomMongoId() }), webhookId: webhook._id.toString(), }), ) .expect(400); }); - it('rejects a chart alert with an unsupported display type', async () => { + it('rejects an inline alert with an unsupported display type', async () => { const { source } = await makeSource(); await agent .post('/alerts') .send( - makeChartAlertInput({ - chartConfig: makeChartAlertConfig({ + makeInlineAlertInput({ + chartConfig: makeAlertChartConfig({ sourceId: source._id.toString(), displayType: DisplayType.Table, }), @@ -1834,13 +1834,13 @@ describe('alerts router', () => { it('validates metric formulas on builder chart configs', async () => { const { source } = await makeSource(); - const base = makeChartAlertConfig({ sourceId: source._id.toString() }); + const base = makeAlertChartConfig({ sourceId: source._id.toString() }); // Formula referencing a nonexistent series (only A exists) await agent .post('/alerts') .send( - makeChartAlertInput({ + makeInlineAlertInput({ chartConfig: { ...base, formulas: [{ expression: 'B * 2' }] }, webhookId: webhook._id.toString(), }), @@ -1851,7 +1851,7 @@ describe('alerts router', () => { await agent .post('/alerts') .send( - makeChartAlertInput({ + makeInlineAlertInput({ chartConfig: { ...base, formulas: [{ expression: 'A +' }] }, webhookId: webhook._id.toString(), }), @@ -1863,7 +1863,7 @@ describe('alerts router', () => { await agent .post('/alerts') .send( - makeChartAlertInput({ + makeInlineAlertInput({ chartConfig: { ...base, seriesReturnType: 'ratio', @@ -1877,7 +1877,7 @@ describe('alerts router', () => { const created = await agent .post('/alerts') .send( - makeChartAlertInput({ + makeInlineAlertInput({ chartConfig: { ...base, formulas: [{ expression: 'A * 2' }] }, webhookId: webhook._id.toString(), }), @@ -1888,7 +1888,7 @@ describe('alerts router', () => { }); }); - it('rejects a raw SQL chart alert whose source is on a different connection', async () => { + it('rejects a raw SQL inline alert whose source is on a different connection', async () => { const { connection, source } = await makeSource(); const otherConnection = await Connection.create({ team: team._id, @@ -1910,7 +1910,7 @@ describe('alerts router', () => { await agent .post('/alerts') .send( - makeChartAlertInput({ + makeInlineAlertInput({ chartConfig: { ...rawSqlConfig, connection: otherConnection._id.toString(), @@ -1923,7 +1923,7 @@ describe('alerts router', () => { await agent .post('/alerts') .send( - makeChartAlertInput({ + makeInlineAlertInput({ chartConfig: { ...rawSqlConfig, connection: connection._id.toString(), @@ -1939,7 +1939,7 @@ describe('alerts router', () => { await agent .post('/alerts') .send( - makeChartAlertInput({ + makeInlineAlertInput({ chartConfig: { ...rawSqlConfig, connection: connection._id.toString().toUpperCase(), @@ -1950,12 +1950,12 @@ describe('alerts router', () => { .expect(200); }); - it('rejects a chart alert with a PromQL config', async () => { + it('rejects an inline alert with a PromQL config', async () => { await agent .post('/alerts') .send({ - ...makeChartAlertInput({ - chartConfig: makeChartAlertConfig({ sourceId: randomMongoId() }), + ...makeInlineAlertInput({ + chartConfig: makeAlertChartConfig({ sourceId: randomMongoId() }), webhookId: webhook._id.toString(), }), chartConfig: { @@ -1967,14 +1967,14 @@ describe('alerts router', () => { .expect(400); }); - it('accepts a raw SQL chart alert and validates its template', async () => { + it('accepts a raw SQL inline alert and validates its template', async () => { const { connection } = await makeSource(); // Missing the required time-filter/interval parameters await agent .post('/alerts') .send( - makeChartAlertInput({ + makeInlineAlertInput({ chartConfig: { configType: 'sql', displayType: DisplayType.Line, @@ -1990,7 +1990,7 @@ describe('alerts router', () => { await agent .post('/alerts') .send( - makeChartAlertInput({ + makeInlineAlertInput({ chartConfig: { configType: 'sql', displayType: DisplayType.Line, @@ -2005,7 +2005,7 @@ describe('alerts router', () => { const created = await agent .post('/alerts') .send( - makeChartAlertInput({ + makeInlineAlertInput({ chartConfig: { configType: 'sql', displayType: DisplayType.Line, diff --git a/packages/api/src/routers/external-api/__tests__/alerts.int.test.ts b/packages/api/src/routers/external-api/__tests__/alerts.int.test.ts index d2e613fded..49c5c7db31 100644 --- a/packages/api/src/routers/external-api/__tests__/alerts.int.test.ts +++ b/packages/api/src/routers/external-api/__tests__/alerts.int.test.ts @@ -658,8 +658,8 @@ describe('External API Alerts', () => { }); describe('Input validation', () => { - it('rejects the internal-only chart alert source', async () => { - // Chart alerts (source: 'chart') are creatable through the internal API + it('rejects the internal-only inline alert source', async () => { + // Inline alerts (source: 'inline') are creatable through the internal API // only; the external v2 contract (OpenAPI docs, Terraform provider) has // not been extended to cover them yet. const webhook = await createTestWebhook(); @@ -667,7 +667,7 @@ describe('External API Alerts', () => { const alertInput = { threshold: 100, interval: '1h', - source: 'chart', + source: 'inline', chartConfig: { name: 'Chart Alert Query', source: new ObjectId().toString(), 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 4aba634291..2273ec3b7f 100644 --- a/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts +++ b/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts @@ -22,7 +22,7 @@ import { DEFAULT_METRICS_TABLE, getServer, getTestFixtureClickHouseClient, - makeChartAlertConfig, + makeAlertChartConfig, makeTile, RAW_SQL_ALERT_TEMPLATE, RAW_SQL_NUMBER_ALERT_TEMPLATE, @@ -1125,11 +1125,11 @@ describe('checkAlerts', () => { }; } - if (overrides.taskType === AlertTaskType.CHART) { + if (overrides.taskType === AlertTaskType.INLINE) { return { ...base, - taskType: AlertTaskType.CHART, - chartConfig: makeChartAlertConfig({ + taskType: AlertTaskType.INLINE, + chartConfig: makeAlertChartConfig({ sourceId: 'fake-source-id', groupBy: overrides.tileGroupBy ?? '', }), @@ -1194,19 +1194,19 @@ describe('checkAlerts', () => { ).toBe(true); }); - it('should return false for chart alert with empty config groupBy', () => { + it('should return false for inline alert with empty config groupBy', () => { expect( alertHasGroupBy( - makeDetails({ taskType: AlertTaskType.CHART, tileGroupBy: '' }), + makeDetails({ taskType: AlertTaskType.INLINE, tileGroupBy: '' }), ), ).toBe(false); }); - it('should return true for chart alert with config groupBy', () => { + it('should return true for inline alert with config groupBy', () => { expect( alertHasGroupBy( makeDetails({ - taskType: AlertTaskType.CHART, + taskType: AlertTaskType.INLINE, tileGroupBy: 'ServiceName', }), ), @@ -1326,20 +1326,20 @@ describe('checkAlerts', () => { value: 5, }; - const chartAlertConfig = makeChartAlertConfig({ + const inlineAlertConfig = makeAlertChartConfig({ sourceId: 'fake-source-id', }); - const defaultChartAlertView: AlertMessageTemplateDefaultView = { + const defaultInlineAlertView: AlertMessageTemplateDefaultView = { alert: { thresholdType: AlertThresholdType.ABOVE, threshold: 1, - source: AlertSource.CHART, + source: AlertSource.INLINE, channel: { type: 'webhook', webhookId: 'fake-webhook-id', }, interval: '1m', - chartConfig: chartAlertConfig, + chartConfig: inlineAlertConfig, }, startTime: new Date('2023-03-17T22:13:03.103Z'), endTime: new Date('2023-03-17T22:13:59.103Z'), @@ -1382,17 +1382,17 @@ describe('checkAlerts', () => { `"http://app:8080/dashboards/id-123?from=1679089083103&granularity=5+minute&to=1679093339103&highlightedTileId=test-tile-id"`, ); - // Chart alerts link to the chart explorer seeded with the persisted + // Inline alerts link to the chart explorer seeded with the persisted // config. Built programmatically — the URL-encoded config JSON makes an // inline snapshot unreadable. const expectedChartAlertUrl = new URL('http://app:8080/chart'); expectedChartAlertUrl.search = new URLSearchParams({ - config: JSON.stringify(chartAlertConfig), + config: JSON.stringify(inlineAlertConfig), from: '1679089083103', to: '1679093339103', }).toString(); expect( - buildAlertMessageTemplateHdxLink(alertProvider, defaultChartAlertView), + buildAlertMessageTemplateHdxLink(alertProvider, defaultInlineAlertView), ).toBe(expectedChartAlertUrl.toString()); }); @@ -1496,10 +1496,10 @@ describe('checkAlerts', () => { ).toMatchInlineSnapshot( `"🚨 Alert for "Test Chart" in "My Dashboard" - 5 meets or exceeds 1"`, ); - // Chart alerts default to the chart config's name + // Inline alerts default to the chart config's name expect( buildAlertMessageTemplateTitle({ - view: defaultChartAlertView, + view: defaultInlineAlertView, }), ).toMatchInlineSnapshot( `"🚨 Alert for "Chart Alert Query" - 5 meets or exceeds 1"`, @@ -2149,7 +2149,7 @@ describe('checkAlerts', () => { dashboard: IDashboard; } | { - taskType: AlertTaskType.CHART; + taskType: AlertTaskType.INLINE; chartConfig: AlertChartConfig; }, ): Promise => { @@ -2863,7 +2863,7 @@ describe('checkAlerts', () => { ); }); - it('CHART alert (events) - slack webhook', async () => { + it('INLINE alert (events) - slack webhook', async () => { const { team, webhook, @@ -2887,7 +2887,7 @@ describe('checkAlerts', () => { ); // The config lives on the alert itself — no saved search or dashboard. - const chartConfig = makeChartAlertConfig({ + const chartConfig = makeAlertChartConfig({ sourceId: source.id, name: 'Error Count', aggCondition: 'ServiceName:api', @@ -2897,7 +2897,7 @@ describe('checkAlerts', () => { team, source, { - source: AlertSource.CHART, + source: AlertSource.INLINE, channel: { type: 'webhook', webhookId: webhook._id.toString(), @@ -2908,7 +2908,7 @@ describe('checkAlerts', () => { chartConfig, }, { - taskType: AlertTaskType.CHART, + taskType: AlertTaskType.INLINE, chartConfig, }, ); @@ -3002,7 +3002,7 @@ describe('checkAlerts', () => { ); }); - it('CHART alert (group by) - notifies per group', async () => { + it('INLINE alert (group by) - notifies per group', async () => { // Two groups fire, so two notifications go out (the shared beforeEach // only mocks a single call). jest.spyOn(slack, 'postMessageToWebhook').mockResolvedValue(null as any); @@ -3034,7 +3034,7 @@ describe('checkAlerts', () => { })), ]); - const chartConfig = makeChartAlertConfig({ + const chartConfig = makeAlertChartConfig({ sourceId: source.id, name: 'Errors by service', groupBy: 'ServiceName', @@ -3044,7 +3044,7 @@ describe('checkAlerts', () => { team, source, { - source: AlertSource.CHART, + source: AlertSource.INLINE, channel: { type: 'webhook', webhookId: webhook._id.toString(), @@ -3055,7 +3055,7 @@ describe('checkAlerts', () => { chartConfig, }, { - taskType: AlertTaskType.CHART, + taskType: AlertTaskType.INLINE, chartConfig, }, ); diff --git a/packages/api/src/tasks/checkAlerts/index.ts b/packages/api/src/tasks/checkAlerts/index.ts index 1d206f9685..46d201c6a4 100644 --- a/packages/api/src/tasks/checkAlerts/index.ts +++ b/packages/api/src/tasks/checkAlerts/index.ts @@ -126,7 +126,7 @@ const alertBatchFailuresCounter = getCounter('hyperdx.alerts.batch_failures', { /** * Determine if an alert has group-by behavior. * For saved search alerts, groupBy is on alert.groupBy. - * For tile and chart alerts, groupBy is on the chart config. + * For tile and inline alerts, groupBy is on the chart config. */ export const alertHasGroupBy = (details: AlertDetails): boolean => { const { alert } = details; @@ -140,7 +140,7 @@ export const alertHasGroupBy = (details: AlertDetails): boolean => { const savedConfig: SavedChartConfig | undefined = details.taskType === AlertTaskType.TILE ? details.tile.config - : details.taskType === AlertTaskType.CHART + : details.taskType === AlertTaskType.INLINE ? details.chartConfig : undefined; if (savedConfig == null) { @@ -544,7 +544,7 @@ const fireChannelEvent = async ({ thresholdMax: alert.thresholdMax, thresholdType: alert.thresholdType, tileId: alert.tileId, - // Chart alerts: the persisted config, used to build the explorer link + // Inline alerts: the persisted config, used to build the explorer link // and the default notification title. chartConfig: alert.chartConfig, }, @@ -683,9 +683,9 @@ const getAlertEvaluationDateRange = ( /** * Assemble the queryable chart config for an alert backed by a saved chart - * config — a dashboard tile's config or a chart alert's persisted config. - * Shared so tile and chart alerts evaluate identically; only the dashboard - * variables differ (chart alerts have no dashboard, so none are declared). + * config — a dashboard tile's config or an inline alert's persisted config. + * Shared so tile and inline alerts evaluate identically; only the dashboard + * variables differ (inline alerts have no dashboard, so none are declared). */ const buildAlertChartConfigFromSavedConfig = ({ alertId, @@ -697,7 +697,7 @@ const buildAlertChartConfigFromSavedConfig = ({ variables, }: { alertId: string; - // AlertChartConfig (a chart alert's persisted config) is assignable here: + // AlertChartConfig (an inline alert's persisted config) is assignable here: // its members are the tile config types minus the embedded alert field. savedConfig: SavedChartConfig; source?: ISource; @@ -740,7 +740,7 @@ const buildAlertChartConfigFromSavedConfig = ({ } if (!source) { - logger.error({ alertId }, 'Source not found for builder chart alert'); + logger.error({ alertId }, 'Source not found for builder inline alert'); return undefined; } @@ -847,8 +847,8 @@ const getChartConfigFromAlert = ( if (config != null) { return config; } - } else if (details.taskType === AlertTaskType.CHART) { - // Chart alerts have no dashboard, so no variables are declared. + } else if (details.taskType === AlertTaskType.INLINE) { + // Inline alerts have no dashboard, so no variables are declared. const config = buildAlertChartConfigFromSavedConfig({ alertId: alert.id, savedConfig: details.chartConfig, diff --git a/packages/api/src/tasks/checkAlerts/providers/__tests__/default.int.test.ts b/packages/api/src/tasks/checkAlerts/providers/__tests__/default.int.test.ts index 8ae40af6d1..27ef9da7c7 100644 --- a/packages/api/src/tasks/checkAlerts/providers/__tests__/default.int.test.ts +++ b/packages/api/src/tasks/checkAlerts/providers/__tests__/default.int.test.ts @@ -5,7 +5,7 @@ import { createAlert } from '@/controllers/alerts'; import { createTeam } from '@/controllers/team'; import { getServer, - makeChartAlertConfig, + makeAlertChartConfig, makeTile, RAW_SQL_ALERT_TEMPLATE, } from '@/fixtures'; @@ -735,7 +735,7 @@ describe('DefaultAlertProvider', () => { expect(result[0].conn.host).toBe('http://localhost:8124'); }); - it('should process a single chart alert', async () => { + it('should process a single inline alert', async () => { const team = await createTeam({ name: 'Test Team' }); const connection = await Connection.create({ @@ -758,14 +758,14 @@ describe('DefaultAlertProvider', () => { connection: connection._id, }); - const chartConfig = makeChartAlertConfig({ + const chartConfig = makeAlertChartConfig({ sourceId: source._id.toString(), }); const alert = await createAlert( team._id, { - source: AlertSource.CHART, + source: AlertSource.INLINE, chartConfig, threshold: 10, thresholdType: AlertThresholdType.ABOVE, @@ -781,13 +781,13 @@ describe('DefaultAlertProvider', () => { const result = await provider.getAlertTasks(); expect(result).toHaveLength(1); - // Builder chart alerts derive their connection from the source + // Builder inline alerts derive their connection from the source expect(result[0].conn.id).toBe(connection.id); expect(result[0].alerts).toHaveLength(1); - expect(result[0].alerts[0].taskType).toBe(AlertTaskType.CHART); + expect(result[0].alerts[0].taskType).toBe(AlertTaskType.INLINE); expect(result[0].alerts[0].alert.id).toBe(alert.id); - if (result[0].alerts[0].taskType === AlertTaskType.CHART) { + if (result[0].alerts[0].taskType === AlertTaskType.INLINE) { expect(result[0].alerts[0].chartConfig).toMatchObject({ source: source._id.toString(), }); @@ -830,14 +830,14 @@ describe('DefaultAlertProvider', () => { return { team, connection, otherConnection, source }; }; - it('drops raw-SQL chart alert source metadata when the source moved to a different connection', async () => { + it('drops raw-SQL inline alert source metadata when the source moved to a different connection', async () => { const { team, connection, otherConnection, source } = await setupRawSqlSourceMove(); await createAlert( team._id, { - source: AlertSource.CHART, + source: AlertSource.INLINE, chartConfig: { configType: 'sql', displayType: DisplayType.Line, @@ -858,8 +858,8 @@ describe('DefaultAlertProvider', () => { let result = await provider.getAlertTasks(); expect(result).toHaveLength(1); - expect(result[0].alerts[0].taskType).toBe(AlertTaskType.CHART); - if (result[0].alerts[0].taskType === AlertTaskType.CHART) { + expect(result[0].alerts[0].taskType).toBe(AlertTaskType.INLINE); + if (result[0].alerts[0].taskType === AlertTaskType.INLINE) { expect(result[0].alerts[0].source?.name).toBe('Test Source'); } @@ -873,7 +873,7 @@ describe('DefaultAlertProvider', () => { // Still executes through the alert's pinned connection... expect(result[0].conn.id).toBe(connection.id); // ...but without the moved source's metadata. - if (result[0].alerts[0].taskType === AlertTaskType.CHART) { + if (result[0].alerts[0].taskType === AlertTaskType.INLINE) { expect(result[0].alerts[0].source).toBeUndefined(); } }); diff --git a/packages/api/src/tasks/checkAlerts/providers/default.ts b/packages/api/src/tasks/checkAlerts/providers/default.ts index 12007dcdea..560637a0c8 100644 --- a/packages/api/src/tasks/checkAlerts/providers/default.ts +++ b/packages/api/src/tasks/checkAlerts/providers/default.ts @@ -259,13 +259,13 @@ async function getTileDetails( ]; } -async function getChartDetails( +async function getInlineAlertDetails( alert: IAlert, ): Promise<[IConnection, PartialAlertDetails] | []> { const chartConfig = alert.chartConfig; if (chartConfig == null) { logger.error({ - message: 'chart alert has no chartConfig', + message: 'inline alert has no chartConfig', alertId: alert.id, }); return []; @@ -276,7 +276,7 @@ async function getChartDetails( logger.warn({ alertId: alert.id, message: - 'skipping chart alert with raw sql chart config, only line/bar/number display types are supported', + 'skipping inline alert with raw sql chart config, only line/bar/number display types are supported', }); return []; } @@ -289,7 +289,7 @@ async function getChartDetails( if (!connection) { logger.error({ - message: 'connection not found for raw sql chart alert', + message: 'connection not found for raw sql inline alert', connectionId: chartConfig.connection, alertId: alert.id, }); @@ -304,7 +304,7 @@ async function getChartDetails( { alert, source, - taskType: AlertTaskType.CHART, + taskType: AlertTaskType.INLINE, chartConfig, }, ]; @@ -343,7 +343,7 @@ async function getChartDetails( { alert, source: { ...sourceProps, connection: connection.id }, - taskType: AlertTaskType.CHART, + taskType: AlertTaskType.INLINE, chartConfig, }, ]; @@ -377,8 +377,8 @@ async function loadAlert( [conn, details] = await getTileDetails(alert); break; - case AlertSource.CHART: - [conn, details] = await getChartDetails(alert); + case AlertSource.INLINE: + [conn, details] = await getInlineAlertDetails(alert); break; default: diff --git a/packages/api/src/tasks/checkAlerts/providers/index.ts b/packages/api/src/tasks/checkAlerts/providers/index.ts index e59c23e6e4..96071696ed 100644 --- a/packages/api/src/tasks/checkAlerts/providers/index.ts +++ b/packages/api/src/tasks/checkAlerts/providers/index.ts @@ -17,7 +17,7 @@ import logger from '@/utils/logger'; export enum AlertTaskType { SAVED_SEARCH, TILE, - CHART, + INLINE, } // Discriminated union of possible alert channel types with populated channel data @@ -50,10 +50,10 @@ export type AlertDetails = { dashboard: IDashboard; } | { - // Chart alerts: the config lives on the alert document. `source` is + // Inline alerts: the config lives on the alert document. `source` is // present for builder configs and optional for raw SQL configs (which // carry the connection directly). - taskType: AlertTaskType.CHART; + taskType: AlertTaskType.INLINE; source?: ISource; chartConfig: AlertChartConfig; } @@ -89,7 +89,7 @@ export interface AlertProvider { }): string; /** - * Link for chart alerts (which have no saved search or dashboard to open): + * Link for inline alerts (which have no saved search or dashboard to open): * the chart explorer with the alert's persisted config over the alerting * window. Works regardless of whether the alert-details page is enabled. */ diff --git a/packages/api/src/tasks/checkAlerts/template.ts b/packages/api/src/tasks/checkAlerts/template.ts index bca6ac08f1..b3800221b1 100644 --- a/packages/api/src/tasks/checkAlerts/template.ts +++ b/packages/api/src/tasks/checkAlerts/template.ts @@ -210,11 +210,11 @@ export const buildAlertMessageTemplateHdxLink = ( startTime, tileId: alert.tileId ?? undefined, }); - } else if (alert.source === AlertSource.CHART) { + } else if (alert.source === AlertSource.INLINE) { if (alert.chartConfig == null) { throw new Error(`Source is ${alert.source} but chartConfig is null`); } - // Chart alerts have no saved search or dashboard to open — link to the + // Inline alerts have no saved search or dashboard to open — link to the // chart explorer seeded with the alert's persisted config. return alertProvider.buildChartExplorerLink({ chartConfig: alert.chartConfig, @@ -270,9 +270,9 @@ export const buildAlertMessageTemplateTitle = ({ : describeThresholdResolution(alert.thresholdType) } ${describeThreshold(alert)}`; return `${emoji}${baseTitle}`; - } else if (alert.source === AlertSource.CHART) { + } else if (alert.source === AlertSource.INLINE) { const formattedValue = formatValueToMatchThreshold(value, alert.threshold); - // Chart alerts have no saved search/tile to name them; the alert's `name` + // Inline alerts have no saved search/tile to name them; the alert's `name` // doubles as the title template, so the default falls back to the chart // config's name. const baseTitle = template @@ -719,7 +719,7 @@ ${targetTemplate} \`\`\``; } else if ( alert.source === AlertSource.TILE || - alert.source === AlertSource.CHART + alert.source === AlertSource.INLINE ) { if (alert.source === AlertSource.TILE && dashboard == null) { throw new Error(`Source is ${alert.source} but dashboard is null`); diff --git a/packages/api/src/utils/zod.ts b/packages/api/src/utils/zod.ts index 46ab345aed..dccf8cba85 100644 --- a/packages/api/src/utils/zod.ts +++ b/packages/api/src/utils/zod.ts @@ -730,30 +730,30 @@ const zTileAlert = z.object({ dashboardId: z.string().min(1), }); -const zChartAlert = z.object({ - source: z.literal(AlertSource.CHART), +const zInlineAlert = z.object({ + source: z.literal(AlertSource.INLINE), // Builder + raw SQL configs only; the schema has no PromQL variant. chartConfig: AlertChartConfigSchema, }); /** - * Metric-formula validation for chart alerts (builder configs only — raw SQL + * Metric-formula validation for inline alerts (builder configs only — raw SQL * configs have no formulas). Dashboard tiles get this through the editor's - * save-time rules and the external tile-config refinement above; chart alerts + * save-time rules and the external tile-config refinement above; inline alerts * are authored through this API directly, so without it a malformed formula * (or one referencing a nonexistent series) persists and then throws on every * evaluation tick instead of being rejected at write time. The helper checks * the external-shape `asRatio`, so map the internal `seriesReturnType: 'ratio'` * onto it. */ -const validateChartAlertFormulas = ( +const validateInlineAlertFormulas = ( alert: { source: AlertSource; chartConfig?: z.infer; }, ctx: z.RefinementCtx, ) => { - if (alert.source !== AlertSource.CHART || alert.chartConfig == null) { + if (alert.source !== AlertSource.INLINE || alert.chartConfig == null) { return; } const chartConfig = alert.chartConfig; @@ -789,16 +789,16 @@ export const alertSchema = alertBaseSchema .superRefine(validateAlertScheduleOffsetMinutes) .superRefine(validateAlertThresholdMax); -// Superset of alertSchema for the internal API only: also accepts chart -// alerts (source: 'chart'), which persist their own chart config. The +// Superset of alertSchema for the internal API only: also accepts inline +// alerts (source: 'inline'), which persist their own chart config. The // external v2 API keeps the narrower alertSchema until its contract (OpenAPI -// docs, Terraform provider) is extended to cover chart alerts. +// docs, Terraform provider) is extended to cover inline alerts. export const internalAlertSchema = alertBaseSchema - .and(zSavedSearchAlert.or(zTileAlert).or(zChartAlert)) + .and(zSavedSearchAlert.or(zTileAlert).or(zInlineAlert)) .superRefine(validateAlertChannelSelection) .superRefine(validateAlertScheduleOffsetMinutes) .superRefine(validateAlertThresholdMax) - .superRefine(validateChartAlertFormulas); + .superRefine(validateInlineAlertFormulas); // ============================== // Webhooks diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index e0565f25d2..095244592f 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -667,10 +667,11 @@ export enum AlertSource { SAVED_SEARCH = 'saved_search', TILE = 'tile', /** - * A "detached" alert that persists its own chart config (the same shape a - * dashboard tile stores) instead of referencing a saved search or tile. + * A "detached" alert whose query definition lives inline on the alert + * document (a chart config, the same shape a dashboard tile stores) + * instead of referencing a saved search or tile. */ - CHART = 'chart', + INLINE = 'inline', } export const AlertIntervalSchema = z.union([ @@ -827,14 +828,14 @@ export const zTileAlert = z.object({ }); /** - * Chart alerts persist their query/chart definition directly on the alert — + * Inline alerts persist their query/chart definition directly on the alert — * the same shape a dashboard tile stores (minus the embedded `alert` field). * Builder and raw SQL configs only; PromQL charts cannot be alerted on. * `z.lazy` defers resolution because the chart-config schemas are declared * later in this module. */ -export const zChartAlert = z.object({ - source: z.literal(AlertSource.CHART), +export const zInlineAlert = z.object({ + source: z.literal(AlertSource.INLINE), chartConfig: z.lazy(() => AlertChartConfigSchema), }); @@ -985,7 +986,7 @@ const ChartAlertBaseValidatedSchema = ChartAlertBaseSchema.superRefine( export const AlertSchema = z.union([ z.intersection(AlertBaseValidatedSchema, zSavedSearchAlert), z.intersection(ChartAlertBaseValidatedSchema, zTileAlert), - z.intersection(ChartAlertBaseValidatedSchema, zChartAlert), + z.intersection(ChartAlertBaseValidatedSchema, zInlineAlert), ]); export type Alert = z.infer; @@ -1757,7 +1758,7 @@ export const SavedChartConfigSchema = z.union([ ]); /** - * The chart config a chart-source alert persists (see `zChartAlert`). Same + * The chart config an inline-source alert persists (see `zInlineAlert`). Same * shape as a dashboard tile's config, but without the embedded `alert` field * (the alert's own document carries those fields) and without the PromQL * variant (PromQL charts cannot be alerted on). @@ -2448,7 +2449,7 @@ export const AlertsPageItemSchema = z.object({ dashboardId: z.string().optional(), savedSearchId: z.string().optional(), tileId: z.string().optional(), - // Chart alerts: the persisted chart config (absent on other sources). + // Inline alerts: the persisted chart config (absent on other sources). chartConfig: AlertChartConfigSchema.optional(), groupBy: z.string().optional(), name: z.string().nullish(), From 49799fafa764411949338c1be64f762bd1132feb Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:15:38 -0700 Subject: [PATCH 6/6] fix(api): omit chartConfig from the alerts list response (HDX-5090) GET /alerts is unpaginated, so attaching every inline alert's full chart config (raw SQL templates included) would bloat every alerts-page load at the 1000+-rule scale this feature targets, and bake in a collection contract that couldn't be paginated away later. formatAlertResponse gains an includeChartConfig flag; only GET /alerts/:id sets it, keeping the full config on the detail response for edit surfaces. A slim list-side summary (name/display type) is deferred to the UI PR that will consume it. --- .../src/routers/api/__tests__/alerts.int.test.ts | 7 ++++--- packages/api/src/routers/api/alerts.ts | 16 ++++++++++++---- packages/common-utils/src/types.ts | 4 +++- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/api/src/routers/api/__tests__/alerts.int.test.ts b/packages/api/src/routers/api/__tests__/alerts.int.test.ts index c2dd2bc68d..acd05b60f8 100644 --- a/packages/api/src/routers/api/__tests__/alerts.int.test.ts +++ b/packages/api/src/routers/api/__tests__/alerts.int.test.ts @@ -1707,11 +1707,12 @@ describe('alerts router', () => { expect(single.body.data.savedSearchId).toBeUndefined(); expect(single.body.data.dashboardId).toBeUndefined(); + // The unpaginated list omits the config — only the detail response + // carries the full query definition. const list = await agent.get('/alerts').expect(200); expect(list.body.data).toHaveLength(1); - expect(list.body.data[0].chartConfig).toMatchObject({ - source: source._id.toString(), - }); + expect(list.body.data[0].source).toBe(AlertSource.INLINE); + expect(list.body.data[0].chartConfig).toBeUndefined(); }); it('updates an inline alert config', async () => { diff --git a/packages/api/src/routers/api/alerts.ts b/packages/api/src/routers/api/alerts.ts index bebe5d490a..386e3d3417 100644 --- a/packages/api/src/routers/api/alerts.ts +++ b/packages/api/src/routers/api/alerts.ts @@ -38,6 +38,7 @@ type EnhancedAlert = NonNullable>>; const formatAlertResponse = ( alert: EnhancedAlert, history: Omit[], + { includeChartConfig = false }: { includeChartConfig?: boolean } = {}, ): PreSerialized => { return { history, @@ -78,9 +79,14 @@ const formatAlertResponse = ( 'tags', ]), }), - // Chart alerts carry their persisted config so edit surfaces can seed the - // chart editor and the detail page can render the query. - ...(alert.chartConfig && { chartConfig: alert.chartConfig }), + // Inline alerts carry their persisted config so edit surfaces can seed + // the chart editor and the detail page can render the query — but only on + // the single-alert response. The list endpoint is unpaginated, so + // attaching every alert's full config (raw SQL templates included) would + // bloat every alerts-page load and create a contract that couldn't be + // paginated away later. + ...(includeChartConfig && + alert.chartConfig && { chartConfig: alert.chartConfig }), ...pick(alert, [ '_id', 'interval', @@ -159,7 +165,9 @@ router.get( limit: 20, }); - const data = formatAlertResponse(alert, history); + const data = formatAlertResponse(alert, history, { + includeChartConfig: true, + }); sendJson(res, { data }); } catch (e) { diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 095244592f..26c6bfc0c5 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -2449,7 +2449,9 @@ export const AlertsPageItemSchema = z.object({ dashboardId: z.string().optional(), savedSearchId: z.string().optional(), tileId: z.string().optional(), - // Inline alerts: the persisted chart config (absent on other sources). + // Inline alerts: the persisted chart config. Only present on the + // single-alert (detail) response — the unpaginated list omits it so every + // alerts-page load doesn't carry every alert's full query definition. chartConfig: AlertChartConfigSchema.optional(), groupBy: z.string().optional(), name: z.string().nullish(),