Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/alert-per-target-notification-timings.md
Original file line number Diff line number Diff line change
@@ -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.
25 changes: 24 additions & 1 deletion packages/api/src/models/alertHistory.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
57 changes: 53 additions & 4 deletions packages/api/src/tasks/checkAlerts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ import {
isRawSqlSavedChartConfig,
} from '@hyperdx/common-utils/dist/guards';
import {
ALERT_NOTIFICATION_TARGETS_LIMIT,
AlertErrorType,
AlertNotificationTargetTiming,
AlertThresholdType,
BuilderChartConfigWithOptDateRange,
ChartConfigWithOptDateRange,
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -546,7 +550,7 @@ const fireChannelEvent = async ({
value: totalCount,
};

const { failures } = await renderAlertTemplate({
const { failures, timings } = await renderAlertTemplate({
alertProvider,
clickhouseClient,
metadata,
Expand All @@ -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
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
34 changes: 33 additions & 1 deletion packages/api/src/tasks/checkAlerts/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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}`);
Expand Down
3 changes: 2 additions & 1 deletion packages/app/src/components/alerts/AlertEvaluationRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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 ? (
Expand Down
81 changes: 81 additions & 0 deletions packages/app/src/components/alerts/NotificationDurationCell.tsx
Original file line number Diff line number Diff line change
@@ -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>
);
}
Loading
Loading