Merge lp://staging/~theiw/txstatsd/refactor-with-gaugemetric into lp://staging/txstatsd
Proposed by
Ian Wilkinson
Status: | Merged |
---|---|
Merged at revision: | 15 |
Proposed branch: | lp://staging/~theiw/txstatsd/refactor-with-gaugemetric |
Merge into: | lp://staging/txstatsd |
Diff against target: |
1808 lines (+999/-636) 22 files modified
example-stats-client.tac (+16/-9) statsd.tac (+1/-1) twisted/plugins/txstatsd_plugin.py (+1/-1) txstatsd/client/client.py (+94/-0) txstatsd/metrics.py (+0/-154) txstatsd/metrics/gaugemetric.py (+22/-0) txstatsd/metrics/meter.py (+154/-0) txstatsd/metrics/metric.py (+47/-0) txstatsd/metrics/metrics.py (+80/-0) txstatsd/processor.py (+0/-140) txstatsd/protocol.py (+0/-82) txstatsd/report.py (+0/-46) txstatsd/server/processor.py (+185/-0) txstatsd/server/protocol.py (+61/-0) txstatsd/server/report.py (+46/-0) txstatsd/server/service.py (+128/-0) txstatsd/service.py (+0/-128) txstatsd/tests/test_meter.py (+72/-0) txstatsd/tests/test_metrics.py (+60/-72) txstatsd/tests/test_processor.py (+30/-1) txstatsd/tests/test_service.py (+1/-1) txstatsd/version.py (+1/-1) |
To merge this branch: | bzr merge lp://staging/~theiw/txstatsd/refactor-with-gaugemetric |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Sidnei da Silva | Needs Fixing | ||
Lucio Torre | Pending | ||
Review via email:
|
Description of the change
Initial refactoring of the metrics (including gauge metric support).
To post a comment you must log in.
[1] I don't see a reason to move service.py into server. Since it uses both
client and server, it should remain at top-level.
[2] Similarly, I don't see the point of moving the client protocol part to client. py. Just moving it to client.py would have been enough.
client/
[3] I like that you added a raw socket UdpStatsDClient, but the naming is
confusing because StatsDClient also uses UDP, though it relies on a twisted
transport. Since StatsDClient takes an optional 'transport' argument, they
are not even interchangeable. I'd just remove it unless you plan to use it
somewhere.
[4] The separation of the Metrics class and the individual Metric classes seems
fairly arbitrary, and I don't think having a reference to the connection is
helping much here. I suggest getting rid of the GaugeMetric class and move
the implementation of 'mark' from GaugeMetric to the 'gauge' method into the
Metrics class, also move 'send' back to 'Metrics'.
[5] Once you do the above, it makes sense to move metrics/metrics.py back to
just metrics.py, IMO.
[6] Please keep process.py and report.py top-level, since they have little
relation to 'server'. In fact, I'm using functions from 'report' on my statsd
clients, so it's weird to import it from 'server'.
[7] I like that you split 'flush_gauge' out of 'flush'. While you're there, counters' and 'flush_timers'.
maybe it makes sense to refactor the counters and timers handling into
'flush_