Skip to content

fix(alert): recover device offline detection - #164

Open
mislayming wants to merge 1 commit into
ongridio:mainfrom
mislayming:codex/fix-device-offline-recovery
Open

mislayming wants to merge 1 commit into
ongridio:mainfrom
mislayming:codex/fix-device-offline-recovery

Conversation

@mislayming

@mislayming mislayming commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a stable device_last_seen_timestamp_seconds{device_id} metric and use time() - max by (device_id)(...) for offline rules
  • refresh the timestamp metric from register/heartbeat/offline paths and prefer device last_seen_at when refreshing compatibility gauges
  • recover active metric_raw incidents from storage when their dedupe keys are absent from the current PromQL result, covering evaluator restarts and query label changes

Tests

  • go test ./internal/pkg/prom ./internal/manager/biz/edge ./internal/manager/biz/alert ./internal/manager/data/alert/store
  • go test -race ./internal/manager/biz/alert ./internal/manager/biz/edge ./internal/manager/data/alert/store
  • npm run typecheck
  • go test ./...

Author confirmation

  • I have read and agree to the project contribution guide in CONTRIBUTING.md.

@singchia

singchia commented Jul 10, 2026

Copy link
Copy Markdown
Member

整体方向没问题:把离线判断改成 device_last_seen_timestamp_seconds,让 PromQL 用 time() - last_seen 算年龄,比以前依赖 evaluator tick 刷 seconds_ago 更稳。冷启动后把已经恢复的旧 incident 关掉,这个也该做。

我建议这里再收一下两点:

  1. device_offline 现在还是 scope_type=global。这不是这次引入的问题,主线原来就是这样。但现在这个规则已经明确按 device_id 分组了,语义上更像 host/device 级别告警。继续用 global 也能按 label 生成不同 dedupe key,但 incident.device_id 不会写进去,后面设备详情、过滤、通知路由、RCA 上下文都会比较别扭。建议改成 host,并把旧的 edge_offline/device_offline 规则一起迁过去。

  2. resolveRecoveredPromQueryIncidents 现在每条 metric_raw 规则每轮都会按 open/ack/silenced 各查一次,limit 10000。规则多一点时,这里会变成固定的周期性扫库。建议至少合并成一次查询,或者只对需要 cold-start reconcile 的规则跑;否则 evaluator 间隔越短,DB 压力越明显。

还有一个小点:heartbeat 路径里更新 device_last_seen_timestamp_seconds 时,尽量用传进来的 heartbeat ts,不要用 time.Now(),这样 DB 里的 last_seen 和 Prom gauge 时间能保持一致。

@singchia

Copy link
Copy Markdown
Member

Reviewed again. I would not merge this as-is yet.

Two blockers:

  1. device_offline is still seeded with global scope. The PromQL result has device_id, but evaluatePromQuery only fills FiringInput.DeviceID for host-scoped rules. That means the offline incident is not reliably attached to the actual Device row, so device-level UI/actions/RCA context can lose the target.

  2. resolveRecoveredPromQueryIncidents scans active incidents on every evaluator tick: 3 statuses x up to 10000 rows x every metric_raw rule. That is too much work for the hot alert loop and will get worse as rules/incidents grow. Please make recovery targeted, for example keep the in-memory previous-firing path for normal operation and use a bounded startup/backfill path, or add a narrower repo query for active incidents by rule/status/dedupe keys instead of broad scans each tick.

After those are fixed, the direction is good: device LastSeenAt should be the source for heartbeat gauges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants