-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat: healthchecks for sentry components #3859
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
base: master
Are you sure you want to change the base?
Conversation
.env
Outdated
HEATLHCHECK_START_PERIOD=10s | ||
HEALTHCHECK_FILE_INTERVAL=60s |
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.
Potential bug: A typo in the `HEATLHCHECK_START_PERIOD` variable name causes the healthcheck grace period to be ignored, potentially leading to premature service restarts.
- Description: A typo in the environment variable
HEATLHCHECK_START_PERIOD
(missing an 'H') in the.env
file and its reference indocker-compose.yml
will cause Docker Compose to substitute an empty string for the healthcheckstart_period
. This leads to the value defaulting to0s
instead of the intended10s
. As a result, multiple services includingpostgres
,redis
,kafka
, andweb
will not have the intended grace period during startup, which could cause them to be marked as unhealthy and restart unnecessarily, impacting system reliability. - Suggested fix: Correct the typo in the environment variable name from
HEATLHCHECK_START_PERIOD
toHEALTHCHECK_START_PERIOD
in the.env
file and update the corresponding reference indocker-compose.yml
.
severity: 0.65, confidence: 0.98
Did we get this right? 👍 / 👎 to inform future reviews.
This reverts commit fc43389.
… compose wait command
@Dav1dde do you have any workarounds for this, or perhaps, how does the health check work on SaaS?
I'll ask around for uptime-checker. For taskbroker, we might leave it there. |
@aldy505 Relay healthcheck is possible on something like the kubernetes, where the orchestrator has the capability to make http request to the running container without the use of any binaries inside the image. Unfortunately docker compose does not have such a functionality. |
@mzglinski I think it's possible to add wget or curl into Relay's container. Let us wait for David. |
1 similar comment
@mzglinski I think it's possible to add wget or curl into Relay's container. Let us wait for David. |
Can you do the health check through a separate container which has curl?
Kubernetes supports HTTP probes.
That won't be possible, the image is completely empty except for Relay and a tiny bit of support libraries. |
Not really, compose does not support multiple containers per service, and the healthcheck is specific to the container/service. The only option I can see would involve a Dockerfile that builds on top of the relay image and adds curl/bash to the local image. Similar to the solution used for Sentry image. |
That's not great, immediate options which come to mind:
|
Let's do this one. |
Now we wait getsentry/relay#5044 |
Resolves #3853
Few services still are missing the healthcheck, here is why:
Proof:
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.