-
Notifications
You must be signed in to change notification settings - Fork 4
Add healthcheck endpoint, move metrics endpoint to main port #297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
d886a3d to
4659bd7
Compare
isomorpheme
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Big +1 on just having the metrics endpoint on the same port and in the same server thread, I think having that separate buys us very little.
Looks good overall, just some style comments, and I think we do want to keep using wai-middleware-prometheus if possible (see other comment).
ca9ff8c to
6867243
Compare
isomorpheme
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
| ghQueueFull <- atomically $ isFullTBQueue ghQueue | ||
| projectQueuesFull <- mapM (atomically . isFullTBQueue) $ fmap projectThreadQueue projectThreadState | ||
|
|
||
| return $ not $ ghQueueFull || or projectQueuesFull |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading the || or slightly confused me for a second. I'm not sure if I actually prefer this, but you could skip the || like so:
| return $ not $ ghQueueFull || or projectQueuesFull | |
| return $ not $ or $ ghQueueFull : projectQueuesFull |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
projectQueuesFull isn't a list, so we can't even nicely rewrite this 🙃
|
@OpsBotPrime merge |
Approved-by: Riscky Priority: Normal Auto-deploy: false
|
Rebased as 8780d43, waiting for CI … |
|
CI job 🟡 started. |
6867243 to
8780d43
Compare
This adds a
/healthendpoint, which checks the application health. For now it only checks if the queues are full.I've also moved
/metricsto the main port. I don't think there is a good reason to advertise two separate ports.