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

[jschaul]

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 #72

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

Thanks.

[jolynch]

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.

[jolynch]

This seems really jank to me because anything that assumes time works is usually a bad design (i.e. you have no guarantee when you'll be scheduled or that Time.new.to_i is at all monotonic). Can we do this with logical clocks somehow?

[jschaul]

You are right about this being bad design. This cleanup thread is not strictly necessary, as that cleanup of files is only necessary if services get renamed or removed from the nerve configuration. I added a note to the README that people should ensure to wipe the configuration directory through configuration management on changes to service names.

[jschaul]

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?