(WIP) (RE-7014) Add support for statsd #150
Conversation
shermdog
force-pushed
the
RE-7014
branch
2 times, most recently
from
c1a4c3d to
40bdd8b
Compare
| $config = config | ||
|
|
||
| # Load logger library | ||
| $logger = logger | ||
|
|
||
| unless graphite.nil? | ||
| # statsd and graphite are mutex in the context of vmpooler | ||
| unless statsd.nil? |
There was a problem hiding this comment.
I've got more to say about this in another comment I'm working on, but nil is already falsey, so you can just use the status of something that might be nil instead of explicitly checking for it.
# No don't do this!
unless graphite.nil?
# Yes do this!
if graphiteThere was a problem hiding this comment.
Word - I generally just follow what ever pattern was done before, but I'm by no means opposed to cleaning this up as we go :)
There was a problem hiding this comment.
I prefer the latter approach if we can try to match general style.
They way we were using graphite was incorrect for the type of data we were sending it. statsd is the appropriate mechanism for our needs. statsd and graphite are mutually exclusive and configuring statsd will take precendence over Graphite. Example of configuration in vmpooler.yaml.example
| @@ -258,6 +263,7 @@ def clone_vm(template, folder, datastore, target) | |||
| $redis.decr('vmpooler__tasks__clone') | |||
|
|
|||
| begin | |||
| $statsd.timing($config[:statsd]['prefix'] + ".clone.#{vm['template']}", finish) if defined? $statsd | |||
| $graphite.log($config[:graphite]['prefix'] + ".clone.#{vm['template']}", finish) if defined? $graphite | |||
There was a problem hiding this comment.
It looks like you're checking on $statsd and $graphite a lot. Consider letting the initializer define both of those values (and letting one fall through to nil, and then wrapping those into a general method (this is by no means complete but if you're interested then I can help you flesh it out):
# @return [SomeKindOfObject, nil] an object representing the defined processing system for metric collection
def metric_collector
$statsd || $graphite
endmetric_collector.increment($config[:statsd]['prefix'] + '.ready.' + pool['name'], $redis.scard('vmpooler__ready__' + pool['name']))(Disregard the use of $config[:statsd]['prefix']; that's part of the fleshed out solution)
shermdog
changed the title
This adds the statsd tracking of pool gets. It is seemingly working on success, but not fail. WIP
|
Random aside, if we are using statsd, it's worth knowing about brubeck: http://githubengineering.com/brubeck/ Cc @heathseals with whom I was talking about this |
|
Superseded by #155 |
They way we were using graphite was incorrect for the type of data we were sending it. statsd is the appropriate mechanism for our needs.
statsd and graphite are mutually exclusive and configuring statsd will take precendence over Graphite. Example of configuration in vmpooler.yaml.example