feat: add health check retry logic and alert cooldown to reduce false… - #468
inderjeet20 wants to merge 1 commit into
Conversation
|
Thank you for opening this PR! Before a maintainer takes a look, it would be really helpful if you could walk through your changes using GitHub's review tools. Please take a moment to:
More information on how to conduct a self review: This helps make the review process smoother and gives us a clearer understanding of your thought process. Once you've added your self-review, we'll continue from our side. Thank you! |
its-me-abhishek
left a comment
There was a problem hiding this comment.
I found 2 issues worth fixing before merging (AI-Generated, please check the feasibility of these, might be incorrect):
- Blocking:
deployment/health-check.shtreats any HTTP 200 as healthy.
- In
check_backend_endpoint(), the code does:curl --silent --show-error --fail --max-time "$CURL_TIMEOUT_SECONDS" "$HEALTH_URL" >/dev/null
- That only checks the status code, not the actual response body.
- A stale page, maintenance response, or a false-positive 200 from an upstream proxy would still be considered healthy, which directly undermines the goal of reducing false-positive alerts.
- Recommendation: validate the returned payload as well, e.g. check for a known healthy marker (
healthy,ok, or JSON status field) instead of only checking for HTTP success.
- Medium: cooldown state is not reset on recovery.
- In
main(), when the checks pass, it resetsconsecutive_failuresbut does not clearlast_alert_at. - That means a later incident can still be suppressed immediately after a recovered run if the previous incident was recent, even though the issue has already cleared.
- Recommendation: clear
last_alert_atwhen the system becomes healthy again, or otherwise scope the cooldown to the active failure window rather than permanently retaining the old timestamp.
These are both in deployment/health-check.sh; the first one is the more important correctness issue.
🚀 Summary
Reduce false-positive backend health alerts by introducing retry logic and cooldown.
🐛 Problem
Health alerts are currently triggered even when the backend is healthy. This appears to be caused by transient failures in the health check.
Additionally, alerting logic is not version-controlled and likely depends on an external VPS script, making it harder to maintain and debug.
✅ Solution
deployment/health-check.sh🎯 Benefits
📝 Notes
🧪 Testing
healthyduring normal operation