revsys / django-health-check

a pluggable app that runs a full check on the deployment, using a number of plugins to check e.g. database, queue server, celery processes, etc.
https://readthedocs.org/projects/django-health-check/
MIT License
1.24k stars 191 forks source link

Conform to Simple Standard Service Endpoints Spec #76

Open morenoh149 opened 7 years ago

morenoh149 commented 7 years ago

Hello, I've been researching the topic of api monitoring and came across your django package. I also came across this spec https://github.com/beamly/SE4/blob/master/SE4.md Was wondering if there's any ideas in the spec your project could borrow from?

best,

codingjoe commented 7 years ago

@morenoh149 we implement ASG, that could be documented I guess. Also @aleksihakli is working on a JSON endpoint. That should Healthcheck I guess. @aleksihakli what do you think?

aleksihakli commented 7 years ago

GTG is implemented functionally as well. Config is ready SE4 spec wise in #80.

I think conforming to the SE4 check spec would be smart and we could gradually extend the current API with SE4 endpoints that we want to offer externally. I think the smart ones would be:

Does that seem reasonable?

morenoh149 commented 7 years ago

In my implementation I only made gtg and asg openly available. Making config available felt like it'd make it too easy to hack the service.

aleksihakli commented 7 years ago

On a closer inspection we currently don't support GTG as we only check if plugins succeed from checks. A failure means the service is in non-operational state. We don't have a "partially working" check. We only offer "working" or "not working".

I guess we could add a text/plain response with "OK" easily and people could register the health check endpoint under /service/asg (or /service/gtg) URLs if they so want.

The question is whether we want to offer text/plain as the default reply if no Accept header is defined - at the moment we render text/html and changing the default response types is breaking the old functionality and warrants a release if we want to change that functionality.

The correct partial implementation of SE4, I guess, would be to offer:

codingjoe commented 7 years ago

@aleksihakli have you seen: https://github.com/mwarkentin/django-watchman I haven't tested it yet, since I just found it. Looks very promising.

aleksihakli commented 7 years ago

@codingjoe Watchman seems pretty neat but it isn't very KISS to be honest. A lot of features and feature duplication in there and you can't toggle some features. The design seems very mature though and I like the schemas they use. I think we should adopt some of them good ideas.

Do you mean the JSON schemas or other things as well?

codingjoe commented 7 years ago

Yeah, I know. Question is, do we want to bloat this package, or keep it very very simple. Furthermore, I don't know if we shouldn't join forces. I don't know if this topic is worth package diversity.

aleksihakli commented 7 years ago

I thought of the same thing about package diversity. One really good package would be a lot better than two decent ones.

A lot of work would, however, go into possibly merging the packages or porting essential functionality between code bases. On the other hand maintaining multiple different packages for the same purpose can be some work.

I think django-watchman offers much of our functionality but at the current moment it

The architectures are a bit different taking into consideration the decorator based check registrations and configuration model.

I do think we should initiate a conversation about the current state of the packages and joining forces, though, if we could manage something like that in near future. I don't know what @mwarkentin has planned for Watchman 1.0 release and other stuff :)

codingjoe commented 7 years ago

Ok, I agree. Honestly I'm happy what this package does so far. It could be better, but it does the job.

KristianOellegaard commented 7 years ago

We are also just using the simplicity of django health check and we're happy with the limited feature set, which on the contrary makes it a super easy component to add to any django app we deploy.