fix: reject alerts on PromQL dashboard tiles

.changeset/reject-promql-tile-alerts.md

@@ -0,0 +1,8 @@
+---
+'@hyperdx/api': patch
+'@hyperdx/common-utils': patch
+---
+
+fix: reject alerts on dashboard tiles the alert checker never evaluates (PromQL
+tiles, and builder tiles with unsupported display types), and drop them when a
+dashboard is saved

packages/api/src/controllers/alerts.ts

@@ -2,9 +2,13 @@ import {
   displayTypeSupportsBuilderAlerts,
   displayTypeSupportsRawSqlAlerts,
   isFormulaSourceKind,
+  savedChartConfigSupportsAlerts,
   validateRawSqlForAlert,
 } from '@hyperdx/common-utils/dist/core/utils';
-import { isRawSqlSavedChartConfig } from '@hyperdx/common-utils/dist/guards';
+import {
+  isPromqlSavedChartConfig,
+  isRawSqlSavedChartConfig,
+} from '@hyperdx/common-utils/dist/guards';
 import { isRangeThresholdType } from '@hyperdx/common-utils/dist/types';
 import { groupBy } from 'lodash';
 import { Types } from 'mongoose';
@@ -91,13 +95,20 @@ export const validateAlertInput = async (
       throw new Api400Error('Tile not found');
     }
 
-    if (tile.config != null && isRawSqlSavedChartConfig(tile.config)) {
-      if (!displayTypeSupportsRawSqlAlerts(tile.config.displayType)) {
-        throw new Api400Error(
-          'Alerts on Raw SQL tiles are only supported for Line, Stacked Bar, or Number display types',
-        );
+    // The alert checker cannot evaluate these tiles, so the alert would never run.
+    if (tile.config != null && !savedChartConfigSupportsAlerts(tile.config)) {
+      if (isPromqlSavedChartConfig(tile.config)) {
+        throw new Api400Error('Alerts are not supported on PromQL tiles');
       }
+      const tileKind = isRawSqlSavedChartConfig(tile.config)
+        ? 'Raw SQL tiles'
+        : 'dashboard tiles';
+      throw new Api400Error(
+        `Alerts on ${tileKind} are only supported for Line, Stacked Bar, or Number display types`,
+      );
+    }
 
+    if (tile.config != null && isRawSqlSavedChartConfig(tile.config)) {
       const { errors } = validateRawSqlForAlert(tile.config);
       if (errors.length > 0) {
         throw new Api400Error(

packages/api/src/controllers/dashboard.ts

@@ -1,3 +1,4 @@
+import { savedChartConfigSupportsAlerts } from '@hyperdx/common-utils/dist/core/utils';
 import {
   DashboardWithoutIdSchema,
   resolveChartPaletteToken,
@@ -20,10 +21,19 @@ import type { AlertDocument, IAlert } from '@/models/alert';
 import Dashboard, { IDashboard } from '@/models/dashboard';
 import { resolveAlertDisplayFields } from '@/utils/alerts';
 
+// Drop alerts on tiles the alert checker cannot evaluate (e.g. PromQL), so
+// they are never stored and any existing ones are deleted on save.
+function getSupportedTileAlert(tile: Tile) {
+  return savedChartConfigSupportsAlerts(tile.config)
+    ? tile.config.alert
+    : undefined;
+}
+
 function pickAlertsByTile(tiles: Tile[]) {
   return tiles.reduce((acc, tile) => {
-    if (tile.config.alert) {
-      acc[tile.id] = tile.config.alert;
+    const alert = getSupportedTileAlert(tile);
+    if (alert) {
+      acc[tile.id] = alert;
     }
     return acc;
   }, {});
@@ -94,7 +104,7 @@ async function syncDashboardAlerts(
 
   const newTilesForAlertSync: TileForAlertSync[] = newTiles.map(t => ({
     id: t.id,
-    config: { alert: t.config.alert },
+    config: { alert: getSupportedTileAlert(t) },
   }));
   const { tileIds: newTileIds, tileIdsWithAlerts: newTileIdsWithAlerts } =
     extractTileAlertData(newTilesForAlertSync);

packages/api/src/routers/api/__tests__/alerts.int.test.ts

@@ -1142,6 +1142,68 @@ describe('alerts router', () => {
       .expect(400);
   });
 
+  it('rejects creating an alert on a builder table tile', async () => {
+    const tile = makeTile();
+    tile.config = { ...tile.config, displayType: DisplayType.Table };
+    const dashboard = await agent
+      .post('/dashboards')
+      .send({ name: 'Test Dashboard', tiles: [tile], tags: [] })
+      .expect(200);
+
+    const res = await agent
+      .post('/alerts')
+      .send(
+        makeAlertInput({
+          dashboardId: dashboard.body.id,
+          tileId: tile.id,
+          webhookId: webhook._id.toString(),
+        }),
+      )
+      .expect(400);
+    expect(JSON.stringify(res.body)).toContain(
+      'Alerts on dashboard tiles are only supported',
+    );
+  });
+
+  it('rejects creating an alert on a PromQL tile', async () => {
+    const promqlTileId = randomMongoId();
+    const dashboard = await Dashboard.create({
+      name: 'PromQL Dashboard',
+      team: team._id,
+      tiles: [
+        {
+          id: promqlTileId,
+          x: 0,
+          y: 0,
+          w: 6,
+          h: 3,
+          config: {
+            configType: 'promql',
+            name: 'Up',
+            displayType: DisplayType.Line,
+            source: randomMongoId(),
+            promqlExpression: [{ expression: 'up' }],
+          },
+        },
+      ],
+    });
+
+    const res = await agent
+      .post('/alerts')
+      .send(
+        makeAlertInput({
+          dashboardId: dashboard._id.toString(),
+          tileId: promqlTileId,
+          webhookId: webhook._id.toString(),
+        }),
+      )
+      .expect(400);
+    expect(JSON.stringify(res.body)).toContain(
+      'Alerts are not supported on PromQL tiles',
+    );
+    expect(await Alert.countDocuments({ team: team._id })).toBe(0);
+  });
+
   it('rejects creating an alert on a raw SQL tile without interval params', async () => {
     const rawSqlTile = makeRawSqlTile({
       sqlTemplate: 'SELECT count() FROM otel_logs',

packages/api/src/routers/api/__tests__/dashboard.int.test.ts

@@ -1,5 +1,6 @@
 import {
   AlertThresholdType,
+  DisplayType,
   MetricsDataType,
   PresetDashboard,
   SourceKind,
@@ -35,6 +36,15 @@ const makeMockAlert = (webhookId: string) => ({
   thresholdType: AlertThresholdType.ABOVE,
 });
 
+const makePromqlConfig = (alert?: ReturnType<typeof makeMockAlert>) => ({
+  configType: 'promql' as const,
+  name: 'Up',
+  displayType: DisplayType.Line,
+  source: 'test-source',
+  promqlExpression: [{ expression: 'up' }],
+  alert,
+});
+
 describe('dashboard router', () => {
   const server = getServer();
   let agent: Awaited<ReturnType<typeof getLoggedInAgent>>['agent'];
@@ -508,6 +518,49 @@ describe('dashboard router', () => {
     expect(alerts.body.data).toEqual([]);
   });
 
+  it('does not create alerts on PromQL tiles when creating a dashboard', async () => {
+    await agent
+      .post('/dashboards')
+      .send({
+        name: 'Test Dashboard',
+        tiles: [
+          {
+            ...makeTile(),
+            config: makePromqlConfig(makeMockAlert(webhook._id.toString())),
+          },
+        ],
+        tags: [],
+      })
+      .expect(200);
+
+    expect(await Alert.countDocuments({ team: team._id })).toBe(0);
+  });
+
+  it('deletes the alert when a tile is switched to PromQL', async () => {
+    const mockAlert = makeMockAlert(webhook._id.toString());
+    const dashboard = await agent
+      .post('/dashboards')
+      .send({
+        name: 'Test Dashboard',
+        tiles: [makeTile({ alert: mockAlert })],
+        tags: [],
+      })
+      .expect(200);
+    expect(await Alert.countDocuments({ team: team._id })).toBe(1);
+
+    await agent
+      .patch(`/dashboards/${dashboard.body.id}`)
+      .send({
+        ...dashboard.body,
+        tiles: [
+          { ...dashboard.body.tiles[0], config: makePromqlConfig(mockAlert) },
+        ],
+      })
+      .expect(200);
+
+    expect(await Alert.countDocuments({ team: team._id })).toBe(0);
+  });
+
   it('alerts are updated when updating dashboard (updating tile alert settings)', async () => {
     const mockAlert = makeMockAlert(webhook._id.toString());
     const dashboard = await agent

packages/api/src/routers/external-api/v2/utils/dashboards.ts

@@ -1,9 +1,8 @@
 import { getHeatmapMode } from '@hyperdx/common-utils/dist/core/heatmap';
 import { getPromqlSeries } from '@hyperdx/common-utils/dist/core/promql';
 import {
-  displayTypeSupportsBuilderAlerts,
-  displayTypeSupportsRawSqlAlerts,
   isFormulaSourceKind,
+  savedChartConfigSupportsAlerts,
   TIME_SERIES_DISPLAY_TYPE_BY_NAME,
 } from '@hyperdx/common-utils/dist/core/utils';
 import {
@@ -1863,22 +1862,9 @@ export async function cleanupDashboardAlerts({
 }) {
   const newTileIdSet = new Set(internalTiles.map(t => t.id));
   const tileIdsToDeleteAlerts = [
-    // Tiles whose config no longer supports alerts. PromQL tiles cannot be
-    // alerted on (HDX-4636), so an alert left on a tile switched to PromQL
-    // would never be checked.
+    // Tiles whose config no longer supports alerts (e.g. switched to PromQL).
     ...internalTiles
-      .filter(tile => {
-        if (isPromqlSavedChartConfig(tile.config)) {
-          return true;
-        }
-        if (isRawSqlSavedChartConfig(tile.config)) {
-          return !displayTypeSupportsRawSqlAlerts(tile.config.displayType);
-        }
-        if (isBuilderSavedChartConfig(tile.config)) {
-          return !displayTypeSupportsBuilderAlerts(tile.config.displayType);
-        }
-        return false;
-      })
+      .filter(tile => !savedChartConfigSupportsAlerts(tile.config))
       .map(tile => tile.id),
     // Tiles that were completely removed.
     ...[...existingTileIds].filter(id => !newTileIdSet.has(id)),

packages/common-utils/src/core/utils.ts

@@ -11,7 +11,11 @@ import {
   isQueryExpressionFilter,
   isStaticListFilter,
 } from '@/filters';
-import { isBuilderSavedChartConfig, isRawSqlSavedChartConfig } from '@/guards';
+import {
+  isBuilderSavedChartConfig,
+  isPromqlSavedChartConfig,
+  isRawSqlSavedChartConfig,
+} from '@/guards';
 import { MacroExpansionError, MalformedMacroArgsError } from '@/macroErrors';
 import {
   getSourceDependentMacrosUsed,
@@ -1675,6 +1679,22 @@ export function displayTypeSupportsPromQLAlerts(
   return displayType ? false : false;
 }
 
+/**
+ * Whether the alert checker can evaluate an alert on a tile with this config.
+ * Shared by every save path so alerts are never stored where they cannot run.
+ */
+export function savedChartConfigSupportsAlerts(
+  config: SavedChartConfig,
+): boolean {
+  if (isPromqlSavedChartConfig(config)) {
+    return displayTypeSupportsPromQLAlerts(config.displayType);
+  }
+  if (isRawSqlSavedChartConfig(config)) {
+    return displayTypeSupportsRawSqlAlerts(config.displayType);
+  }
+  return displayTypeSupportsBuilderAlerts(config.displayType);
+}
+
 /** Expand the chart's macros, returning failures instead of throwing. */
 function resolveRawSqlMacros(
   chartConfig: RawSqlChartConfig,