feat: attribute alert notification time to each target (#3003)

Author: jordan-simonovski

.changeset/alert-per-target-notification-timings.md

@@ -0,0 +1,11 @@
+---
+'@hyperdx/api': minor
+'@hyperdx/app': minor
+'@hyperdx/common-utils': minor
+---
+
+Record and show which notification target an evaluation's delivery time went to. `webhookDurationMs` was a single number covering the whole delivery, and because targets are dispatched concurrently the slowest one sets it — so a multi-target alert reported a figure with no way to tell which webhook was responsible, or that the other targets were fine.
+
+Each dispatch is now timed individually and aggregated per target across the evaluation, since a grouped alert notifies the same target once per firing group and again on resolve. One entry per distinct target carries its webhook id, display name, summed duration, how many dispatches it took, and how many failed. The evaluation history's "Notification duration" cell expands in place to show the breakdown.
+
+Stored per evaluation rather than per dispatch: a 50-group alert notifying 10 targets would otherwise write 500 entries onto every history row. The array is capped at `ALERT_NOTIFICATION_TARGETS_LIMIT` and sorted slowest-first, so the cap drops the least interesting rows. Records written before this change keep rendering their total with nothing to expand.

packages/api/src/models/alertHistory.ts

@@ -1,4 +1,7 @@
-import { AlertErrorType } from '@hyperdx/common-utils/dist/types';
+import {
+  AlertErrorType,
+  AlertNotificationTargetTiming,
+} from '@hyperdx/common-utils/dist/types';
 import mongoose, { Schema } from 'mongoose';
 import ms from 'ms';
 
@@ -28,6 +31,12 @@ export interface IAlertHistoryAnalytics {
    * (expected buckets − 1). 0 in steady state.
    */
   backfilledBuckets?: number;
+  /**
+   * Per-target breakdown of `webhookDurationMs`, one entry per distinct
+   * target, slowest first. Targets dispatch concurrently, so these do not sum
+   * to `webhookDurationMs`. Absent when the evaluation sent nothing.
+   */
+  notificationTargets?: AlertNotificationTargetTiming[];
 }
 
 export interface IAlertHistory {
@@ -105,6 +114,20 @@ const AlertHistorySchema = new Schema<IAlertHistory>({
       queryDurationMs: { type: Number, required: false },
       webhookDurationMs: { type: Number, required: false },
       backfilledBuckets: { type: Number, required: false },
+      notificationTargets: {
+        type: [
+          {
+            _id: false,
+            targetId: { type: String, required: true },
+            target: { type: String, required: true },
+            durationMs: { type: Number, required: true },
+            dispatches: { type: Number, required: true },
+            failures: { type: Number, required: true },
+          },
+        ],
+        required: false,
+        default: undefined,
+      },
     },
     required: false,
     default: undefined,

packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts

@@ -3345,6 +3345,17 @@ describe('checkAlerts', () => {
         expect(normalHistories[0].analytics!.webhookDurationMs).toEqual(
           expect.any(Number),
         );
+        // Per-target breakdown of that total: one entry for the alert's single
+        // configured webhook, named so the UI can attribute the time. Read
+        // field by field — these come back as Mongoose subdocuments, which
+        // don't deep-equal a plain object literal.
+        const targets = normalHistories[0].analytics!.notificationTargets;
+        expect(targets).toHaveLength(1);
+        expect(targets![0].targetId).toBe(webhook._id.toString());
+        expect(targets![0].target).toBe(webhook.name);
+        expect(targets![0].durationMs).toEqual(expect.any(Number));
+        expect(targets![0].dispatches).toBe(1);
+        expect(targets![0].failures).toBe(0);
       });
 
       it('keeps ERROR rows from older windows when a later window succeeds', async () => {

packages/api/src/tasks/checkAlerts/index.ts

@@ -33,7 +33,9 @@ import {
   isRawSqlSavedChartConfig,
 } from '@hyperdx/common-utils/dist/guards';
 import {
+  ALERT_NOTIFICATION_TARGETS_LIMIT,
   AlertErrorType,
+  AlertNotificationTargetTiming,
   AlertThresholdType,
   BuilderChartConfigWithOptDateRange,
   ChartConfigWithOptDateRange,
@@ -80,7 +82,9 @@ import {
   AlertMessageTemplateDefaultView,
   buildAlertMessageTemplateTitle,
   NotificationFailure,
+  NotificationTiming,
   renderAlertTemplate,
+  RenderedAlert,
 } from '@/tasks/checkAlerts/template';
 import { handleSendGenericWebhook } from '@/tasks/checkAlerts/transports';
 import { tasksTracer } from '@/tasks/tracer';
@@ -494,7 +498,7 @@ const fireChannelEvent = async ({
   totalCount: number;
   windowSizeInMins: number;
   teamWebhooksById: Map<string, IWebhook>;
-}): Promise<NotificationFailure[]> => {
+}): Promise<Pick<RenderedAlert, 'failures' | 'timings'>> => {
   const team = alert.team;
   if (team == null) {
     throw new Error('Team not found');
@@ -546,7 +550,7 @@ const fireChannelEvent = async ({
     value: totalCount,
   };
 
-  const { failures } = await renderAlertTemplate({
+  const { failures, timings } = await renderAlertTemplate({
     alertProvider,
     clickhouseClient,
     metadata,
@@ -561,7 +565,7 @@ const fireChannelEvent = async ({
     teamId,
     teamWebhooksById,
   });
-  return failures;
+  return { failures, timings };
 };
 
 // Use a delimiter that's unlikely to appear in alert IDs or group names
@@ -925,6 +929,45 @@ export const processAlert = async (
   // (query duration, webhook delivery time, backfilled buckets). Populated
   // progressively; hoisted so the catch blocks can attach what was measured.
   const evaluationAnalytics: IAlertHistoryAnalytics = {};
+  // Per-target notification timings, keyed by webhook id so the same target
+  // notified for several groups (and again on resolve) aggregates into one
+  // entry rather than one per dispatch.
+  const notificationTimings = new Map<string, AlertNotificationTargetTiming>();
+  const recordNotificationTimings = (timings: NotificationTiming[]) => {
+    for (const timing of timings) {
+      const existing = notificationTimings.get(timing.key);
+      if (existing == null) {
+        notificationTimings.set(timing.key, {
+          targetId: timing.key,
+          target: timing.target,
+          durationMs: timing.durationMs,
+          dispatches: 1,
+          failures: timing.ok ? 0 : 1,
+        });
+        continue;
+      }
+      existing.durationMs += timing.durationMs;
+      existing.dispatches += 1;
+      existing.failures += timing.ok ? 0 : 1;
+    }
+  };
+  /**
+   * Fold the aggregated timings onto the analytics object. Called before the
+   * records are written, from both the success and the error path, so a
+   * failed evaluation still reports what it managed to deliver.
+   */
+  const flushNotificationTimings = () => {
+    if (notificationTimings.size === 0) {
+      return;
+    }
+    evaluationAnalytics.notificationTargets = Array.from(
+      notificationTimings.values(),
+    )
+      // Slowest first: the point of the breakdown is finding what dominated
+      // the total, and the cap below should drop the least interesting rows.
+      .sort((a, b) => b.durationMs - a.durationMs)
+      .slice(0, ALERT_NOTIFICATION_TARGETS_LIMIT);
+  };
   try {
     const windowSizeInMins = ms(alert.interval) / 60000;
     const scheduleStartAt = normalizeScheduleStartAt({
@@ -1232,7 +1275,7 @@ export const processAlert = async (
         // alert logic requiring large, nested objects. We should look at
         // cleaning this up next. fireChannelEvent guards against null values
         // for these properties.
-        const failures = await fireChannelEvent({
+        const { failures, timings } = await fireChannelEvent({
           alert,
           alertProvider,
           attributes,
@@ -1250,6 +1293,7 @@ export const processAlert = async (
           windowSizeInMins,
           teamWebhooksById,
         });
+        recordNotificationTimings(timings);
         // Each entry is a target that didn't end up delivered: unresolvable,
         // capped, or (for the inline dispatcher) an actual send rejection —
         // see renderAlertTemplate.
@@ -1373,6 +1417,7 @@ export const processAlert = async (
 
       // Single-value evaluations always cover exactly the current window.
       evaluationAnalytics.backfilledBuckets = 0;
+      flushNotificationTimings();
       const historyRecords = Array.from(histories.values());
       for (const record of historyRecords) {
         record.analytics = evaluationAnalytics;
@@ -1595,6 +1640,7 @@ export const processAlert = async (
     }
 
     // Save all history records and update alert state
+    flushNotificationTimings();
     const historyRecords = Array.from(histories.values());
     for (const record of historyRecords) {
       record.analytics = evaluationAnalytics;
@@ -1623,6 +1669,9 @@ export const processAlert = async (
       e instanceof InvalidAlertError
         ? AlertErrorType.INVALID_ALERT
         : AlertErrorType.UNKNOWN;
+    // An evaluation can notify some targets and then fail; report what it
+    // managed to deliver rather than dropping the timings with the error.
+    flushNotificationTimings();
     try {
       await alertProvider.recordAlertErrors(
         alert.id,

packages/api/src/tasks/checkAlerts/template.ts

@@ -341,11 +341,30 @@ const channelKey = (c: PopulatedAlertChannel) =>
 const channelLabel = (c: PopulatedAlertChannel) =>
   c.type === 'webhook' ? c.channel.name : c.type;
 
+/**
+ * One dispatch's wall time. Emitted per target per event, so a grouped alert
+ * produces one of these per (group, target); the caller aggregates.
+ */
+export type NotificationTiming = {
+  /** Stable identity for aggregation across events — the webhook id. */
+  key: string;
+  /** Display label: the webhook's name. */
+  target: string;
+  durationMs: number;
+  ok: boolean;
+};
+
 export type RenderedAlert = {
   /** The rendered message body, as delivered to every target. */
   body: string;
   /** One entry per target that did not end up delivered — see NotificationFailure. */
   failures: NotificationFailure[];
+  /**
+   * One entry per target that reached the dispatcher, delivered or not.
+   * Targets that failed before dispatch have no timing — there was nothing to
+   * time — so this is not the complement of `failures`.
+   */
+  timings: NotificationTiming[];
 };
 
 // this method will build the body of the alert message and will be used to send the alert to the channel
@@ -733,11 +752,17 @@ ${targetTemplate}`;
     // queued dispatcher resolves after enqueue and never rejects here; it
     // reports delivery outcomes through its own logs/metrics instead (see
     // agent_docs/observability.md).
+    const timings: NotificationTiming[] = [];
     await Promise.all(
       jobs.map(async job => {
+        // Per-job, not around the Promise.all: the whole point is attributing
+        // the total to a target, and the dispatches overlap.
+        const startedAt = performance.now();
+        let ok = true;
         try {
           await dispatcher.dispatch(job);
         } catch (e) {
+          ok = false;
           logger.error(
             {
               alertId: alert.id,
@@ -751,11 +776,18 @@ ${targetTemplate}`;
             type: job.populatedChannel.type,
             error: e,
           });
+        } finally {
+          timings.push({
+            key: channelKey(job.populatedChannel),
+            target: channelLabel(job.populatedChannel),
+            durationMs: Math.round(performance.now() - startedAt),
+            ok,
+          });
         }
       }),
     );
 
-    return { body, failures };
+    return { body, failures, timings };
   }
 
   throw new Error(`Unsupported alert source: ${alert.source}`);

packages/app/src/components/alerts/AlertEvaluationRow.tsx

@@ -15,6 +15,7 @@ import {
   AlertErrorsContent,
 } from '@/components/alerts/AlertHistoryCards';
 import { AlertStateBadge } from '@/components/alerts/AlertStateBadge';
+import { NotificationDurationCell } from '@/components/alerts/NotificationDurationCell';
 import { FormatTime } from '@/useFormatTime';
 import { formatDurationMs } from '@/utils';
 
@@ -184,7 +185,7 @@ export function AlertEvaluationRow({
         </Table.Td>
         <Table.Td>{durationCell(history.analytics?.queryDurationMs)}</Table.Td>
         <Table.Td>
-          {durationCell(history.analytics?.webhookDurationMs)}
+          <NotificationDurationCell analytics={history.analytics} />
         </Table.Td>
         <Table.Td>
           {hasErrors ? (

packages/app/src/components/alerts/NotificationDurationCell.tsx

@@ -0,0 +1,81 @@
+import * as React from 'react';
+import type { AlertHistoryAnalytics } from '@hyperdx/common-utils/dist/types';
+import { Collapse, Group, Stack, Text, UnstyledButton } from '@mantine/core';
+import { IconChevronDown, IconChevronRight } from '@tabler/icons-react';
+
+import { formatDurationMs } from '@/utils';
+
+/**
+ * The evaluation's notification wall time, expandable in place into a
+ * per-target breakdown.
+ *
+ * It expands *within* the cell rather than adding child rows: the parent row
+ * already owns a chevron for groups and errors, and a second row-level
+ * expander competing with it would be ambiguous to click.
+ */
+export function NotificationDurationCell({
+  analytics,
+}: {
+  analytics?: AlertHistoryAnalytics;
+}) {
+  const [expanded, setExpanded] = React.useState(false);
+  const total = analytics?.webhookDurationMs;
+  const targets = analytics?.notificationTargets ?? [];
+
+  if (total == null) {
+    return <>–</>;
+  }
+
+  // Records written before per-target timing existed have the total but no
+  // breakdown, so there is nothing to expand into.
+  if (targets.length === 0) {
+    return <Text size="sm">{formatDurationMs(total)}</Text>;
+  }
+
+  return (
+    <Stack gap={2} align="flex-start">
+      <UnstyledButton
+        // The parent row toggles its own expansion on click; without this the
+        // cell's expander would fire both.
+        onClick={event => {
+          event.stopPropagation();
+          setExpanded(value => !value);
+        }}
+        aria-expanded={expanded}
+        data-testid="notification-duration-toggle"
+      >
+        <Group gap={2} wrap="nowrap">
+          <Text size="sm">{formatDurationMs(total)}</Text>
+          {expanded ? (
+            <IconChevronDown size={12} />
+          ) : (
+            <IconChevronRight size={12} />
+          )}
+        </Group>
+      </UnstyledButton>
+      <Collapse expanded={expanded}>
+        <Stack gap={2} pt={2} data-testid="notification-duration-breakdown">
+          {targets.map(target => (
+            // Keyed on the id, not the label: two webhooks can share a name.
+            <Group key={target.targetId} gap="xs" wrap="nowrap">
+              <Text size="xs" c="dimmed">
+                {target.target}
+              </Text>
+              <Text size="xs">{formatDurationMs(target.durationMs)}</Text>
+              {target.dispatches > 1 && (
+                <Text size="xs" c="dimmed">
+                  ×{target.dispatches}
+                </Text>
+              )}
+              {target.failures > 0 && (
+                <Text size="xs" c="var(--color-text-danger)">
+                  {target.failures} failed
+                </Text>
+              )}
+            </Group>
+          ))}
+        </Stack>
+      </Collapse>
+    </Stack>
+  );
+}