[QENG-4075] Unify graphite and statsd for the pool manager

Prior to this, the `pool_manager.rb` library could take handles for both
graphite and statsd endpoints (which were considered mutually exclusive),
and then would use one. There was a bevy of conditional logic around sending
metrics to the graphite/statsd handles (and actually at least one bug of
omission).

Here we refactor more, building on earlier work:

 - Our graphite class comes into line with the API of our Statsd and DummyStatsd classes
 - In `pool_manager.rb` we now accept a single "metrics" handle, and we drop all the conditional logic around statsd vs. graphite
 - We move the inconsistent error handling out of the calling classes and into our metrics classes, actually logging to `$stderr` when we can't publish metrics
 - We unify the setup code to use `config` to determine whether statsd, graphite, or a dummy metrics handle should be used, and make that happen.
 - Cleaned up some tests. We could probably stand to do a bit more work in this area.

lib/vmpooler.rb

@@ -54,10 +54,6 @@ module Vmpooler
       end
     end
 
-    if parsed_config[:graphite]['server']
-      parsed_config[:graphite]['prefix'] ||= 'vmpooler'
-    end
-
     if parsed_config[:tagfilter]
       parsed_config[:tagfilter].keys.each do |tag|
         parsed_config[:tagfilter][tag] = Regexp.new(parsed_config[:tagfilter][tag])
@@ -65,7 +61,6 @@ module Vmpooler
     end
 
     parsed_config[:uptime] = Time.now
-
     parsed_config
   end
 
@@ -77,19 +72,16 @@ module Vmpooler
     Vmpooler::Logger.new logfile
   end
 
-  def self.new_graphite(server)
-    if server.nil? or server.empty? or server.length == 0
-      nil
+  def self.new_metrics(params)
+    if config[:statsd]
+      Vmpooler::Statsd.new(config[:statsd])
+    elsif config[:graphite]
+      Vmpooler::Graphite.new(config[:graphite])
     else
-      Vmpooler::Graphite.new server
+      Vmpooler::DummyStatsd.new
     end
   end
 
-  def self.new_statsd(params)
-    return DummyStatsd.new unless params
-    Statsd.new params
-  end
-
   def self.pools(conf)
     conf[:pools]
   end

lib/vmpooler/graphite.rb

@@ -2,18 +2,41 @@ require 'rubygems' unless defined?(Gem)
 
 module Vmpooler
   class Graphite
-    def initialize(
-      s = 'graphite'
-    )
-      @server = s
+    attr_reader :server, :port, :prefix
+
+    def initialize(params = {})
+      if params[:server].nil? || params[:server].empty?
+        raise ArgumentError, "Graphite server is required. Config: #{params.inspect}"
+      end
+
+      @server = params[:server]
+      @port   = params[:port]   || 2003
+      @prefix = params[:prefix] || "vmpooler"
+    end
+
+    def increment(label)
+      log label, 1
+    end
+
+    def gauge(label, value)
+      log label, value
+    end
+
+    def timing(label, duration)
+      log label, duration
     end
 
     def log(path, value)
       Thread.new do
-        socket = TCPSocket.new(@server, 2003)
-        socket.puts "#{path} #{value} #{Time.now.to_i}"
-        socket.close
+        socket = TCPSocket.new(server, port)
+        begin
+          socket.puts "#{prefix}.#{path} #{value} #{Time.now.to_i}"
+        ensure
+          socket.close
+        end
       end
+    rescue => err
+      $stderr.puts "Failure logging #{path} to graphite server [#{server}:#{port}]: #{err}"
     end
   end
 end

lib/vmpooler/pool_manager.rb

@@ -1,17 +1,13 @@
 module Vmpooler
   class PoolManager
-    def initialize(config, logger, redis, graphite = nil, statsd = nil)
+    def initialize(config, logger, redis, metrics = nil)
       $config = config
 
       # Load logger library
       $logger = logger
 
-      # statsd and graphite are mutex in the context of vmpooler
-      if statsd
-        $statsd = statsd
-      elsif graphite
-        $graphite = graphite
-      end
+      # metrics logging handle
+      $metrics = metrics
 
       # Connect to Redis
       $redis = redis
@@ -66,7 +62,7 @@ module Vmpooler
           (host.summary.guest.hostName == vm)
 
         begin
-          Socket.getaddrinfo(vm, nil)
+          Socket.getaddrinfo(vm, nil)  # WTF?
         rescue
         end
 
@@ -260,11 +256,7 @@ module Vmpooler
 
         $redis.decr('vmpooler__tasks__clone')
 
-        begin
-          $statsd.timing($config[:statsd]['prefix'] + ".clone.#{vm['template']}", finish) if $statsd
-          $graphite.log($config[:graphite]['prefix'] + ".clone.#{vm['template']}", finish) if $graphite
-        rescue
-        end
+        $metrics.timing("clone.#{vm['template']}", finish)
       end
     end
 
@@ -297,8 +289,7 @@ module Vmpooler
           finish = '%.2f' % (Time.now - start)
 
           $logger.log('s', "[-] [#{pool}] '#{vm}' destroyed in #{finish} seconds")
-
-          $graphite.log($config[:graphite]['prefix'] + ".destroy.#{pool}", finish) if $graphite
+          $metrics.log("destroy.#{pool}", finish)
         end
       end
     end
@@ -568,16 +559,8 @@ module Vmpooler
       ready = $redis.scard('vmpooler__ready__' + pool['name'])
       total = $redis.scard('vmpooler__pending__' + pool['name']) + ready
 
-      begin
-        if $statsd
-          $statsd.gauge($config[:statsd]['prefix'] + '.ready.' + pool['name'], $redis.scard('vmpooler__ready__' + pool['name']))
-          $statsd.gauge($config[:statsd]['prefix'] + '.running.' + pool['name'], $redis.scard('vmpooler__running__' + pool['name']))
-        elsif $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
-      rescue
-      end
+      $metrics.gauge('ready.' + pool['name'], $redis.scard('vmpooler__ready__' + pool['name']))
+      $metrics.gauge('running.' + pool['name'], $redis.scard('vmpooler__running__' + pool['name']))
 
       if $redis.get('vmpooler__empty__' + pool['name'])
         unless ready == 0

lib/vmpooler/statsd.rb

@@ -10,21 +10,27 @@ module Vmpooler
       end
 
       host    = params[:server]
-      port    = params[:port]   || 8125
+      @port    = params[:port]   || 8125
       @prefix = params[:prefix] || 'vmpooler'
-      @server = Statsd.new(host, port)
+      @server = Statsd.new(host, @port)
     end
 
     def increment(label)
       server.increment(prefix + "." + label)
+    rescue => err
+      $stderr.puts "Failure incrementing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{err}"
     end
 
     def gauge(label, value)
       server.gauge(prefix + "." + label, value)
+    rescue => err
+      $stderr.puts "Failure updating gauge #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{err}"
     end
 
     def timing(label, duration)
       server.timing(prefix + "." + label, duration)
+    rescue => err
+      $stderr.puts "Failure updating timing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{err}"
     end
   end