From b9834720883283e4031e617b11be2ac5300839d2 Mon Sep 17 00:00:00 2001 From: Rick Sherman Date: Tue, 7 Jun 2016 23:01:03 -0500 Subject: [PATCH] (RE-7014) Add tracking of vm gets via statsd Add the tracking of successful, failed, invalid, and empty pool vm gets. It is possible we may want to tweak this, but have validated with spec tests and pcaps. ``` vmpooler-tmp-dev.ready.debian-7-x86_64:1|c vmpooler-tmp-dev.running.debian-7-x86_64:1|c vmpooler-tmp-dev.checkout.invalid:1|c vmpooler-tmp-dev.checkout.success.debian-7-x86_64:1|c vmpooler-tmp-dev.checkout.empty:1|c vmpooler-tmp-dev.running.debian-7-x86_64:1|c vmpooler-tmp-dev.clone.debian-7-x86_64:12.10|ms vmpooler-tmp-dev.ready.debian-7-x86_64:1|c ``` --- lib/vmpooler/api.rb | 3 ++- lib/vmpooler/api/v1.rb | 47 +++++++++++++++++++++++++++------ spec/vmpooler/api/v1_spec.rb | 51 +++++++++++++++++++++++++++++++----- vmpooler | 6 ++++- 4 files changed, 90 insertions(+), 17 deletions(-) diff --git a/lib/vmpooler/api.rb b/lib/vmpooler/api.rb index ee51cbc..45a9bfa 100644 --- a/lib/vmpooler/api.rb +++ b/lib/vmpooler/api.rb @@ -42,9 +42,10 @@ module Vmpooler use Vmpooler::API::Reroute use Vmpooler::API::V1 - def configure(config, redis, environment = :production) + def configure(config, redis, statsd, environment = :production) self.settings.set :config, config self.settings.set :redis, redis + self.settings.set :statsd, statsd self.settings.set :environment, environment end diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 140bfb8..42f092c 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -12,6 +12,16 @@ module Vmpooler Vmpooler::API.settings.redis end + def statsd + Vmpooler::API.settings.statsd + end + + def statsd_prefix + if Vmpooler::API.settings.statsd + Vmpooler::API.settings.config[:statsd]['prefix']? Vmpooler::API.settings.config[:statsd]['prefix'] : 'vmpooler' + end + end + def config Vmpooler::API.settings.config[:config] end @@ -32,13 +42,16 @@ module Vmpooler newhash = {} hash.each do |key, val| + if Vmpooler::API.settings.config[:alias][key] + key = Vmpooler::API.settings.config[:alias][key] + end + if backend.exists('vmpooler__ready__' + key) newhash[key] = val + elsif backend.exists('vmpooler__empty__' + key) + newhash['empty'] = (newhash['empty'] || 0) + val.to_i else - if Vmpooler::API.settings.config[:alias][key] - newkey = Vmpooler::API.settings.config[:alias][key] - newhash[newkey] = val - end + newhash['invalid'] = (newhash['invalid'] || 0) + val.to_i end end @@ -94,8 +107,10 @@ module Vmpooler vm = fetch_single_vm(template) if !vm failed = true + statsd.increment(statsd_prefix + '.checkout.fail.' + template, 1) break else + statsd.increment(statsd_prefix + '.checkout.success.' + template, 1) vms << [ template, vm ] end end @@ -375,8 +390,16 @@ module Vmpooler content_type :json result = { 'ok' => false } - if jdata and !jdata.empty? - result = atomically_allocate_vms(jdata) + if jdata + empty = jdata.delete('empty') + invalid = jdata.delete('invalid') + statsd.increment(statsd_prefix + '.checkout.empty', empty) if !empty.nil? + statsd.increment(statsd_prefix + '.checkout.invalid', invalid) if !invalid.nil? + if !jdata.empty? + result = atomically_allocate_vms(jdata) + else + status 404 + end else status 404 end @@ -400,8 +423,16 @@ module Vmpooler content_type :json result = { 'ok' => false } - if payload and !payload.empty? - result = atomically_allocate_vms(payload) + if payload + empty = payload.delete('empty') + invalid = payload.delete('invalid') + statsd.increment(statsd_prefix + '.checkout.empty', empty) if !empty.nil? + statsd.increment(statsd_prefix + '.checkout.invalid', invalid) if !invalid.nil? + if !payload.empty? + result = atomically_allocate_vms(payload) + else + status 404 + end else status 404 end diff --git a/spec/vmpooler/api/v1_spec.rb b/spec/vmpooler/api/v1_spec.rb index fac75de..a269f60 100644 --- a/spec/vmpooler/api/v1_spec.rb +++ b/spec/vmpooler/api/v1_spec.rb @@ -180,6 +180,7 @@ describe Vmpooler::API::V1 do describe '/vm' do let(:redis) { double('redis') } + let(:statsd) { double('stats') } let(:prefix) { '/api/v1' } let(:config) { { config: { @@ -190,20 +191,27 @@ describe Vmpooler::API::V1 do {'name' => 'pool1', 'size' => 5}, {'name' => 'pool2', 'size' => 10} ], - alias: { 'poolone' => 'pool1' } + alias: { 'poolone' => 'pool1' }, + statsd: { 'prefix' => 'vmpooler' } } } before do app.settings.set :config, config app.settings.set :redis, redis + app.settings.set :statsd, statsd - allow(redis).to receive(:exists).and_return '1' + allow(redis).to receive(:exists).with('vmpooler__token__abcdefghijklmnopqrstuvwxyz012345').and_return '1' + allow(redis).to receive(:exists).with('vmpooler__ready__pool1').and_return '1' + allow(redis).to receive(:exists).with('vmpooler__ready__pool2').and_return '1' allow(redis).to receive(:hget).with('vmpooler__token__abcdefghijklmnopqrstuvwxyz012345', 'user').and_return 'jdoe' allow(redis).to receive(:hset).and_return '1' allow(redis).to receive(:sadd).and_return '1' allow(redis).to receive(:scard).and_return '5' allow(redis).to receive(:spop).with('vmpooler__ready__pool1').and_return 'abcdefghijklmnop' allow(redis).to receive(:spop).with('vmpooler__ready__pool2').and_return 'qrstuvwxyz012345' + allow(statsd).to receive(:increment).with('vmpooler.checkout.success.pool1', 1) + allow(statsd).to receive(:increment).with('vmpooler.checkout.success.pool2', 1) + allow(statsd).to receive(:increment).with('vmpooler.checkout.fail.pool2', 1) end describe 'POST /vm' do @@ -223,7 +231,7 @@ describe Vmpooler::API::V1 do end it 'returns a single VM for an alias' do - expect(redis).to receive(:exists).with("vmpooler__ready__poolone").and_return(false) + expect(redis).to receive(:exists).with("vmpooler__ready__pool1").and_return(1) post "#{prefix}/vm", '{"poolone":"1"}' @@ -241,12 +249,22 @@ describe Vmpooler::API::V1 do it 'fails on nonexistent pools' do expect(redis).to receive(:exists).with("vmpooler__ready__poolpoolpool").and_return(false) - + expect(redis).to receive(:exists).with("vmpooler__empty__poolpoolpool").and_return(false) + expect(statsd).to receive(:increment).with('vmpooler.checkout.invalid', 1) post "#{prefix}/vm", '{"poolpoolpool":"1"}' expect_json(ok = false, http = 404) end + it 'fails on empty pools' do + expect(redis).to receive(:exists).with("vmpooler__ready__emptypool").and_return(false) + expect(redis).to receive(:exists).with("vmpooler__empty__emptypool").and_return(true) + expect(statsd).to receive(:increment).with('vmpooler.checkout.empty', 1) + post "#{prefix}/vm", '{"emptypool":"1"}' + + expect_json(ok = false, http = 404) + end + it 'returns multiple VMs' do post "#{prefix}/vm", '{"pool1":"1","pool2":"1"}' @@ -446,6 +464,7 @@ describe Vmpooler::API::V1 do describe '/vm/:template' do let(:redis) { double('redis') } + let(:statsd) { double('stats') } let(:prefix) { '/api/v1' } let(:config) { { config: { @@ -456,20 +475,27 @@ describe Vmpooler::API::V1 do {'name' => 'pool1', 'size' => 5}, {'name' => 'pool2', 'size' => 10} ], - alias: { 'poolone' => 'pool1' } + alias: { 'poolone' => 'pool1' }, + statsd: { 'prefix' => 'vmpooler' } } } before do app.settings.set :config, config app.settings.set :redis, redis + app.settings.set :statsd, statsd - allow(redis).to receive(:exists).and_return '1' + allow(redis).to receive(:exists).with('vmpooler__token__abcdefghijklmnopqrstuvwxyz012345').and_return '1' + allow(redis).to receive(:exists).with('vmpooler__ready__pool1').and_return '1' + allow(redis).to receive(:exists).with('vmpooler__ready__pool2').and_return '1' allow(redis).to receive(:hget).with('vmpooler__token__abcdefghijklmnopqrstuvwxyz012345', 'user').and_return 'jdoe' allow(redis).to receive(:hset).and_return '1' allow(redis).to receive(:sadd).and_return '1' allow(redis).to receive(:scard).and_return '5' allow(redis).to receive(:spop).with('vmpooler__ready__pool1').and_return 'abcdefghijklmnop' allow(redis).to receive(:spop).with('vmpooler__ready__pool2').and_return 'qrstuvwxyz012345' + allow(statsd).to receive(:increment).with('vmpooler.checkout.success.pool1', 1) + allow(statsd).to receive(:increment).with('vmpooler.checkout.success.pool2', 1) + allow(statsd).to receive(:increment).with('vmpooler.checkout.fail.pool2', 1) end describe 'POST /vm/:template' do @@ -489,7 +515,7 @@ describe Vmpooler::API::V1 do end it 'returns a single VM for an alias' do - expect(redis).to receive(:exists).with("vmpooler__ready__poolone").and_return(false) + expect(redis).to receive(:exists).with("vmpooler__ready__pool1").and_return(1) post "#{prefix}/vm/poolone", '' @@ -507,12 +533,23 @@ describe Vmpooler::API::V1 do it 'fails on nonexistent pools' do expect(redis).to receive(:exists).with("vmpooler__ready__poolpoolpool").and_return(false) + expect(redis).to receive(:exists).with("vmpooler__empty__poolpoolpool").and_return(false) + expect(statsd).to receive(:increment).with('vmpooler.checkout.invalid', 1) post "#{prefix}/vm/poolpoolpool", '' expect_json(ok = false, http = 404) end + it 'fails on empty pools' do + expect(redis).to receive(:exists).with("vmpooler__ready__emptypool").and_return(false) + expect(redis).to receive(:exists).with("vmpooler__empty__emptypool").and_return(true) + expect(statsd).to receive(:increment).with('vmpooler.checkout.empty', 1) + post "#{prefix}/vm/emptypool", '' + + expect_json(ok = false, http = 404) + end + it 'returns multiple VMs' do post "#{prefix}/vm/pool1+pool2", '' diff --git a/vmpooler b/vmpooler index 96b46a1..44ef89c 100755 --- a/vmpooler +++ b/vmpooler @@ -17,7 +17,11 @@ end api = Thread.new { thr = Vmpooler::API.new - thr.helpers.configure(config, Vmpooler.new_redis(redis_host)) + if statsd + thr.helpers.configure(config, Vmpooler.new_redis(redis_host), Vmpooler.new_statsd(statsd, statsd_port)) + else + thr.helpers.configure(config, Vmpooler.new_redis(redis_host), statsd=nil) + end thr.helpers.execute! }