Skip to content

Commit 78f4ed4

Browse files
Enhance metric alerts observability (#8232)
1 parent c0873ac commit 78f4ed4

4 files changed

Lines changed: 198 additions & 21 deletions

File tree

.changeset/itchy-otters-lead.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'hive': patch
3+
---
4+
5+
Metric Alerts: enhance Grafana with additional panels for tracking filtered vs unfiltered rule and
6+
group population and attaches additional data to evaluate-group span

deployment/grafana-dashboards/Metric-Alerts.json

Lines changed: 136 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -264,17 +264,143 @@
264264
"datasource": { "type": "prometheus", "uid": "PROM_DATASOURCE_UID" },
265265
"editorMode": "code",
266266
"expr": "hive_workflow_job_duration_seconds{task_identifier=\"evaluateMetricAlertRules\",quantile=\"0.99\"}",
267-
"legendFormat": "p99",
267+
"legendFormat": "p99 {{pod}}",
268268
"range": true,
269269
"refId": "A"
270270
}
271271
],
272272
"title": "evaluateMetricAlertRules task duration (p99)",
273273
"type": "timeseries"
274274
},
275+
{
276+
"datasource": { "type": "prometheus", "uid": "PROM_DATASOURCE_UID" },
277+
"description": "Enabled query groups (one ClickHouse query each) split by whether a saved filter is attached. Filtered groups read the heavier legacy tables (cost scales with the target's traffic cardinality), so watch this for growth in the more expensive with-filter population.",
278+
"fieldConfig": {
279+
"defaults": {
280+
"color": { "mode": "palette-classic" },
281+
"custom": {
282+
"axisCenteredZero": false,
283+
"axisPlacement": "auto",
284+
"axisSoftMin": 0,
285+
"drawStyle": "line",
286+
"fillOpacity": 10,
287+
"lineInterpolation": "linear",
288+
"lineWidth": 1,
289+
"stacking": { "group": "A", "mode": "none" },
290+
"thresholdsStyle": { "mode": "off" }
291+
},
292+
"mappings": [],
293+
"min": 0,
294+
"noValue": "0",
295+
"thresholds": { "mode": "absolute", "steps": [{ "color": "green", "value": null }] },
296+
"unit": "short"
297+
},
298+
"overrides": [
299+
{
300+
"matcher": { "id": "byName", "options": "with filter" },
301+
"properties": [{ "id": "color", "value": { "fixedColor": "orange", "mode": "fixed" } }]
302+
}
303+
]
304+
},
305+
"gridPos": { "h": 8, "w": 12, "x": 0, "y": 16 },
306+
"id": 5,
307+
"options": {
308+
"legend": {
309+
"calcs": ["last", "max"],
310+
"displayMode": "table",
311+
"placement": "bottom",
312+
"showLegend": true
313+
},
314+
"tooltip": { "mode": "multi", "sort": "desc" }
315+
},
316+
"pluginVersion": "9.3.6",
317+
"targets": [
318+
{
319+
"datasource": { "type": "prometheus", "uid": "PROM_DATASOURCE_UID" },
320+
"editorMode": "code",
321+
"expr": "hive_metric_alert_rule_groups{filtered=\"true\"}",
322+
"legendFormat": "with filter",
323+
"range": true,
324+
"refId": "A"
325+
},
326+
{
327+
"datasource": { "type": "prometheus", "uid": "PROM_DATASOURCE_UID" },
328+
"editorMode": "code",
329+
"expr": "hive_metric_alert_rule_groups{filtered=\"false\"}",
330+
"legendFormat": "no filter",
331+
"range": true,
332+
"refId": "B"
333+
}
334+
],
335+
"title": "Alert query groups: filtered vs unfiltered",
336+
"type": "timeseries"
337+
},
338+
{
339+
"datasource": { "type": "prometheus", "uid": "PROM_DATASOURCE_UID" },
340+
"description": "Enabled alert rules split by whether a saved filter is attached. Companion to the query-groups panel (a group can hold several rules that share one query).",
341+
"fieldConfig": {
342+
"defaults": {
343+
"color": { "mode": "palette-classic" },
344+
"custom": {
345+
"axisCenteredZero": false,
346+
"axisPlacement": "auto",
347+
"axisSoftMin": 0,
348+
"drawStyle": "line",
349+
"fillOpacity": 10,
350+
"lineInterpolation": "linear",
351+
"lineWidth": 1,
352+
"stacking": { "group": "A", "mode": "none" },
353+
"thresholdsStyle": { "mode": "off" }
354+
},
355+
"mappings": [],
356+
"min": 0,
357+
"noValue": "0",
358+
"thresholds": { "mode": "absolute", "steps": [{ "color": "green", "value": null }] },
359+
"unit": "short"
360+
},
361+
"overrides": [
362+
{
363+
"matcher": { "id": "byName", "options": "with filter" },
364+
"properties": [{ "id": "color", "value": { "fixedColor": "orange", "mode": "fixed" } }]
365+
}
366+
]
367+
},
368+
"gridPos": { "h": 8, "w": 12, "x": 12, "y": 16 },
369+
"id": 6,
370+
"options": {
371+
"legend": {
372+
"calcs": ["last", "max"],
373+
"displayMode": "table",
374+
"placement": "bottom",
375+
"showLegend": true
376+
},
377+
"tooltip": { "mode": "multi", "sort": "desc" }
378+
},
379+
"pluginVersion": "9.3.6",
380+
"targets": [
381+
{
382+
"datasource": { "type": "prometheus", "uid": "PROM_DATASOURCE_UID" },
383+
"editorMode": "code",
384+
"expr": "hive_metric_alert_enabled_rules{filtered=\"true\"}",
385+
"legendFormat": "with filter",
386+
"range": true,
387+
"refId": "A"
388+
},
389+
{
390+
"datasource": { "type": "prometheus", "uid": "PROM_DATASOURCE_UID" },
391+
"editorMode": "code",
392+
"expr": "hive_metric_alert_enabled_rules{filtered=\"false\"}",
393+
"legendFormat": "no filter",
394+
"range": true,
395+
"refId": "B"
396+
}
397+
],
398+
"title": "Enabled rules: filtered vs unfiltered",
399+
"type": "timeseries"
400+
},
275401
{
276402
"collapsed": false,
277-
"gridPos": { "h": 1, "w": 24, "x": 0, "y": 16 },
403+
"gridPos": { "h": 1, "w": 24, "x": 0, "y": 24 },
278404
"id": 100,
279405
"panels": [],
280406
"title": "Traces",
@@ -296,7 +422,7 @@
296422
},
297423
"overrides": []
298424
},
299-
"gridPos": { "h": 9, "w": 12, "x": 0, "y": 17 },
425+
"gridPos": { "h": 9, "w": 12, "x": 0, "y": 25 },
300426
"id": 101,
301427
"options": {
302428
"cellHeight": "sm",
@@ -320,7 +446,7 @@
320446
},
321447
{
322448
"datasource": { "type": "tempo", "uid": "TEMPO_DATASOURCE_UID" },
323-
"description": "Per-target group evaluations slower than $slowGroupThreshold. Inspect target.id, rules.in_group, time_window_minutes on the row; click to see the ClickHouse query and per-rule PG transactions inside the group.",
449+
"description": "Per-target group evaluations slower than $slowGroupThreshold. The row's filter.applied, window.needs_previous, and metric.needs_percentiles explain the query cost (filtered legacy-table path, 2x window scan, heavy percentile blob); click to see the ClickHouse query and per-rule PG transactions inside the group.",
324450
"fieldConfig": {
325451
"defaults": {
326452
"color": { "mode": "fixed" },
@@ -334,7 +460,7 @@
334460
},
335461
"overrides": []
336462
},
337-
"gridPos": { "h": 9, "w": 12, "x": 12, "y": 17 },
463+
"gridPos": { "h": 9, "w": 12, "x": 12, "y": 25 },
338464
"id": 102,
339465
"options": {
340466
"cellHeight": "sm",
@@ -346,7 +472,7 @@
346472
"datasource": { "type": "tempo", "uid": "TEMPO_DATASOURCE_UID" },
347473
"filters": [{ "id": "slow-groups-filter", "operator": "=", "scope": "span" }],
348474
"limit": 100,
349-
"query": "{name=\"evaluate-group\" && duration>$slowGroupThreshold} | select(.target.id, .rules.in_group, .time_window_minutes)",
475+
"query": "{name=\"evaluate-group\" && duration>$slowGroupThreshold} | select(span.target.id, span.rules.in_group, span.time_window_minutes, span.filter.applied, span.window.needs_previous, span.metric.needs_percentiles)",
350476
"queryType": "traceql",
351477
"refId": "A",
352478
"spss": 20,
@@ -389,7 +515,7 @@
389515
}
390516
]
391517
},
392-
"gridPos": { "h": 9, "w": 12, "x": 0, "y": 26 },
518+
"gridPos": { "h": 9, "w": 12, "x": 0, "y": 34 },
393519
"id": 104,
394520
"options": {
395521
"legend": {
@@ -443,7 +569,7 @@
443569
},
444570
"overrides": []
445571
},
446-
"gridPos": { "h": 9, "w": 12, "x": 12, "y": 26 },
572+
"gridPos": { "h": 9, "w": 12, "x": 12, "y": 34 },
447573
"id": 105,
448574
"options": {
449575
"legend": {
@@ -497,7 +623,7 @@
497623
},
498624
"overrides": []
499625
},
500-
"gridPos": { "h": 9, "w": 24, "x": 0, "y": 35 },
626+
"gridPos": { "h": 9, "w": 24, "x": 0, "y": 43 },
501627
"id": 106,
502628
"options": {
503629
"legend": {
@@ -541,7 +667,7 @@
541667
},
542668
"overrides": []
543669
},
544-
"gridPos": { "h": 9, "w": 24, "x": 0, "y": 44 },
670+
"gridPos": { "h": 9, "w": 24, "x": 0, "y": 52 },
545671
"id": 103,
546672
"options": {
547673
"cellHeight": "sm",

packages/services/workflows/src/metrics.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,16 @@ export const metricAlertClickHouseQueryDuration = new metrics.Histogram({
5454
buckets: [0.05, 0.1, 0.25, 0.5, 1, 2, 5, 10],
5555
labelNames: ['outcome'],
5656
});
57+
58+
// Enabled rules/query-groups split by whether they filter (the heavier
59+
// legacy-table path). Tracks how much of that expensive population exists.
60+
export const metricAlertEnabledRules = new metrics.Gauge({
61+
name: 'hive_metric_alert_enabled_rules',
62+
help: 'Enabled metric alert rules, by whether a saved filter is attached',
63+
labelNames: ['filtered'],
64+
});
65+
export const metricAlertRuleGroups = new metrics.Gauge({
66+
name: 'hive_metric_alert_rule_groups',
67+
help: 'Metric alert query groups (one ClickHouse query each), by whether a saved filter is attached',
68+
labelNames: ['filtered'],
69+
});

packages/services/workflows/src/tasks/evaluate-metric-alert-rules.ts

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import pLimit from 'p-limit';
22
import { z } from 'zod';
3+
import type { SqlValue } from '@hive/clickhouse';
34
import { psql } from '@hive/postgres';
45
import { SpanKind, SpanStatusCode, trace } from '@hive/service-common';
56
import { defineTask, implementTask } from '../kit.js';
@@ -11,6 +12,7 @@ import {
1112
isRuleDue,
1213
queryClickHouseWindows,
1314
} from '../lib/metric-alert-evaluator.js';
15+
import { metricAlertEnabledRules, metricAlertRuleGroups } from '../metrics.js';
1416

1517
// How many groups to evaluate in parallel. Each group = 1 ClickHouse round-trip
1618
// plus a per-rule state-machine evaluation that holds a Postgres connection for
@@ -58,6 +60,30 @@ export const task = implementTask(EvaluateMetricAlertRulesTask, async args => {
5860
// Evaluate every enabled rule. The org feature flag gates rule creation in
5961
// the API, not evaluation here; the per-rule `enabled` column is the gate.
6062
const rules = await fetchEnabledRules(context.pg);
63+
const groups = groupRulesByQuery(rules);
64+
65+
// Parse each group's saved filter once, reused by the gauges below and by
66+
// processGroup, so a filtered group isn't re-parsed within the same tick.
67+
const groupFilters = [...groups.values()].map(group => ({
68+
group,
69+
filterConditions: buildSavedFilterConditions(group[0].savedFilterFilters, logger),
70+
}));
71+
72+
// Population gauges for the expensive with-filter rules. "Filtered" means the
73+
// filter yields query conditions (the legacy-table path), not just that a
74+
// saved_filter_id is set. Both labels set each tick (incl. 0) to avoid staleness.
75+
let filteredGroups = 0;
76+
let filteredRules = 0;
77+
for (const { group, filterConditions } of groupFilters) {
78+
if (filterConditions.length > 0) {
79+
filteredGroups += 1;
80+
filteredRules += group.length;
81+
}
82+
}
83+
metricAlertRuleGroups.set({ filtered: 'true' }, filteredGroups);
84+
metricAlertRuleGroups.set({ filtered: 'false' }, groups.size - filteredGroups);
85+
metricAlertEnabledRules.set({ filtered: 'true' }, filteredRules);
86+
metricAlertEnabledRules.set({ filtered: 'false' }, rules.length - filteredRules);
6187

6288
if (rules.length === 0) {
6389
logger.debug('No enabled metric alert rules found');
@@ -69,8 +95,7 @@ export const task = implementTask(EvaluateMetricAlertRulesTask, async args => {
6995
// Evaluate only groups with at least one due member. Members of a group share
7096
// a window (so one cadence), and the batched UPDATE below keeps their
7197
// last_evaluated_at aligned, so a group is due as a unit.
72-
const groups = groupRulesByQuery(rules);
73-
const dueGroupList = [...groups.values()].filter(group =>
98+
const dueGroupList = groupFilters.filter(({ group }) =>
7499
group.some(rule => isRuleDue(rule, evaluationTime)),
75100
);
76101

@@ -82,18 +107,15 @@ export const task = implementTask(EvaluateMetricAlertRulesTask, async args => {
82107
return;
83108
}
84109

85-
async function processGroup(groupRules: (typeof rules)[number][]): Promise<{
110+
async function processGroup(
111+
groupRules: (typeof rules)[number][],
112+
filterConditions: SqlValue[],
113+
): Promise<{
86114
failed: boolean;
87115
evaluatedIds: string[];
88116
}> {
89117
const representative = groupRules[0];
90118

91-
// All rules in a group share the same saved filter (it's part of the group key),
92-
// so build the ClickHouse conditions once from the representative.
93-
// A malformed filter yields no conditions (evaluates unfiltered) and is logged,
94-
// isolating the failure to this group.
95-
const filterConditions = buildSavedFilterConditions(representative.savedFilterFilters, logger);
96-
97119
// Only PERCENTAGE_CHANGE rules need the prior window; if none in the group do,
98120
// skip it (half the scan) and persist a null previousValue.
99121
const needsPreviousWindow = groupRules.some(r => r.thresholdType === 'PERCENTAGE_CHANGE');
@@ -116,6 +138,14 @@ export const task = implementTask(EvaluateMetricAlertRulesTask, async args => {
116138
'target.id': representative.targetId,
117139
'rules.in_group': groupRules.length,
118140
time_window_minutes: representative.timeWindowMinutes,
141+
// Rule intent, so a slow trace is findable in TraceQL without reading
142+
// db.query. filter.applied uses filterConditions (matching the gauge):
143+
// true only when the filter yields the heavier legacy-table query.
144+
'metric.types': [...new Set(groupRules.map(r => r.type))],
145+
'threshold.types': [...new Set(groupRules.map(r => r.thresholdType))],
146+
'filter.applied': filterConditions.length > 0,
147+
'window.needs_previous': needsPreviousWindow,
148+
'metric.needs_percentiles': needsPercentiles,
119149
},
120150
},
121151
async span => {
@@ -205,7 +235,7 @@ export const task = implementTask(EvaluateMetricAlertRulesTask, async args => {
205235
'rules.count': rules.length,
206236
'groups.count': groups.size,
207237
'groups.due': dueGroupList.length,
208-
'rules.due': dueGroupList.reduce((total, group) => total + group.length, 0),
238+
'rules.due': dueGroupList.reduce((total, { group }) => total + group.length, 0),
209239
'evaluation.time': evaluationTime.toISOString(),
210240
},
211241
},
@@ -220,7 +250,9 @@ export const task = implementTask(EvaluateMetricAlertRulesTask, async args => {
220250
let groupsFailed = 0;
221251
const evaluatedRuleIds: string[] = [];
222252
const results = await Promise.allSettled(
223-
dueGroupList.map(group => limit(() => processGroup(group))),
253+
dueGroupList.map(({ group, filterConditions }) =>
254+
limit(() => processGroup(group, filterConditions)),
255+
),
224256
);
225257
for (const r of results) {
226258
if (r.status === 'rejected') {

0 commit comments

Comments
 (0)