(POOLER-160) Revise Metrics Classwork

Review changes suggested to revise the Metrics related files into a more
logical class structure.

Also fixup grammar typos in docs strings and any trailing metrics that
have been recently added to vmpooler.

lib/vmpooler/metrics.rb

@@ -1,24 +1,24 @@
 # frozen_string_literal: true
 
+require 'vmpooler/metrics/statsd'
+require 'vmpooler/metrics/graphite'
+require 'vmpooler/metrics/promstats'
+require 'vmpooler/metrics/dummy_statsd'
+
 module Vmpooler
   class Metrics
     # static class instantiate appropriate metrics object.
     def self.init(logger, params)
       if params[:statsd]
-        metrics = Vmpooler::Statsd.new(logger, params[:statsd])
+        metrics = Vmpooler::Metrics::Statsd.new(logger, params[:statsd])
       elsif params[:graphite]
-        metrics = Vmpooler::Graphite.new(logger, params[:graphite])
+        metrics = Vmpooler::Metrics::Graphite.new(logger, params[:graphite])
       elsif params[:prometheus]
-        metrics = Vmpooler::Promstats.new(logger, params[:prometheus])
+        metrics = Vmpooler::Metrics::Promstats.new(logger, params[:prometheus])
       else
-        metrics = Vmpooler::DummyStatsd.new
+        metrics = Vmpooler::Metrics::DummyStatsd.new
       end
       metrics
     end
   end
 end
-
-require 'vmpooler/metrics/statsd'
-require 'vmpooler/metrics/dummy_statsd'
-require 'vmpooler/metrics/graphite'
-require 'vmpooler/metrics/promstats'

lib/vmpooler/metrics/dummy_statsd.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 module Vmpooler
+  class Metrics
     class DummyStatsd < Metrics
       attr_reader :server, :port, :prefix
 
@@ -20,3 +21,4 @@ module Vmpooler
       end
     end
   end
+end

lib/vmpooler/metrics/graphite.rb

@@ -3,6 +3,7 @@
 require 'rubygems' unless defined?(Gem)
 
 module Vmpooler
+  class Metrics
     class Graphite < Metrics
       attr_reader :server, :port, :prefix
 
@@ -43,3 +44,4 @@ module Vmpooler
       end
     end
   end
+end

lib/vmpooler/metrics/promstats.rb

@@ -3,6 +3,7 @@
 require 'prometheus/client'
 
 module Vmpooler
+  class Metrics
     class Promstats < Metrics
       attr_reader :prefix, :endpoint, :metrics_prefix
 
@@ -14,7 +15,7 @@ module Vmpooler
 
       # Customised Bucket set to use for the Pooler clone times set to more appropriate intervals.
       POOLER_TIME_BUCKETS = [1.0, 2.5, 5.0, 10.0, 20.0, 50.0, 100.0, 200.0, 500.0, 1000.0, 2000.0].freeze
-    # Same for Redis connection times - this is the same as the current Prometheus Default.
+      # Same for redis connection times - this is the same as the current Prometheus Default.
       # https://github.com/prometheus/client_ruby/blob/master/lib/prometheus/client/histogram.rb#L14
       REDIS_CONNECT_BUCKETS = [0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10].freeze
 
@@ -35,23 +36,30 @@ module Vmpooler
         {
           errors: {
             mtype: M_COUNTER,
-          docstring: 'Count of Errors for pool',
+            docstring: 'Count of errors for pool',
             prom_metric_prefix: "#{@metrics_prefix}_errors",
             metric_suffixes: {
-            markedasfailed: 'Timeout waiting for instance to initialise',
-            duplicatehostname: 'Unable to create instance due to duplicate hostname'
+              markedasfailed: 'timeout waiting for instance to initialise',
+              duplicatehostname: 'unable to create instance due to duplicate hostname',
+              staledns: 'unable to create instance due to duplicate DNS record'
             },
             param_labels: %i[template_name]
           },
           user: {
             mtype: M_COUNTER,
-          docstring: 'Number of Pool Instances this user created created',
+            docstring: 'Number of pool instances this user created created',
             prom_metric_prefix: "#{@metrics_prefix}_user",
             param_labels: %i[user poolname]
           },
+          usage_litmus: {
+            mtype: M_COUNTER,
+            docstring: 'Pools by Litmus job usage',
+            prom_metric_prefix: "#{@metrics_prefix}_usage_litmus",
+            param_labels: %i[user poolname]
+          },
           usage_jenkins_instance: {
             mtype: M_COUNTER,
-          docstring: 'Pools by Jenkins Instance usage',
+            docstring: 'Pools by Jenkins instance usage',
             prom_metric_prefix: "#{@metrics_prefix}_usage_jenkins_instance",
             param_labels: %i[jenkins_instance value_stream poolname]
           },
@@ -69,33 +77,33 @@ module Vmpooler
           },
           checkout: {
             mtype: M_COUNTER,
-          docstring: 'Pool Checkout counts',
+            docstring: 'Pool checkout counts',
             prom_metric_prefix: "#{@metrics_prefix}_checkout",
             metric_suffixes: {
-            nonresponsive: 'Checkout Failed - Non Responsive Machine',
-            empty: 'Checkout Failed - no machine',
-            success: 'Successful Checkout',
-            invalid: 'Checkout Failed - Invalid Template'
+              nonresponsive: 'checkout failed - non responsive machine',
+              empty: 'checkout failed - no machine',
+              success: 'successful checkout',
+              invalid: 'checkout failed - invalid template'
             },
             param_labels: %i[poolname]
           },
           config: {
             mtype: M_COUNTER,
-          docstring: 'vmpooler Pool Configuration Request',
+            docstring: 'vmpooler pool configuration request',
             prom_metric_prefix: "#{@metrics_prefix}_config",
             metric_suffixes: { invalid: 'Invalid' },
             param_labels: %i[poolname]
           },
           poolreset: {
             mtype: M_COUNTER,
-          docstring: 'Pool Reset Counter',
+            docstring: 'Pool reset counter',
             prom_metric_prefix: "#{@metrics_prefix}_poolreset",
             metric_suffixes: { invalid: 'Invalid Pool' },
             param_labels: %i[poolname]
           },
           connect: {
             mtype: M_COUNTER,
-          docstring: 'vmpooler Connect (to vSphere)',
+            docstring: 'vmpooler connect (to vSphere)',
             prom_metric_prefix: "#{@metrics_prefix}_connect",
             metric_suffixes: {
               open: 'Connect Succeeded',
@@ -105,31 +113,37 @@ module Vmpooler
           },
           migrate_from: {
             mtype: M_COUNTER,
-          docstring: 'vmpooler Machine Migrated from',
+            docstring: 'vmpooler machine migrated from',
             prom_metric_prefix: "#{@metrics_prefix}_migrate_from",
             param_labels: %i[host_name]
           },
           migrate_to: {
             mtype: M_COUNTER,
-          docstring: 'vmpooler Machine Migrated to',
+            docstring: 'vmpooler machine migrated to',
             prom_metric_prefix: "#{@metrics_prefix}_migrate_to",
             param_labels: %i[host_name]
           },
+          api_vm: {
+            mtype: M_COUNTER,
+            docstring: 'Total number of HTTP request/sub-operations handled by the Rack application under the /vm endpoint',
+            prom_metric_prefix: "#{@metrics_prefix}_http_requests_vm_total",
+            param_labels: %i[method subpath operation]
+          },
           ready: {
             mtype: M_GAUGE,
-          docstring: 'vmpooler Number of Machines in Ready State',
+            docstring: 'vmpooler number of machines in ready State',
             prom_metric_prefix: "#{@metrics_prefix}_ready",
             param_labels: %i[poolname]
           },
           running: {
             mtype: M_GAUGE,
-          docstring: 'vmpooler Number of Machines Running',
+            docstring: 'vmpooler number of machines running',
             prom_metric_prefix: "#{@metrics_prefix}_running",
             param_labels: %i[poolname]
           },
           connection_available: {
             mtype: M_GAUGE,
-          docstring: 'vmpooler Redis Connections Available',
+            docstring: 'vmpooler redis connections available',
             prom_metric_prefix: "#{@metrics_prefix}_connection_available",
             param_labels: %i[type provider]
           },
@@ -143,28 +157,28 @@ module Vmpooler
           migrate: {
             mtype: M_HISTOGRAM,
             buckets: POOLER_TIME_BUCKETS,
-          docstring: 'vmpooler Time taken to migrate machine for pool',
+            docstring: 'vmpooler time taken to migrate machine for pool',
             prom_metric_prefix: "#{@metrics_prefix}_migrate",
             param_labels: %i[poolname]
           },
           clone: {
             mtype: M_HISTOGRAM,
             buckets: POOLER_TIME_BUCKETS,
-          docstring: 'vmpooler Time taken to Clone Machine',
+            docstring: 'vmpooler time taken to clone machine',
             prom_metric_prefix: "#{@metrics_prefix}_clone",
             param_labels: %i[poolname]
           },
           destroy: {
             mtype: M_HISTOGRAM,
             buckets: POOLER_TIME_BUCKETS,
-          docstring: 'vmpooler Time taken to Destroy Machine',
+            docstring: 'vmpooler time taken to destroy machine',
             prom_metric_prefix: "#{@metrics_prefix}_destroy",
             param_labels: %i[poolname]
           },
           connection_waited: {
             mtype: M_HISTOGRAM,
             buckets: REDIS_CONNECT_BUCKETS,
-          docstring: 'vmpooler Redis Connection Wait Time',
+            docstring: 'vmpooler redis connection wait time',
             prom_metric_prefix: "#{@metrics_prefix}_connection_waited",
             param_labels: %i[type provider]
           }
@@ -305,3 +319,4 @@ module Vmpooler
       end
     end
   end
+end

lib/vmpooler/metrics/statsd.rb

@@ -4,6 +4,7 @@ require 'rubygems' unless defined?(Gem)
 require 'statsd'
 
 module Vmpooler
+  class Metrics
     class Statsd < Metrics
       attr_reader :server, :port, :prefix
 
@@ -36,3 +37,4 @@ module Vmpooler
       end
     end
   end
+end

spec/integration/api/v1/config_spec.rb

@@ -34,7 +34,7 @@ describe Vmpooler::API::V1 do
 
   describe '/config/pooltemplate' do
     let(:prefix) { '/api/v1' }
-    let(:metrics) { Vmpooler::DummyStatsd.new }
+    let(:metrics) { Vmpooler::Metrics::DummyStatsd.new }
 
     let(:current_time) { Time.now }
 

spec/integration/api/v1/ondemandvm_spec.rb

@@ -16,7 +16,7 @@ describe Vmpooler::API::V1 do
 
   describe '/ondemandvm' do
     let(:prefix) { '/api/v1' }
-    let(:metrics) { Vmpooler::DummyStatsd.new }
+    let(:metrics) { Vmpooler::Metrics::DummyStatsd.new }
     let(:config) {
       {
         config: {

spec/integration/api/v1/poolreset.rb

@@ -31,7 +31,7 @@ describe Vmpooler::API::V1 do
 
   describe '/poolreset' do
     let(:prefix) { '/api/v1' }
-    let(:metrics) { Vmpooler::DummyStatsd.new }
+    let(:metrics) { Vmpooler::Metrics::DummyStatsd.new }
 
     let(:current_time) { Time.now }
 

spec/integration/api/v1/vm_hostname_spec.rb

@@ -21,7 +21,7 @@ describe Vmpooler::API::V1 do
 
   describe '/vm/:hostname' do
     let(:prefix) { '/api/v1' }
-    let(:metrics) { Vmpooler::DummyStatsd.new }
+    let(:metrics) { Vmpooler::Metrics::DummyStatsd.new }
 
     let(:config) {
       {

spec/integration/api/v1/vm_spec.rb

@@ -17,7 +17,7 @@ describe Vmpooler::API::V1 do
 
   describe '/vm' do
     let(:prefix) { '/api/v1' }
-    let(:metrics) { Vmpooler::DummyStatsd.new }
+    let(:metrics) { Vmpooler::Metrics::DummyStatsd.new }
     let(:config) {
       {
         config: {

spec/integration/api/v1/vm_template_spec.rb

@@ -17,7 +17,7 @@ describe Vmpooler::API::V1 do
 
   describe '/vm/:template' do
     let(:prefix) { '/api/v1' }
-    let(:metrics) { Vmpooler::DummyStatsd.new }
+    let(:metrics) { Vmpooler::Metrics::DummyStatsd.new }
     let(:config) {
       {
         config: {

spec/unit/pool_manager_spec.rb

@@ -10,7 +10,7 @@ RSpec::Matchers.define :a_pool_with_name_of do |value|
 end
 describe 'Pool Manager' do
   let(:logger) { MockLogger.new }
-  let(:metrics) { Vmpooler::DummyStatsd.new }
+  let(:metrics) { Vmpooler::Metrics::DummyStatsd.new }
   let(:pool) { 'pool1' }
   let(:vm) { 'vm1' }
   let(:timeout) { 5 }

spec/unit/promstats_spec.rb

@@ -5,12 +5,12 @@ require 'spec_helper'
 describe 'prometheus' do
   logger = MockLogger.new
   params = { 'prefix': 'test', 'metrics_prefix': 'mtest', 'endpoint': 'eptest' }
-  subject = Vmpooler::Promstats.new(logger, params)
+  subject = Vmpooler::Metrics::Promstats.new(logger, params)
   let(:logger) { MockLogger.new }
 
   describe '#initialise' do
     it 'returns a Metrics object' do
-      expect(Vmpooler::Promstats.new(logger)).to be_a(Vmpooler::Metrics)
+      expect(Vmpooler::Metrics::Promstats.new(logger)).to be_a(Vmpooler::Metrics)
     end
   end
 
@@ -142,6 +142,13 @@ describe 'prometheus' do
           po.get(labels: metric[:labels])
         }.by(1)
       end
+      it 'Increments errors.staledns.#{pool_name}' do
+        pool_name = 'test-pool'
+        expect { subject.increment("errors.staledns.#{pool_name}") }.to change {
+          metric, po = subject.get("errors.staledns.#{pool_name}")
+          po.get(labels: metric[:labels])
+        }.by(1)
+      end
       it 'Increments user.#{user}.#{poolname}' do
         user = 'myuser'
         poolname = 'test-pool'
@@ -150,6 +157,14 @@ describe 'prometheus' do
           po.get(labels: metric[:labels])
         }.by(1)
       end
+      it 'Increments usage_litmus.#{user}.#{poolname}' do
+        user = 'myuser'
+        poolname = 'test-pool'
+        expect { subject.increment("usage_litmus.#{user}.#{poolname}") }.to change {
+          metric, po = subject.get("usage_litmus.#{user}.#{poolname}")
+          po.get(labels: metric[:labels])
+        }.by(1)
+      end
       it 'Increments label usage_jenkins_instance.#{jenkins_instance}.#{value_stream}.#{poolname}' do
         jenkins_instance = 'jenkins_test_instance'
         value_stream = 'notional_value'
@@ -177,7 +192,6 @@ describe 'prometheus' do
           po.get(labels: metric[:labels])
         }.by(1)
       end
-
       it 'Increments connect.open' do
         expect { subject.increment('connect.open') }.to change {
           metric, po = subject.get('connect.open')
@@ -204,6 +218,16 @@ describe 'prometheus' do
           po.get(labels: metric[:labels])
         }.by(1)
       end
+      it 'Increments label api_vm.#{method}.#{subpath}.#{operation}' do
+        method = 'get'
+        subpath = 'template'
+        operation = 'something'
+        expect { subject.increment("api_vm.#{method}.#{subpath}.#{operation}") }.to change {
+          metric, po = subject.get("api_vm.#{method}.#{subpath}.#{operation}")
+          po.get(labels: metric[:labels])
+        }.by(1)
+      end
+
     end
 
     describe '#gauge' do

spec/unit/providers/base_spec.rb

@@ -6,7 +6,7 @@ require 'vmpooler/providers/base'
 
 describe 'Vmpooler::PoolManager::Provider::Base' do
   let(:logger) { MockLogger.new }
-  let(:metrics) { Vmpooler::DummyStatsd.new }
+  let(:metrics) { Vmpooler::Metrics::DummyStatsd.new }
   let(:config) { {} }
   let(:provider_name) { 'base' }
   let(:provider_options) { { 'param' => 'value' } }

spec/unit/providers/dummy_spec.rb

@@ -3,7 +3,7 @@ require 'vmpooler/providers/dummy'
 
 describe 'Vmpooler::PoolManager::Provider::Dummy' do
   let(:logger) { MockLogger.new }
-  let(:metrics) { Vmpooler::DummyStatsd.new }
+  let(:metrics) { Vmpooler::Metrics::DummyStatsd.new }
   let(:pool_name) { 'pool1' }
   let(:other_pool_name) { 'pool2' }
   let(:vm_name) { 'vm1' }

spec/unit/providers/vsphere_spec.rb

@@ -41,7 +41,7 @@ end
 
 describe 'Vmpooler::PoolManager::Provider::VSphere' do
   let(:logger) { MockLogger.new }
-  let(:metrics) { Vmpooler::DummyStatsd.new }
+  let(:metrics) { Vmpooler::Metrics::DummyStatsd.new }
   let(:poolname) { 'pool1'}
   let(:provider_options) { { 'param' => 'value' } }
   let(:datacenter_name) { 'MockDC' }