separate flush_stats code from the backend #103

Closed
smarden1 opened this Issue Jun 14, 2012 · 2 comments

Comments

Projects
None yet
2 participants
@smarden1
Contributor

smarden1 commented Jun 14, 2012

I think we should consider separating the flush_stats code from the backend into a different location. If there are going to be multiple backends, then there should only be one place that handles all of the metrics. This way there is less duplication of the code and it makes testing the actual metrics creation a bit easier. Also, these are really two different ideas, the graphite backend should be responsible for sending data to graphite and the console backend should be responsible for sending data to the console, but they should be sending the same data.

This allows makes it easier to change how the metrics are actual created and open's up the door for better testing both in tests and in a local(console) mode.

For example, if I want to add median to all timers, it would currently be hard to test this in a local mode and my tests would involve a graphite backend. If someone had a fork of statsd with a different backend, then they would also have to re-implement all of my code on their new backend.

@mrtazz

This comment has been minimized.

Show comment Hide comment
@mrtazz

mrtazz Jun 18, 2012

Member

That's a good idea. Since the statistics calculations are pretty much a part of StatsD and not the backends, it makes sense to have it there. However while pulling more calculations into the main loop, we should keep an eye on not blocking the event loop too much. It maybe makes sense to move the calculations into a module.

Member

mrtazz commented Jun 18, 2012

That's a good idea. Since the statistics calculations are pretty much a part of StatsD and not the backends, it makes sense to have it there. However while pulling more calculations into the main loop, we should keep an eye on not blocking the event loop too much. It maybe makes sense to move the calculations into a module.

@mrtazz

This comment has been minimized.

Show comment Hide comment
@mrtazz

mrtazz Oct 20, 2012

Member

closing this in favour of #167

Member

mrtazz commented Oct 20, 2012

closing this in favour of #167

@mrtazz mrtazz closed this Oct 20, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment