diff --git a/.changeset/itchy-otters-lead.md b/.changeset/itchy-otters-lead.md new file mode 100644 index 00000000000..72a9028ac16 --- /dev/null +++ b/.changeset/itchy-otters-lead.md @@ -0,0 +1,6 @@ +--- +'hive': patch +--- + +Metric Alerts: enhance Grafana with additional panels for tracking filtered vs unfiltered rule and +group population and attaches additional data to evaluate-group span diff --git a/deployment/grafana-dashboards/Metric-Alerts.json b/deployment/grafana-dashboards/Metric-Alerts.json index 0825b5f254b..b02cc4b18a2 100644 --- a/deployment/grafana-dashboards/Metric-Alerts.json +++ b/deployment/grafana-dashboards/Metric-Alerts.json @@ -264,7 +264,7 @@ "datasource": { "type": "prometheus", "uid": "PROM_DATASOURCE_UID" }, "editorMode": "code", "expr": "hive_workflow_job_duration_seconds{task_identifier=\"evaluateMetricAlertRules\",quantile=\"0.99\"}", - "legendFormat": "p99", + "legendFormat": "p99 {{pod}}", "range": true, "refId": "A" } @@ -272,9 +272,135 @@ "title": "evaluateMetricAlertRules task duration (p99)", "type": "timeseries" }, + { + "datasource": { "type": "prometheus", "uid": "PROM_DATASOURCE_UID" }, + "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.", + "fieldConfig": { + "defaults": { + "color": { "mode": "palette-classic" }, + "custom": { + "axisCenteredZero": false, + "axisPlacement": "auto", + "axisSoftMin": 0, + "drawStyle": "line", + "fillOpacity": 10, + "lineInterpolation": "linear", + "lineWidth": 1, + "stacking": { "group": "A", "mode": "none" }, + "thresholdsStyle": { "mode": "off" } + }, + "mappings": [], + "min": 0, + "noValue": "0", + "thresholds": { "mode": "absolute", "steps": [{ "color": "green", "value": null }] }, + "unit": "short" + }, + "overrides": [ + { + "matcher": { "id": "byName", "options": "with filter" }, + "properties": [{ "id": "color", "value": { "fixedColor": "orange", "mode": "fixed" } }] + } + ] + }, + "gridPos": { "h": 8, "w": 12, "x": 0, "y": 16 }, + "id": 5, + "options": { + "legend": { + "calcs": ["last", "max"], + "displayMode": "table", + "placement": "bottom", + "showLegend": true + }, + "tooltip": { "mode": "multi", "sort": "desc" } + }, + "pluginVersion": "9.3.6", + "targets": [ + { + "datasource": { "type": "prometheus", "uid": "PROM_DATASOURCE_UID" }, + "editorMode": "code", + "expr": "hive_metric_alert_rule_groups{filtered=\"true\"}", + "legendFormat": "with filter", + "range": true, + "refId": "A" + }, + { + "datasource": { "type": "prometheus", "uid": "PROM_DATASOURCE_UID" }, + "editorMode": "code", + "expr": "hive_metric_alert_rule_groups{filtered=\"false\"}", + "legendFormat": "no filter", + "range": true, + "refId": "B" + } + ], + "title": "Alert query groups: filtered vs unfiltered", + "type": "timeseries" + }, + { + "datasource": { "type": "prometheus", "uid": "PROM_DATASOURCE_UID" }, + "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).", + "fieldConfig": { + "defaults": { + "color": { "mode": "palette-classic" }, + "custom": { + "axisCenteredZero": false, + "axisPlacement": "auto", + "axisSoftMin": 0, + "drawStyle": "line", + "fillOpacity": 10, + "lineInterpolation": "linear", + "lineWidth": 1, + "stacking": { "group": "A", "mode": "none" }, + "thresholdsStyle": { "mode": "off" } + }, + "mappings": [], + "min": 0, + "noValue": "0", + "thresholds": { "mode": "absolute", "steps": [{ "color": "green", "value": null }] }, + "unit": "short" + }, + "overrides": [ + { + "matcher": { "id": "byName", "options": "with filter" }, + "properties": [{ "id": "color", "value": { "fixedColor": "orange", "mode": "fixed" } }] + } + ] + }, + "gridPos": { "h": 8, "w": 12, "x": 12, "y": 16 }, + "id": 6, + "options": { + "legend": { + "calcs": ["last", "max"], + "displayMode": "table", + "placement": "bottom", + "showLegend": true + }, + "tooltip": { "mode": "multi", "sort": "desc" } + }, + "pluginVersion": "9.3.6", + "targets": [ + { + "datasource": { "type": "prometheus", "uid": "PROM_DATASOURCE_UID" }, + "editorMode": "code", + "expr": "hive_metric_alert_enabled_rules{filtered=\"true\"}", + "legendFormat": "with filter", + "range": true, + "refId": "A" + }, + { + "datasource": { "type": "prometheus", "uid": "PROM_DATASOURCE_UID" }, + "editorMode": "code", + "expr": "hive_metric_alert_enabled_rules{filtered=\"false\"}", + "legendFormat": "no filter", + "range": true, + "refId": "B" + } + ], + "title": "Enabled rules: filtered vs unfiltered", + "type": "timeseries" + }, { "collapsed": false, - "gridPos": { "h": 1, "w": 24, "x": 0, "y": 16 }, + "gridPos": { "h": 1, "w": 24, "x": 0, "y": 24 }, "id": 100, "panels": [], "title": "Traces", @@ -296,7 +422,7 @@ }, "overrides": [] }, - "gridPos": { "h": 9, "w": 12, "x": 0, "y": 17 }, + "gridPos": { "h": 9, "w": 12, "x": 0, "y": 25 }, "id": 101, "options": { "cellHeight": "sm", @@ -320,7 +446,7 @@ }, { "datasource": { "type": "tempo", "uid": "TEMPO_DATASOURCE_UID" }, - "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.", + "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.", "fieldConfig": { "defaults": { "color": { "mode": "fixed" }, @@ -334,7 +460,7 @@ }, "overrides": [] }, - "gridPos": { "h": 9, "w": 12, "x": 12, "y": 17 }, + "gridPos": { "h": 9, "w": 12, "x": 12, "y": 25 }, "id": 102, "options": { "cellHeight": "sm", @@ -346,7 +472,7 @@ "datasource": { "type": "tempo", "uid": "TEMPO_DATASOURCE_UID" }, "filters": [{ "id": "slow-groups-filter", "operator": "=", "scope": "span" }], "limit": 100, - "query": "{name=\"evaluate-group\" && duration>$slowGroupThreshold} | select(.target.id, .rules.in_group, .time_window_minutes)", + "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)", "queryType": "traceql", "refId": "A", "spss": 20, @@ -389,7 +515,7 @@ } ] }, - "gridPos": { "h": 9, "w": 12, "x": 0, "y": 26 }, + "gridPos": { "h": 9, "w": 12, "x": 0, "y": 34 }, "id": 104, "options": { "legend": { @@ -443,7 +569,7 @@ }, "overrides": [] }, - "gridPos": { "h": 9, "w": 12, "x": 12, "y": 26 }, + "gridPos": { "h": 9, "w": 12, "x": 12, "y": 34 }, "id": 105, "options": { "legend": { @@ -497,7 +623,7 @@ }, "overrides": [] }, - "gridPos": { "h": 9, "w": 24, "x": 0, "y": 35 }, + "gridPos": { "h": 9, "w": 24, "x": 0, "y": 43 }, "id": 106, "options": { "legend": { @@ -541,7 +667,7 @@ }, "overrides": [] }, - "gridPos": { "h": 9, "w": 24, "x": 0, "y": 44 }, + "gridPos": { "h": 9, "w": 24, "x": 0, "y": 52 }, "id": 103, "options": { "cellHeight": "sm", diff --git a/packages/services/workflows/src/metrics.ts b/packages/services/workflows/src/metrics.ts index 27b1429a0a7..80538fecf22 100644 --- a/packages/services/workflows/src/metrics.ts +++ b/packages/services/workflows/src/metrics.ts @@ -54,3 +54,16 @@ export const metricAlertClickHouseQueryDuration = new metrics.Histogram({ buckets: [0.05, 0.1, 0.25, 0.5, 1, 2, 5, 10], labelNames: ['outcome'], }); + +// Enabled rules/query-groups split by whether they filter (the heavier +// legacy-table path). Tracks how much of that expensive population exists. +export const metricAlertEnabledRules = new metrics.Gauge({ + name: 'hive_metric_alert_enabled_rules', + help: 'Enabled metric alert rules, by whether a saved filter is attached', + labelNames: ['filtered'], +}); +export const metricAlertRuleGroups = new metrics.Gauge({ + name: 'hive_metric_alert_rule_groups', + help: 'Metric alert query groups (one ClickHouse query each), by whether a saved filter is attached', + labelNames: ['filtered'], +}); diff --git a/packages/services/workflows/src/tasks/evaluate-metric-alert-rules.ts b/packages/services/workflows/src/tasks/evaluate-metric-alert-rules.ts index ef7ef9f90a3..ad83820ca68 100644 --- a/packages/services/workflows/src/tasks/evaluate-metric-alert-rules.ts +++ b/packages/services/workflows/src/tasks/evaluate-metric-alert-rules.ts @@ -1,5 +1,6 @@ import pLimit from 'p-limit'; import { z } from 'zod'; +import type { SqlValue } from '@hive/clickhouse'; import { psql } from '@hive/postgres'; import { SpanKind, SpanStatusCode, trace } from '@hive/service-common'; import { defineTask, implementTask } from '../kit.js'; @@ -11,6 +12,7 @@ import { isRuleDue, queryClickHouseWindows, } from '../lib/metric-alert-evaluator.js'; +import { metricAlertEnabledRules, metricAlertRuleGroups } from '../metrics.js'; // How many groups to evaluate in parallel. Each group = 1 ClickHouse round-trip // plus a per-rule state-machine evaluation that holds a Postgres connection for @@ -58,6 +60,30 @@ export const task = implementTask(EvaluateMetricAlertRulesTask, async args => { // Evaluate every enabled rule. The org feature flag gates rule creation in // the API, not evaluation here; the per-rule `enabled` column is the gate. const rules = await fetchEnabledRules(context.pg); + const groups = groupRulesByQuery(rules); + + // Parse each group's saved filter once, reused by the gauges below and by + // processGroup, so a filtered group isn't re-parsed within the same tick. + const groupFilters = [...groups.values()].map(group => ({ + group, + filterConditions: buildSavedFilterConditions(group[0].savedFilterFilters, logger), + })); + + // Population gauges for the expensive with-filter rules. "Filtered" means the + // filter yields query conditions (the legacy-table path), not just that a + // saved_filter_id is set. Both labels set each tick (incl. 0) to avoid staleness. + let filteredGroups = 0; + let filteredRules = 0; + for (const { group, filterConditions } of groupFilters) { + if (filterConditions.length > 0) { + filteredGroups += 1; + filteredRules += group.length; + } + } + metricAlertRuleGroups.set({ filtered: 'true' }, filteredGroups); + metricAlertRuleGroups.set({ filtered: 'false' }, groups.size - filteredGroups); + metricAlertEnabledRules.set({ filtered: 'true' }, filteredRules); + metricAlertEnabledRules.set({ filtered: 'false' }, rules.length - filteredRules); if (rules.length === 0) { logger.debug('No enabled metric alert rules found'); @@ -69,8 +95,7 @@ export const task = implementTask(EvaluateMetricAlertRulesTask, async args => { // Evaluate only groups with at least one due member. Members of a group share // a window (so one cadence), and the batched UPDATE below keeps their // last_evaluated_at aligned, so a group is due as a unit. - const groups = groupRulesByQuery(rules); - const dueGroupList = [...groups.values()].filter(group => + const dueGroupList = groupFilters.filter(({ group }) => group.some(rule => isRuleDue(rule, evaluationTime)), ); @@ -82,18 +107,15 @@ export const task = implementTask(EvaluateMetricAlertRulesTask, async args => { return; } - async function processGroup(groupRules: (typeof rules)[number][]): Promise<{ + async function processGroup( + groupRules: (typeof rules)[number][], + filterConditions: SqlValue[], + ): Promise<{ failed: boolean; evaluatedIds: string[]; }> { const representative = groupRules[0]; - // All rules in a group share the same saved filter (it's part of the group key), - // so build the ClickHouse conditions once from the representative. - // A malformed filter yields no conditions (evaluates unfiltered) and is logged, - // isolating the failure to this group. - const filterConditions = buildSavedFilterConditions(representative.savedFilterFilters, logger); - // Only PERCENTAGE_CHANGE rules need the prior window; if none in the group do, // skip it (half the scan) and persist a null previousValue. const needsPreviousWindow = groupRules.some(r => r.thresholdType === 'PERCENTAGE_CHANGE'); @@ -116,6 +138,14 @@ export const task = implementTask(EvaluateMetricAlertRulesTask, async args => { 'target.id': representative.targetId, 'rules.in_group': groupRules.length, time_window_minutes: representative.timeWindowMinutes, + // Rule intent, so a slow trace is findable in TraceQL without reading + // db.query. filter.applied uses filterConditions (matching the gauge): + // true only when the filter yields the heavier legacy-table query. + 'metric.types': [...new Set(groupRules.map(r => r.type))], + 'threshold.types': [...new Set(groupRules.map(r => r.thresholdType))], + 'filter.applied': filterConditions.length > 0, + 'window.needs_previous': needsPreviousWindow, + 'metric.needs_percentiles': needsPercentiles, }, }, async span => { @@ -205,7 +235,7 @@ export const task = implementTask(EvaluateMetricAlertRulesTask, async args => { 'rules.count': rules.length, 'groups.count': groups.size, 'groups.due': dueGroupList.length, - 'rules.due': dueGroupList.reduce((total, group) => total + group.length, 0), + 'rules.due': dueGroupList.reduce((total, { group }) => total + group.length, 0), 'evaluation.time': evaluationTime.toISOString(), }, }, @@ -220,7 +250,9 @@ export const task = implementTask(EvaluateMetricAlertRulesTask, async args => { let groupsFailed = 0; const evaluatedRuleIds: string[] = []; const results = await Promise.allSettled( - dueGroupList.map(group => limit(() => processGroup(group))), + dueGroupList.map(({ group, filterConditions }) => + limit(() => processGroup(group, filterConditions)), + ), ); for (const r of results) { if (r.status === 'rejected') {