-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(WIP) (RE-7014) Add support for statsd #150
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,18 @@ | ||
source ENV['GEM_SOURCE'] || 'https://rubygems.org' | ||
|
||
gem 'json', '>= 1.8' | ||
gem 'net-ldap', '<= 0.11' | ||
gem 'rack', '>= 1.6' | ||
gem 'rake', '>= 10.4' | ||
gem 'rbvmomi', '>= 1.8' | ||
gem 'redis', '>= 3.2' | ||
gem 'sinatra', '>= 1.4' | ||
gem 'statsd-ruby', '>= 1.3.0' | ||
|
||
# Test deps | ||
group :test do | ||
gem 'rack-test', '>= 0.6' | ||
gem 'rspec', '>= 3.2' | ||
gem 'simplecov', '>= 0.11.2' | ||
gem 'yarjuf', '>= 2.0' | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,17 @@ | ||
module Vmpooler | ||
class PoolManager | ||
def initialize(config, logger, redis, graphite=nil) | ||
def initialize(config, logger, redis, graphite=nil, statsd=nil) | ||
$config = config | ||
|
||
# Load logger library | ||
$logger = logger | ||
|
||
unless graphite.nil? | ||
# statsd and graphite are mutex in the context of vmpooler | ||
unless statsd.nil? | ||
$statsd = statsd | ||
end | ||
|
||
unless graphite.nil? || !statsd.nil? | ||
$graphite = graphite | ||
end | ||
|
||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like you're checking on # @return [SomeKindOfObject, nil] an object representing the defined processing system for metric collection
def metric_collector
$statsd || $graphite
end metric_collector.increment($config[:statsd]['prefix'] + '.ready.' + pool['name'], $redis.scard('vmpooler__ready__' + pool['name'])) (Disregard the use of |
||
rescue | ||
end | ||
|
@@ -565,7 +571,10 @@ def _check_pool(pool) | |
total = $redis.scard('vmpooler__pending__' + pool['name']) + ready | ||
|
||
begin | ||
if defined? $graphite | ||
if defined? $statsd | ||
$statsd.increment($config[:statsd]['prefix'] + '.ready.' + pool['name'], $redis.scard('vmpooler__ready__' + pool['name'])) | ||
$statsd.increment($config[:statsd]['prefix'] + '.running.' + pool['name'], $redis.scard('vmpooler__running__' + pool['name'])) | ||
elsif defined? $graphite | ||
$graphite.log($config[:graphite]['prefix'] + '.ready.' + pool['name'], $redis.scard('vmpooler__ready__' + pool['name'])) | ||
$graphite.log($config[:graphite]['prefix'] + '.running.' + pool['name'], $redis.scard('vmpooler__running__' + pool['name'])) | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
require 'rubygems' unless defined?(Gem) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can go away. |
||
|
||
module Vmpooler | ||
class Statsd | ||
def initialize( | ||
s = 'statsd', | ||
port = 8125 | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A nitpick and a must-do: Imperative: Don't declare single letter variable names. Ever. Maybe in iterators, but definitely never in initializers. |
||
@server = Statsd.new s, port | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 benil
instead of explicitly checking for it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the latter approach if we can try to match general style.