diff --git a/.changeset/inline-alerts-external-api-mcp.md b/.changeset/inline-alerts-external-api-mcp.md new file mode 100644 index 0000000000..398b782c73 --- /dev/null +++ b/.changeset/inline-alerts-external-api-mcp.md @@ -0,0 +1,23 @@ +--- +'@hyperdx/api': minor +--- + +Support inline chart alerts (source `inline` + `chartConfig`) in the external +API v2 and the MCP `clickstack_save_alert` / `clickstack_get_alert` tools. +Inline alerts can now be created, updated, listed, and deleted through +`/api/v2/alerts` using the same tile-config dialect as v2 dashboards, with the +same validation rules as the internal API: display-type allowlist, metric +formula validation, the formula source-kind gate, raw SQL template validation, +and team-scoped source/connection ownership. Passing a `chartConfig` to a +`tile` or `saved_search` alert is now rejected instead of silently dropped, +and reading an inline alert whose config carries internal-only fields omits +`chartConfig` rather than returning a lossy approximation. + +Also fixes three pre-existing issues on the external v2 dashboards path: a +gauge tile saved through `clickstack_save_dashboard` with `isDelta: true` was +persisted as a non-delta (the flag was dropped converting MCP tiles to the +internal shape); a tile config carrying an unrecognized `configType` (e.g. +`promql`) was silently stored as a builder config and skipped the formula and +number-select rules, and is now rejected; and validation errors on alert +bodies no longer collapse to `Invalid input` — the schema reports the failing +branch's own message. diff --git a/packages/api/openapi.json b/packages/api/openapi.json index 6ceb379443..e24340448e 100644 --- a/packages/api/openapi.json +++ b/packages/api/openapi.json @@ -92,9 +92,75 @@ "type": "string", "enum": [ "saved_search", - "tile" + "tile", + "inline" ], - "description": "Alert source type." + "description": "Alert source type. \"saved_search\" alerts monitor a saved search, \"tile\" alerts monitor a dashboard tile, and \"inline\" alerts carry their own chart configuration (see AlertChartConfig) without requiring a saved search or dashboard.\n" + }, + "AlertChartConfigOverlay": { + "type": "object", + "description": "Fields an alert's chart config carries on top of the dashboard tile config dialect. Unlike a tile (whose name lives on the tile, not its config), an inline alert's config is standalone.\n", + "properties": { + "name": { + "type": "string", + "description": "Display name for the alert query. Used in notification titles and as an alert-name fallback when the alert itself has no name.\n", + "example": "Error Rate Query" + }, + "where": { + "type": "string", + "maxLength": 10000, + "description": "Chart-level filter applied on top of every select item's own \"where\" (combined via AND). Builder variants only; rejected on Raw SQL variants (filter inside the sqlTemplate instead).\n", + "example": "ServiceName:api" + }, + "whereLanguage": { + "$ref": "#/components/schemas/QueryLanguage", + "description": "Language of the chart-level \"where\" filter." + } + } + }, + "AlertLineChartConfig": { + "allOf": [ + { + "$ref": "#/components/schemas/LineChartConfig" + }, + { + "$ref": "#/components/schemas/AlertChartConfigOverlay" + } + ] + }, + "AlertBarChartConfig": { + "allOf": [ + { + "$ref": "#/components/schemas/BarChartConfig" + }, + { + "$ref": "#/components/schemas/AlertChartConfigOverlay" + } + ] + }, + "AlertNumberChartConfig": { + "allOf": [ + { + "$ref": "#/components/schemas/NumberChartConfig" + }, + { + "$ref": "#/components/schemas/AlertChartConfigOverlay" + } + ] + }, + "AlertChartConfig": { + "description": "The chart configuration an inline alert evaluates, in the same dialect as dashboard tile configs plus the alert-only fields in AlertChartConfigOverlay. Only the display types the alert evaluator supports are accepted: line, stacked_bar, and number, in both builder and Raw SQL (configType \"sql\") variants. Raw SQL templates must reference the evaluation window via the time-filter and interval macros (e.g. $__timeFilter and $__timeInterval), and the referenced source/connection must belong to the team.\n", + "oneOf": [ + { + "$ref": "#/components/schemas/AlertLineChartConfig" + }, + { + "$ref": "#/components/schemas/AlertBarChartConfig" + }, + { + "$ref": "#/components/schemas/AlertNumberChartConfig" + } + ] }, "AlertState": { "type": "string", @@ -240,6 +306,10 @@ "nullable": true, "example": "ServiceName" }, + "chartConfig": { + "$ref": "#/components/schemas/AlertChartConfig", + "description": "Chart configuration for inline alerts. Required when source is \"inline\" and rejected otherwise. Returned on single-alert responses (GET by ID, POST, PUT); the list endpoint omits it.\n" + }, "threshold": { "type": "number", "description": "Threshold value for triggering the alert. For between and not_between threshold types, this is the lower bound.", @@ -5523,6 +5593,31 @@ ], "name": "Error Spike Alert" } + }, + "inlineAlert": { + "summary": "Create an inline alert that carries its own chart config", + "value": { + "source": "inline", + "chartConfig": { + "displayType": "line", + "sourceId": "65f5e4a3b9e77c001a123456", + "select": [ + { + "aggFn": "count", + "where": "level:error", + "whereLanguage": "lucene" + } + ] + }, + "threshold": 100, + "interval": "5m", + "thresholdType": "above", + "channel": { + "type": "webhook", + "webhookId": "65f5e4a3b9e77c001a789012" + }, + "name": "Error log spike" + } } } } diff --git a/packages/api/src/controllers/alerts.ts b/packages/api/src/controllers/alerts.ts index feba0444f9..44fe6662e3 100644 --- a/packages/api/src/controllers/alerts.ts +++ b/packages/api/src/controllers/alerts.ts @@ -1,6 +1,7 @@ import { displayTypeSupportsBuilderAlerts, displayTypeSupportsRawSqlAlerts, + isFormulaSourceKind, validateRawSqlForAlert, } from '@hyperdx/common-utils/dist/core/utils'; import { isRawSqlSavedChartConfig } from '@hyperdx/common-utils/dist/guards'; @@ -189,6 +190,19 @@ export const validateAlertInput = async ( if (source == null) { throw new Api400Error('Source not found'); } + + // Same formula source-kind gate as dashboard tiles ("Add Formula" is + // disabled in the editor for these kinds), so the API cannot persist a + // config the editor refuses. + if ( + 'formulas' in chartConfig && + (chartConfig.formulas?.length ?? 0) > 0 && + !isFormulaSourceKind(source.kind) + ) { + throw new Api400Error( + 'Alerts with formulas require a Metric, Log, or Trace source', + ); + } } } diff --git a/packages/api/src/mcp/__tests__/alerts.int.test.ts b/packages/api/src/mcp/__tests__/alerts.int.test.ts index 5fbd0d87cd..1ff0e730ac 100644 --- a/packages/api/src/mcp/__tests__/alerts.int.test.ts +++ b/packages/api/src/mcp/__tests__/alerts.int.test.ts @@ -3,6 +3,7 @@ import { SourceKind, } from '@hyperdx/common-utils/dist/types'; import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import mongoose from 'mongoose'; import * as config from '@/config'; import { @@ -10,6 +11,7 @@ import { DEFAULT_TRACES_TABLE, getLoggedInAgent, getServer, + RAW_SQL_ALERT_TEMPLATE, } from '@/fixtures'; import { McpContext } from '@/mcp/tools/types'; import Alert, { AlertSource, AlertState } from '@/models/alert'; @@ -513,6 +515,354 @@ describe('MCP Alert Tools', () => { }); }); + describe('inline alerts', () => { + // External tile-config dialect, same as the dashboard tools. + const makeChartConfig = (overrides: Record = {}) => ({ + displayType: 'line', + sourceId: traceSource._id.toString(), + select: [ + { + aggFn: 'count', + where: 'StatusCode:STATUS_CODE_ERROR', + whereLanguage: 'lucene', + }, + ], + ...overrides, + }); + + const makeInlineInput = ( + webhookId: string, + overrides: Record = {}, + ) => ({ + source: 'inline', + chartConfig: makeChartConfig(), + threshold: 10, + thresholdType: 'above', + interval: '5m', + channel: { type: 'webhook', webhookId }, + name: 'Inline MCP Alert', + ...overrides, + }); + + it('should create an inline alert and echo the external-dialect chartConfig', async () => { + const webhook = await createTestWebhook(); + + const result = await callTool( + client, + 'clickstack_save_alert', + makeInlineInput(webhook._id.toString()), + ); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.source).toBe('inline'); + expect(output.savedSearchId).toBeUndefined(); + expect(output.dashboardId).toBeUndefined(); + expect(output.chartConfig).toMatchObject({ + displayType: 'line', + sourceId: traceSource._id.toString(), + select: [ + { + aggFn: 'count', + where: 'StatusCode:STATUS_CODE_ERROR', + whereLanguage: 'lucene', + }, + ], + }); + + // Mongo persists the internal dialect the check-alerts task reads. + const stored = await Alert.findById(output.id); + expect(stored!.source).toBe(AlertSource.INLINE); + expect(stored!.chartConfig).toMatchObject({ + source: traceSource._id.toString(), + select: [ + { + aggFn: 'count', + aggCondition: 'StatusCode:STATUS_CODE_ERROR', + aggConditionLanguage: 'lucene', + }, + ], + }); + }); + + it('should update an inline alert chart config', async () => { + const webhook = await createTestWebhook(); + + const createResult = await callTool( + client, + 'clickstack_save_alert', + makeInlineInput(webhook._id.toString()), + ); + const created = JSON.parse(getFirstText(createResult)); + + const updateResult = await callTool(client, 'clickstack_save_alert', { + ...makeInlineInput(webhook._id.toString(), { + chartConfig: makeChartConfig({ groupBy: 'ServiceName' }), + threshold: 42, + }), + id: created.id, + }); + + expect(updateResult.isError).toBeFalsy(); + const updated = JSON.parse(getFirstText(updateResult)); + expect(updated.threshold).toBe(42); + expect(updated.chartConfig).toMatchObject({ groupBy: 'ServiceName' }); + + const stored = await Alert.findById(created.id); + expect(stored!.threshold).toBe(42); + expect(stored!.chartConfig).toMatchObject({ groupBy: 'ServiceName' }); + }); + + it('should reject inline source without chartConfig', async () => { + const webhook = await createTestWebhook(); + + const result = await callTool( + client, + 'clickstack_save_alert', + makeInlineInput(webhook._id.toString(), { chartConfig: undefined }), + ); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('chartConfig is required'); + }); + + it('should reject a chartConfig on a saved-search alert', async () => { + const savedSearch = await createTestSavedSearch(); + const webhook = await createTestWebhook(); + + const result = await callTool(client, 'clickstack_save_alert', { + source: 'saved_search', + savedSearchId: savedSearch._id.toString(), + chartConfig: makeChartConfig(), + threshold: 10, + thresholdType: 'above', + interval: '5m', + channel: { type: 'webhook', webhookId: webhook._id.toString() }, + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain( + 'only supported when source is "inline"', + ); + }); + + it('should reject a malformed metric formula', async () => { + const webhook = await createTestWebhook(); + + const result = await callTool( + client, + 'clickstack_save_alert', + makeInlineInput(webhook._id.toString(), { + chartConfig: makeChartConfig({ + formulas: [{ expression: 'B * 2' }], + }), + }), + ); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('Invalid chartConfig'); + }); + + it("should reject a source that does not belong to the team's sources", async () => { + const webhook = await createTestWebhook(); + + const result = await callTool( + client, + 'clickstack_save_alert', + makeInlineInput(webhook._id.toString(), { + chartConfig: makeChartConfig({ + sourceId: new mongoose.Types.ObjectId().toString(), + }), + }), + ); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('Source not found'); + }); + + it('should create a raw SQL inline alert and validate the displayType allowlist', async () => { + const webhook = await createTestWebhook(); + + // Raw SQL charts allow alertable display types only: the tool's own + // input schema narrows displayType, so 'table' is rejected by MCP + // argument validation before the handler runs. + const rejected = await callTool( + client, + 'clickstack_save_alert', + makeInlineInput(webhook._id.toString(), { + chartConfig: { + configType: 'sql', + displayType: 'table', + connectionId: connection._id.toString(), + sqlTemplate: RAW_SQL_ALERT_TEMPLATE, + }, + }), + ); + expect(rejected.isError).toBe(true); + expect(getFirstText(rejected)).toContain('displayType'); + + const result = await callTool( + client, + 'clickstack_save_alert', + makeInlineInput(webhook._id.toString(), { + chartConfig: { + configType: 'sql', + displayType: 'line', + connectionId: connection._id.toString(), + sqlTemplate: RAW_SQL_ALERT_TEMPLATE, + }, + }), + ); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.chartConfig).toMatchObject({ + configType: 'sql', + displayType: 'line', + connectionId: connection._id.toString(), + sqlTemplate: RAW_SQL_ALERT_TEMPLATE, + }); + + // Stored with the internal field names. + const stored = await Alert.findById(output.id); + expect(stored!.chartConfig).toMatchObject({ + configType: 'sql', + connection: connection._id.toString(), + }); + }); + + it('preserves the gauge delta flag across the isDelta/periodAggFn dialect bridge', async () => { + const webhook = await createTestWebhook(); + + // MCP spells the flag `isDelta`; the external dialect (and the + // persisted internal shape) must not silently drop it — a lost flag + // makes the alert evaluate raw gauge values instead of deltas. + const createResult = await callTool( + client, + 'clickstack_save_alert', + makeInlineInput(webhook._id.toString(), { + chartConfig: makeChartConfig({ + select: [ + { + aggFn: 'avg', + metricType: 'gauge', + metricName: 'system.cpu.utilization', + isDelta: true, + }, + ], + }), + }), + ); + expect(createResult.isError).toBeFalsy(); + const created = JSON.parse(getFirstText(createResult)); + expect(created.chartConfig.select[0]).toMatchObject({ + periodAggFn: 'delta', + }); + + const stored = await Alert.findById(created.id); + expect(stored!.chartConfig).toMatchObject({ + select: [{ isDelta: true }], + }); + + // Read-then-resend: the detail response spells the flag + // periodAggFn: 'delta'; resending that config verbatim must keep it. + const detail = await callTool(client, 'clickstack_get_alert', { + id: created.id, + }); + const detailOutput = JSON.parse(getFirstText(detail)); + expect(detailOutput.chartConfig.select[0]).toMatchObject({ + periodAggFn: 'delta', + }); + + const resent = await callTool(client, 'clickstack_save_alert', { + ...makeInlineInput(webhook._id.toString(), { + chartConfig: detailOutput.chartConfig, + }), + id: created.id, + }); + expect(resent.isError).toBeFalsy(); + const storedAfter = await Alert.findById(created.id); + expect(storedAfter!.chartConfig).toMatchObject({ + select: [{ isDelta: true }], + }); + }); + + it('accepts the alert-only name and chart-level where fields', async () => { + const webhook = await createTestWebhook(); + + const result = await callTool( + client, + 'clickstack_save_alert', + makeInlineInput(webhook._id.toString(), { + chartConfig: makeChartConfig({ + name: 'Error Rate Query', + where: 'ServiceName:api', + whereLanguage: 'lucene', + }), + }), + ); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.chartConfig).toMatchObject({ + name: 'Error Rate Query', + where: 'ServiceName:api', + }); + + const stored = await Alert.findById(output.id); + expect(stored!.chartConfig).toMatchObject({ + name: 'Error Rate Query', + where: 'ServiceName:api', + whereLanguage: 'lucene', + }); + }); + + it('clickstack_get_alert list entries stay slim for inline alerts', async () => { + const webhook = await createTestWebhook(); + await callTool( + client, + 'clickstack_save_alert', + makeInlineInput(webhook._id.toString()), + ); + + const list = await callTool(client, 'clickstack_get_alert', {}); + expect(list.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(list)); + expect(output).toHaveLength(1); + expect(output[0].source).toBe('inline'); + expect(output[0].name).toBe('Inline MCP Alert'); + expect(output[0].chartConfig).toBeUndefined(); + }); + + it('clickstack_get_alert detail includes chartConfig and falls back to its name', async () => { + const webhook = await createTestWebhook(); + + const createResult = await callTool( + client, + 'clickstack_save_alert', + makeInlineInput(webhook._id.toString(), { name: undefined }), + ); + const created = JSON.parse(getFirstText(createResult)); + + // Seed a chartConfig name directly (as the internal UI flow would) + // to exercise the name fallback. + await Alert.findByIdAndUpdate(created.id, { + $set: { 'chartConfig.name': 'Error Rate Query' }, + }); + + const detail = await callTool(client, 'clickstack_get_alert', { + id: created.id, + }); + expect(detail.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(detail)); + expect(output.chartConfig).toMatchObject({ + displayType: 'line', + sourceId: traceSource._id.toString(), + }); + expect(output.name).toBe('Error Rate Query'); + }); + }); + describe('multiple channels', () => { const namedWebhook = (name: string) => Webhook.create({ diff --git a/packages/api/src/mcp/__tests__/dashboards/saveDashboard.int.test.ts b/packages/api/src/mcp/__tests__/dashboards/saveDashboard.int.test.ts index ebc9ceb9fb..250621e5d9 100644 --- a/packages/api/src/mcp/__tests__/dashboards/saveDashboard.int.test.ts +++ b/packages/api/src/mcp/__tests__/dashboards/saveDashboard.int.test.ts @@ -3363,4 +3363,96 @@ describe('MCP Dashboard Tools - clickstack_save_dashboard', () => { expect(text).toContain(ghostDashboardId); }); }); + + describe('metric tiles', () => { + const makeMetricSource = () => + Source.create({ + kind: SourceKind.Metric, + team: ctx.team._id, + from: { databaseName: DEFAULT_DATABASE, tableName: '' }, + metricTables: { + [MetricsDataType.Gauge.toLowerCase()]: 'otel_metrics_gauge', + [MetricsDataType.Sum.toLowerCase()]: 'otel_metrics_sum', + [MetricsDataType.Histogram.toLowerCase()]: 'otel_metrics_histogram', + }, + timestampValueExpression: 'TimeUnix', + connection: ctx.connection._id, + name: 'Metrics', + }); + + const gaugeTile = ( + selectItem: Record, + sourceId: string, + ) => ({ + name: 'CPU delta', + x: 0, + y: 0, + w: 12, + h: 4, + config: { + displayType: 'line', + sourceId, + select: [ + { + aggFn: 'avg', + metricType: 'gauge', + metricName: 'system.cpu.utilization', + valueExpression: 'Value', + where: '', + ...selectItem, + }, + ], + }, + }); + + it.each([ + ['isDelta', { isDelta: true }], + ['periodAggFn', { periodAggFn: 'delta' }], + ])( + 'persists the gauge delta flag on a saved tile spelled via %s', + async (_label, deltaFlag) => { + // The tool accepts both spellings and the conversion to the internal + // tile config must keep the flag: a dropped isDelta silently makes + // the tile chart raw gauge values instead of per-bucket deltas. + const metricSource = await makeMetricSource(); + const result = await callTool( + ctx.client!, + 'clickstack_save_dashboard', + { + name: `Gauge delta dashboard (${_label})`, + tiles: [gaugeTile(deltaFlag, metricSource._id.toString())], + }, + ); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + + const stored = await Dashboard.findById(output.id); + expect(stored!.tiles[0].config).toMatchObject({ + select: [{ isDelta: true }], + }); + }, + ); + + it('rejects a delta on a non-gauge metric', async () => { + const metricSource = await makeMetricSource(); + const result = await callTool(ctx.client!, 'clickstack_save_dashboard', { + name: 'Sum delta dashboard', + tiles: [ + gaugeTile( + { + metricType: 'sum', + metricName: 'http.server.requests', + aggFn: 'sum', + isDelta: true, + }, + metricSource._id.toString(), + ), + ], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('only valid for gauge metrics'); + }); + }); }); diff --git a/packages/api/src/mcp/tools/alerts/__tests__/schemas.test.ts b/packages/api/src/mcp/tools/alerts/__tests__/schemas.test.ts index 55a09951b9..ab280b7f69 100644 --- a/packages/api/src/mcp/tools/alerts/__tests__/schemas.test.ts +++ b/packages/api/src/mcp/tools/alerts/__tests__/schemas.test.ts @@ -1,5 +1,6 @@ import { McpSaveAlertInput, + mcpSaveAlertSchema, validateSaveAlertInput, } from '@/mcp/tools/alerts/schemas'; @@ -82,3 +83,129 @@ describe('validateSaveAlertInput channel comparison', () => { expect(result).toBeNull(); }); }); + +describe('validateSaveAlertInput inline alerts', () => { + const chartConfig: NonNullable = { + displayType: 'line', + sourceId: 'source-1', + select: [ + { + aggFn: 'count', + where: '', + whereLanguage: 'lucene', + }, + ], + fillNulls: true, + }; + + it('requires chartConfig when source is inline', () => { + const result = validateSaveAlertInput({ + ...baseInput, + source: 'inline', + savedSearchId: undefined, + channel: wh('a'), + }); + + expect(result).toContain('chartConfig is required'); + }); + + it('accepts an inline alert with a chartConfig', () => { + const result = validateSaveAlertInput({ + ...baseInput, + source: 'inline', + savedSearchId: undefined, + chartConfig, + channel: wh('a'), + }); + + expect(result).toBeNull(); + }); + + it('rejects a chartConfig on non-inline sources instead of silently dropping it', () => { + const result = validateSaveAlertInput({ + ...baseInput, + chartConfig, + channel: wh('a'), + }); + + expect(result).toContain('only supported when source is "inline"'); + }); +}); + +describe('mcpSaveAlertSchema isDelta dialect bridge', () => { + const parseSelectItem = (selectItem: Record) => { + const parsed = mcpSaveAlertSchema.parse({ + ...baseInput, + source: 'inline', + savedSearchId: undefined, + channel: wh('a'), + chartConfig: { + displayType: 'line', + sourceId: 'source-1', + select: [selectItem], + }, + }); + const config = parsed.chartConfig; + if (!config || !('select' in config)) { + throw new Error('expected a builder chartConfig'); + } + return config.select[0]; + }; + + const gaugeItem = { + aggFn: 'avg', + metricType: 'gauge', + metricName: 'system.cpu.utilization', + }; + + // Both spellings are emitted in agreement: `isDelta` for direct MCP + // consumers, `periodAggFn` because the external REST schema this config is + // re-parsed through knows only that one and strips the other. + it.each([ + ['periodAggFn: "delta"', { periodAggFn: 'delta' }], + ['isDelta: true', { isDelta: true }], + ])('emits both delta spellings for %s', (_label, deltaFlag) => { + expect(parseSelectItem({ ...gaugeItem, ...deltaFlag })).toMatchObject({ + isDelta: true, + periodAggFn: 'delta', + }); + }); + + it('emits neither spelling when the item is not a delta', () => { + const item = parseSelectItem(gaugeItem); + expect(item).not.toHaveProperty('isDelta'); + expect(item).not.toHaveProperty('periodAggFn'); + }); + + it('lets an explicit isDelta: false win over periodAggFn: "delta"', () => { + // The refinement and the transform must agree on precedence. When they + // disagreed, this body validated as non-delta (passing the gauge-only + // rule) and then persisted as a delta because the stale periodAggFn + // survived. + const item = parseSelectItem({ + ...gaugeItem, + isDelta: false, + periodAggFn: 'delta', + }); + expect(item).not.toHaveProperty('isDelta'); + expect(item).not.toHaveProperty('periodAggFn'); + }); + + it.each([ + ['isDelta', { isDelta: true }], + ['periodAggFn', { periodAggFn: 'delta' }], + ])( + 'rejects a delta on a non-gauge metric spelled via %s', + (_label, deltaFlag) => { + expect(() => + parseSelectItem({ + aggFn: 'sum', + metricType: 'sum', + metricName: 'http.server.requests', + valueExpression: 'Value', + ...deltaFlag, + }), + ).toThrow('isDelta is only valid for gauge metrics'); + }, + ); +}); diff --git a/packages/api/src/mcp/tools/alerts/getAlert.ts b/packages/api/src/mcp/tools/alerts/getAlert.ts index 457ba4573d..6e8e53b5de 100644 --- a/packages/api/src/mcp/tools/alerts/getAlert.ts +++ b/packages/api/src/mcp/tools/alerts/getAlert.ts @@ -11,13 +11,14 @@ import { mcpUserError, validateObjectId } from '@/mcp/utils/errors'; import Alert from '@/models/alert'; import type { IDashboard } from '@/models/dashboard'; import type { ISavedSearch } from '@/models/savedSearch'; -import { translateAlertDocumentToExternalAlert } from '@/utils/externalApi'; +import { translateAlertDocumentToExternalAlertWithChartConfig } from '@/routers/external-api/v2/utils/alertChartConfig'; function deriveAlertName(alert: { name?: string | null; tileId?: string | null; savedSearch?: ISavedSearch | null; dashboard?: IDashboard | null; + chartConfig?: { name?: string } | null; }): string | null { // Prefer explicit alert name if (alert.name) { @@ -29,6 +30,11 @@ function deriveAlertName(alert: { return alert.savedSearch.name; } + // Inline alerts: fall back to the persisted chart config's name + if (alert.chartConfig?.name) { + return alert.chartConfig.name; + } + // Fall back to dashboard tile name or dashboard name if (alert.dashboard?.name) { if (alert.tileId) { @@ -118,7 +124,8 @@ export function registerGetAlert({ return mcpUserError('Alert not found'); } - const external = translateAlertDocumentToExternalAlert(alert); + const external = + translateAlertDocumentToExternalAlertWithChartConfig(alert); // Populate refs so deriveAlertName can fall back to the // saved search / dashboard name when the alert has no explicit name. diff --git a/packages/api/src/mcp/tools/alerts/saveAlert.ts b/packages/api/src/mcp/tools/alerts/saveAlert.ts index 3434b57d61..aa00c19d7c 100644 --- a/packages/api/src/mcp/tools/alerts/saveAlert.ts +++ b/packages/api/src/mcp/tools/alerts/saveAlert.ts @@ -10,16 +10,27 @@ import { } from '@/controllers/alerts'; import type { ToolRegistrar } from '@/mcp/tools/types'; import { + formatZodIssues, mcpServerError, mcpUserError, validateObjectId, } from '@/mcp/utils/errors'; import { AlertSource } from '@/models/alert'; +import { + convertExternalAlertChartConfigToInternal, + translateAlertDocumentToExternalAlertWithChartConfig, +} from '@/routers/external-api/v2/utils/alertChartConfig'; import { BaseError } from '@/utils/errors'; -import { translateAlertDocumentToExternalAlert } from '@/utils/externalApi'; +import { externalAlertChartConfigSchema } from '@/utils/zod'; import { mcpSaveAlertSchema, validateSaveAlertInput } from './schemas'; +const MCP_SOURCE_TO_ALERT_SOURCE = { + saved_search: AlertSource.SAVED_SEARCH, + tile: AlertSource.TILE, + inline: AlertSource.INLINE, +} as const; + export function registerSaveAlert({ context, registerTool, @@ -34,8 +45,10 @@ export function registerSaveAlert({ annotations: { destructiveHint: true }, description: 'Create a new alert (omit id) or update an existing one (provide id). ' + - 'Alerts monitor a saved search or dashboard tile and fire when the ' + - 'metric crosses a threshold. At least one webhook notification channel ' + + 'Alerts monitor a saved search, a dashboard tile, or an inline chart ' + + 'config (source "inline" + chartConfig — no saved search or dashboard ' + + 'needed) and fire when the metric crosses a threshold. At least one ' + + 'webhook notification channel ' + 'is required: pass "channels" for 1-10 targets, or the legacy singular ' + '"channel" for one. Updates replace the alert configuration rather than ' + 'merging it, so read the alert first and resend its full "channels" ' + @@ -56,9 +69,32 @@ export function registerSaveAlert({ if (idError) return idError; } + // Inline alerts: run the chart config through the shared external + // schema (same one the v2 REST API parses) so formula validation and + // the number single-select rule cannot drift between the surfaces, + // then convert to the internal shape the controllers persist. + let internalChartConfig: AlertInput['chartConfig']; + if (input.source === 'inline') { + // The MCP tile dialect spells the gauge delta flag `isDelta`; the + // shared external schema spells it `periodAggFn: 'delta'` and strips + // unknown keys. The MCP select-item schema already emits both + // spellings in agreement (see mcpTileSelectItemSchema), so the flag + // survives this parse. + const parsed = externalAlertChartConfigSchema.safeParse( + input.chartConfig, + ); + if (!parsed.success) { + return mcpUserError( + `Invalid chartConfig:\n${formatZodIssues(parsed.error)}`, + ); + } + internalChartConfig = convertExternalAlertChartConfigToInternal( + parsed.data, + ); + } + // Build the alert input matching the shape expected by controllers. - const source = - input.source === 'tile' ? AlertSource.TILE : AlertSource.SAVED_SEARCH; + const source = MCP_SOURCE_TO_ALERT_SOURCE[input.source]; const alertInput: AlertInput = { source, // `channel` is omitted; makeAlert mirrors it from channels[0]. @@ -75,6 +111,7 @@ export function registerSaveAlert({ savedSearchId: input.savedSearchId, dashboardId: input.dashboardId, tileId: input.tileId, + chartConfig: internalChartConfig, }; // ── Validate referenced entities exist ── @@ -107,7 +144,9 @@ export function registerSaveAlert({ type: 'text' as const, text: JSON.stringify( { - ...translateAlertDocumentToExternalAlert(updated), + ...translateAlertDocumentToExternalAlertWithChartConfig( + updated, + ), ...(frontendUrl ? { url: `${frontendUrl}/alerts` } : {}), }, null, @@ -130,7 +169,9 @@ export function registerSaveAlert({ type: 'text' as const, text: JSON.stringify( { - ...translateAlertDocumentToExternalAlert(created), + ...translateAlertDocumentToExternalAlertWithChartConfig( + created, + ), ...(frontendUrl ? { url: `${frontendUrl}/alerts` } : {}), }, null, diff --git a/packages/api/src/mcp/tools/alerts/schemas.ts b/packages/api/src/mcp/tools/alerts/schemas.ts index 57c90f09cd..d03b39f4aa 100644 --- a/packages/api/src/mcp/tools/alerts/schemas.ts +++ b/packages/api/src/mcp/tools/alerts/schemas.ts @@ -4,9 +4,17 @@ import { checkAlertChannelSelection, isRangeThresholdType, MAX_ALERT_CHANNELS, + SearchConditionTrimmedLanguageSchema, } from '@hyperdx/common-utils/dist/types'; import { z } from 'zod'; +import { + mcpBarTileSchema, + mcpLineTileSchema, + mcpNumberTileSchema, + mcpSqlTileSchema, +} from '@/mcp/tools/dashboards/schemas'; + // --------------------------------------------------------------------------- // MCP-compatible flat Zod schema for clickstack_save_alert. // @@ -16,6 +24,83 @@ import { z } from 'zod'; // validateSaveAlertInput(). // --------------------------------------------------------------------------- +// Inline alerts carry their chart config in the same dialect as dashboard +// tile configs, restricted to the display types the alert evaluator can run +// as a time series: line, stacked_bar, and number (builder or raw SQL). +// Reuses the dashboard tile config shapes so agents author both from one +// vocabulary; the raw SQL variant narrows displayType and drops the +// tile-only onClick affordance. Full validation (formulas, number +// single-select rule, source/connection ownership) runs at save time via the +// shared external alert schema and validateAlertInput. +// +// Unlike tiles, an alert's config carries its own `name` (used in +// notification titles) and — builder variants only — a chart-level +// `where`/`whereLanguage` filter ANDed into every series, so those override +// the tile schemas' rejected-never fields here. +const mcpAlertChartConfigNameSchema = z + .string() + .optional() + .describe( + 'Display name for the alert query, used in notification titles when the ' + + 'alert has no explicit name.', + ); + +const mcpAlertChartLevelWhereSchema = z + .string() + .max(10000) + .optional() + .describe( + 'Chart-level filter applied to every select item (combined with each ' + + "item's own `where` via AND). Lucene syntax by default.", + ); + +const rejectedRawSqlAlertWhereField = z + .never({ + invalid_type_error: + 'Raw SQL alert configs have no chart-level where; filter inside the sqlTemplate instead', + }) + .optional() + .describe( + 'Not supported on Raw SQL alert configs. Filter inside the sqlTemplate.', + ); + +const mcpAlertChartConfigSchema = z + .union([ + mcpLineTileSchema.shape.config.extend({ + name: mcpAlertChartConfigNameSchema, + where: mcpAlertChartLevelWhereSchema, + whereLanguage: SearchConditionTrimmedLanguageSchema.optional(), + }), + mcpBarTileSchema.shape.config.extend({ + name: mcpAlertChartConfigNameSchema, + where: mcpAlertChartLevelWhereSchema, + whereLanguage: SearchConditionTrimmedLanguageSchema.optional(), + }), + mcpNumberTileSchema.shape.config.extend({ + name: mcpAlertChartConfigNameSchema, + where: mcpAlertChartLevelWhereSchema, + whereLanguage: SearchConditionTrimmedLanguageSchema.optional(), + }), + mcpSqlTileSchema.shape.config.omit({ onClick: true }).extend({ + name: mcpAlertChartConfigNameSchema, + where: rejectedRawSqlAlertWhereField, + whereLanguage: rejectedRawSqlAlertWhereField, + displayType: z + .enum(['line', 'stacked_bar', 'number']) + .describe( + 'How to render the SQL results. Alerts evaluate line, stacked_bar, or number charts only.', + ), + }), + ]) + .describe( + 'Chart configuration for inline alerts (required when source is "inline"). ' + + 'Same shape as a dashboard tile config, limited to displayType line, ' + + 'stacked_bar, or number. Omit configType for the builder variant ' + + '(sourceId + select); set configType to "sql" for the Raw SQL variant ' + + '(connectionId + sqlTemplate; the template must use the $__timeFilter ' + + 'and $__timeInterval macros so each evaluation window can be queried).', + ); + const mcpAlertChannelSchema = z .object({ type: z @@ -37,8 +122,12 @@ export const mcpSaveAlertSchema = z.object({ // Source source: z - .enum(['saved_search', 'tile']) - .describe('Alert source type: saved_search or tile.'), + .enum(['saved_search', 'tile', 'inline']) + .describe( + 'Alert source type: "saved_search" monitors a saved search, "tile" ' + + 'monitors a dashboard tile, and "inline" carries its own chartConfig ' + + 'without requiring a saved search or dashboard.', + ), savedSearchId: z .string() .optional() @@ -57,6 +146,7 @@ export const mcpSaveAlertSchema = z.object({ .string() .optional() .describe('Group-by key for saved search alerts.'), + chartConfig: mcpAlertChartConfigSchema.optional(), // Threshold threshold: z.number().describe('Threshold value for triggering the alert.'), @@ -161,6 +251,14 @@ export function validateSaveAlertInput(data: McpSaveAlertInput): string | null { return 'savedSearchId is required when source is "saved_search"'; } } + if (data.source === 'inline' && data.chartConfig == null) { + return 'chartConfig is required when source is "inline"'; + } + // Reject rather than silently drop a whole config: the caller clearly + // intended an inline alert. + if (data.source !== 'inline' && data.chartConfig != null) { + return 'chartConfig is only supported when source is "inline"'; + } // Threshold range checks if (isRangeThresholdType(data.thresholdType)) { diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index 1a1740635b..2c123c9ef3 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -199,6 +199,21 @@ const mcpNumberFormatSchema = z.object({ .describe('Suffix appended to the value (e.g. " req/s")'), }); +/** + * The delta flag a select item resolves to, given that the tool accepts both + * spellings (`isDelta` and the REST dialect's `periodAggFn: 'delta'`). An + * explicit `isDelta` wins; `periodAggFn` only fills in when it is absent. + * Shared by the refinement and the transform so a body carrying both cannot + * validate as one value and persist as the other. + */ +const resolveIsDelta = (data: { + isDelta?: unknown; + periodAggFn?: unknown; +}): boolean | undefined => + typeof data.isDelta === 'boolean' + ? data.isDelta + : data.periodAggFn === 'delta' || undefined; + const mcpTileSelectItemSchema = z .object({ aggFn: AggregateFunctionSchema.describe( @@ -273,6 +288,18 @@ const mcpTileSelectItemSchema = z 'METRIC SOURCES ONLY (gauge metrics). When true, computes the Prometheus-style ' + 'delta over each bucket. Default false.', ), + // The REST v2 dialect's spelling of the same flag. Accepted so a select + // item read back from the external API (or clickstack_get_alert / + // clickstack_get_dashboard, which emit the REST dialect) can be resent + // verbatim without silently clearing the delta flag; normalized onto + // `isDelta` in the transform below. + periodAggFn: z + .enum(['delta']) + .optional() + .describe( + 'Alias for isDelta accepted for round-tripping configs read from ' + + 'get tools: "delta" is equivalent to isDelta: true.', + ), }) .superRefine((data, ctx) => { const narrow = { @@ -281,7 +308,11 @@ const mcpTileSelectItemSchema = z typeof data.metricType === 'string' ? data.metricType : undefined, metricName: typeof data.metricName === 'string' ? data.metricName : undefined, - isDelta: typeof data.isDelta === 'boolean' ? data.isDelta : undefined, + // Must use the same precedence as the transform below, or a body + // spelling both flags (`isDelta: false` + `periodAggFn: 'delta'`) is + // validated as non-delta — passing the gauge-only rule — and then + // persisted as a delta. + isDelta: resolveIsDelta(data), level: typeof data.level === 'number' ? data.level : undefined, valueExpression: typeof data.valueExpression === 'string' @@ -296,11 +327,24 @@ const mcpTileSelectItemSchema = z }); } }) - .transform(data => - data.metricType && data.aggFn !== 'count' && !data.valueExpression - ? { ...data, valueExpression: 'Value' } - : data, - ); + .transform(data => { + // Emit BOTH spellings, agreeing, so neither downstream consumer can lose + // the flag: MCP tiles are re-parsed through the external REST schema + // (createDashboardBodySchema / externalAlertChartConfigSchema), which + // knows only `periodAggFn` and strips `isDelta`, while direct MCP + // consumers read `isDelta`. Writing the resolved value to both also + // clears a stale `periodAggFn: 'delta'` when `isDelta: false` wins, + // which would otherwise persist a delta the refinement never checked. + const { isDelta: _isDelta, periodAggFn: _periodAggFn, ...rest } = data; + const withDelta = resolveIsDelta(data) + ? { ...rest, isDelta: true as const, periodAggFn: 'delta' as const } + : rest; + return withDelta.metricType && + withDelta.aggFn !== 'count' && + !withDelta.valueExpression + ? { ...withDelta, valueExpression: 'Value' } + : withDelta; + }); // ─── Chart formulas ────────────────────────────────────────────────────────── @@ -620,7 +664,7 @@ const mcpTileLayoutSchema = z.object({ ), }); -const mcpLineTileSchema = mcpTileLayoutSchema.extend({ +export const mcpLineTileSchema = mcpTileLayoutSchema.extend({ config: z.object({ ...rejectedTileWhereFields, displayType: z.literal('line').describe('Line chart over time'), @@ -666,7 +710,7 @@ const mcpLineTileSchema = mcpTileLayoutSchema.extend({ }), }); -const mcpBarTileSchema = mcpTileLayoutSchema.extend({ +export const mcpBarTileSchema = mcpTileLayoutSchema.extend({ config: z.object({ ...rejectedTileWhereFields, displayType: z @@ -736,7 +780,7 @@ const mcpTableTileSchema = mcpTileLayoutSchema.extend({ }), }); -const mcpNumberTileSchema = mcpTileLayoutSchema.extend({ +export const mcpNumberTileSchema = mcpTileLayoutSchema.extend({ config: z.object({ ...rejectedTileWhereFields, displayType: z.literal('number').describe('Single aggregate scalar value'), @@ -980,7 +1024,7 @@ const mcpMarkdownTileSchema = mcpTileLayoutSchema.extend({ }), }); -const mcpSqlTileSchema = mcpTileLayoutSchema.extend({ +export const mcpSqlTileSchema = mcpTileLayoutSchema.extend({ config: z.object({ configType: z .literal('sql') 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..2c549bac7c 100644 --- a/packages/api/src/routers/api/__tests__/alerts.int.test.ts +++ b/packages/api/src/routers/api/__tests__/alerts.int.test.ts @@ -1838,7 +1838,7 @@ describe('alerts router', () => { const base = makeAlertChartConfig({ sourceId: source._id.toString() }); // Formula referencing a nonexistent series (only A exists) - await agent + const unknownSeries = await agent .post('/alerts') .send( makeInlineAlertInput({ @@ -1847,6 +1847,10 @@ describe('alerts router', () => { }), ) .expect(400); + // The alert body's source union must be discriminated for this to + // surface: a plain `.or()` reports only a generic union failure and + // buries the real issue in unionErrors. + expect(JSON.stringify(unknownSeries.body)).toContain('Unknown series'); // Malformed expression await agent 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 49c5c7db31..4006fc6988 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 @@ -1,4 +1,4 @@ -import { AlertErrorType } from '@hyperdx/common-utils/dist/types'; +import { AlertErrorType, SourceKind } from '@hyperdx/common-utils/dist/types'; import _ from 'lodash'; import { ObjectId } from 'mongodb'; import request from 'supertest'; @@ -11,8 +11,10 @@ import { } from '@/fixtures'; import { AlertSource, AlertThresholdType } from '@/models/alert'; import Alert from '@/models/alert'; +import Connection from '@/models/connection'; import Dashboard from '@/models/dashboard'; import { SavedSearch } from '@/models/savedSearch'; +import { Source } from '@/models/source'; import Webhook, { WebhookService } from '@/models/webhook'; // Constants @@ -657,21 +659,797 @@ describe('External API Alerts', () => { }); }); - describe('Input validation', () => { - 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. + describe('Inline alerts', () => { + // Team-scoped connection + source the inline chart configs reference. + 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 }; + }; + + // External tile-config dialect (sourceId, per-select where) — the + // internal API's dialect (source, aggCondition) is rejected here. + const makeBuilderChartConfig = ( + sourceId: string, + overrides: Record = {}, + ) => ({ + displayType: 'line', + sourceId, + select: [ + { + aggFn: 'count', + where: 'level:error', + whereLanguage: 'lucene', + }, + ], + ...overrides, + }); + + const makeInlineAlertInput = ( + chartConfig: unknown, + webhookId: string, + overrides: Record = {}, + ) => ({ + source: 'inline', + chartConfig, + threshold: 100, + interval: '1h', + thresholdType: AlertThresholdType.ABOVE, + channel: { + type: 'webhook', + webhookId, + }, + ...overrides, + }); + + it('creates an inline alert, persists the internal dialect, and round-trips chartConfig on single-alert responses', async () => { + const { source } = await makeSource(); const webhook = await createTestWebhook(); - const alertInput = { - threshold: 100, - interval: '1h', - source: 'inline', + const created = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + makeBuilderChartConfig(source._id.toString()), + webhook._id.toString(), + { name: 'Inline Alert' }, + ), + ) + .expect(200); + + expect(created.body.data.source).toBe(AlertSource.INLINE); + expect(created.body.data.savedSearchId).toBeUndefined(); + expect(created.body.data.dashboardId).toBeUndefined(); + expect(created.body.data.chartConfig).toMatchObject({ + displayType: 'line', + sourceId: source._id.toString(), + select: [ + { + aggFn: 'count', + where: 'level:error', + whereLanguage: 'lucene', + }, + ], + }); + + // Mongo persists the internal dialect the check-alerts task evaluates. + const stored = await Alert.findById(created.body.data.id); + expect(stored!.chartConfig).toMatchObject({ + displayType: 'line', + source: source._id.toString(), + select: [ + { + aggFn: 'count', + aggCondition: 'level:error', + aggConditionLanguage: 'lucene', + }, + ], + }); + + const single = await authRequest( + 'get', + `${ALERTS_BASE_URL}/${created.body.data.id}`, + ).expect(200); + expect(single.body.data.chartConfig).toMatchObject({ + displayType: 'line', + sourceId: source._id.toString(), + }); + + // The list endpoint stays lean: no chartConfig per row. + const list = await authRequest('get', ALERTS_BASE_URL).expect(200); + const listed = list.body.data.find(a => a.id === created.body.data.id); + expect(listed.source).toBe(AlertSource.INLINE); + expect(listed.chartConfig).toBeUndefined(); + }); + + it('updates an inline alert config and maps asRatio to the internal ratio return type', async () => { + const { source } = await makeSource(); + const webhook = await createTestWebhook(); + + const created = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + makeBuilderChartConfig(source._id.toString()), + webhook._id.toString(), + ), + ) + .expect(200); + + const updated = await authRequest( + 'put', + `${ALERTS_BASE_URL}/${created.body.data.id}`, + ) + .send( + makeInlineAlertInput( + makeBuilderChartConfig(source._id.toString(), { + groupBy: 'ServiceName', + asRatio: true, + select: [ + { aggFn: 'count', where: 'level:error' }, + { aggFn: 'count', where: '' }, + ], + }), + webhook._id.toString(), + { threshold: 42 }, + ), + ) + .expect(200); + + expect(updated.body.data.threshold).toBe(42); + expect(updated.body.data.chartConfig).toMatchObject({ + groupBy: 'ServiceName', + asRatio: true, + }); + + const stored = await Alert.findById(created.body.data.id); + expect(stored!.threshold).toBe(42); + expect(stored!.chartConfig).toMatchObject({ + groupBy: 'ServiceName', + seriesReturnType: 'ratio', + }); + }); + + it('clears source-specific references when switching between inline and tile sources', async () => { + const { source } = await makeSource(); + const webhook = await createTestWebhook(); + const dashboard = await createTestDashboard(); + const tileId = dashboard.tiles[0].id; + + const created = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + makeBuilderChartConfig(source._id.toString()), + webhook._id.toString(), + ), + ) + .expect(200); + + await authRequest('put', `${ALERTS_BASE_URL}/${created.body.data.id}`) + .send({ + source: AlertSource.TILE, + dashboardId: dashboard._id.toString(), + tileId, + threshold: 100, + interval: '1h', + thresholdType: AlertThresholdType.ABOVE, + channel: { type: 'webhook', 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._id.toString()); + expect(stored!.tileId).toBe(tileId); + + await authRequest('put', `${ALERTS_BASE_URL}/${created.body.data.id}`) + .send( + makeInlineAlertInput( + makeBuilderChartConfig(source._id.toString()), + 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 an inline alert whose source does not exist in the team', async () => { + const webhook = await createTestWebhook(); + await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + makeBuilderChartConfig(new ObjectId().toString()), + webhook._id.toString(), + ), + ) + .expect(400); + }); + + it('rejects an inline alert with an unsupported display type', async () => { + const { source } = await makeSource(); + const webhook = await createTestWebhook(); + await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + makeBuilderChartConfig(source._id.toString(), { + displayType: 'table', + }), + webhook._id.toString(), + ), + ) + .expect(400); + }); + + it('rejects an inline alert with a PromQL config', async () => { + const webhook = await createTestWebhook(); + await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { + configType: 'promql', + promqlQuery: 'up', + sourceId: new ObjectId().toString(), + }, + webhook._id.toString(), + ), + ) + .expect(400); + }); + + it('rejects an unsupported configType instead of routing it to the builder dialect', async () => { + // An otherwise-valid builder body carrying configType: 'promql' parses + // against the builder union (the unknown keys are stripped), so without + // an explicit check it would persist as a builder config AND skip the + // builder-only rules — here a formula referencing a nonexistent series, + // which then throws on every check-alerts evaluation. + const { source } = await makeSource(); + const webhook = await createTestWebhook(); + const base = makeBuilderChartConfig(source._id.toString()); + + const withFormula = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { + ...base, + configType: 'promql', + formulas: [{ expression: 'B * 2' }], + }, + webhook._id.toString(), + ), + ) + .expect(400); + expect(withFormula.body.message).toContain( + 'configType must be "sql" or omitted', + ); + + // Same bypass for the number single-select rule. + const numberConfig = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { + ...base, + configType: 'promql', + displayType: 'number', + select: [ + { aggFn: 'count', where: 'level:error' }, + { aggFn: 'count', where: '' }, + ], + }, + webhook._id.toString(), + ), + ) + .expect(400); + expect(numberConfig.body.message).toContain( + 'configType must be "sql" or omitted', + ); + }); + + it('validates metric formulas on builder chart configs', async () => { + const { source } = await makeSource(); + const webhook = await createTestWebhook(); + const base = makeBuilderChartConfig(source._id.toString()); + + // Formula referencing a nonexistent series (only A exists) + const unknownSeries = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { ...base, formulas: [{ expression: 'B * 2' }] }, + webhook._id.toString(), + ), + ) + .expect(400); + // The schema-level message must survive the alert body's source union — + // a plain `.or()` collapses every branch issue to "Invalid input". + expect(unknownSeries.body.message).toContain('Unknown series "B"'); + + // Malformed expression + await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { ...base, formulas: [{ expression: 'A +' }] }, + webhook._id.toString(), + ), + ) + .expect(400); + + // Formulas are mutually exclusive with the ratio toggle + await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { + ...base, + asRatio: true, + select: [ + { aggFn: 'count', where: '' }, + { aggFn: 'count', where: 'level:error' }, + ], + formulas: [{ expression: 'A * 2' }], + }, + webhook._id.toString(), + ), + ) + .expect(400); + + const created = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { ...base, formulas: [{ expression: 'A * 2' }] }, + webhook._id.toString(), + ), + ) + .expect(200); + expect(created.body.data.chartConfig).toMatchObject({ + formulas: [{ expression: 'A * 2' }], + }); + }); + + it('accepts a raw SQL inline alert and validates its template and connection ownership', async () => { + const { connection } = await makeSource(); + const webhook = await createTestWebhook(); + + // Missing the required time-filter/interval macros + await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { + configType: 'sql', + displayType: 'line', + sqlTemplate: 'SELECT 1', + connectionId: connection._id.toString(), + }, + webhook._id.toString(), + ), + ) + .expect(400); + + // A connection outside the team is rejected + await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { + configType: 'sql', + displayType: 'line', + sqlTemplate: RAW_SQL_ALERT_TEMPLATE, + connectionId: new ObjectId().toString(), + }, + webhook._id.toString(), + ), + ) + .expect(400); + + const created = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { + configType: 'sql', + displayType: 'line', + sqlTemplate: RAW_SQL_ALERT_TEMPLATE, + connectionId: connection._id.toString(), + }, + webhook._id.toString(), + ), + ) + .expect(200); + expect(created.body.data.chartConfig).toMatchObject({ + configType: 'sql', + displayType: 'line', + connectionId: connection._id.toString(), + sqlTemplate: RAW_SQL_ALERT_TEMPLATE, + }); + + // Stored internally with the internal field names + const stored = await Alert.findById(created.body.data.id); + expect(stored!.chartConfig).toMatchObject({ + configType: 'sql', + connection: connection._id.toString(), + }); + }); + + it('rejects a raw SQL inline alert whose source is on a different connection', async () => { + const { connection, source } = await makeSource(); + const webhook = await createTestWebhook(); + const otherConnection = await Connection.create({ + team: team._id, + name: 'Other', + host: 'http://localhost:8124', + username: 'default', + password: '', + }); + + const rawSqlConfig = { + configType: 'sql', + displayType: 'line', + sqlTemplate: RAW_SQL_ALERT_TEMPLATE, + sourceId: source._id.toString(), + }; + + // The source belongs to `connection`, not `otherConnection` — the + // worker would execute on one and expand $__sourceTable from the other. + await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { ...rawSqlConfig, connectionId: otherConnection._id.toString() }, + webhook._id.toString(), + ), + ) + .expect(400); + + await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { ...rawSqlConfig, connectionId: connection._id.toString() }, + webhook._id.toString(), + ), + ) + .expect(200); + }); + + it('rejects an inline alert without a chartConfig', async () => { + const webhook = await createTestWebhook(); + await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput(undefined, webhook._id.toString(), { + chartConfig: undefined, + }), + ) + .expect(400); + }); + + it('accepts a number chart and enforces the single-select rule', async () => { + const { source } = await makeSource(); + const webhook = await createTestWebhook(); + + const numberConfig = { + displayType: 'number', + sourceId: source._id.toString(), + select: [{ aggFn: 'count', where: 'level:error' }], + }; + + const created = await authRequest('post', ALERTS_BASE_URL) + .send(makeInlineAlertInput(numberConfig, webhook._id.toString())) + .expect(200); + expect(created.body.data.chartConfig).toMatchObject({ + displayType: 'number', + sourceId: source._id.toString(), + }); + + // A number chart without formulas displays a single select item; + // extra items are only valid as formula operands. + const multiSelect = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { + ...numberConfig, + select: [ + { aggFn: 'count', where: 'level:error' }, + { aggFn: 'count', where: '' }, + ], + }, + webhook._id.toString(), + ), + ) + .expect(400); + expect(multiSelect.body.message).toContain( + 'Number charts support a single select item', + ); + + // With a formula, the extra select items are its operands. + const withFormula = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { + ...numberConfig, + select: [ + { aggFn: 'count', where: 'level:error' }, + { aggFn: 'count', where: '' }, + ], + formulas: [{ expression: 'A / B * 100' }], + }, + webhook._id.toString(), + ), + ) + .expect(200); + expect(withFormula.body.data.chartConfig).toMatchObject({ + formulas: [{ expression: 'A / B * 100' }], + }); + }); + + it('rejects formulas on a formula-incapable source kind (session)', async () => { + // Same gate as dashboard tiles: the editor disables "Add Formula" for + // session sources, so the alert API must refuse the config too. + const { connection, source } = await makeSource(); + const webhook = await createTestWebhook(); + + const sessionSource = await Source.create({ + kind: SourceKind.Session, + team: team._id, + from: { databaseName: 'default', tableName: 'rrweb_events' }, + timestampValueExpression: 'Timestamp', + traceSourceId: source._id.toString(), + connection: connection._id, + name: 'Sessions', + }); + + const withFormulas = (sourceId: string) => + makeInlineAlertInput( + { + ...makeBuilderChartConfig(sourceId), + select: [ + { aggFn: 'count', where: 'level:error' }, + { aggFn: 'count', where: '' }, + ], + formulas: [{ expression: 'A / B * 100' }], + }, + webhook._id.toString(), + ); + + const rejected = await authRequest('post', ALERTS_BASE_URL) + .send(withFormulas(sessionSource._id.toString())) + .expect(400); + expect(JSON.stringify(rejected.body)).toContain('formulas require'); + + // The identical config on a formula-capable source is accepted. + await authRequest('post', ALERTS_BASE_URL) + .send(withFormulas(source._id.toString())) + .expect(200); + }); + + it('rejects asRatio without exactly two select items', async () => { + const { source } = await makeSource(); + const webhook = await createTestWebhook(); + + const res = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + makeBuilderChartConfig(source._id.toString(), { asRatio: true }), + webhook._id.toString(), + ), + ) + .expect(400); + expect(res.body.message).toContain( + 'asRatio can only be used with exactly two select items', + ); + }); + + it('round-trips a single-alert GET body back through PUT unchanged', async () => { + // The Terraform-provider flow: read an alert, generate config from the + // response, apply it back. The echoed body must be valid PUT input and + // must not mutate the stored chart config. + const { source } = await makeSource(); + const webhook = await createTestWebhook(); + + const created = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + makeBuilderChartConfig(source._id.toString(), { + groupBy: 'ServiceName', + }), + webhook._id.toString(), + { name: 'Echo Test' }, + ), + ) + .expect(200); + const storedBefore = await Alert.findById(created.body.data.id); + + const single = await authRequest( + 'get', + `${ALERTS_BASE_URL}/${created.body.data.id}`, + ).expect(200); + + const echoed = await authRequest( + 'put', + `${ALERTS_BASE_URL}/${created.body.data.id}`, + ) + .send(single.body.data) + .expect(200); + expect(echoed.body.data.chartConfig).toEqual( + single.body.data.chartConfig, + ); + + const storedAfter = await Alert.findById(created.body.data.id); + expect(storedAfter!.chartConfig).toEqual(storedBefore!.chartConfig); + }); + + it('strips unknown chartConfig fields instead of persisting them', async () => { + const { source } = await makeSource(); + const webhook = await createTestWebhook(); + + const created = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { + ...makeBuilderChartConfig(source._id.toString()), + unknownField: 'dropped', + }, + webhook._id.toString(), + ), + ) + .expect(200); + + expect(created.body.data.chartConfig).not.toHaveProperty('unknownField'); + const stored = await Alert.findById(created.body.data.id); + expect(stored!.chartConfig).not.toHaveProperty('unknownField'); + }); + + it('rejects a chartConfig on non-inline sources instead of silently dropping it', async () => { + const { source } = await makeSource(); + const webhook = await createTestWebhook(); + const dashboard = await createTestDashboard(); + const savedSearch = await createTestSavedSearch(); + const chartConfig = makeBuilderChartConfig(source._id.toString()); + + const tileRes = await authRequest('post', ALERTS_BASE_URL) + .send({ + source: AlertSource.TILE, + dashboardId: dashboard._id.toString(), + tileId: dashboard.tiles[0].id, + chartConfig, + threshold: 100, + interval: '1h', + thresholdType: AlertThresholdType.ABOVE, + channel: { type: 'webhook', webhookId: webhook._id.toString() }, + }) + .expect(400); + expect(tileRes.body.message).toContain( + 'chartConfig is only supported when source is "inline"', + ); + + const savedSearchRes = await authRequest('post', ALERTS_BASE_URL) + .send({ + source: AlertSource.SAVED_SEARCH, + savedSearchId: savedSearch._id.toString(), + chartConfig, + threshold: 100, + interval: '1h', + thresholdType: AlertThresholdType.ABOVE, + channel: { type: 'webhook', webhookId: webhook._id.toString() }, + }) + .expect(400); + expect(savedSearchRes.body.message).toContain( + 'chartConfig is only supported when source is "inline"', + ); + + // PUT applies the same rule. + const { alert } = await createTestAlert(); + await authRequest('put', `${ALERTS_BASE_URL}/${alert.id}`) + .send({ + source: AlertSource.TILE, + dashboardId: alert.dashboardId, + tileId: alert.tileId, + chartConfig, + threshold: 100, + interval: '1h', + thresholdType: AlertThresholdType.ABOVE, + channel: alert.channel, + }) + .expect(400); + }); + + it('round-trips the config name and chart-level where', async () => { + const { source } = await makeSource(); + const webhook = await createTestWebhook(); + + const created = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + makeBuilderChartConfig(source._id.toString(), { + name: 'Error Rate Query', + where: 'ServiceName:api', + whereLanguage: 'lucene', + }), + webhook._id.toString(), + ), + ) + .expect(200); + + expect(created.body.data.chartConfig).toMatchObject({ + name: 'Error Rate Query', + where: 'ServiceName:api', + whereLanguage: 'lucene', + }); + + // Persisted on the internal shape the evaluator reads. + const stored = await Alert.findById(created.body.data.id); + expect(stored!.chartConfig).toMatchObject({ + name: 'Error Rate Query', + where: 'ServiceName:api', + whereLanguage: 'lucene', + }); + + const single = await authRequest( + 'get', + `${ALERTS_BASE_URL}/${created.body.data.id}`, + ).expect(200); + expect(single.body.data.chartConfig).toMatchObject({ + name: 'Error Rate Query', + where: 'ServiceName:api', + }); + + // Echoing the GET body through PUT keeps them intact — the + // read-modify-write loop must not degrade notification titles or + // silently broaden the alert's query. + await authRequest('put', `${ALERTS_BASE_URL}/${created.body.data.id}`) + .send(single.body.data) + .expect(200); + const storedAfter = await Alert.findById(created.body.data.id); + expect(storedAfter!.chartConfig).toEqual(stored!.chartConfig); + }); + + it('rejects a chart-level where on raw SQL configs', async () => { + const { connection } = await makeSource(); + const webhook = await createTestWebhook(); + + const res = await authRequest('post', ALERTS_BASE_URL) + .send( + makeInlineAlertInput( + { + configType: 'sql', + displayType: 'line', + sqlTemplate: RAW_SQL_ALERT_TEMPLATE, + connectionId: connection._id.toString(), + where: 'ServiceName:api', + }, + webhook._id.toString(), + ), + ) + .expect(400); + expect(res.body.message).toContain( + 'Raw SQL alert configs have no chart-level where', + ); + }); + + it('omits chartConfig for internally-authored configs the dialect cannot express, and a blind echo-PUT fails loudly', async () => { + // An alert created through the internal API can persist fields the + // external dialect has no spelling for (chart-level `filters` here). + // Emitting a lossy approximation would let a GET -> PUT loop silently + // broaden the alert's query, so the GET omits chartConfig entirely and + // re-PUTting the echoed body 400s on the missing config. + const { source } = await makeSource(); + + const alert = await createTestAlertDirectly({ + source: AlertSource.INLINE, + dashboardId: undefined, + tileId: undefined, chartConfig: { - name: 'Chart Alert Query', - source: new ObjectId().toString(), displayType: 'line', + source: source._id.toString(), select: [ { aggFn: 'count', @@ -682,17 +1460,143 @@ describe('External API Alerts', () => { ], where: '', whereLanguage: 'lucene', + filters: [{ type: 'sql', condition: "ServiceName = 'api'" }], }, - thresholdType: AlertThresholdType.ABOVE, - channel: { - type: 'webhook', - webhookId: webhook._id.toString(), + }); + + const single = await authRequest( + 'get', + `${ALERTS_BASE_URL}/${alert._id}`, + ).expect(200); + expect(single.body.data.source).toBe(AlertSource.INLINE); + expect(single.body.data.chartConfig).toBeUndefined(); + + await authRequest('put', `${ALERTS_BASE_URL}/${alert._id}`) + .send(single.body.data) + .expect(400); + + // The stored config is untouched by the failed write. + const stored = await Alert.findById(alert._id); + expect(stored!.chartConfig).toMatchObject({ + filters: [{ type: 'sql', condition: "ServiceName = 'api'" }], + }); + }); + + it('omits chartConfig when the config would convert lossily or violate the external write schema', async () => { + // Two classes the key-classification gate alone cannot catch: + // an array groupBy (the tile converter silently drops it, so an + // echo-PUT would strip the grouping from a live alert) and an + // aggCondition beyond the external cap (converts cleanly but the + // emitted body would 400 on echo-PUT). Both are legal internally. + const { source } = await makeSource(); + + const configs = [ + { + label: 'array groupBy', + chartConfig: { + displayType: 'line', + source: source._id.toString(), + select: [ + { + aggFn: 'count', + aggCondition: '', + aggConditionLanguage: 'lucene', + valueExpression: '', + }, + ], + where: '', + whereLanguage: 'lucene', + groupBy: [{ valueExpression: 'ServiceName' }], + }, }, - }; + { + label: 'over-cap aggCondition', + chartConfig: { + displayType: 'line', + source: source._id.toString(), + select: [ + { + aggFn: 'count', + aggCondition: 'a'.repeat(10001), + aggConditionLanguage: 'lucene', + valueExpression: '', + }, + ], + where: '', + whereLanguage: 'lucene', + }, + }, + ]; + + for (const { chartConfig } of configs) { + const alert = await createTestAlertDirectly({ + source: AlertSource.INLINE, + dashboardId: undefined, + tileId: undefined, + chartConfig, + }); + + const single = await authRequest( + 'get', + `${ALERTS_BASE_URL}/${alert._id}`, + ).expect(200); + expect(single.body.data.chartConfig).toBeUndefined(); - await authRequest('post', ALERTS_BASE_URL).send(alertInput).expect(400); + // The echoed body has no chartConfig, so the write fails loudly + // instead of persisting a rewritten query. + await authRequest('put', `${ALERTS_BASE_URL}/${alert._id}`) + .send(single.body.data) + .expect(400); + + const stored = await Alert.findById(alert._id); + expect(stored!.chartConfig).toEqual(chartConfig); + } }); + it('emits chartConfig for internally-authored configs the dialect can express', async () => { + // Guards against over-refusal: the shapes the UI's chart explorer + // persists (name, empty chart-level where, count select, and its + // always-written granularity: 'auto' — evaluation derives the bucket + // size from alert.interval) must keep round-tripping. + const { source } = await makeSource(); + + const alert = await createTestAlertDirectly({ + source: AlertSource.INLINE, + dashboardId: undefined, + tileId: undefined, + chartConfig: { + name: 'Chart Alert Query', + displayType: 'line', + source: source._id.toString(), + select: [ + { + aggFn: 'count', + aggCondition: 'level:error', + aggConditionLanguage: 'lucene', + valueExpression: '', + }, + ], + where: '', + whereLanguage: 'lucene', + seriesReturnType: 'column', + granularity: 'auto', + }, + }); + + const single = await authRequest( + 'get', + `${ALERTS_BASE_URL}/${alert._id}`, + ).expect(200); + expect(single.body.data.chartConfig).toMatchObject({ + displayType: 'line', + name: 'Chart Alert Query', + sourceId: source._id.toString(), + select: [{ aggFn: 'count', where: 'level:error' }], + }); + }); + }); + + describe('Input validation', () => { describe('webhook validation', () => { it('should reject a non-existent webhook', async () => { const dashboard = await createTestDashboard(); diff --git a/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts b/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts index 2558cf609d..2d25aa4506 100644 --- a/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts +++ b/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts @@ -6603,6 +6603,18 @@ describe('External API v2 Dashboards - new format', () => { ); }); + it('rejects an unsupported configType instead of routing it to the builder dialect', async () => { + // A body carrying an unrecognized configType parses fine against the + // builder union (the unknown keys are stripped), so without an explicit + // check it would persist as a builder tile AND skip the builder-only + // rules — here a formula referencing a nonexistent series. + const res = await postTile({ + configType: 'promql', + formulas: [{ expression: 'C * 2' }], + }).expect(400); + expect(res.body.message).toContain('configType must be "sql" or omitted'); + }); + it('round-trips formulas on a log/trace event source', async () => { // Event-source formulas are accepted and round-trip just like // metric formulas. diff --git a/packages/api/src/routers/external-api/v2/alerts.ts b/packages/api/src/routers/external-api/v2/alerts.ts index 693d21d64b..a1b44d7468 100644 --- a/packages/api/src/routers/external-api/v2/alerts.ts +++ b/packages/api/src/routers/external-api/v2/alerts.ts @@ -11,6 +11,11 @@ import { updateAlert, validateAlertInput, } from '@/controllers/alerts'; +import { AlertSource } from '@/models/alert'; +import { + convertExternalAlertChartConfigToInternal, + translateAlertDocumentToExternalAlertWithChartConfig, +} from '@/routers/external-api/v2/utils/alertChartConfig'; import { processRequestWithEnhancedErrors as processRequest, validateRequestWithEnhancedErrors as validateRequest, @@ -21,7 +26,29 @@ import { paginationMeta, paginationQuerySchema, } from '@/utils/pagination'; -import { alertSchema, objectIdSchema } from '@/utils/zod'; +import { + alertSchema, + ExternalAlertInput, + InternalAlertInput, + objectIdSchema, +} from '@/utils/zod'; + +/** + * Maps a parsed external alert request body onto the internal alert input + * shape: inline alerts arrive with a chartConfig in the external tile-config + * dialect and are converted to the internal AlertChartConfig the controllers + * and check-alerts task operate on. + */ +function toInternalAlertInput(body: ExternalAlertInput): InternalAlertInput { + if (body.source === AlertSource.INLINE) { + const { chartConfig, ...rest } = body; + return { + ...rest, + chartConfig: convertExternalAlertChartConfigToInternal(chartConfig), + }; + } + return body; +} /** * @openapi @@ -44,8 +71,63 @@ import { alertSchema, objectIdSchema } from '@/utils/zod'; * description: Threshold comparison direction. * AlertSource: * type: string - * enum: [saved_search, tile] - * description: Alert source type. + * enum: [saved_search, tile, inline] + * description: > + * Alert source type. "saved_search" alerts monitor a saved search, + * "tile" alerts monitor a dashboard tile, and "inline" alerts carry + * their own chart configuration (see AlertChartConfig) without + * requiring a saved search or dashboard. + * AlertChartConfigOverlay: + * type: object + * description: > + * Fields an alert's chart config carries on top of the dashboard tile + * config dialect. Unlike a tile (whose name lives on the tile, not + * its config), an inline alert's config is standalone. + * properties: + * name: + * type: string + * description: > + * Display name for the alert query. Used in notification titles + * and as an alert-name fallback when the alert itself has no + * name. + * example: "Error Rate Query" + * where: + * type: string + * maxLength: 10000 + * description: > + * Chart-level filter applied on top of every select item's own + * "where" (combined via AND). Builder variants only; rejected on + * Raw SQL variants (filter inside the sqlTemplate instead). + * example: "ServiceName:api" + * whereLanguage: + * $ref: '#/components/schemas/QueryLanguage' + * description: Language of the chart-level "where" filter. + * AlertLineChartConfig: + * allOf: + * - $ref: '#/components/schemas/LineChartConfig' + * - $ref: '#/components/schemas/AlertChartConfigOverlay' + * AlertBarChartConfig: + * allOf: + * - $ref: '#/components/schemas/BarChartConfig' + * - $ref: '#/components/schemas/AlertChartConfigOverlay' + * AlertNumberChartConfig: + * allOf: + * - $ref: '#/components/schemas/NumberChartConfig' + * - $ref: '#/components/schemas/AlertChartConfigOverlay' + * AlertChartConfig: + * description: > + * The chart configuration an inline alert evaluates, in the same + * dialect as dashboard tile configs plus the alert-only fields in + * AlertChartConfigOverlay. Only the display types the alert evaluator + * supports are accepted: line, stacked_bar, and number, in both + * builder and Raw SQL (configType "sql") variants. Raw SQL templates + * must reference the evaluation window via the time-filter and + * interval macros (e.g. $__timeFilter and $__timeInterval), and the + * referenced source/connection must belong to the team. + * oneOf: + * - $ref: '#/components/schemas/AlertLineChartConfig' + * - $ref: '#/components/schemas/AlertBarChartConfig' + * - $ref: '#/components/schemas/AlertNumberChartConfig' * AlertState: * type: string * enum: [ALERT, OK, INSUFFICIENT_DATA, DISABLED, PENDING] @@ -148,6 +230,12 @@ import { alertSchema, objectIdSchema } from '@/utils/zod'; * description: Group-by key for saved search alerts. * nullable: true * example: "ServiceName" + * chartConfig: + * $ref: '#/components/schemas/AlertChartConfig' + * description: > + * Chart configuration for inline alerts. Required when source is + * "inline" and rejected otherwise. Returned on single-alert + * responses (GET by ID, POST, PUT); the list endpoint omits it. * threshold: * type: number * description: Threshold value for triggering the alert. For between and not_between threshold types, this is the lower bound. @@ -395,7 +483,7 @@ router.get( } return res.json({ - data: translateAlertDocumentToExternalAlert(alert), + data: translateAlertDocumentToExternalAlertWithChartConfig(alert), }); } catch (e) { next(e); @@ -551,6 +639,24 @@ router.get( * - type: "webhook" * webhookId: "65f5e4a3b9e77c001a789013" * name: "Error Spike Alert" + * inlineAlert: + * summary: Create an inline alert that carries its own chart config + * value: + * source: "inline" + * chartConfig: + * displayType: "line" + * sourceId: "65f5e4a3b9e77c001a123456" + * select: + * - aggFn: "count" + * where: "level:error" + * whereLanguage: "lucene" + * threshold: 100 + * interval: "5m" + * thresholdType: "above" + * channel: + * type: "webhook" + * webhookId: "65f5e4a3b9e77c001a789012" + * name: "Error log spike" * responses: * '200': * description: Successfully created alert @@ -589,13 +695,15 @@ router.post( return res.status(403).json({ message: 'Forbidden' }); } try { - const alertInput = req.body; + const alertInput = toInternalAlertInput(req.body); await validateAlertInput(teamId, alertInput); const createdAlert = await createAlert(teamId, alertInput, userId); return res.json({ - data: translateAlertDocumentToExternalAlert(createdAlert), + data: translateAlertDocumentToExternalAlertWithChartConfig( + createdAlert, + ), }); } catch (e) { next(e); @@ -689,7 +797,7 @@ router.put( } const { id } = req.params; - const alertInput = req.body; + const alertInput = toInternalAlertInput(req.body); await validateAlertInput(teamId, alertInput); const alert = await updateAlert(id, teamId, alertInput); @@ -699,7 +807,7 @@ router.put( } res.json({ - data: translateAlertDocumentToExternalAlert(alert), + data: translateAlertDocumentToExternalAlertWithChartConfig(alert), }); } catch (e) { next(e); diff --git a/packages/api/src/routers/external-api/v2/utils/__tests__/alertChartConfig.test.ts b/packages/api/src/routers/external-api/v2/utils/__tests__/alertChartConfig.test.ts new file mode 100644 index 0000000000..c4da87435f --- /dev/null +++ b/packages/api/src/routers/external-api/v2/utils/__tests__/alertChartConfig.test.ts @@ -0,0 +1,616 @@ +import { + AlertChartConfig, + AlertChartConfigSchema, + DisplayType, +} from '@hyperdx/common-utils/dist/types'; + +import { + convertAlertChartConfigToExternal, + convertExternalAlertChartConfigToInternal, + EVALUATION_INERT_CONFIG_KEYS, + EXTERNAL_TO_INTERNAL_KEY, + KNOWN_LOSSY_CONFIG_KEYS, +} from '@/routers/external-api/v2/utils/alertChartConfig'; +import { + externalAlertBuilderChartConfigSchema, + ExternalAlertChartConfig, + externalAlertChartConfigSchema, + externalAlertRawSqlChartConfigSchema, +} from '@/utils/zod'; + +const SOURCE_ID = '65f5e4a3b9e77c001a123456'; +const CONNECTION_ID = '65f5e4a3b9e77c001a789012'; + +const countItem = (where = '') => ({ + aggFn: 'count' as const, + where, + whereLanguage: 'lucene' as const, + valueExpression: '', +}); + +const baseInternal = ( + overrides: Partial> = {}, +): AlertChartConfig => + ({ + displayType: DisplayType.Line, + source: SOURCE_ID, + select: [ + { + aggFn: 'count', + aggCondition: '', + aggConditionLanguage: 'lucene', + valueExpression: '', + }, + ], + where: '', + whereLanguage: 'lucene', + ...overrides, + }) as AlertChartConfig; + +describe('externalAlertChartConfigSchema configType routing', () => { + const builderBody = (overrides: Record = {}) => ({ + displayType: DisplayType.Line, + sourceId: SOURCE_ID, + select: [{ aggFn: 'count', where: '' }], + ...overrides, + }); + + const messages = (body: unknown) => { + const result = externalAlertChartConfigSchema.safeParse(body); + return result.success + ? [] + : result.error.issues.map(i => `${i.path.join('.')}: ${i.message}`); + }; + + it('rejects a configType the router does not recognize', () => { + // Routing keys on configType === 'sql', so an unrecognized value would + // otherwise parse as a builder config (unknown keys stripped) and skip + // the builder-only rules below. + expect(messages(builderBody({ configType: 'promql' }))).toEqual([ + 'configType: configType must be "sql" or omitted (builder configs carry no configType)', + ]); + }); + + it('still applies the builder rules to a config carrying an unsupported configType', () => { + // The bypass this guards: both bodies parse against the builder union. + expect( + messages( + builderBody({ + configType: 'promql', + formulas: [{ expression: 'B * 2' }], + }), + ).join(' '), + ).toContain('configType must be "sql" or omitted'); + expect( + messages( + builderBody({ + configType: 'promql', + displayType: DisplayType.Number, + select: [ + { aggFn: 'count', where: '' }, + { aggFn: 'count', where: 'level:error' }, + ], + }), + ).join(' '), + ).toContain('configType must be "sql" or omitted'); + }); + + it('applies the builder rules when configType is omitted', () => { + expect( + messages(builderBody({ formulas: [{ expression: 'B * 2' }] })).join(' '), + ).toContain('Unknown series "B"'); + expect( + messages( + builderBody({ + displayType: DisplayType.Number, + select: [ + { aggFn: 'count', where: '' }, + { aggFn: 'count', where: 'level:error' }, + ], + }), + ).join(' '), + ).toContain('Number charts support a single select item'); + }); + + it('accepts the raw SQL dialect', () => { + expect( + messages({ + configType: 'sql', + displayType: DisplayType.Line, + connectionId: CONNECTION_ID, + sqlTemplate: + 'SELECT $__timeInterval(Timestamp) AS ts, count() FROM t WHERE $__timeFilter(Timestamp) GROUP BY ts', + }), + ).toEqual([]); + }); +}); + +describe('convertExternalAlertChartConfigToInternal', () => { + it('maps the external dialect onto the internal one', () => { + const internal = convertExternalAlertChartConfigToInternal({ + displayType: DisplayType.Line, + sourceId: SOURCE_ID, + name: 'Error Rate Query', + where: 'ServiceName:api', + whereLanguage: 'lucene', + groupBy: 'ServiceName', + asRatio: true, + select: [ + { + aggFn: 'avg', + valueExpression: 'Value', + where: 'level:error', + whereLanguage: 'lucene', + alias: 'CPU', + metricType: 'gauge', + metricName: 'system.cpu.utilization', + periodAggFn: 'delta', + }, + countItem(), + ], + } as ExternalAlertChartConfig); + + expect(internal).toMatchObject({ + displayType: DisplayType.Line, + source: SOURCE_ID, + name: 'Error Rate Query', + where: 'ServiceName:api', + whereLanguage: 'lucene', + groupBy: 'ServiceName', + seriesReturnType: 'ratio', + select: [ + { + aggFn: 'avg', + valueExpression: 'Value', + aggCondition: 'level:error', + aggConditionLanguage: 'lucene', + alias: 'CPU', + metricType: 'gauge', + metricName: 'system.cpu.utilization', + isDelta: true, + }, + { aggFn: 'count', aggCondition: '', isDelta: false }, + ], + }); + expect('alert' in internal).toBe(false); + }); + + it('omits the synthetic tile name and never persists an alert field', () => { + const internal = convertExternalAlertChartConfigToInternal({ + displayType: DisplayType.Line, + sourceId: SOURCE_ID, + select: [countItem()], + } as ExternalAlertChartConfig); + + expect('name' in internal).toBe(false); + expect('alert' in internal).toBe(false); + }); +}); + +describe('external <-> internal round-trip', () => { + it('round-trips a builder line config, including alert-only and metric fields', () => { + const external = { + displayType: DisplayType.Line, + sourceId: SOURCE_ID, + name: 'Error Rate Query', + where: 'ServiceName:api', + whereLanguage: 'lucene' as const, + groupBy: 'ServiceName', + asRatio: true, + seriesLimit: 5, + fillNulls: false, + numberFormat: { output: 'number' as const }, + select: [ + { + aggFn: 'avg' as const, + valueExpression: 'Value', + where: 'level:error', + whereLanguage: 'lucene' as const, + alias: 'CPU', + metricType: 'gauge' as const, + metricName: 'system.cpu.utilization', + periodAggFn: 'delta' as const, + }, + countItem(), + ], + } as ExternalAlertChartConfig; + + const roundTripped = convertAlertChartConfigToExternal( + convertExternalAlertChartConfigToInternal(external), + ); + + expect(roundTripped).toEqual(external); + }); + + it('round-trips a quantile select item with its level', () => { + const external = { + displayType: DisplayType.StackedBar, + sourceId: SOURCE_ID, + select: [ + { + aggFn: 'quantile' as const, + level: 0.95 as const, + valueExpression: 'Duration', + where: '', + whereLanguage: 'lucene' as const, + }, + ], + } as ExternalAlertChartConfig; + + const roundTripped = convertAlertChartConfigToExternal( + convertExternalAlertChartConfigToInternal(external), + ); + + expect(roundTripped).toMatchObject({ + displayType: DisplayType.StackedBar, + select: [{ aggFn: 'quantile', level: 0.95, valueExpression: 'Duration' }], + }); + }); + + it('round-trips a raw SQL config', () => { + const external = { + configType: 'sql' as const, + displayType: DisplayType.Line, + connectionId: CONNECTION_ID, + sourceId: SOURCE_ID, + sqlTemplate: 'SELECT $__timeInterval(Timestamp) AS ts, count() ...', + name: 'Raw SQL Alert Query', + fillNulls: true, + } as ExternalAlertChartConfig; + + const roundTripped = convertAlertChartConfigToExternal( + convertExternalAlertChartConfigToInternal(external), + ); + + expect(roundTripped).toEqual(external); + }); +}); + +describe('convertAlertChartConfigToExternal', () => { + it('emits a number config', () => { + const external = convertAlertChartConfigToExternal( + baseInternal({ + displayType: DisplayType.Number, + select: [ + { + aggFn: 'count', + aggCondition: '', + aggConditionLanguage: 'lucene', + valueExpression: '', + }, + ], + }), + ); + + expect(external).toMatchObject({ + displayType: DisplayType.Number, + sourceId: SOURCE_ID, + }); + }); + + it('still emits configs carrying evaluation-inert extras', () => { + const external = convertAlertChartConfigToExternal( + baseInternal({ + seriesReturnType: 'column', + fillNulls: 0, + markdown: '', + groupByColumnsOnLeft: true, + compareToPreviousPeriod: true, + color: 'chart-blue', + select: [ + { + aggFn: 'count', + aggCondition: '', + aggConditionLanguage: 'lucene', + valueExpression: '', + color: 'chart-red', + }, + ], + }), + ); + + expect(external).toMatchObject({ displayType: DisplayType.Line }); + }); + + it('still emits configs carrying a stored granularity', () => { + // The chart explorer persists granularity: 'auto' on every inline alert + // it authors, and the alert task derives the real bucket size from + // alert.interval — refusing on it would blank chartConfig for the most + // common authoring path. The field is dropped (the external dialect has + // no spelling for it), which only affects the unattached chart-editor + // preview. + for (const granularity of ['auto', '5 minute']) { + const external = convertAlertChartConfigToExternal( + baseInternal({ granularity }), + ); + expect(external).toMatchObject({ displayType: DisplayType.Line }); + expect(external).not.toHaveProperty('granularity'); + } + }); + + it.each([ + [ + 'filters', + { filters: [{ type: 'sql', condition: "ServiceName = 'api'" }] }, + ], + ['having', { having: 'count() > 5', havingLanguage: 'sql' }], + ['orderBy', { orderBy: 'Timestamp DESC' }], + ['limit', { limit: { limit: 10 } }], + ['ratioMode', { ratioMode: 'total' }], + ['selectGroupBy', { selectGroupBy: true }], + ['implicitColumnExpression', { implicitColumnExpression: 'Body' }], + ['sampleWeightExpression', { sampleWeightExpression: 'SampleRate' }], + ['a raw SQL select expression string', { select: 'count() as count' }], + [ + 'an unknown future field', + { someFutureEvaluationKnob: 'on' } as Record, + ], + ])('refuses to emit a config carrying %s', (_label, overrides) => { + expect( + convertAlertChartConfigToExternal(baseInternal(overrides)), + ).toBeUndefined(); + }); + + // The internal schema leaves these strings uncapped / this shape legal; + // the post-conversion parse against externalAlertChartConfigSchema (the + // exact schema PUT enforces) is what refuses them, so an emitted body is + // valid PUT input by construction. + it('refuses a chart-level where longer than the external cap', () => { + expect( + convertAlertChartConfigToExternal( + baseInternal({ where: 'a'.repeat(10001) }), + ), + ).toBeUndefined(); + }); + + it('refuses select-item strings longer than the external caps', () => { + expect( + convertAlertChartConfigToExternal( + baseInternal({ + select: [ + { + aggFn: 'count', + aggCondition: 'a'.repeat(10001), + aggConditionLanguage: 'lucene', + valueExpression: '', + }, + ], + }), + ), + ).toBeUndefined(); + + expect( + convertAlertChartConfigToExternal( + baseInternal({ + select: [ + { + aggFn: 'avg', + aggCondition: '', + aggConditionLanguage: 'lucene', + valueExpression: 'a'.repeat(10001), + }, + ], + }), + ), + ).toBeUndefined(); + + expect( + convertAlertChartConfigToExternal( + baseInternal({ + select: [ + { + aggFn: 'count', + aggCondition: '', + aggConditionLanguage: 'lucene', + valueExpression: '', + alias: 'a'.repeat(10001), + }, + ], + }), + ), + ).toBeUndefined(); + }); + + it('refuses a non-count aggregation with an empty valueExpression', () => { + // Legal internally, but the external select-item schema requires a value + // expression for non-count aggregations — emitting it would produce a + // body that 400s on echo-PUT. + expect( + convertAlertChartConfigToExternal( + baseInternal({ + select: [ + { + aggFn: 'avg', + aggCondition: '', + aggConditionLanguage: 'lucene', + valueExpression: '', + }, + ], + }), + ), + ).toBeUndefined(); + }); + + it('refuses an array groupBy the tile dialect cannot carry', () => { + // renderSelectList evaluates array groupBys, so the alert task honors + // them; the tile converter silently omits the field, which would let an + // echo-PUT strip the grouping from a live alert. + expect( + convertAlertChartConfigToExternal( + baseInternal({ + groupBy: [{ valueExpression: 'ServiceName' }], + }), + ), + ).toBeUndefined(); + + // The string form the tile dialect does carry still round-trips. + expect( + convertAlertChartConfigToExternal( + baseInternal({ groupBy: 'ServiceName' }), + ), + ).toMatchObject({ groupBy: 'ServiceName' }); + }); + + it('refuses a select item whose aggFn the external enum cannot express', () => { + // The internal schema admits aggregations the external one does not + // (histogram, quantileMerge, the *Merge combinators). + // convertToExternalSelectItem would emit aggFn: 'none' for these, so a + // GET -> PUT would silently rewrite the aggregation. + for (const aggFn of ['sumIfMerge', 'quantileMerge', 'histogram']) { + expect( + convertAlertChartConfigToExternal( + baseInternal({ + select: [ + { + aggFn, + aggCondition: '', + aggConditionLanguage: 'lucene', + valueExpression: 'Value', + ...(aggFn === 'quantileMerge' ? { level: 0.95 } : {}), + }, + ], + }), + ), + ).toBeUndefined(); + } + }); + + it('refuses a quantile level outside the external set', () => { + // The internal `level` is any number; the external dialect admits only + // 0.5/0.9/0.95/0.99, and the converter drops anything else — emitting a + // quantile with no level. + expect( + convertAlertChartConfigToExternal( + baseInternal({ + select: [ + { + aggFn: 'quantile', + level: 0.75, + aggCondition: '', + aggConditionLanguage: 'lucene', + valueExpression: 'Duration', + }, + ], + }), + ), + ).toBeUndefined(); + + // A level the dialect does admit still round-trips. + expect( + convertAlertChartConfigToExternal( + baseInternal({ + select: [ + { + aggFn: 'quantile', + level: 0.95, + aggCondition: '', + aggConditionLanguage: 'lucene', + valueExpression: 'Duration', + }, + ], + }), + ), + ).toMatchObject({ select: [{ aggFn: 'quantile', level: 0.95 }] }); + }); + + it('refuses an empty select instead of throwing on a number config', () => { + // A persisted number config with select: [] used to TypeError in the + // tile converter (select[0] is undefined) and surface as a 500 from + // GET /api/v2/alerts/:id. + expect( + convertAlertChartConfigToExternal( + baseInternal({ displayType: DisplayType.Number, select: [] }), + ), + ).toBeUndefined(); + expect( + convertAlertChartConfigToExternal(baseInternal({ select: [] })), + ).toBeUndefined(); + }); + + it('refuses more than 20 select items', () => { + expect( + convertAlertChartConfigToExternal( + baseInternal({ + select: Array.from({ length: 21 }, () => ({ + aggFn: 'count', + aggCondition: '', + aggConditionLanguage: 'lucene', + valueExpression: '', + })), + }), + ), + ).toBeUndefined(); + }); + + it('refuses a formula-less number config with multiple select items', () => { + expect( + convertAlertChartConfigToExternal( + baseInternal({ + displayType: DisplayType.Number, + select: [ + { + aggFn: 'count', + aggCondition: '', + aggConditionLanguage: 'lucene', + valueExpression: '', + }, + { + aggFn: 'count', + aggCondition: 'level:error', + aggConditionLanguage: 'lucene', + valueExpression: '', + }, + ], + }), + ), + ).toBeUndefined(); + }); + + it('refuses unsupported display types and PromQL configs', () => { + expect( + convertAlertChartConfigToExternal( + baseInternal({ displayType: DisplayType.Table }), + ), + ).toBeUndefined(); + expect( + convertAlertChartConfigToExternal({ + configType: 'promql', + promqlQuery: 'up', + source: SOURCE_ID, + } as unknown as AlertChartConfig), + ).toBeUndefined(); + }); +}); + +// Drift guard: every field the internal alert chart config can persist must +// be deliberately classified as externally representable, evaluation-inert, +// or known-lossy. A new field on the internal schema breaks this test and +// forces a decision instead of silently leaking through the external +// round-trip (unknown fields refuse at runtime either way — this test is +// about intent, not safety). +describe('internal schema field classification', () => { + const representableInternalKeys = new Set( + [ + ...externalAlertBuilderChartConfigSchema.options, + ...externalAlertRawSqlChartConfigSchema.options, + ].flatMap(member => + Object.keys(member.shape).map( + key => EXTERNAL_TO_INTERNAL_KEY[key] ?? key, + ), + ), + ); + + it.each( + AlertChartConfigSchema.options.flatMap((member, i) => + Object.keys(member.shape).map( + key => [i === 0 ? 'builder' : 'raw SQL', key] as const, + ), + ), + )('classifies the internal %s config field "%s"', (_variant, key) => { + const classified = + representableInternalKeys.has(key) || + EVALUATION_INERT_CONFIG_KEYS.has(key) || + KNOWN_LOSSY_CONFIG_KEYS.has(key); + expect(classified).toBe(true); + }); +}); diff --git a/packages/api/src/routers/external-api/v2/utils/alertChartConfig.ts b/packages/api/src/routers/external-api/v2/utils/alertChartConfig.ts new file mode 100644 index 0000000000..51bf1ac86c --- /dev/null +++ b/packages/api/src/routers/external-api/v2/utils/alertChartConfig.ts @@ -0,0 +1,402 @@ +import { + AggregateFunctionSchema, + AlertChartConfig, + DisplayType, +} from '@hyperdx/common-utils/dist/types'; +import { omit } from 'lodash'; + +import { AlertDocument, AlertSource, IAlert } from '@/models/alert'; +import { + ExternalAlert, + translateAlertDocumentToExternalAlert, +} from '@/utils/externalApi'; +import logger from '@/utils/logger'; +import { + externalAlertBuilderChartConfigSchema, + ExternalAlertChartConfig, + externalAlertChartConfigSchema, + externalAlertRawSqlChartConfigSchema, + externalQuantileLevelSchema, +} from '@/utils/zod'; + +import { + convertToExternalTileChartConfig, + convertToInternalTileConfig, +} from './dashboards'; + +/** + * Converts an inline alert's external-dialect chart config (the tile-config + * dialect v2 dashboards use: `sourceId`, per-select `where`, `asRatio`, + * `connectionId` + `sqlTemplate`) into the internal AlertChartConfig shape the + * alert document persists and the check-alerts task evaluates. + * + * Reuses the dashboard tile converter through a synthetic tile so the two + * surfaces cannot drift on field mapping (`where` -> `aggCondition`, + * `asRatio` -> `seriesReturnType`, `connectionId` -> `connection`, fillNulls + * defaulting, ...). Layout fields are placeholders — only `config` is kept. + * The alert-only fields the tile dialect lacks (`name`, chart-level + * `where`/`whereLanguage`) are threaded through on top. + */ +export function convertExternalAlertChartConfigToInternal( + external: ExternalAlertChartConfig, +): AlertChartConfig { + const { config } = convertToInternalTileConfig({ + id: '', + // The tile converter persists the tile's name into the config, which is + // exactly where an alert's config name lives. + name: external.name ?? '', + x: 0, + y: 0, + w: 1, + h: 1, + config: external, + }); + // The converter never emits a PromQL variant or an embedded `alert` for the + // display types the alert schema admits, so dropping `alert` yields a valid + // AlertChartConfig. + const internal = omit(config, ['alert']) as AlertChartConfig; + if (!external.name) { + // Synthetic-tile artifact: the converter wrote name: ''. + delete internal.name; + } + if (!('configType' in external) && !('configType' in internal)) { + // The tile converter hardcodes where: '' (the tile dialect has no + // chart-level filter); the alert dialect does, so apply it. + if (external.where) { + internal.where = external.where; + internal.whereLanguage = external.whereLanguage ?? 'lucene'; + } + } + return internal; +} + +const EXTERNAL_ALERT_DISPLAY_TYPES = [ + DisplayType.Line, + DisplayType.StackedBar, + DisplayType.Number, +] as const; + +type ExternalAlertDisplayType = (typeof EXTERNAL_ALERT_DISPLAY_TYPES)[number]; + +/** + * External dialect spelling -> internal spelling for top-level config keys. + * Exported for the schema drift-guard test. + */ +export const EXTERNAL_TO_INTERNAL_KEY: Record = { + sourceId: 'source', + connectionId: 'connection', + asRatio: 'seriesReturnType', +}; + +/** + * Internal fields the external round-trip drops but which have no effect on + * how the check-alerts task evaluates the config (render/UI-only concerns). + * Losing them on an external GET -> PUT does not change alert behavior. + * Exported for the schema drift-guard test. + */ +export const EVALUATION_INERT_CONFIG_KEYS: ReadonlySet = new Set([ + 'markdown', + // Only meaningful together with `filters`, which is itself lossy. + 'filtersLogicalOperator', + 'groupByColumnsOnLeft', + 'color', + 'colorRules', + 'backgroundChart', + 'compareToPreviousPeriod', + 'fitYAxisToData', + 'onClick', + 'alternateRowBackground', + // Client-side null-bucket rendering; the alert task ignores it. + 'fillNulls', + // Evaluation-relevant only with a groupBy, which (on variants whose shape + // lacks seriesLimit, i.e. number) is itself refused when present. + 'seriesLimit', + // The alert task derives granularity from alert.interval + // (tasks/checkAlerts/index.ts: `${windowSizeInMins} minute`) and the alert + // detail chart from intervalToGranularity, so the stored value only affects + // the unattached chart-editor preview. The chart explorer persists + // granularity: 'auto' on every inline alert it authors — refusing on it + // would blank chartConfig for the most common authoring path. + 'granularity', +]); + +/** + * Internal fields that DO affect evaluation and that the external dialect + * cannot express. A non-empty value on any of these makes the config + * unrepresentable, so the GET response omits chartConfig and a blind echo-PUT + * fails loudly ("chartConfig is required") instead of silently rewriting the + * alert's query. Exported for the schema drift-guard test, which asserts every + * field of the internal AlertChartConfigSchema is deliberately classified. + */ +export const KNOWN_LOSSY_CONFIG_KEYS: ReadonlySet = new Set([ + 'filters', + 'having', + 'havingLanguage', + 'orderBy', + 'limit', + 'ratioMode', + 'selectGroupBy', + 'implicitColumnExpression', + 'sampleWeightExpression', + 'eventTableSelect', + 'bodyExpression', + 'useTextIndexForImplicitColumn', + 'metricTables', +]); + +/** Select-item fields the external dialect round-trips. */ +const REPRESENTABLE_SELECT_ITEM_KEYS: ReadonlySet = new Set([ + 'aggFn', + 'aggCondition', + 'aggConditionLanguage', + 'valueExpression', + 'alias', + 'level', + 'metricType', + 'metricName', + 'numberFormat', + 'isDelta', +]); + +/** + * Select-item fields that are inert on line/stacked_bar/number charts + * (heatmap- or table-cell-only affordances) and may be dropped. + */ +const EVALUATION_INERT_SELECT_ITEM_KEYS: ReadonlySet = new Set([ + 'color', + 'colorRules', + 'countExpression', + 'heatmapScaleType', +]); + +const isEmptyValue = (value: unknown): boolean => + value === undefined || + value === null || + value === '' || + (Array.isArray(value) && value.length === 0); + +/** Values equivalent to the field being absent. */ +const isDefaultValue = (key: string, value: unknown): boolean => { + switch (key) { + case 'seriesReturnType': + return value === 'column'; + case 'whereLanguage': + case 'aggConditionLanguage': + case 'havingLanguage': + return value === 'lucene'; + default: + // Persisted boolean flags are off-by-default; fillNulls (number|false) + // is evaluation-inert and classified above, so `false` here only ever + // describes a flag in its default state. + return value === false; + } +}; + +/** + * Whether the persisted internal config survives an external GET -> PUT + * round-trip without changing what the alert evaluates. + * + * This gate only covers what the post-conversion + * `externalAlertChartConfigSchema.safeParse` in + * `convertAlertChartConfigToExternal` cannot: SILENT losses, where the tile + * converter drops, substitutes, or crashes on an internal value and the + * converted output then parses cleanly (an inexpressible field vanishing, an + * out-of-enum aggFn becoming 'none', an array groupBy becoming no groupBy). + * Value-level contract rules the converter passes through verbatim (string + * caps, select-item shape, formula rules) are deliberately NOT re-implemented + * here — the final parse enforces them from the single source of truth. + * + * Allowlist semantics: unknown future fields refuse by default rather than + * leaking through silently. + */ +function isRepresentableExternally( + config: AlertChartConfig, + displayType: ExternalAlertDisplayType, +): boolean { + const isRawSql = 'configType' in config && config.configType === 'sql'; + const memberSchema = ( + isRawSql + ? externalAlertRawSqlChartConfigSchema + : externalAlertBuilderChartConfigSchema + ).optionsMap.get(displayType); + if (memberSchema == null) { + return false; + } + // The representable set is derived from the external member schema itself, + // so it cannot drift when the external dialect gains fields. + const representableKeys = new Set( + Object.keys(memberSchema.shape).map( + key => EXTERNAL_TO_INTERNAL_KEY[key] ?? key, + ), + ); + + for (const [key, value] of Object.entries(config)) { + if (isEmptyValue(value) || isDefaultValue(key, value)) continue; + if (representableKeys.has(key)) continue; + if (EVALUATION_INERT_CONFIG_KEYS.has(key)) continue; + return false; + } + + if (isRawSql) { + return true; + } + + // The internal groupBy is a select list (string or array of derived + // columns) and the alert task evaluates both forms via renderSelectList; + // the tile dialect only carries the string form, and the converter + // silently omits an array. Emitting a groupBy-less body would let an + // echo-PUT strip the grouping from a live alert. + if ('groupBy' in config && Array.isArray(config.groupBy)) { + return false; + } + + const select = 'select' in config ? config.select : undefined; + // A raw SQL select expression string has no external representation — the + // tile converter would silently substitute a default count() item. + if (!Array.isArray(select)) { + return false; + } + // An empty select would make the number branch of the tile converter throw + // (it reads select[0]); every variant's external schema requires >= 1 item + // anyway, so refuse before converting. + if (select.length === 0) { + return false; + } + // The external converter keeps only select[0] on formula-less number + // configs, so extra items would be silently discarded. + if ( + displayType === DisplayType.Number && + select.length > 1 && + !('formulas' in config && (config.formulas?.length ?? 0) > 0) + ) { + return false; + } + for (const item of select) { + // Values, not just key names: `convertToExternalSelectItem` falls back to + // `aggFn: 'none'` for an aggregation the external enum does not admit + // (the internal schema also allows `histogram`, `quantileMerge`, and the + // *Merge combinators) and drops a `level` outside the external quantile + // set. Emitting either would silently rewrite the aggregation on a + // GET -> PUT, which is what this gate exists to prevent. + if (!AggregateFunctionSchema.safeParse(item.aggFn).success) { + return false; + } + if ( + item.aggFn === 'quantile' && + !externalQuantileLevelSchema.safeParse( + 'level' in item ? item.level : undefined, + ).success + ) { + return false; + } + + for (const [key, value] of Object.entries(item)) { + if (isEmptyValue(value) || isDefaultValue(key, value)) continue; + // Inert leftovers the external converter heals away: a level on a + // non-quantile agg and a valueExpression on a count are ignored by the + // renderer (see convertToExternalSelectItem). + if (key === 'level' && item.aggFn !== 'quantile') continue; + if (key === 'valueExpression' && item.aggFn === 'count') continue; + if (REPRESENTABLE_SELECT_ITEM_KEYS.has(key)) continue; + if (EVALUATION_INERT_SELECT_ITEM_KEYS.has(key)) continue; + return false; + } + } + + return true; +} + +/** + * Converts an inline alert's persisted internal chart config to the external + * tile-config dialect for API responses. Returns undefined when the external + * contract cannot represent the config faithfully — an unsupported display + * type, a PromQL config, a config carrying evaluation-affecting fields the + * dialect cannot express (see isRepresentableExternally), or a converted body + * the external write schema would reject. Callers omit the field, so a blind + * echo-PUT fails loudly instead of silently rewriting the alert's query, and + * anything that IS emitted is valid PUT input by construction. + */ +export function convertAlertChartConfigToExternal( + config: AlertChartConfig, +): ExternalAlertChartConfig | undefined { + const displayType = config.displayType; + if ( + displayType == null || + !(EXTERNAL_ALERT_DISPLAY_TYPES as readonly DisplayType[]).includes( + displayType, + ) || + !isRepresentableExternally(config, displayType as ExternalAlertDisplayType) + ) { + logger.warn( + { displayType }, + 'Inline alert chart config has no faithful external representation; omitting chartConfig', + ); + return undefined; + } + + const external = convertToExternalTileChartConfig(config); + switch (external?.displayType) { + case DisplayType.Line: + case DisplayType.StackedBar: + case DisplayType.Number: { + const name = config.name ? { name: config.name } : {}; + const candidate = + 'configType' in external + ? { ...external, ...name } + : { + ...external, + ...name, + // Chart-level filter: builder configs only (the raw SQL + // internal shape has no `where`). + ...(!('configType' in config) && config.where + ? { + where: config.where, + whereLanguage: config.whereLanguage ?? 'lucene', + } + : {}), + }; + // The emitted body must be valid PUT input by construction: run it + // through the exact schema the write path parses. The internal schema + // is looser in places the converter passes through verbatim (uncapped + // aggCondition/valueExpression/alias, an empty valueExpression on a + // non-count agg, >20 select items), so a stored config can convert + // cleanly yet still violate the external contract. + const parsed = externalAlertChartConfigSchema.safeParse(candidate); + if (!parsed.success) { + logger.warn( + { displayType, issues: parsed.error.issues }, + 'Inline alert chart config converts to a body the external schema rejects; omitting chartConfig', + ); + return undefined; + } + return parsed.data; + } + default: + logger.error( + { config }, + 'Inline alert chart config has no external representation', + ); + return undefined; + } +} + +/** + * `translateAlertDocumentToExternalAlert` plus the inline alert's chartConfig + * (external dialect) when one is faithfully representable. Single-alert + * responses (GET by id, POST, PUT, MCP detail) use this; list responses use + * the bare translator so they stay lean. Lives here (not utils/externalApi) + * to keep the utils -> router-utils import direction one-way. + */ +export function translateAlertDocumentToExternalAlertWithChartConfig( + alert: AlertDocument, +): ExternalAlert { + const external = translateAlertDocumentToExternalAlert(alert); + const alertObj: Pick = alert.toJSON + ? alert.toJSON() + : alert; + const chartConfig = + alertObj.source === AlertSource.INLINE && alertObj.chartConfig != null + ? convertAlertChartConfigToExternal(alertObj.chartConfig) + : undefined; + return { ...external, ...(chartConfig && { chartConfig }) }; +} diff --git a/packages/api/src/routers/external-api/v2/utils/dashboards.ts b/packages/api/src/routers/external-api/v2/utils/dashboards.ts index 85615a1a33..fa3a4eb311 100644 --- a/packages/api/src/routers/external-api/v2/utils/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/utils/dashboards.ts @@ -201,7 +201,9 @@ const toExternalColorRules = ( return resolved.length > 0 ? resolved : undefined; }; -const convertToExternalTileChartConfig = ( +// Exported for the inline-alert converter (v2/utils/alertChartConfig.ts), +// which reuses the tile-config translation for an alert's persisted config. +export const convertToExternalTileChartConfig = ( config: SavedChartConfig, ): ExternalDashboardTileConfig | undefined => { if (isRawSqlSavedChartConfig(config)) { @@ -650,7 +652,12 @@ export function convertToExternalDashboard( // -------------------------------------------------------------------------------- const convertToInternalSelectItem = ( - item: ExternalDashboardSelectItem, + // `isDelta` is the MCP tile dialect's spelling of the gauge delta flag + // (`periodAggFn: 'delta'` in the REST dialect). MCP tiles are cast to + // ExternalDashboardTileWithId before reaching this converter + // (saveDashboard.ts), so honoring both spellings here is what keeps an + // MCP-authored gauge-delta series from silently evaluating raw values. + item: ExternalDashboardSelectItem & { isDelta?: boolean }, ): Exclude => { return { ...pick(item, [ @@ -663,7 +670,7 @@ const convertToInternalSelectItem = ( ]), aggCondition: item.where, aggConditionLanguage: item.whereLanguage, - isDelta: item.periodAggFn === 'delta', + isDelta: item.periodAggFn === 'delta' || item.isDelta === true, valueExpression: item.valueExpression ?? '', }; }; diff --git a/packages/api/src/utils/__tests__/externalApi.test.ts b/packages/api/src/utils/__tests__/externalApi.test.ts index 0b1f61aff6..e0d79dad55 100644 --- a/packages/api/src/utils/__tests__/externalApi.test.ts +++ b/packages/api/src/utils/__tests__/externalApi.test.ts @@ -7,6 +7,7 @@ import { AlertState, AlertThresholdType, } from '@/models/alert'; +import { translateAlertDocumentToExternalAlertWithChartConfig } from '@/routers/external-api/v2/utils/alertChartConfig'; import { translateAlertDocumentToExternalAlert } from '@/utils/externalApi'; // A channel type this repo doesn't define -- see @@ -105,6 +106,128 @@ describe('utils/externalApi', () => { }); }); + describe('chartConfig handling', () => { + const internalChartConfig = { + displayType: 'line', + source: '65f5e4a3b9e77c001a123456', + select: [ + { + aggFn: 'count', + aggCondition: 'level:error', + aggConditionLanguage: 'lucene', + valueExpression: '', + }, + ], + where: '', + whereLanguage: 'lucene', + seriesReturnType: 'ratio', + }; + + it('omits chartConfig by default (list responses stay lean)', () => { + const alert = createAlertDocument({ + source: AlertSource.INLINE, + chartConfig: internalChartConfig, + }); + + const translated = translateAlertDocumentToExternalAlert(alert); + + expect(translated.chartConfig).toBeUndefined(); + expect('chartConfig' in translated).toBe(false); + }); + + it('emits chartConfig in the external tile-config dialect when requested', () => { + const alert = createAlertDocument({ + source: AlertSource.INLINE, + chartConfig: internalChartConfig, + }); + + const translated = + translateAlertDocumentToExternalAlertWithChartConfig(alert); + + // seriesReturnType 'ratio' becomes asRatio only with exactly two + // select items — this single-select config maps to false. + expect(translated.chartConfig).toMatchObject({ + displayType: 'line', + sourceId: '65f5e4a3b9e77c001a123456', + asRatio: false, + select: [ + { + aggFn: 'count', + where: 'level:error', + whereLanguage: 'lucene', + }, + ], + }); + }); + + it('emits raw SQL chartConfig with external field names', () => { + const alert = createAlertDocument({ + source: AlertSource.INLINE, + chartConfig: { + configType: 'sql', + displayType: 'line', + sqlTemplate: 'SELECT 1', + connection: '65f5e4a3b9e77c001a789012', + source: '65f5e4a3b9e77c001a123456', + }, + }); + + const translated = + translateAlertDocumentToExternalAlertWithChartConfig(alert); + + expect(translated.chartConfig).toMatchObject({ + configType: 'sql', + displayType: 'line', + sqlTemplate: 'SELECT 1', + connectionId: '65f5e4a3b9e77c001a789012', + sourceId: '65f5e4a3b9e77c001a123456', + }); + }); + + it('does not emit chartConfig for non-inline alerts even when requested', () => { + const alert = createAlertDocument({ + source: AlertSource.TILE, + chartConfig: internalChartConfig, + }); + + const translated = + translateAlertDocumentToExternalAlertWithChartConfig(alert); + + expect(translated.chartConfig).toBeUndefined(); + }); + + it('does not throw on an inline alert missing its chartConfig', () => { + // Reachable via a direct DB write or a partial legacy document; the + // translate layer must not crash a whole list response over one row. + const alert = createAlertDocument({ + source: AlertSource.INLINE, + chartConfig: null, + }); + + const translated = + translateAlertDocumentToExternalAlertWithChartConfig(alert); + + expect(translated.chartConfig).toBeUndefined(); + expect('chartConfig' in translated).toBe(false); + }); + + it('omits chartConfig when the persisted config has no external representation', () => { + const alert = createAlertDocument({ + source: AlertSource.INLINE, + chartConfig: { + configType: 'promql', + promqlQuery: 'up', + source: '65f5e4a3b9e77c001a123456', + }, + }); + + const translated = + translateAlertDocumentToExternalAlertWithChartConfig(alert); + + expect(translated.chartConfig).toBeUndefined(); + }); + }); + describe('channel mirroring', () => { // translateAlertDocumentToExternalAlert mirrors channels[0] into // `channel`, same as makeAlert (controllers/__tests__/alerts.test.ts). diff --git a/packages/api/src/utils/externalApi.ts b/packages/api/src/utils/externalApi.ts index bf9c8b4ec7..332ec32a94 100644 --- a/packages/api/src/utils/externalApi.ts +++ b/packages/api/src/utils/externalApi.ts @@ -23,7 +23,10 @@ import { } from '@/models/alert'; import type { DashboardDocument } from '@/models/dashboard'; import { SeriesTile } from '@/routers/external-api/v2/utils/dashboards'; -import { ExternalDashboardFilterWithId } from '@/utils/zod'; +import { + ExternalAlertChartConfig, + ExternalDashboardFilterWithId, +} from '@/utils/zod'; /** Returns a new object containing only the truthy, requested keys from the original object */ const pickIfTruthy = (obj: T, keys: K[]): Partial => { @@ -287,6 +290,12 @@ export type ExternalAlert = { dashboardId?: string; savedSearchId?: string; groupBy?: string; + /** + * Inline alerts only, and only on single-alert responses (the list endpoint + * stays lean): the alert's persisted chart config in the external + * tile-config dialect. + */ + chartConfig?: ExternalAlertChartConfig; silenced?: { by?: string; at: string; @@ -358,6 +367,12 @@ function transformErrorsToExternalErrors( })); } +// Note: this translator does not attach an inline alert's `chartConfig`. +// Single-alert responses (GET by id, POST, PUT, MCP detail) attach it via +// `translateAlertDocumentToExternalAlertWithChartConfig` (v2 router util) — +// keeping the converter out of here avoids a runtime import cycle with the +// v2 utils, and list responses stay lean so a team with hundreds of raw-SQL +// inline alerts does not ship every template on each page. export function translateAlertDocumentToExternalAlert( alert: AlertDocument, ): ExternalAlert { diff --git a/packages/api/src/utils/zod.ts b/packages/api/src/utils/zod.ts index 6cd71bae75..348ff00ad9 100644 --- a/packages/api/src/utils/zod.ts +++ b/packages/api/src/utils/zod.ts @@ -574,21 +574,116 @@ export type ExternalDashboardRawSqlTileConfig = z.infer< typeof externalDashboardRawSqlTileConfigSchema >; +/** + * Routing predicate for the configType-routed config schemas below + * (dashboard tiles and inline alert configs): raw SQL configs carry + * `configType: 'sql'`, everything else is the builder dialect. + */ +const isRawSqlRoutedConfig = (data: unknown): boolean => + data !== null && + typeof data === 'object' && + 'configType' in data && + data.configType === 'sql'; + +/** + * Rejects a `configType` the routing predicate does not recognize. Without + * this, an unknown value (e.g. `configType: 'promql'`) routes to the builder + * union, which strips the unrecognized keys and parses a body that asked for + * something else — so the caller's PromQL config would silently persist as a + * builder config. Returns true when an issue was raised (caller stops). + */ +const addUnsupportedConfigTypeIssue = ( + data: unknown, + ctx: z.RefinementCtx, +): boolean => { + if ( + data !== null && + typeof data === 'object' && + 'configType' in data && + data.configType !== 'sql' + ) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: + 'configType must be "sql" or omitted (builder configs carry no configType)', + path: ['configType'], + }); + return true; + } + return false; +}; + +/** + * The shared post-parse rule set for configType-routed config schemas + * (dashboard tiles and inline alert configs) — kept in one place so the two + * surfaces cannot drift: + * - `asRatio` requires exactly two select items. + * - Builder configs: metric-formula validation (expression parse + + * series-ref range checks, asRatio mutual exclusion, number single-formula + * cap) — shared with the chart editor's save-time rules via common-utils. + * - Builder number configs without formulas display their single select + * item; the relaxed `.min(1).max(20)` on the number schema exists only so + * formula operands fit (the editor enforces the same in + * `validateChartForm`). + * + * The builder-only rules gate on `isRawSqlRoutedConfig` — the same predicate + * that picks the parsing schema — so a config parsed as builder always gets + * the builder rules. + */ +const addConfigTypeRoutedIssues = ( + data: { + displayType?: string; + configType?: string; + select?: unknown; + formulas?: { expression: string }[]; + asRatio?: boolean; + }, + ctx: z.RefinementCtx, + { numberSelectMessage }: { numberSelectMessage: string }, +): void => { + if ( + data.asRatio && + (!Array.isArray(data.select) || data.select.length !== 2) + ) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'asRatio can only be used with exactly two select items', + }); + } + + if (!isRawSqlRoutedConfig(data)) { + validateChartConfigFormulas(data, ctx); + + if ( + data.displayType === 'number' && + (data.formulas?.length ?? 0) === 0 && + Array.isArray(data.select) && + data.select.length !== 1 + ) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: numberSelectMessage, + path: ['select'], + }); + } + } +}; + const externalDashboardTileConfigSchema = z .custom< ExternalDashboardRawSqlTileConfig | ExternalDashboardBuilderTileConfig >() .superRefine((data, ctx) => { + if (addUnsupportedConfigTypeIssue(data, ctx)) { + return; + } + // Route to the correct sub-schema based on configType so Zod's // discriminatedUnion can produce targeted field-level errors rather // than a generic union failure. - const schema = - data !== null && - typeof data === 'object' && - 'configType' in data && - data.configType === 'sql' - ? externalDashboardRawSqlTileConfigSchema - : externalDashboardBuilderTileConfigSchema; + const schema = isRawSqlRoutedConfig(data) + ? externalDashboardRawSqlTileConfigSchema + : externalDashboardBuilderTileConfigSchema; const result = schema.safeParse(data); if (!result.success) { @@ -598,55 +693,18 @@ const externalDashboardTileConfigSchema = z return; } - if ( - 'asRatio' in data && - data.asRatio && - (!Array.isArray(data.select) || data.select.length !== 2) - ) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: 'asRatio can only be used with exactly two select items', - }); - } - - if (!('configType' in data)) { - // Metric formulas (builder tiles only): expression parse + series-ref - // range checks, asRatio mutual exclusion, and the number-tile - // single-formula cap — shared with the chart editor's save-time rules - // via common-utils. - validateChartConfigFormulas(data, ctx); - - // A number tile without formulas displays its single select item; the - // relaxed `.min(1).max(20)` on the number schema exists only so - // formula operands fit (the editor enforces the same in - // `validateChartForm`). - if ( - data.displayType === 'number' && - !('formulas' in data && (data.formulas?.length ?? 0) > 0) && - Array.isArray(data.select) && - data.select.length !== 1 - ) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: - 'Number tiles support a single select item unless formulas are used (extra items are formula operands)', - path: ['select'], - }); - } - } + addConfigTypeRoutedIssues(data, ctx, { + numberSelectMessage: + 'Number tiles support a single select item unless formulas are used (extra items are formula operands)', + }); }) .transform(data => { // Re-parse through the appropriate sub-schema to strip unknown fields. // Safe to call .parse() here — superRefine already validated the data, // so this is guaranteed to succeed. - const schema = - data !== null && - typeof data === 'object' && - 'configType' in data && - data.configType === 'sql' - ? externalDashboardRawSqlTileConfigSchema - : externalDashboardBuilderTileConfigSchema; - return schema.parse(data); + return isRawSqlRoutedConfig(data) + ? externalDashboardRawSqlTileConfigSchema.parse(data) + : externalDashboardBuilderTileConfigSchema.parse(data); }); export type ExternalDashboardTileConfig = z.infer< @@ -794,6 +852,143 @@ const validateInlineAlertFormulas = ( ); }; +// Inline-alert chart configs on the external surface use the same external +// tile-config dialect v2 dashboards use (`sourceId`, per-select `where`, +// `asRatio`, `connectionId` + `sqlTemplate`), restricted to the display types +// the alert task can evaluate as a time series: Line, Stacked Bar, and +// Number. Builder + raw SQL variants; no PromQL (PromQL charts cannot be +// alerted on). +// +// Unlike dashboard tiles, an alert's chart config additionally carries fields +// that live on the tile (not its config) or are deliberately hidden from the +// tile dialect, but are meaningful for a standalone alert: +// - `name`: the config's display name, used in notification titles +// (buildAlertMessageTemplateTitle) and as an alert-name fallback. +// - `where`/`whereLanguage` (builder only): the chart-level filter that +// renderChartConfig ANDs into every series. The tile dialect omits it +// because the tile editor cannot display it, but the chart-explorer +// editor that authors inline alerts does; without it a GET -> PUT +// round-trip would silently broaden the alert's query. +const alertChartConfigNameSchema = z.string().optional(); + +// Raw SQL configs have no chart-level filter internally +// (RawSqlBaseChartConfigSchema); reject rather than silently strip so a +// caller who meant to filter learns the field does not exist here. +const rejectedAlertRawSqlWhereField = z + .never({ + invalid_type_error: + 'Raw SQL alert configs have no chart-level where; filter inside the sqlTemplate instead', + }) + .optional(); + +export const externalAlertBuilderChartConfigSchema = z.discriminatedUnion( + 'displayType', + [ + externalDashboardLineChartConfigSchema.extend({ + name: alertChartConfigNameSchema, + where: z.string().max(10000).optional(), + whereLanguage: whereLanguageSchema, + }), + externalDashboardBarChartConfigSchema.extend({ + name: alertChartConfigNameSchema, + where: z.string().max(10000).optional(), + whereLanguage: whereLanguageSchema, + }), + externalDashboardNumberChartConfigSchema.extend({ + name: alertChartConfigNameSchema, + where: z.string().max(10000).optional(), + whereLanguage: whereLanguageSchema, + }), + ], +); + +export const externalAlertRawSqlChartConfigSchema = z.discriminatedUnion( + 'displayType', + [ + externalDashboardLineRawSqlChartConfigSchema.extend({ + name: alertChartConfigNameSchema, + where: rejectedAlertRawSqlWhereField, + whereLanguage: rejectedAlertRawSqlWhereField, + }), + externalDashboardBarRawSqlChartConfigSchema.extend({ + name: alertChartConfigNameSchema, + where: rejectedAlertRawSqlWhereField, + whereLanguage: rejectedAlertRawSqlWhereField, + }), + externalDashboardNumberRawSqlChartConfigSchema.extend({ + name: alertChartConfigNameSchema, + where: rejectedAlertRawSqlWhereField, + whereLanguage: rejectedAlertRawSqlWhereField, + }), + ], +); + +export type ExternalAlertChartConfig = + | z.infer + | z.infer; + +// Mirrors the route-by-configType superRefine/transform pattern of +// externalDashboardTileConfigSchema above (same routing predicate and the +// shared addConfigTypeRoutedIssues rule set), so inline alert configs get +// the same targeted field-level errors, formula validation, and +// unknown-field stripping as dashboard tile configs. +export const externalAlertChartConfigSchema = z + .custom() + .superRefine((data, ctx) => { + if (addUnsupportedConfigTypeIssue(data, ctx)) { + return; + } + + const schema = isRawSqlRoutedConfig(data) + ? externalAlertRawSqlChartConfigSchema + : externalAlertBuilderChartConfigSchema; + + const result = schema.safeParse(data); + if (!result.success) { + for (const issue of result.error.issues) { + ctx.addIssue(issue); + } + return; + } + + addConfigTypeRoutedIssues(data, ctx, { + numberSelectMessage: + 'Number charts support a single select item unless formulas are used (extra items are formula operands)', + }); + }) + .transform(data => { + // Re-parse through the appropriate sub-schema to strip unknown fields. + // Safe to call .parse() here — superRefine already validated the data. + return isRawSqlRoutedConfig(data) + ? externalAlertRawSqlChartConfigSchema.parse(data) + : externalAlertBuilderChartConfigSchema.parse(data); + }); + +const zExternalInlineAlert = z.object({ + source: z.literal(AlertSource.INLINE), + chartConfig: externalAlertChartConfigSchema, +}); + +// The branch schemas are strip-mode z.objects, so a stray chartConfig on a +// non-inline alert would be silently discarded before any superRefine could +// see it. Declaring the field as a rejected never (same pattern as the MCP +// tile-level `where` rejection) turns that into a 400, matching both the +// OpenAPI contract ("rejected otherwise") and the MCP surface. External-only: +// the internal schema keeps its historical strip semantics. +const rejectedNonInlineChartConfigField = z + .never({ + invalid_type_error: 'chartConfig is only supported when source is "inline"', + }) + .optional(); + +const zExternalSavedSearchAlert = zSavedSearchAlert.extend({ + chartConfig: rejectedNonInlineChartConfigField, +}); + +const zExternalTileAlert = zTileAlert.extend({ + chartConfig: rejectedNonInlineChartConfigField, +}); + const alertBaseSchema = z.object({ channel: zAlertChannel.optional(), channels: zAlertChannels.optional(), @@ -810,23 +1005,48 @@ const alertBaseSchema = z.object({ numConsecutiveWindows: z.number().int().min(1).nullish(), }); +// External v2 alert schema. Inline alerts carry their chart config in the +// external tile-config dialect (see externalAlertChartConfigSchema); the +// router converts it to the internal AlertChartConfig shape before +// validateAlertInput/createAlert. +// Discriminated on `source` (not a plain `.or()`): a failing `.or()` branch +// collapses to a single "Invalid input" issue with the real messages buried +// in unionErrors, which the external API's error formatter drops — so every +// chartConfig rule below would surface as "Body validation failed: Invalid +// input". Discriminating reports the matching branch's own issues. export const alertSchema = alertBaseSchema - .and(zSavedSearchAlert.or(zTileAlert)) + .and( + z.discriminatedUnion('source', [ + zExternalSavedSearchAlert, + zExternalTileAlert, + zExternalInlineAlert, + ]), + ) .superRefine(validateAlertChannelSelection) .superRefine(validateAlertScheduleOffsetMinutes) .superRefine(validateAlertThresholdMax); -// 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 inline alerts. +export type ExternalAlertInput = z.infer; + +// Internal-API variant: inline alerts carry the persisted internal +// AlertChartConfig shape directly (per-select `aggCondition`, +// `seriesReturnType: 'ratio'`, `source`/`connection`) instead of the external +// dialect above. export const internalAlertSchema = alertBaseSchema - .and(zSavedSearchAlert.or(zTileAlert).or(zInlineAlert)) + .and( + z.discriminatedUnion('source', [ + zSavedSearchAlert, + zTileAlert, + zInlineAlert, + ]), + ) .superRefine(validateAlertChannelSelection) .superRefine(validateAlertScheduleOffsetMinutes) .superRefine(validateAlertThresholdMax) .superRefine(validateInlineAlertFormulas); +export type InternalAlertInput = z.infer; + // ============================== // Webhooks // ============================== diff --git a/packages/common-utils/src/__tests__/iac.test.ts b/packages/common-utils/src/__tests__/iac.test.ts index 7133e74b4a..a152fe6ecc 100644 --- a/packages/common-utils/src/__tests__/iac.test.ts +++ b/packages/common-utils/src/__tests__/iac.test.ts @@ -76,6 +76,12 @@ describe('isImportableAlert', () => { expect(isImportableAlert({ source: 'tile' })).toBe(false); }); + // The provider's alert resource does not model a chartConfig, so inline + // alerts stay excluded even though external API v2 can create/read them. + it('rejects an inline alert', () => { + expect(isImportableAlert({ source: 'inline' })).toBe(false); + }); + it('rejects an alert with no source and no saved search', () => { expect(isImportableAlert({})).toBe(false); }); diff --git a/packages/common-utils/src/iacEligibility.ts b/packages/common-utils/src/iacEligibility.ts index 9694105297..e975a29b0e 100644 --- a/packages/common-utils/src/iacEligibility.ts +++ b/packages/common-utils/src/iacEligibility.ts @@ -10,8 +10,11 @@ import { AlertSource, DisplayType, SourceKind } from './types'; /** * The provider models only saved-search alerts — a tile alert has no * corresponding resource, so offering one for import produces a command that - * fails. Shared by the bulk export and the per-alert popover so the two cannot - * disagree about what is eligible. + * fails. Inline alerts (source: 'inline') are excluded for the same reason: + * although the external API v2 can now create/read them, the provider's alert + * resource does not model a chartConfig yet, so importing one would generate + * a config the provider cannot apply. Shared by the bulk export and the + * per-alert popover so the two cannot disagree about what is eligible. */ export function isImportableAlert(alert: { source?: string;