feat(collector): implement crash-loop safety and panic recovery - #159
feat(collector): implement crash-loop safety and panic recovery#159wazer24 wants to merge 4 commits into
Conversation
Signed-off-by: wazer24 <24wazer@rbunagpur.in>
Signed-off-by: wazer24 <24wazer@rbunagpur.in>
|
🚀 First PR — welcome aboard! A few things to expect:
If you get stuck, reply here or jump to Discussions. We want this PR to land. |
github-actions
Bot
added
level:critical
Signed-off-by: wazer24 <24wazer@rbunagpur.in>
btwshivam
left a comment
btwshivam
left a comment
There was a problem hiding this comment.
good redo, the stale-base regressions from the last attempt are mostly gone (cgroup-memory case intact, no os.Exit in cli). two things: it reintroduces interface{}, and the daemon exit-code is picked by string-sniffing the error message.
| func main() { | ||
| if err := cli.New().Execute(); err != nil { | ||
| fmt.Fprintf(os.Stderr, "Error: %v\n", err) | ||
| if err.Error() != "" && err.Error()[0] == 'd' && err.Error()[:6] == "daemon" { |
There was a problem hiding this comment.
picking exit code 2 by string-matching the error message is brittle, any future error starting with daemon trips it. return a typed exit-code error from runStart (or a sentinel + errors.Is) and assert on that instead of sniffing err.Error().
| w.Header().Set("Content-Type", "application/json") | ||
| w.WriteHeader(http.StatusOK) | ||
| json.NewEncoder(w).Encode(map[string]any{ | ||
| json.NewEncoder(w).Encode(map[string]interface{}{ |
There was a problem hiding this comment.
reverts map[string]any back to map[string]interface{}. #54 swept the codebase to any, keep it.
| // HandlePanic processes a recovered panic. It writes the stack trace to a file | ||
| // (if logging is enabled), logs the error, and returns whether the collector should | ||
| // be permanently disabled due to crash-looping. | ||
| func (h *PanicHandler) HandlePanic(component string, r interface{}, logger *slog.Logger) bool { |
There was a problem hiding this comment.
r interface{} should be r any per the repo convention (same on line 108).
| // If we reach here, the collector loop exited normally. | ||
| // We don't restart; we just let the goroutine exit. | ||
| }() | ||
| } |
There was a problem hiding this comment.
on panic the goroutine just exits with no restart, which is the safe choice (no lock-held-on-panic loop), but it means one panic silently kills that collector for the daemon's life. worth a one-line doc comment stating the no-restart contract so callers don't assume self-healing.
| now := time.Now() | ||
|
|
||
| // Determine panic reason | ||
| reason := "unknown" |
There was a problem hiding this comment.
one file per panic under the log dir is unbounded on disk over the daemon lifetime. add a cap or rotation, or note the operator is responsible.
2 participants
What
Implemented crash-loop safety and panic recovery mechanisms for the collector goroutines, while resolving architectural feedback on graceful daemon shutdown and Prometheus cardinality. Also, restored missing BPF collector map wirings and updated modern Go syntax across the package.
Why
Fixes #95
How
RunSafeCollectorGoroutineincollector.goto recover from panics, log them, and permanently disable the offending collector instead of endlessly looping to prevent deadlocks.os.Exit(2)calls from library code (internal/cli/start.go). Daemon panic errors are now bubbled up gracefully and handled inmain.go.KERNO_PANIC_LOG_DIR. Leaving this environment variable unset disables file-based panic logging, defaulting safely to stdout/stderr.reasonlabel fromCollectorPanicsTotalto prevent excessive Prometheus memory consumption.CgroupMemorywiring inRegistry.Signalsthat was accidentally dropped in an older base branch.internal/collector/*.goSnapshot methods frominterface{}toany.Testing
go build ./...passesgo test ./...passesgo vet ./...passesgolangci-lint run ./...passesgo test -v ./internal/collector -run TestCollectorPanicRecoverysudo ./bin/bpf-verify --read 5sconfirms 6/6 programs still load./scripts/verify.shpasses (or specific phase:./scripts/verify.sh quality)Checklist
feat(scope): subject)git commit -s)scripts/verify.sh