From 11ae3ff207c836b5c11dcd41ea1b397917235a64 Mon Sep 17 00:00:00 2001 From: Jordan Simonovski Date: Thu, 3 Sep 2026 09:21:22 +1000 Subject: [PATCH 1/5] fix(api): report the range upper bound and inline query in webhook templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two of the enriched webhook template variables did not carry what they advertised. `{{sourceQuery}}` read only `savedSearch.where`, so an inline-query alert — whose query lives on the alert's own `chartConfig` — always rendered empty. And a `between`/`outside` alert exposed only `threshold`, so a receiver saw one bound of two. `{{thresholdMax}}` is gated on the comparator rather than read straight off the alert: nothing clears a persisted `thresholdMax` when an alert is switched off a range comparator, and a stale bound would advertise a range that no longer fires. The documented guard for an optional numeric slot was `{{#if thresholdMax}}`, which is falsy on a legitimate bound of 0. It now compares against `undefined` via the registered `eq` helper. --- ...webhook-template-range-and-inline-query.md | 14 +++ docs/alert-webhook-template-variables.md | 28 ++++- packages/api/src/fixtures.ts | 16 ++- .../__tests__/renderAlertTemplate.int.test.ts | 104 +++++++++++++++++- .../api/src/tasks/checkAlerts/template.ts | 30 ++++- .../transports/__tests__/generic.test.ts | 25 +++++ .../tasks/checkAlerts/transports/generic.ts | 1 + .../src/tasks/checkAlerts/transports/types.ts | 4 +- 8 files changed, 211 insertions(+), 11 deletions(-) create mode 100644 .changeset/webhook-template-range-and-inline-query.md diff --git a/.changeset/webhook-template-range-and-inline-query.md b/.changeset/webhook-template-range-and-inline-query.md new file mode 100644 index 0000000000..fcb8cdb561 --- /dev/null +++ b/.changeset/webhook-template-range-and-inline-query.md @@ -0,0 +1,14 @@ +--- +'@hyperdx/api': minor +--- + +Fill in two gaps in the Generic and incident.io webhook template variables. A +`between` or `outside` alert now carries its upper bound as `{{thresholdMax}}`, +so a receiver can reconstruct the condition instead of seeing one bound of two. +An inline-query alert now reports its own builder `where` expression or raw SQL +in `{{sourceQuery}}`, which was previously always empty because only a saved +search's query was read. + +`{{thresholdMax}}` is gated on the comparator rather than read straight off the +alert: switching an alert off a range comparator leaves the old bound on the +document, and reporting it would advertise a range that no longer fires. diff --git a/docs/alert-webhook-template-variables.md b/docs/alert-webhook-template-variables.md index f9fcbece9b..bf98326cbe 100644 --- a/docs/alert-webhook-template-variables.md +++ b/docs/alert-webhook-template-variables.md @@ -17,7 +17,8 @@ a HyperDX alert without parsing the human-readable message body. | `{{status}}` | string | `firing`, `resolved`, `no_data`, `pending` or `error`. | | `{{alertType}}` | string | `search`, `dashboard_chart` or `inline_query`. | | `{{comparator}}` | string | `>=`, `>`, `<`, `<=`, `=`, `!=`, `between`, `outside`. | -| `{{threshold}}` | number | The configured threshold. | +| `{{threshold}}` | number | The configured threshold. For `between`/`outside`, the lower bound. | +| `{{thresholdMax}}` | number | Upper bound of a `between`/`outside` condition. Always unset for every other comparator, even if the alert was once a range, so guard it (see below). | | `{{value}}` | number | The value that triggered or resolved the alert. | | `{{groupKey}}` | string | The breaching group, for a grouped alert. | | `{{sourceQuery}}` | string | The search expression or SQL behind the alert. | @@ -25,10 +26,27 @@ a HyperDX alert without parsing the human-readable message body. | `{{note}}` | string | The alert's freeform note — commonly a runbook link. | Strings are JSON-escaped, so they are safe to drop into a quoted slot. -Numbers (`startTime`, `endTime`, `threshold`, `value`) are emitted raw for -unquoted slots. Every enriched variable is optional and renders as an empty -string when the alert doesn't carry it — an alert with no group has an empty -`{{groupKey}}`, and a dashboard-tile alert has an empty `{{sourceQuery}}`. +Numbers (`startTime`, `endTime`, `threshold`, `thresholdMax`, `value`) are +emitted raw for unquoted slots. Every enriched variable is optional and renders +as an empty string when the alert doesn't carry it — an alert with no group has +an empty `{{groupKey}}`, and a dashboard-tile alert has an empty +`{{sourceQuery}}` because its query lives on the tile. A saved-search alert +reports the search's `where` expression; an inline alert reports its own +builder `where` or raw SQL. + +An empty string is not valid JSON in an unquoted numeric slot, so guard any +number that may be absent. Compare against `undefined` rather than using +`{{#if}}`, which treats a legitimate bound of `0` as absent: + +``` +{ + "threshold": {{threshold}}{{#unless (eq thresholdMax undefined)}}, + "threshold_max": {{thresholdMax}}{{/unless}} +} +``` + +Keep a newline or space between `{{/unless}}` and a closing `}` — Handlebars +reads `}}}` as a triple-stache and fails to compile the template. ## Example diff --git a/packages/api/src/fixtures.ts b/packages/api/src/fixtures.ts index 26ad95229c..20c15cb1ca 100644 --- a/packages/api/src/fixtures.ts +++ b/packages/api/src/fixtures.ts @@ -878,6 +878,7 @@ export const makeAlertChartConfig = (opts: { displayType?: DisplayType; aggCondition?: string; groupBy?: string; + where?: string; }): AlertChartConfig => ({ name: opts.name ?? 'Chart Alert Query', source: opts.sourceId, @@ -890,11 +891,24 @@ export const makeAlertChartConfig = (opts: { valueExpression: '', }, ], - where: '', + where: opts.where ?? '', whereLanguage: 'lucene', ...(opts.groupBy != null && { groupBy: opts.groupBy }), }); +/** Raw SQL counterpart to {@link makeAlertChartConfig}. */ +export const makeRawSqlAlertChartConfig = (opts?: { + name?: string; + sqlTemplate?: string; + connectionId?: string; +}): AlertChartConfig => ({ + name: opts?.name ?? 'Raw SQL Alert Query', + configType: 'sql', + displayType: DisplayType.Line, + sqlTemplate: opts?.sqlTemplate ?? RAW_SQL_ALERT_TEMPLATE, + connection: opts?.connectionId ?? 'test-connection', +}); + export const makeInlineAlertInput = ({ chartConfig, interval = '15m', diff --git a/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts b/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts index d19ec269c9..0c89a33f57 100644 --- a/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts +++ b/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts @@ -5,7 +5,12 @@ import { } from '@hyperdx/common-utils/dist/types'; import mongoose from 'mongoose'; -import { makeTile } from '@/fixtures'; +import { + makeAlertChartConfig, + makeRawSqlAlertChartConfig, + makeTile, + RAW_SQL_ALERT_TEMPLATE, +} from '@/fixtures'; import { AlertSource } from '@/models/alert'; import type { IWebhook } from '@/models/webhook'; import { @@ -172,6 +177,42 @@ const makeTileView = ( value: overrides.value ?? 10, }); +const makeInlineView = ( + overrides: Partial & { + thresholdType?: AlertThresholdType; + threshold?: number; + thresholdMax?: number; + value?: number; + group?: string; + where?: string; + rawSql?: boolean; + } = {}, +): AlertMessageTemplateDefaultView => ({ + alert: { + thresholdType: overrides.thresholdType ?? AlertThresholdType.ABOVE, + threshold: overrides.threshold ?? 5, + thresholdMax: overrides.thresholdMax, + source: AlertSource.INLINE, + channel: { type: null }, + interval: '1m', + chartConfig: overrides.rawSql + ? makeRawSqlAlertChartConfig() + : makeAlertChartConfig({ + sourceId: 'fake-source-id', + where: overrides.where ?? 'ServiceName: "checkout"', + }), + }, + attributes: {}, + granularity: '1m', + group: overrides.group, + isGroupedAlert: false, + startTime, + endTime, + value: overrides.value ?? 10, +}); + +type ViewOverrides = Parameters[0]; + const render = async ( view: AlertMessageTemplateDefaultView, state: AlertState, @@ -435,7 +476,10 @@ describe('renderAlertTemplate', () => { describe('enriched message fields', () => { const renderWithWebhook = async ( state: AlertState, - viewOverrides: Parameters[0] = {}, + viewOverrides: ViewOverrides = {}, + makeView: ( + overrides: ViewOverrides, + ) => AlertMessageTemplateDefaultView = makeSearchView, ) => { const webhook = castWebhook({ _id: new mongoose.Types.ObjectId(), @@ -445,7 +489,7 @@ describe('enriched message fields', () => { url: 'https://hooks.slack.com/services/x', }); const { dispatcher, dispatched } = makeRecordingDispatcher(); - const base = makeSearchView(viewOverrides); + const base = makeView(viewOverrides); const result = await renderAlertTemplate({ alertProvider, @@ -491,6 +535,60 @@ describe('enriched message fields', () => { expect(dispatched[0].message).toMatchObject({ status: 'resolved' }); }); + + it('carries both bounds of a range condition', async () => { + const { dispatched } = await renderWithWebhook(AlertState.ALERT, { + thresholdType: AlertThresholdType.BETWEEN, + threshold: 5, + thresholdMax: 20, + value: 12, + }); + + expect(dispatched[0].message).toMatchObject({ + comparator: 'between', + threshold: 5, + thresholdMax: 20, + }); + }); + + // Switching an alert off a range comparator leaves the old bound on the + // document, and reporting it would advertise a range that no longer fires. + it('drops a stale thresholdMax for a single-bound condition', async () => { + const { dispatched } = await renderWithWebhook(AlertState.ALERT, { + thresholdType: AlertThresholdType.ABOVE, + threshold: 5, + thresholdMax: 20, + }); + + expect(dispatched[0].message).toMatchObject({ comparator: '>=' }); + expect(dispatched[0].message.thresholdMax).toBeUndefined(); + }); + + it("reports an inline alert's query from its own chart config", async () => { + const { dispatched } = await renderWithWebhook( + AlertState.ALERT, + {}, + makeInlineView, + ); + + expect(dispatched[0].message).toMatchObject({ + alertType: 'inline_query', + sourceQuery: 'ServiceName: "checkout"', + }); + }); + + it('reports the SQL of a raw SQL inline alert', async () => { + const { dispatched } = await renderWithWebhook( + AlertState.ALERT, + { rawSql: true }, + makeInlineView, + ); + + expect(dispatched[0].message).toMatchObject({ + alertType: 'inline_query', + sourceQuery: RAW_SQL_ALERT_TEMPLATE, + }); + }); }); describe('buildAlertMessageTemplateTitle', () => { diff --git a/packages/api/src/tasks/checkAlerts/template.ts b/packages/api/src/tasks/checkAlerts/template.ts index 598c45791f..17cc034523 100644 --- a/packages/api/src/tasks/checkAlerts/template.ts +++ b/packages/api/src/tasks/checkAlerts/template.ts @@ -105,6 +105,27 @@ const describeThreshold = (alert: AlertInput): string => { : `${alert.threshold}`; }; +// An inline alert keeps its query on the alert itself rather than behind a +// saved search, so reading only `savedSearch.where` leaves `{{sourceQuery}}` +// empty for it. A tile alert stays empty: its query lives on the dashboard +// tile, which the render path doesn't load. +const describeSourceQuery = ( + alert: AlertInput, + savedSearch?: ISavedSearch | null, +): string => { + if (alert.source === AlertSource.INLINE) { + const config = alert.chartConfig; + if (config == null) { + return ''; + } + // Same discriminator as isRawSqlSavedChartConfig, inlined because that + // guard narrows to a SavedChartConfig member and so cannot narrow this + // union's builder branch. + return ('configType' in config ? config.sqlTemplate : config.where) ?? ''; + } + return savedSearch?.where ?? ''; +}; + // Mappings for the enriched webhook template variables. These turn internal // enums into stable, consumer-friendly strings a receiver can branch on // without knowing HyperDX's internals. @@ -556,9 +577,16 @@ export const renderAlertTemplate = async ({ alertType: alert.source ? ALERT_TYPE_BY_SOURCE[alert.source] : '', comparator: COMPARATOR_BY_THRESHOLD_TYPE[alert.thresholdType], threshold: alert.threshold, + // Gated on the comparator, not just read off the alert: nothing clears + // a persisted `thresholdMax` when an alert is switched off a range + // comparator, and a stale bound would advertise a range that no longer + // fires. + thresholdMax: isRangeThresholdType(alert.thresholdType) + ? alert.thresholdMax + : undefined, value, groupKey: group ?? '', - sourceQuery: savedSearch?.where ?? '', + sourceQuery: describeSourceQuery(alert, savedSearch), teamId, note: alert.note ?? '', }, diff --git a/packages/api/src/tasks/checkAlerts/transports/__tests__/generic.test.ts b/packages/api/src/tasks/checkAlerts/transports/__tests__/generic.test.ts index 54c75f932f..1d9e159d28 100644 --- a/packages/api/src/tasks/checkAlerts/transports/__tests__/generic.test.ts +++ b/packages/api/src/tasks/checkAlerts/transports/__tests__/generic.test.ts @@ -4,6 +4,7 @@ import { ObjectId } from 'mongodb'; import { AlertState } from '@/models/alert'; import { buildWebhookTemplateVariables, + createHandlebarsWithHelpers, getWebhookFetchTimeoutMs, handleSendGenericWebhook, } from '@/tasks/checkAlerts/transports/generic'; @@ -98,6 +99,7 @@ describe('buildWebhookTemplateVariables', () => { alertType: 'search', comparator: '>=', threshold: 5, + thresholdMax: 20, value: 42, groupKey: 'checkout', sourceQuery: 'Body: "error"', @@ -113,6 +115,7 @@ describe('buildWebhookTemplateVariables', () => { alertType: 'search', comparator: '>=', threshold: 5, + thresholdMax: 20, value: 42, groupKey: 'checkout', teamId: 'team-1', @@ -128,6 +131,28 @@ describe('buildWebhookTemplateVariables', () => { expect(vars.alertId).toBe(''); expect(vars.status).toBe(''); expect(vars.note).toBe(''); + expect(vars.thresholdMax).toBeUndefined(); expect(vars.startTimeISO).toBe(new Date(0).toISOString()); }); }); + +// The guard published in docs/alert-webhook-template-variables.md is the only +// way a receiver can put an optional number in an unquoted JSON slot, so a +// change to the helper set that breaks it would break every template using it. +describe('the documented guard for an optional numeric variable', () => { + const render = (thresholdMax?: number) => + createHandlebarsWithHelpers().compile( + '{"threshold": {{threshold}}{{#unless (eq thresholdMax undefined)}}, "threshold_max": {{thresholdMax}}{{/unless}}\n}', + )( + buildWebhookTemplateVariables({ ...message, threshold: 5, thresholdMax }), + ); + + it.each([ + [undefined, { threshold: 5 }], + [20, { threshold: 5, threshold_max: 20 }], + // A range bounded at zero is real, and `{{#if}}` would drop it. + [0, { threshold: 5, threshold_max: 0 }], + ])('renders valid JSON for thresholdMax=%s', (thresholdMax, expected) => { + expect(JSON.parse(render(thresholdMax))).toEqual(expected); + }); +}); diff --git a/packages/api/src/tasks/checkAlerts/transports/generic.ts b/packages/api/src/tasks/checkAlerts/transports/generic.ts index 1d82ce7713..75763eb467 100644 --- a/packages/api/src/tasks/checkAlerts/transports/generic.ts +++ b/packages/api/src/tasks/checkAlerts/transports/generic.ts @@ -100,6 +100,7 @@ export const buildWebhookTemplateVariables = (message: Message) => ({ status: escapeJsonString(message.status ?? ''), teamId: escapeJsonString(message.teamId ?? ''), threshold: message.threshold, + thresholdMax: message.thresholdMax, value: message.value, }); diff --git a/packages/api/src/tasks/checkAlerts/transports/types.ts b/packages/api/src/tasks/checkAlerts/transports/types.ts index 4fc161ef4c..5c12bd7739 100644 --- a/packages/api/src/tasks/checkAlerts/transports/types.ts +++ b/packages/api/src/tasks/checkAlerts/transports/types.ts @@ -16,9 +16,11 @@ export interface Message { // Optional so existing callers and non-enriched templates are unaffected. alertId?: string; status?: string; // firing | resolved | no_data | pending - alertType?: string; // search | dashboard_chart + alertType?: string; // search | dashboard_chart | inline_query comparator?: string; // >=, >, <=, <, =, !=, between, outside threshold?: number; + // Upper bound of a `between`/`outside` condition; unset for the others. + thresholdMax?: number; value?: number; // the value that triggered/resolved the alert groupKey?: string; sourceQuery?: string; // the search expr / SQL that defines the alert From e63337bf4199ad407e23ea2613c2b76c79d9009f Mon Sep 17 00:00:00 2001 From: Jordan Simonovski Date: Thu, 3 Sep 2026 09:42:37 +1000 Subject: [PATCH 2/5] fix(api): report a tile alert's query in {{sourceQuery}} too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tile's config is already on the render view — fireChannelEvent puts the whole dashboard there, and the title builder reads the tile's name off it — so returning an empty string for the most common chart-alert type was wrong, as was the comment claiming the render path doesn't load it. The per-config-kind query lookup is now shared with the inline path, and covers PromQL as well: a tile config can be any of the three kinds, where an inline alert's cannot be PromQL. --- ...webhook-template-range-and-inline-query.md | 7 +- docs/alert-webhook-template-variables.md | 14 ++-- packages/api/src/fixtures.ts | 4 +- .../__tests__/renderAlertTemplate.int.test.ts | 73 ++++++++++++------- .../api/src/tasks/checkAlerts/template.ts | 42 +++++++---- 5 files changed, 91 insertions(+), 49 deletions(-) diff --git a/.changeset/webhook-template-range-and-inline-query.md b/.changeset/webhook-template-range-and-inline-query.md index fcb8cdb561..6696f1cbbe 100644 --- a/.changeset/webhook-template-range-and-inline-query.md +++ b/.changeset/webhook-template-range-and-inline-query.md @@ -5,9 +5,10 @@ Fill in two gaps in the Generic and incident.io webhook template variables. A `between` or `outside` alert now carries its upper bound as `{{thresholdMax}}`, so a receiver can reconstruct the condition instead of seeing one bound of two. -An inline-query alert now reports its own builder `where` expression or raw SQL -in `{{sourceQuery}}`, which was previously always empty because only a saved -search's query was read. +And `{{sourceQuery}}` now reports the query for every alert source: it read +only a saved search's `where` expression, leaving dashboard-tile and +inline-query alerts empty. Raw SQL and PromQL charts report their SQL template +and PromQL expression respectively. `{{thresholdMax}}` is gated on the comparator rather than read straight off the alert: switching an alert off a range comparator leaves the old bound on the diff --git a/docs/alert-webhook-template-variables.md b/docs/alert-webhook-template-variables.md index bf98326cbe..f055e27a44 100644 --- a/docs/alert-webhook-template-variables.md +++ b/docs/alert-webhook-template-variables.md @@ -21,7 +21,7 @@ a HyperDX alert without parsing the human-readable message body. | `{{thresholdMax}}` | number | Upper bound of a `between`/`outside` condition. Always unset for every other comparator, even if the alert was once a range, so guard it (see below). | | `{{value}}` | number | The value that triggered or resolved the alert. | | `{{groupKey}}` | string | The breaching group, for a grouped alert. | -| `{{sourceQuery}}` | string | The search expression or SQL behind the alert. | +| `{{sourceQuery}}` | string | The query behind the alert — search expression, SQL or PromQL (see below). | | `{{teamId}}` | string | Team the alert belongs to. | | `{{note}}` | string | The alert's freeform note — commonly a runbook link. | @@ -29,10 +29,14 @@ Strings are JSON-escaped, so they are safe to drop into a quoted slot. Numbers (`startTime`, `endTime`, `threshold`, `thresholdMax`, `value`) are emitted raw for unquoted slots. Every enriched variable is optional and renders as an empty string when the alert doesn't carry it — an alert with no group has -an empty `{{groupKey}}`, and a dashboard-tile alert has an empty -`{{sourceQuery}}` because its query lives on the tile. A saved-search alert -reports the search's `where` expression; an inline alert reports its own -builder `where` or raw SQL. +an empty `{{groupKey}}`, for instance. + +`{{sourceQuery}}` reads whichever field the alert's query lives in: a +saved-search alert reports the search's `where` expression, a dashboard-tile +alert its tile's, and an inline alert its own. That is a Lucene or SQL search +expression for a builder chart, the SQL template for a raw SQL chart, and the +PromQL expression for a PromQL chart. Nothing in the variable set says which of +those you got, so treat it as opaque text rather than something to parse. An empty string is not valid JSON in an unquoted numeric slot, so guard any number that may be absent. Compare against `undefined` rather than using diff --git a/packages/api/src/fixtures.ts b/packages/api/src/fixtures.ts index 20c15cb1ca..1406586077 100644 --- a/packages/api/src/fixtures.ts +++ b/packages/api/src/fixtures.ts @@ -677,6 +677,7 @@ export const makeTile = (opts?: { id?: string; alert?: BuilderSavedChartConfig['alert']; sourceId?: string; + where?: string; }): Tile => ({ id: opts?.id ?? randomMongoId(), x: 1, @@ -690,6 +691,7 @@ export const makeChartConfig = (opts?: { id?: string; alert?: BuilderSavedChartConfig['alert']; sourceId?: string; + where?: string; }): SavedChartConfig => ({ name: 'Test Chart', source: opts?.sourceId ?? 'test-source', @@ -702,7 +704,7 @@ export const makeChartConfig = (opts?: { valueExpression: '', }, ], - where: '', + where: opts?.where ?? '', whereLanguage: 'lucene', granularity: 'auto', implicitColumnExpression: 'Body', diff --git a/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts b/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts index 0c89a33f57..caee57282f 100644 --- a/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts +++ b/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts @@ -2,12 +2,14 @@ import { AlertState, AlertThresholdType, SourceKind, + Tile, } from '@hyperdx/common-utils/dist/types'; import mongoose from 'mongoose'; import { makeAlertChartConfig, makeRawSqlAlertChartConfig, + makeRawSqlAlertTile, makeTile, RAW_SQL_ALERT_TEMPLATE, } from '@/fixtures'; @@ -89,14 +91,21 @@ const mockClickhouseClient = { const startTime = new Date('2023-03-17T22:10:00.000Z'); const endTime = new Date('2023-03-17T22:15:00.000Z'); +// Shared by the three view builders below, so a knob added for one is +// accepted by renderWithWebhook's `makeView` parameter for all of them. +type ViewOverrides = Partial & { + thresholdType?: AlertThresholdType; + threshold?: number; + thresholdMax?: number; + value?: number; + group?: string; + where?: string; + rawSql?: boolean; + tile?: Tile; +}; + const makeSearchView = ( - overrides: Partial & { - thresholdType?: AlertThresholdType; - threshold?: number; - thresholdMax?: number; - value?: number; - group?: string; - } = {}, + overrides: ViewOverrides = {}, ): AlertMessageTemplateDefaultView => ({ alert: { thresholdType: overrides.thresholdType ?? AlertThresholdType.ABOVE, @@ -141,13 +150,7 @@ const makeSearchView = ( const testTile = makeTile({ id: 'test-tile-id' }); const makeTileView = ( - overrides: Partial & { - thresholdType?: AlertThresholdType; - threshold?: number; - thresholdMax?: number; - value?: number; - group?: string; - } = {}, + overrides: ViewOverrides = {}, ): AlertMessageTemplateDefaultView => ({ alert: { thresholdType: overrides.thresholdType ?? AlertThresholdType.ABOVE, @@ -156,13 +159,13 @@ const makeTileView = ( source: AlertSource.TILE, channel: { type: null }, interval: '1m', - tileId: 'test-tile-id', + tileId: (overrides.tile ?? testTile).id, }, dashboard: { _id: new mongoose.Types.ObjectId(), id: 'id-123', name: 'My Dashboard', - tiles: [testTile], + tiles: [overrides.tile ?? testTile], team: 'team-123' as any, tags: ['test'], createdAt: new Date(), @@ -178,15 +181,7 @@ const makeTileView = ( }); const makeInlineView = ( - overrides: Partial & { - thresholdType?: AlertThresholdType; - threshold?: number; - thresholdMax?: number; - value?: number; - group?: string; - where?: string; - rawSql?: boolean; - } = {}, + overrides: ViewOverrides = {}, ): AlertMessageTemplateDefaultView => ({ alert: { thresholdType: overrides.thresholdType ?? AlertThresholdType.ABOVE, @@ -211,8 +206,6 @@ const makeInlineView = ( value: overrides.value ?? 10, }); -type ViewOverrides = Parameters[0]; - const render = async ( view: AlertMessageTemplateDefaultView, state: AlertState, @@ -589,6 +582,32 @@ describe('enriched message fields', () => { sourceQuery: RAW_SQL_ALERT_TEMPLATE, }); }); + + it("reports a tile alert's query from its tile", async () => { + const { dispatched } = await renderWithWebhook( + AlertState.ALERT, + { tile: makeTile({ id: 'tile-1', where: 'SeverityText: "error"' }) }, + makeTileView, + ); + + expect(dispatched[0].message).toMatchObject({ + alertType: 'dashboard_chart', + sourceQuery: 'SeverityText: "error"', + }); + }); + + it('reports the SQL of a raw SQL tile alert', async () => { + const { dispatched } = await renderWithWebhook( + AlertState.ALERT, + { tile: makeRawSqlAlertTile({ id: 'tile-1' }) }, + makeTileView, + ); + + expect(dispatched[0].message).toMatchObject({ + alertType: 'dashboard_chart', + sourceQuery: RAW_SQL_ALERT_TEMPLATE, + }); + }); }); describe('buildAlertMessageTemplateTitle', () => { diff --git a/packages/api/src/tasks/checkAlerts/template.ts b/packages/api/src/tasks/checkAlerts/template.ts index 17cc034523..3ee072e953 100644 --- a/packages/api/src/tasks/checkAlerts/template.ts +++ b/packages/api/src/tasks/checkAlerts/template.ts @@ -4,11 +4,13 @@ import { renderChartConfig } from '@hyperdx/common-utils/dist/core/renderChartCo import { formatDate, objectHash } from '@hyperdx/common-utils/dist/core/utils'; import { AlertChannelType, + AlertChartConfig, AlertThresholdType, ChartConfigWithOptDateRange, DisplayType, isRangeThresholdType, pickSampleWeightExpressionProps, + SavedChartConfig, SourceKind, zAlertChannelType, } from '@hyperdx/common-utils/dist/types'; @@ -105,23 +107,37 @@ const describeThreshold = (alert: AlertInput): string => { : `${alert.threshold}`; }; -// An inline alert keeps its query on the alert itself rather than behind a -// saved search, so reading only `savedSearch.where` leaves `{{sourceQuery}}` -// empty for it. A tile alert stays empty: its query lives on the dashboard -// tile, which the render path doesn't load. +// Each chart config kind states its query in a different field. Discriminated +// on `configType` the same way isRawSqlSavedChartConfig does, inlined because +// that guard narrows to a SavedChartConfig member and so cannot narrow the +// builder branch of an AlertChartConfig. +const describeChartConfigQuery = ( + config: AlertChartConfig | SavedChartConfig, +): string => { + if (!('configType' in config)) { + return config.where ?? ''; + } + return ( + (config.configType === 'sql' + ? config.sqlTemplate + : config.promqlExpression) ?? '' + ); +}; + +// Only a saved-search alert states its query as `savedSearch.where`. A tile +// alert's lives on the dashboard tile and an inline alert's on the alert +// itself, so both were reported as empty before. const describeSourceQuery = ( alert: AlertInput, savedSearch?: ISavedSearch | null, + dashboard?: IDashboard | null, ): string => { if (alert.source === AlertSource.INLINE) { - const config = alert.chartConfig; - if (config == null) { - return ''; - } - // Same discriminator as isRawSqlSavedChartConfig, inlined because that - // guard narrows to a SavedChartConfig member and so cannot narrow this - // union's builder branch. - return ('configType' in config ? config.sqlTemplate : config.where) ?? ''; + return alert.chartConfig ? describeChartConfigQuery(alert.chartConfig) : ''; + } + if (alert.source === AlertSource.TILE) { + const tile = dashboard?.tiles.find(t => t.id === alert.tileId); + return tile ? describeChartConfigQuery(tile.config) : ''; } return savedSearch?.where ?? ''; }; @@ -586,7 +602,7 @@ export const renderAlertTemplate = async ({ : undefined, value, groupKey: group ?? '', - sourceQuery: describeSourceQuery(alert, savedSearch), + sourceQuery: describeSourceQuery(alert, savedSearch, dashboard), teamId, note: alert.note ?? '', }, From 550e43be8437ab4d0dc7b0d518caf1dcb5aa0a52 Mon Sep 17 00:00:00 2001 From: Jordan Simonovski Date: Thu, 3 Sep 2026 10:33:19 +1000 Subject: [PATCH 3/5] fix(api): clear a stale thresholdMax at the write path, and address review nits The reader-side gate only stopped the webhook variable reporting a bound the alert no longer has -- the alerts REST and external APIs served the same stale value. updateAlert now $unsets the path for a non-range comparator. It clears rather than writing null because the create and update responses return the document directly, so a null would surface there and a client round-tripping one back into an update would fail input validation. Also: describeChartConfigQuery uses the shared chart-config guards instead of a hand-rolled configType check, makeRawSqlAlertTile delegates to makeRawSqlAlertChartConfig rather than restating the literal, and the "Send test" message carries sample enriched values -- without them {{threshold}} and {{value}} render empty and a template that is correct for real alerts produces invalid JSON only on a test send. --- ...webhook-template-range-and-inline-query.md | 5 +++ docs/alert-webhook-template-variables.md | 4 +++ packages/api/src/controllers/alerts.ts | 18 ++++++++-- packages/api/src/fixtures.ts | 33 ++++++++--------- .../routers/api/__tests__/alerts.int.test.ts | 36 +++++++++++++++++++ packages/api/src/routers/api/webhooks.ts | 17 ++++++++- .../api/src/tasks/checkAlerts/template.ts | 30 ++++++++-------- 7 files changed, 106 insertions(+), 37 deletions(-) diff --git a/.changeset/webhook-template-range-and-inline-query.md b/.changeset/webhook-template-range-and-inline-query.md index 6696f1cbbe..b74e45520b 100644 --- a/.changeset/webhook-template-range-and-inline-query.md +++ b/.changeset/webhook-template-range-and-inline-query.md @@ -13,3 +13,8 @@ and PromQL expression respectively. `{{thresholdMax}}` is gated on the comparator rather than read straight off the alert: switching an alert off a range comparator leaves the old bound on the document, and reporting it would advertise a range that no longer fires. + +Editing an alert off a range comparator now clears the stored `thresholdMax` +instead of leaving the old bound on the document, where it was also served by +the alerts APIs. The "Send test" payload carries sample values for the enriched +variables, so a body template using them can be checked before an alert fires. diff --git a/docs/alert-webhook-template-variables.md b/docs/alert-webhook-template-variables.md index f055e27a44..830c695de3 100644 --- a/docs/alert-webhook-template-variables.md +++ b/docs/alert-webhook-template-variables.md @@ -52,6 +52,10 @@ number that may be absent. Compare against `undefined` rather than using Keep a newline or space between `{{/unless}}` and a closing `}` — Handlebars reads `}}}` as a triple-stache and fails to compile the template. +The **Send test** button on the webhook form fills these variables with sample +values, so a template that uses them can be checked before an alert fires. The +sample is a `>=` alert, so `{{thresholdMax}}` is absent there. + ## Example Routing by severity and deduping on the alert rather than the firing: diff --git a/packages/api/src/controllers/alerts.ts b/packages/api/src/controllers/alerts.ts index feba0444f9..f2cd15cecd 100644 --- a/packages/api/src/controllers/alerts.ts +++ b/packages/api/src/controllers/alerts.ts @@ -4,6 +4,7 @@ import { validateRawSqlForAlert, } from '@hyperdx/common-utils/dist/core/utils'; import { isRawSqlSavedChartConfig } from '@hyperdx/common-utils/dist/guards'; +import { isRangeThresholdType } from '@hyperdx/common-utils/dist/types'; import { groupBy } from 'lodash'; import { Types } from 'mongoose'; import { z } from 'zod'; @@ -253,7 +254,13 @@ export const makeAlert = ( }), source: alert.source, threshold: alert.threshold, - thresholdMax: alert.thresholdMax, + // Omitted rather than set to undefined for a non-range comparator, so + // updateAlert can $unset the path. Writing null instead would surface on + // the create/update responses, which return the document directly, and a + // client round-tripping one back into an update would fail validation. + ...(isRangeThresholdType(alert.thresholdType) && { + thresholdMax: alert.thresholdMax, + }), thresholdType: alert.thresholdType, ...(userId && { createdBy: userId }), @@ -301,12 +308,19 @@ export const updateAlert = async ( alertInput: AlertInput, ) => { // should consider clearing AlertHistory when updating an alert? + // Mongoose drops an undefined key from an update rather than clearing the + // path, so an alert edited off `between` would keep its old upper bound and + // report a range condition it no longer has. $unset clears it. + const isRange = isRangeThresholdType(alertInput.thresholdType); return Alert.findOneAndUpdate( { _id: id, team: teamId, }, - makeAlert(alertInput), + { + $set: makeAlert(alertInput), + ...(!isRange && { $unset: { thresholdMax: 1 } }), + }, { returnDocument: 'after', }, diff --git a/packages/api/src/fixtures.ts b/packages/api/src/fixtures.ts index 1406586077..abb55f388b 100644 --- a/packages/api/src/fixtures.ts +++ b/packages/api/src/fixtures.ts @@ -784,6 +784,19 @@ export const RAW_SQL_ALERT_TEMPLATE = [ ' GROUP BY ts ORDER BY ts', ].join(''); +/** Raw SQL counterpart to {@link makeAlertChartConfig}. */ +export const makeRawSqlAlertChartConfig = (opts?: { + name?: string; + sqlTemplate?: string; + connectionId?: string; +}): AlertChartConfig => ({ + name: opts?.name ?? 'Raw SQL Alert Query', + configType: 'sql', + displayType: DisplayType.Line, + sqlTemplate: opts?.sqlTemplate ?? RAW_SQL_ALERT_TEMPLATE, + connection: opts?.connectionId ?? 'test-connection', +}); + export const makeRawSqlAlertTile = (opts?: { id?: string; connectionId?: string; @@ -794,12 +807,7 @@ export const makeRawSqlAlertTile = (opts?: { y: 1, w: 1, h: 1, - config: { - configType: 'sql', - displayType: DisplayType.Line, - sqlTemplate: opts?.sqlTemplate ?? RAW_SQL_ALERT_TEMPLATE, - connection: opts?.connectionId ?? 'test-connection', - } satisfies RawSqlSavedChartConfig, + config: makeRawSqlAlertChartConfig(opts), }); export const RAW_SQL_NUMBER_ALERT_TEMPLATE = [ @@ -898,19 +906,6 @@ export const makeAlertChartConfig = (opts: { ...(opts.groupBy != null && { groupBy: opts.groupBy }), }); -/** Raw SQL counterpart to {@link makeAlertChartConfig}. */ -export const makeRawSqlAlertChartConfig = (opts?: { - name?: string; - sqlTemplate?: string; - connectionId?: string; -}): AlertChartConfig => ({ - name: opts?.name ?? 'Raw SQL Alert Query', - configType: 'sql', - displayType: DisplayType.Line, - sqlTemplate: opts?.sqlTemplate ?? RAW_SQL_ALERT_TEMPLATE, - connection: opts?.connectionId ?? 'test-connection', -}); - export const makeInlineAlertInput = ({ chartConfig, interval = '15m', 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 acd05b60f8..5cd0c086ee 100644 --- a/packages/api/src/routers/api/__tests__/alerts.int.test.ts +++ b/packages/api/src/routers/api/__tests__/alerts.int.test.ts @@ -135,6 +135,42 @@ describe('alerts router', () => { expect(allAlerts.body.data[0].threshold).toBe(10); }); + it('clears thresholdMax when an alert is moved off a range comparator', async () => { + const dashboard = await agent + .post('/dashboards') + .send(MOCK_DASHBOARD) + .expect(200); + const alert = await agent + .post('/alerts') + .send({ + ...makeAlertInput({ + dashboardId: dashboard.body.id, + tileId: MOCK_TILES[0].id, + webhookId: webhook._id.toString(), + }), + thresholdType: AlertThresholdType.BETWEEN, + threshold: 5, + thresholdMax: 20, + }) + .expect(200); + expect(alert.body.data.thresholdMax).toBe(20); + + await agent + .put(`/alerts/${alert.body.data._id}`) + .send({ + ...alert.body.data, + dashboardId: dashboard.body.id, + thresholdType: AlertThresholdType.ABOVE, + thresholdMax: undefined, + }) + .expect(200); + + const updated = await agent + .get(`/alerts/${alert.body.data._id}`) + .expect(200); + expect(updated.body.data.thresholdMax).toBeUndefined(); + }); + it('returns channel.webhookId, name, and message in GET list and GET single', async () => { const dashboard = await agent .post('/dashboards') diff --git a/packages/api/src/routers/api/webhooks.ts b/packages/api/src/routers/api/webhooks.ts index aeab7c793f..00463b26bb 100644 --- a/packages/api/src/routers/api/webhooks.ts +++ b/packages/api/src/routers/api/webhooks.ts @@ -459,7 +459,12 @@ router.post( body, }); - // Send test message + // Send test message. The enriched fields carry sample values so a body + // template that uses them renders the same shape here as it will on a + // real alert -- left unset, `{{threshold}}` and friends render empty and + // a correct template produces invalid JSON only on the test send. + // Deliberately a `>=` alert, so `thresholdMax` is absent the way it is + // for any non-range comparator. const testMessage = { hdxLink: 'https://hyperdx.io', title: 'Test Webhook from HyperDX', @@ -468,6 +473,16 @@ router.post( endTime: Date.now(), state: AlertState.INSUFFICIENT_DATA, eventId: 'test-event-id', + alertId: 'test-alert-id', + status: 'no_data', + alertType: 'search', + comparator: '>=', + threshold: 10, + value: 42, + groupKey: '', + sourceQuery: 'level:error', + teamId: teamId.toString(), + note: '', }; const testChannel = { type: 'webhook' as const, channel: testWebhook }; diff --git a/packages/api/src/tasks/checkAlerts/template.ts b/packages/api/src/tasks/checkAlerts/template.ts index 3ee072e953..024c7bc763 100644 --- a/packages/api/src/tasks/checkAlerts/template.ts +++ b/packages/api/src/tasks/checkAlerts/template.ts @@ -2,9 +2,12 @@ import { ClickhouseClient } from '@hyperdx/common-utils/dist/clickhouse/node'; import { Metadata } from '@hyperdx/common-utils/dist/core/metadata'; import { renderChartConfig } from '@hyperdx/common-utils/dist/core/renderChartConfig'; import { formatDate, objectHash } from '@hyperdx/common-utils/dist/core/utils'; +import { + isPromqlSavedChartConfig, + isRawSqlSavedChartConfig, +} from '@hyperdx/common-utils/dist/guards'; import { AlertChannelType, - AlertChartConfig, AlertThresholdType, ChartConfigWithOptDateRange, DisplayType, @@ -107,21 +110,18 @@ const describeThreshold = (alert: AlertInput): string => { : `${alert.threshold}`; }; -// Each chart config kind states its query in a different field. Discriminated -// on `configType` the same way isRawSqlSavedChartConfig does, inlined because -// that guard narrows to a SavedChartConfig member and so cannot narrow the -// builder branch of an AlertChartConfig. -const describeChartConfigQuery = ( - config: AlertChartConfig | SavedChartConfig, -): string => { - if (!('configType' in config)) { - return config.where ?? ''; +// Each chart config kind states its query in a different field. Typed as +// SavedChartConfig so the shared guards narrow: an AlertChartConfig is +// assignable to it (its members are the tile config types minus the embedded +// alert field), the same way buildAlertChartConfigFromSavedConfig takes one. +const describeChartConfigQuery = (config: SavedChartConfig): string => { + if (isRawSqlSavedChartConfig(config)) { + return config.sqlTemplate ?? ''; } - return ( - (config.configType === 'sql' - ? config.sqlTemplate - : config.promqlExpression) ?? '' - ); + if (isPromqlSavedChartConfig(config)) { + return config.promqlExpression ?? ''; + } + return config.where ?? ''; }; // Only a saved-search alert states its query as `savedSearch.where`. A tile From c52e4418fb50e88792c4c404e75c3fbbe4170597 Mon Sep 17 00:00:00 2001 From: Jordan Simonovski Date: Thu, 3 Sep 2026 11:36:41 +1000 Subject: [PATCH 4/5] fix(api): clear a stale thresholdMax on the dashboard save path too updateAlert was not the only update path through makeAlert: createOrUpdateDashboardAlerts writes tile alerts saved from the dashboard editor, and it kept the old bound because makeAlert now omits the field rather than writing it. The tile editor only ever sets thresholdMax, so switching a tile alert off `between` left the stale value on the document and in the alerts APIs. Both callers now build the update through makeAlertUpdate, so a third one can't miss the clear. $unset is ignored on an upsert insert, which is what the dashboard path relies on. --- packages/api/src/controllers/alerts.ts | 29 +++++++----- .../api/__tests__/dashboard.int.test.ts | 45 +++++++++++++++++++ 2 files changed, 62 insertions(+), 12 deletions(-) diff --git a/packages/api/src/controllers/alerts.ts b/packages/api/src/controllers/alerts.ts index f2cd15cecd..378cc81a76 100644 --- a/packages/api/src/controllers/alerts.ts +++ b/packages/api/src/controllers/alerts.ts @@ -290,6 +290,18 @@ export const makeAlert = ( }; }; +// makeAlert omits thresholdMax for a non-range comparator, and Mongoose drops +// an omitted key from an update rather than clearing the path, so the bound has +// to be unset explicitly or an alert edited off `between` keeps a stale one and +// reports a range condition it no longer has. Every update path needs this; +// $unset is ignored on an upsert insert, so it is safe there too. +const makeAlertUpdate = (alertInput: AlertInput, userId?: ObjectId) => ({ + $set: makeAlert(alertInput, userId), + ...(!isRangeThresholdType(alertInput.thresholdType) && { + $unset: { thresholdMax: 1 }, + }), +}); + export const createAlert = async ( teamId: ObjectId, alertInput: z.infer, @@ -308,19 +320,12 @@ export const updateAlert = async ( alertInput: AlertInput, ) => { // should consider clearing AlertHistory when updating an alert? - // Mongoose drops an undefined key from an update rather than clearing the - // path, so an alert edited off `between` would keep its old upper bound and - // report a range condition it no longer has. $unset clears it. - const isRange = isRangeThresholdType(alertInput.thresholdType); return Alert.findOneAndUpdate( { _id: id, team: teamId, }, - { - $set: makeAlert(alertInput), - ...(!isRange && { $unset: { thresholdMax: 1 } }), - }, + makeAlertUpdate(alertInput), { returnDocument: 'after', }, @@ -396,12 +401,12 @@ export const createOrUpdateDashboardAlerts = async ( tileId, }; const oldAlert = await Alert.findOne(filter); - const alertValues = + const alertUpdate = oldAlert && oldAlert.createdBy - ? makeAlert(alertInput) - : makeAlert(alertInput, userId); + ? makeAlertUpdate(alertInput) + : makeAlertUpdate(alertInput, userId); - return await Alert.findOneAndUpdate(filter, alertValues, { + return await Alert.findOneAndUpdate(filter, alertUpdate, { new: true, upsert: true, }); diff --git a/packages/api/src/routers/api/__tests__/dashboard.int.test.ts b/packages/api/src/routers/api/__tests__/dashboard.int.test.ts index 527b38dfc3..9a68528de8 100644 --- a/packages/api/src/routers/api/__tests__/dashboard.int.test.ts +++ b/packages/api/src/routers/api/__tests__/dashboard.int.test.ts @@ -303,6 +303,51 @@ describe('dashboard router', () => { }); }); + // The dashboard editor only ever sets thresholdMax, so switching a tile alert + // off a range comparator has to clear the stored bound on this path too. + it('clears thresholdMax when a tile alert is moved off a range comparator', async () => { + const mockAlert = makeMockAlert(webhook._id.toString()); + const tile = makeTile({ + alert: { + ...mockAlert, + thresholdType: AlertThresholdType.BETWEEN, + threshold: 5, + thresholdMax: 20, + }, + }); + const dashboard = await agent + .post('/dashboards') + .send({ name: 'Test Dashboard', tiles: [tile], tags: [] }) + .expect(200); + + const created = await agent.get(`/alerts`).expect(200); + expect(created.body.data[0].thresholdMax).toBe(20); + + await agent + .patch(`/dashboards/${dashboard.body.id}`) + .send({ + ...dashboard.body, + tiles: [ + { + ...dashboard.body.tiles[0], + config: { + ...dashboard.body.tiles[0].config, + alert: { + ...mockAlert, + thresholdType: AlertThresholdType.ABOVE, + threshold: 5, + }, + }, + }, + ], + }) + .expect(200); + + const updated = await agent.get(`/alerts`).expect(200); + expect(updated.body.data[0].thresholdType).toBe(AlertThresholdType.ABOVE); + expect(updated.body.data[0].thresholdMax).toBeUndefined(); + }); + it('rejects a tile alert with no notification channel', async () => { await agent .post('/dashboards') From ae1d4b4250ad9f78a90f46872953b795e920ed4d Mon Sep 17 00:00:00 2001 From: Jordan Simonovski Date: Thu, 3 Sep 2026 11:42:23 +1000 Subject: [PATCH 5/5] fix: publish the webhook template variable list from one place The webhook form's "body supports the following variables" callout reused the array that builds the default body template, so it advertised only the original seven and told users {{thresholdMax}} and the rest of the enriched set did not exist. Appending to that array would have bloated the default body for every new webhook, so the two lists are now separate and both derive from common-utils. buildWebhookTemplateVariables is typed `satisfies Record< WebhookTemplateVariable, ...>`, so a name added to the published list must be built and a variable built here must be published -- checked by adding a canary entry and confirming the API fails to compile. The API's fallback body template derives from the same default list, retiring a third copy. --- ...webhook-template-range-and-inline-query.md | 8 +++ .../tasks/checkAlerts/transports/generic.ts | 59 +++++++++++-------- .../components/TeamSettings/WebhookForm.tsx | 31 +++++----- packages/common-utils/src/types.ts | 39 ++++++++++++ 4 files changed, 97 insertions(+), 40 deletions(-) diff --git a/.changeset/webhook-template-range-and-inline-query.md b/.changeset/webhook-template-range-and-inline-query.md index b74e45520b..3b2c843a33 100644 --- a/.changeset/webhook-template-range-and-inline-query.md +++ b/.changeset/webhook-template-range-and-inline-query.md @@ -1,5 +1,7 @@ --- '@hyperdx/api': minor +'@hyperdx/app': minor +'@hyperdx/common-utils': minor --- Fill in two gaps in the Generic and incident.io webhook template variables. A @@ -18,3 +20,9 @@ Editing an alert off a range comparator now clears the stored `thresholdMax` instead of leaving the old bound on the document, where it was also served by the alerts APIs. The "Send test" payload carries sample values for the enriched variables, so a body template using them can be checked before an alert fires. + +The webhook form's "body supports the following variables" list now advertises +the full set rather than the original seven. It and the API's fallback body +template both derive from one list in common-utils, which +`buildWebhookTemplateVariables` is typed against, so a variable cannot be added +without appearing in both places. diff --git a/packages/api/src/tasks/checkAlerts/transports/generic.ts b/packages/api/src/tasks/checkAlerts/transports/generic.ts index 75763eb467..4a313c343c 100644 --- a/packages/api/src/tasks/checkAlerts/transports/generic.ts +++ b/packages/api/src/tasks/checkAlerts/transports/generic.ts @@ -1,5 +1,9 @@ import { objectHash } from '@hyperdx/common-utils/dist/core/utils'; -import { WebhookService } from '@hyperdx/common-utils/dist/types'; +import { + DEFAULT_WEBHOOK_TEMPLATE_VARIABLES, + WebhookService, + WebhookTemplateVariable, +} from '@hyperdx/common-utils/dist/types'; import Handlebars from 'handlebars'; import { performance } from 'perf_hooks'; import { serializeError } from 'serialize-error'; @@ -63,8 +67,9 @@ export const logBlockedWebhookDelivery = ( // the default template the UI form applies (WebhookForm.tsx) so a webhook // created via the API/MCP (where body is optional) still fires with a sensible // payload instead of crashing Handlebars.compile on an undefined template. -const DEFAULT_GENERIC_WEBHOOK_BODY_TEMPLATE = - '{"text": "{{title}} | {{body}} | {{link}} | {{state}} | {{startTime}} | {{endTime}} | {{eventId}}"}'; +const DEFAULT_GENERIC_WEBHOOK_BODY_TEMPLATE = `{"text": "${DEFAULT_WEBHOOK_TEMPLATE_VARIABLES.map( + v => `{{${v}}}`, +).join(' | ')}"}`; // Renders a Unix-ms timestamp as an ISO-8601 string, or '' if it isn't a valid // time (so a template slot never becomes the literal "Invalid Date"). @@ -81,28 +86,32 @@ const toIsoTimestamp = (t: number): string => { * comparator, sourceQuery, …) enable routing/dedup in receivers without * parsing the message body. */ -export const buildWebhookTemplateVariables = (message: Message) => ({ - body: escapeJsonString(message.body), - endTime: message.endTime, - endTimeISO: escapeJsonString(toIsoTimestamp(message.endTime)), - eventId: message.eventId, - link: escapeJsonString(message.hdxLink), - startTime: message.startTime, - startTimeISO: escapeJsonString(toIsoTimestamp(message.startTime)), - state: message.state, - title: escapeJsonString(message.title), - alertId: escapeJsonString(message.alertId ?? ''), - alertType: escapeJsonString(message.alertType ?? ''), - comparator: escapeJsonString(message.comparator ?? ''), - groupKey: escapeJsonString(message.groupKey ?? ''), - note: escapeJsonString(message.note ?? ''), - sourceQuery: escapeJsonString(message.sourceQuery ?? ''), - status: escapeJsonString(message.status ?? ''), - teamId: escapeJsonString(message.teamId ?? ''), - threshold: message.threshold, - thresholdMax: message.thresholdMax, - value: message.value, -}); +// `satisfies` rather than a return type, so callers keep the precise value +// types while the published variable list stays enforced in both directions: a +// name added there must be built here, and one built here must be published. +export const buildWebhookTemplateVariables = (message: Message) => + ({ + body: escapeJsonString(message.body), + endTime: message.endTime, + endTimeISO: escapeJsonString(toIsoTimestamp(message.endTime)), + eventId: message.eventId, + link: escapeJsonString(message.hdxLink), + startTime: message.startTime, + startTimeISO: escapeJsonString(toIsoTimestamp(message.startTime)), + state: message.state, + title: escapeJsonString(message.title), + alertId: escapeJsonString(message.alertId ?? ''), + alertType: escapeJsonString(message.alertType ?? ''), + comparator: escapeJsonString(message.comparator ?? ''), + groupKey: escapeJsonString(message.groupKey ?? ''), + note: escapeJsonString(message.note ?? ''), + sourceQuery: escapeJsonString(message.sourceQuery ?? ''), + status: escapeJsonString(message.status ?? ''), + teamId: escapeJsonString(message.teamId ?? ''), + threshold: message.threshold, + thresholdMax: message.thresholdMax, + value: message.value, + }) satisfies Record; /** * Creates a Handlebars instance with common helpers registered. diff --git a/packages/app/src/components/TeamSettings/WebhookForm.tsx b/packages/app/src/components/TeamSettings/WebhookForm.tsx index 4bc4d3886c..ffda5ab9ae 100644 --- a/packages/app/src/components/TeamSettings/WebhookForm.tsx +++ b/packages/app/src/components/TeamSettings/WebhookForm.tsx @@ -6,6 +6,8 @@ import { json, jsonParseLinter } from '@codemirror/lang-json'; import { linter } from '@codemirror/lint'; import { AlertState, + DEFAULT_WEBHOOK_TEMPLATE_VARIABLES, + WEBHOOK_TEMPLATE_VARIABLES, WebhookApiData, WebhookService, } from '@hyperdx/common-utils/dist/types'; @@ -30,17 +32,16 @@ import api from '@/api'; import { useBrandDisplayName } from '@/theme/ThemeProvider'; import { isValidUrl } from '@/utils'; -const DEFAULT_GENERIC_WEBHOOK_BODY = [ - '{{title}}', - '{{body}}', - '{{link}}', - '{{state}}', - '{{startTime}}', - '{{endTime}}', - '{{eventId}}', -]; -const DEFAULT_GENERIC_WEBHOOK_BODY_TEMPLATE = - DEFAULT_GENERIC_WEBHOOK_BODY.join(' | '); +// Both lists come from common-utils so this form, the API's fallback body and +// docs/alert-webhook-template-variables.md cannot drift: the default template +// uses the original seven, while the info callout advertises every variable a +// body template may reference. +const asHandlebars = (names: readonly string[]) => + names.map(name => `{{${name}}}`); +const DEFAULT_GENERIC_WEBHOOK_BODY_TEMPLATE = asHandlebars( + DEFAULT_WEBHOOK_TEMPLATE_VARIABLES, +).join(' | '); +const TEMPLATE_VARIABLES = asHandlebars(WEBHOOK_TEMPLATE_VARIABLES); const jsonLinterWithEmptyCheck = () => (editorView: EditorView) => { const text = editorView.state.doc.toString().trim(); @@ -407,10 +408,10 @@ export function WebhookForm({
- {DEFAULT_GENERIC_WEBHOOK_BODY.map((body, index) => ( - - {body} - {index < DEFAULT_GENERIC_WEBHOOK_BODY.length - 1 && ', '} + {TEMPLATE_VARIABLES.map((variable, index) => ( + + {variable} + {index < TEMPLATE_VARIABLES.length - 1 && ', '} ))} diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 69771a1a5a..1061ca9285 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -568,6 +568,45 @@ export enum WebhookService { IncidentIO = 'incidentio', } +/** + * Every variable a Generic/incident.io webhook body template can reference. + * The one published list: `buildWebhookTemplateVariables` (packages/api) is + * typed against it so a variable cannot be built without appearing here, and + * the webhook form renders it as the in-product list. Documented in + * docs/alert-webhook-template-variables.md. + * + * The first seven are also the default body template the form applies. + */ +export const DEFAULT_WEBHOOK_TEMPLATE_VARIABLES = [ + 'title', + 'body', + 'link', + 'state', + 'startTime', + 'endTime', + 'eventId', +] as const; + +export const WEBHOOK_TEMPLATE_VARIABLES = [ + ...DEFAULT_WEBHOOK_TEMPLATE_VARIABLES, + 'startTimeISO', + 'endTimeISO', + 'alertId', + 'status', + 'alertType', + 'comparator', + 'threshold', + 'thresholdMax', + 'value', + 'groupKey', + 'sourceQuery', + 'teamId', + 'note', +] as const; + +export type WebhookTemplateVariable = + (typeof WEBHOOK_TEMPLATE_VARIABLES)[number]; + /** * Base webhook schema (matches backend IWebhook but with JSON-serialized types). * When making changes here, consider if they need to be made to the external