feat(relay): Healthcheck command for self hosted and devservices#5044
feat(relay): Healthcheck command for self hosted and devservices#5044Dav1dde merged 7 commits intogetsentry:masterfrom
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
relay/src/healthcheck.rs
Outdated
| if mode != "live" && mode != "ready" { | ||
| return Err(format_err!( | ||
| "Invalid mode. Expected `live` or `ready`, got `{}`.", | ||
| mode | ||
| )); | ||
| } |
There was a problem hiding this comment.
Can we have clap validate this as an enum and then here just extract the enum?
relay/src/healthcheck.rs
Outdated
| match response { | ||
| Ok(response) => { | ||
| if response.status().is_success() { | ||
| println!("Relay is healthy."); |
There was a problem hiding this comment.
We generally avoid prints, if they are useful we can keep them, otherwise I would just only use the status code and instead work more with some log messages.
Co-authored-by: David Herberth <github@dav1d.de>
There was a problem hiding this comment.
Mind adding an integration test (maybe a new test_healthcheck.py) which asserts that the command has a bad exit code when Relay is not running but a successful one when it is running?
Bonus: a test which asserts a bad exit code when Relay is unhealthy (possible to configure through e.g. memory thresholds).
The get_relay_binary should give you the necessary binary to run the command via subprocess.
|
@Dav1dde Can I do that on a separate PR? I'm trying to chase the next release schedule on the 15th. |
|
Sure, if you're certain what you added works ;) |
Similar work to getsentry/relay#5044 due to distroless migration
Similar work to getsentry/relay#5044 due to distroless migration This works on relay and this is making CI's red right now. Changes made: - Adds a new `healthcheck` sub-command - Defaults to the values from the config - Changes logger to use `stderr` to still allow for logging and parsing of the healthcheck output (Relay uses the same mechanism here) --------- Co-authored-by: David Herberth <david.herberth@sentry.io>
We're (on self-hosted) trying to add healthcheck on Relay since David changed the base image to distroless, hence the only thing on the Docker image is just the relay binary.
I don't know how to pass in the config file while making it optional. If this is specific for self-hosted, I don't think reading from the config file is mandatory since we will be executing this on Docker Compose's healthcheck mechanism.