-
Notifications
You must be signed in to change notification settings - Fork 13
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
is details a really good name? #20
Comments
Thank you, Martin. You make an interesting point about objection to |
As for the spec pushing metric everywhere - I will have to disagree with it. Main reason: Metrics are optional. You can absolutely use this spec to get the level of detail that stops as "can I use or not". However, there are cases when this simple "pass" or "fail" approach isn't sufficient. Such approach, while simple and often enough, is very reactive - you will only know things are bad once things start to "fail". There are plenty of cases when you may want to start noticing thing deteriorating before "fail" so appropriate measures can be taken. The spec merely allows such operation, without mandating it or making it necessary, so in that regard I think it hits a good middle ground. |
Will any of this be included in new version of RFC version (regarding Regarding metrics; I am still convinced that health-check should return boolean only instead of array of metrics; example https://status.github.com/api/status.json https://docs.gitlab.com/ee/user/admin_area/monitoring/health_check.html; I guess it depends who is going to use the service. Maybe I am mistaking it for Perhaps include an example without the use of metrics (in the RFC)? (Maybe I just do not know how to properly read RFCs, and do not discard optional things.) Thank you for the time! |
I had to publish the RFC since it had expired, but happy to hear suggestion for the better naming of "details". Regarding your other feedback: it is totally fine if you want to just return a boolean for a downstream dependency and not provide more details. This is how your example outputs would look like per RFC: Original from github status{"status":"good","last_updated":"2018-10-08T22:39:11Z"} RFC version of Github status{"status" : "pass"} (and caching/freshness negotiation is achieved using standard HTTP headers) Original from Gitlab status{
"queues_check" : {
"status" : "ok"
},
"redis_check" : {
"status" : "ok"
},
"shared_state_check" : {
"status" : "ok"
},
"db_check" : {
"status" : "ok"
},
"cache_check" : {
"status" : "ok"
}
} RFC version of the equivalent for Gitlab status{
"status": "pass",
"details": {
"queues_check": {"status": "pass"},
"redis_check": {"status": "pass"},
"shared_state_check": {"status": "pass"},
"db_check": {"status": "pass"},
"cache_check": {"status": "pass"}
}
} |
I really cringe when I read
details
in the response, I feel it should be namedservices
orcomponents
. In the RFC itself there is description ofdetails
:Emphasis mine
Overall I would think that
details
encapsulates human readablereason
with computer readable reason (code
) why such and such is not in pristine condition (as well aslinks
to documentation for specific code I tried to compose minimal example thus its not included):And of course each sub-component / service (not a detail) shall have its own
details
.Also RFC is really pushing metrics everywhere, I think that health-check is simple boolean kind of endpoint that answers the only question:
Can I use the API right now?
I quickly googled around and found an example
source
The text was updated successfully, but these errors were encountered: