uber / ringpop-go

Scalable, fault-tolerant application-layer sharding for Go applications
http://www.uber.com
MIT License
835 stars 83 forks source link

Update testpop to emit stats to network or file #144

Closed severb closed 8 years ago

severb commented 8 years ago

In preparation for automated perf. tests we want to be able to emit stats to a file/network from testpop.

This PR adds a new flag to testpop to control where the stats should go; if the flag is omitted, no stats are emitted.

mennopruijssers commented 8 years ago

It looks like builds are failing to due a missing dependency:

scripts/testpop/statter.go:70: undefined: statsd.NewClientWithSender
severb commented 8 years ago

Yes, indeed. The build needs the latest (master tip) version of the statsd client library. That's because they added a new constructor that can be used to pass in your own custom Sender implementation.

motiejus commented 8 years ago

I don't see tests that different stats parameters show different behaviors. Are you going to add integration tests for it?

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.2%) to 94.169% when pulling 9db99fdfc1a593b3053f2a20054b00a3f204eb7b on feature/testpop-stats into f739dac7b28ce778cd0b74ca4479354a87548daa on dev.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.07%) to 94.263% when pulling 6a3ffb9cd875ba4c36359188064cdab1e2fb07f7 on feature/testpop-stats into f739dac7b28ce778cd0b74ca4479354a87548daa on dev.

CorgiMan commented 8 years ago

Looks good! EDIT: actually not done yet..

CorgiMan commented 8 years ago

I still have some comments

btromanova commented 8 years ago

lgtm

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.02%) to 94.333% when pulling 80279849b7ddfa7a88958235856f4af8a2c45502 on feature/testpop-stats into 5ac47e8ed00bb2960de5a42035f342747e257fa0 on dev.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.02%) to 94.38% when pulling d528bb2fff79449b59571c653f0e17d2c5d0d512 on feature/testpop-stats into 5ac47e8ed00bb2960de5a42035f342747e257fa0 on dev.

CorgiMan commented 8 years ago

Excellent! Looks good.