Reporter "methods"

[bakins]

Simple (and untested) indirection between the core and reporters based on how its done in synapse. Should allow non-zk reporters to be added in the future.

Quick code - just wanted to get the idea out there. I'm interested in using etcd with expiring keys rather than zk.

[igor47]

this is definitely a good idea; i would support adding a 'reporter' parameter the service configuration, and defaulting it to zk using the zk_ params from the service hash as the defaults.

a note on testing: you can use our integration testing environment. it is as simple as running vagrant up and waiting 5 minutes for the environment to be created and tests to run. additional testing just takes a 30-second vagrant provision; learn how to do that here: https://github.com/airbnb/smartstack-cookbook#dev-and-testing

for your purposes, we might want to add etcd to the test environment.

[bakins]

My plan is to do the change in two steps: first, add the "reporter" parameter and default to zk. Second, add the etcd stuff. Sound reasonable?

I can change the code to make the parameter name "reporter" and do some testing.

Thanks.

[igor47]

i fully support this plan. include both the refactor and your new etcd reporter class in the same PR. that way, it's more clear what you're intending to do/why you're doing it.

[igor47]

ping @bakins; thanks for the original PR! any plans to come back and do the refactor we discussed?

[bakins]

@igor47 I just haven't had time. I am somewhat worried about using etcd as it so young and the ruby clients aren't that great, so I haven't invested much effort here. Maybe once I get the system up on nerve/synapse + zk, I can focus on etcd.

[bakins]

I took this in a slightly different direction. User can add additional reporters without hacking core. Defaults to zk.

[jolynch]

@bakins I think that #44 gives the functionality that this PR does, so I'm going to close this. If you think I missed something please let me know and we can re-open.