search

airbnb / nerve

A service registration daemon that performs health checks; companion to airbnb/synapse
MIT License
942 stars 151 forks source link

add serf reporter by https://github.com/patrickviet #82

Open jschaul opened 8 years ago

jschaul commented 8 years ago

Hi,

It would be nice if nerve would support serf as a reporter out of the box. I've been using a fork by https://github.com/patrickviet and this has worked well for us so far. The original commit is here: https://github.com/getyourguide/nerve/blob/bb1734211b7d5b162ad9e4d62f33cb2834bdf481/lib/nerve/reporter/serf.rb

Your contributing guidelines say to open a PR onto the pull_requests branch; though that one seems very much out-of-date, so I'm opening the PR to master. Let me know if I should re-open the PR to another branch.

also related to https://github.com/airbnb/nerve/issues/72

If you accept this I can also open a PR to synapse for serf support.

Thanks.

jolynch commented 8 years ago

Hello. Yea sorry about the contributing guidelines, I'm fixing them in #83

As for this PR, at the minimum we need some documentation in the README about what the parameters to this reporter are. It seems that it has an external dependency on serf running on the same box, which is cool (I haven't worked with serf but I grok it does registrations somewhere for you), but that needs to be documented too.

I'll comment inline on the diff itself.

jschaul commented 8 years ago

Hi @jolynch, thank you for your comments, sorry for the delay in getting back to this PR. I added a paragraph to the README on how to use this. Do you have any further comments / things I should change?