feat(metrics): report startup crash cause in not-running alarm#168
Conversation
When the collector dies during startup (e.g. sqlite3 native module failing to load under npm12), the updater only reported a generic "collector process is not running", losing the real cause. Record the cause as a breadcrumb the updater can read and surface it in the alarm message: - collector-daemon.js: write last-startup-crash.json (phase=module_load) in the bootstrap import().catch — the only place that observes a fatal module-graph load failure before main() runs. - index.ts: write the breadcrumb (phase=startup) in main().catch and clear it after a healthy start, so a lingering breadcrumb always reflects the most recent failed startup (PID/version-independent, survives auto-update). - updater-metrics: after the existing alibaba#133 debounce confirms absence, classify the breadcrumb (native_module_missing / module_not_found / config_error / permission_or_disk / unknown) and append cause/detail/phase/version to the SERVICE_NOT_RUNNING_ALARM message. Message-only enrichment: no new alarm type and no AlarmEntry schema change. Complements alibaba#164 (validate sqlite3 before activation) by making the failure observable when it still happens. Adds unit tests for the breadcrumb writer, classifier, and alarm enrichment.
| } | ||
| if ( | ||
| phase === 'startup' | ||
| && (text.includes('json') || text.includes('unexpected token') || text.includes('config')) |
There was a problem hiding this comment.
[Medium] config_error 分类过宽:此处对 text.includes(...) 匹配的 text 是 error_message + error_stack_head 的全量 lowercase(见 classifyStartupCrash 的 haystack)。启动期错误消息/栈里出现 json/config 子串(如解析 manifest/state 的 Unexpected end of JSON input、消息含 config 路径/字段名)会被误标为 config_error;且该判定排在下方 permission_or_disk 之前,导致「startup 期 EACCES 且消息含 config」被误标为配置错误,削弱本 PR 的分诊价值。
影响: 运维按 cause= 分诊被误导,config_error 假阳性偏高。
建议: (1) 仅对 error_message(不含 stack)匹配;(2) 用错误类型/码(SyntaxError 且含 JSON)替代裸子串 config;(3) 将 permission_or_disk(eacces/erofs/enospc)判定移到 config_error 之前。
Generated by LoongSuite-Pilot Code Review Agent
|
|
||
| // Resolve the data dir the updater will read the breadcrumb from (honors override). | ||
| function resolveDataDir() { | ||
| const raw = process.env.LOONGSUITE_PILOT_DATA_DIR; |
There was a problem hiding this comment.
[Medium] dataDir 三处解析不一致。这里的 resolveDataDir() 与 src/updater/index.ts 都只认 env(LOONGSUITE_PILOT_DATA_DIR) ?? ~/.loongsuite-pilot,而 src/index.ts 的写入/清除用 resolveHome(config.dataDir),其中 config.dataDir = env ?? file.dataDir ?? default(config-loader.ts:208)。当用户仅在 config.json 设置 dataDir(不设 env)时三者发散:
- (A)
module_load面包屑由本文件写到默认目录,但健康启动后clearStartupCrash(config.dataDir)清的是自定义目录 → 面包屑永不被清除,违反「lingering breadcrumb = 最近一次失败启动」不变式;后续一次无关的 not-running(手动停止/OOM)告警会读到陈旧面包屑并误标cause=native_module_missing。 - (B)
startupphase 面包屑写到自定义config.dataDir,但 updater 从默认目录读 → 读不到,增强静默失效。
影响: config.json 覆盖 dataDir 场景下,清除失效导致陈旧原因误报 + startup-phase 增强丢失。
建议: 收敛到单一 dataDir 解析源——让本文件与 updater/index.ts 也读取 config.json 的 file.dataDir(与 config-loader 对齐),或让 index.ts 的写/清除改用相同的 env ?? default 解析。务必让「写/清除/读」落在同一目录。
Generated by LoongSuite-Pilot Code Review Agent
🔍 Code Review Summary
Lifecycle Verdict
状态恢复正确性 FAIL 证据: PR 中心不变式「lingering breadcrumb 始终反映最近一次失败启动」在 总体结论不阻断合入,无 Critical/High。核心默认路径(默认 dataDir + sqlite3 主要风险集中在边界与分诊精度,建议合入前处理两条 Medium(已行内标注):
可后续改进(Low): Highlights(正向实践)
评审报告详见: |
…ause classification Medium alibaba#1 (dataDir divergence): the breadcrumb reader (updater) and the daemon writer both use env-or-default, but index.ts used config.dataDir (which includes file.dataDir). When dataDir is set only in config.json, clear-on-success and the startup-phase write missed the reader's directory, breaking the "lingering breadcrumb = latest failed startup" invariant. Introduce resolveBreadcrumbDataDir() (env ?? ~/.loongsuite-pilot) and use it for all write/clear in index.ts. Medium alibaba#2 (config_error too broad): reorder permission_or_disk before config_error, and match config_error only on a JSON-parse signature in the error message (not the full stack, which routinely contains config-loader.ts paths). Drop bare "config"/"json" substrings. Low alibaba#1: clear the breadcrumb on the `!config.enabled` deliberate-exit path. Low alibaba#3: sanitize detailHead (strip quotes/control chars, collapse whitespace) so it is safe to embed in the alarm `detail="..."`. Update classifier tests for the narrowed rules, ordering, stack-path safety and detail sanitization.
✅ Fixes Confirmed — Round 2 (
|
| Finding | 严重度 | 状态 | 说明 |
|---|---|---|---|
#1 dataDir 三处解析不一致 |
Medium | ✅ fixed | 新增 resolveBreadcrumbDataDir()（env-or-default），index.ts 写/清除改用它，与 daemon writer + updater reader 对齐；不再用 config.dataDir。不变式在 config.json 覆盖 dataDir 场景下现已成立 |
#2 config_error 分类过宽 |
Medium | ✅ fixed | 收紧为仅匹配 message（不含 stack）中的 JSON-parse 签名；permission_or_disk 前置到 config_error 之前 |
#3 !config.enabled 早退不清除 |
Low | ✅ fixed | disabled 分支补上 clearStartupCrash(...) |
#5 detail="..." 未转义 |
Low | ✅ fixed | 新增 sanitizeDetail() 去引号/控制字符 |
#4 ts 新鲜度约束 |
Low | ⬜ 未做（可接受） | #1/#3 修复后陈旧面包屑窗口已显著收窄，残余风险低，属可选优化 |
测试：分类器新增 4 个针对性用例（bare "config" 不触发、config-loader.ts 栈路径不误判、EACCES+config.json 优先 permission、detail 转义）。本地 vitest run（node 22）：33 passed / 3 files。
Lifecycle Verdict：状态恢复正确性 FAIL → PASS。无阻断项，LGTM ✅
Generated by LoongSuite-Pilot Code Review Agent
2 participants
Summary
When the collector dies during startup — most notably when the
sqlite3native module fails to load under npm12's stricter install-script policy — the only signal today is a genericSERVICE_NOT_RUNNING_ALARMwith the messageloongsuite-pilot collector process is not running. The real cause is written to the local service log and then lost, so operators cannot distinguish a native-module failure from an OOM kill, a corrupt config, or an intentional stop.The root problem is that the process that knows the cause (the dying collector) is not the one that sends the alarm (the updater), and the fatal case happens during ESM module-graph resolution — before
main()runs — so no in-process telemetry is available.This PR bridges that gap with a lightweight crash breadcrumb.
What changes
scripts/collector-daemon.js: in the bootstrapimport().catch, write~/.loongsuite-pilot/logs/last-startup-crash.json(phase=module_load). This is the only place that can observe a fatal module-load failure (e.g. sqlite3). Best-effort, atomic write; never masks the original error.src/index.ts: write the breadcrumb (phase=startup) inmain().catch, and clear it after a healthy start. A lingering breadcrumb therefore always reflects the most recent failed startup — PID/version-independent, so it stays correct across auto-update version swaps. Clean SIGTERM shutdown writes no breadcrumb.src/updater/updater-metrics.ts: after the existing fix(metrics): reduce not-running alarm false positives #133 debounce (startup grace + consecutive-failure threshold + cooldown) has confirmed the collector is genuinely absent, read the breadcrumb, classify the raw error into a stablereason, and appendcause / detail / phase / versionto theSERVICE_NOT_RUNNING_ALARMmessage.Reason labels:
native_module_missing(sqlite3/npm12),module_not_found,config_error,permission_or_disk,unknown(carries the raw message head verbatim).Design notes:
SERVICE_NOT_RUNNING_ALARM; no new alarm type, noAlarmEntryschema change.validate sqlite3 before activation): Fix sqlite3 installation under npm 12 #164 helps prevent rolling forward to a broken version; this PR makes the failure observable with a precise cause when it still occurs.Example enriched message:
Test plan
npm run typecheck— no new errors from these files (one pre-existing unrelated error inotlp-trace-flusher.ts).tests/unit/utils/crash-breadcrumb.test.ts,tests/unit/updater/startup-crash-classifier.test.ts, and a new enrichment case intests/unit/updater/updater-metrics.test.ts(29 tests pass).collector-daemon.jsagainst an isolated data dir with a simulated sqlite3 load failure → breadcrumb written withphase=module_load; verified the updater read+classify producedcause=native_module_missing; healthy start clears the breadcrumb.