From 5c38ba240a82bd9d9020c7d068eb706b6e15d917 Mon Sep 17 00:00:00 2001 From: John O'Connor Date: Fri, 22 May 2020 17:07:33 +0100 Subject: [PATCH] (POOLER-160) Revise some smelly logger code Need the logger code in promstats.rb so move logger initialisation to the top of the vmpooler script, remove the class method from vmpooler.rb and make appropriate downstream changes to metrics and pooler manger handling. This also means we can start decent logging in the API if we wish do. Fixed up to rebase on top of Matts POOLER-158 changes --- bin/vmpooler | 5 +++-- lib/vmpooler.rb | 10 ---------- lib/vmpooler/metrics.rb | 8 ++++---- lib/vmpooler/metrics/graphite.rb | 5 +++-- lib/vmpooler/metrics/promstats.rb | 6 ++---- lib/vmpooler/metrics/statsd.rb | 9 +++++---- 6 files changed, 17 insertions(+), 26 deletions(-) diff --git a/bin/vmpooler b/bin/vmpooler index 17578ac..214480e 100755 --- a/bin/vmpooler +++ b/bin/vmpooler @@ -11,7 +11,8 @@ redis_connection_pool_size = config[:redis]['connection_pool_size'] redis_connection_pool_timeout = config[:redis]['connection_pool_timeout'] logger_file = config[:config]['logfile'] -metrics = Vmpooler::Metrics.init(config) +logger = Vmpooler::Logger.new logger_file +metrics = Vmpooler::Metrics.init(logger, config) torun_threads = [] if ARGV.count == 0 @@ -41,7 +42,7 @@ if torun.include? 'manager' manager = Thread.new do Vmpooler::PoolManager.new( config, - Vmpooler.new_logger(logger_file), + logger, Vmpooler.redis_connection_pool(redis_host, redis_port, redis_password, redis_connection_pool_size, redis_connection_pool_timeout, metrics), metrics ).execute! diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index b7dff93..4c12362 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -106,12 +106,6 @@ module Vmpooler parsed_config[:graphite]['prefix'] = ENV['GRAPHITE_PREFIX'] if ENV['GRAPHITE_PREFIX'] parsed_config[:graphite]['port'] = string_to_int(ENV['GRAPHITE_PORT']) if ENV['GRAPHITE_PORT'] - if parsed_config.key? :prometheus -# parsed_config[:prometheus]['endpoint'] = ENV['PROMETHEUS_ENDPOINT'] if ENV['PROMETHEUS_ENDPOINT'] -# parsed_config[:prometheus]['prefix'] = ENV['PROMETHEUS_PREFIX'] if ENV['PROMETHEUS_PREFIX'] -# parsed_config[:prometheus]['metrics_prefix'] = ENV['PROMETHEUS_METRICS_PREFIX'] if ENV['PROMETHEUS_METRICS_PREFIX'] - end - parsed_config[:auth] = parsed_config[:auth] || {} if ENV['AUTH_PROVIDER'] if parsed_config.key? :auth parsed_config[:auth]['provider'] = ENV['AUTH_PROVIDER'] if ENV['AUTH_PROVIDER'] @@ -189,10 +183,6 @@ module Vmpooler Redis.new(host: host, port: port, password: password) end - def self.new_logger(logfile) - Vmpooler::Logger.new logfile - end - def self.pools(conf) conf[:pools] end diff --git a/lib/vmpooler/metrics.rb b/lib/vmpooler/metrics.rb index 7d704e0..241dc18 100644 --- a/lib/vmpooler/metrics.rb +++ b/lib/vmpooler/metrics.rb @@ -3,13 +3,13 @@ module Vmpooler class Metrics # static class instantiate appropriate metrics object. - def self.init(params) + def self.init(logger, params) if params[:statsd] - metrics = Vmpooler::Statsd.new(params[:statsd]) + metrics = Vmpooler::Statsd.new(logger, params[:statsd]) elsif params[:graphite] - metrics = Vmpooler::Graphite.new(params[:graphite]) + metrics = Vmpooler::Graphite.new(logger, params[:graphite]) elsif params[:prometheus] - metrics = Vmpooler::Promstats.new(params[:prometheus]) + metrics = Vmpooler::Promstats.new(logger, params[:prometheus]) else metrics = Vmpooler::DummyStatsd.new end diff --git a/lib/vmpooler/metrics/graphite.rb b/lib/vmpooler/metrics/graphite.rb index 23740ed..e94e77a 100644 --- a/lib/vmpooler/metrics/graphite.rb +++ b/lib/vmpooler/metrics/graphite.rb @@ -6,12 +6,13 @@ module Vmpooler class Graphite < Metrics attr_reader :server, :port, :prefix - def initialize(params = {}) + def initialize(logger, params = {}) raise ArgumentError, "Graphite server is required. Config: #{params.inspect}" if params['server'].nil? || params['server'].empty? @server = params['server'] @port = params['port'] || 2003 @prefix = params['prefix'] || 'vmpooler' + @logger = logger end def increment(label) @@ -38,7 +39,7 @@ module Vmpooler rescue Errno::EADDRNOTAVAIL => e warn "Could not assign address to graphite server #{server}: #{e}" rescue StandardError => e - warn "Failure logging #{path} to graphite server [#{server}:#{port}]: #{e}" + @logger.log('s', "[!] Failure logging #{path} to graphite server [#{server}:#{port}]: #{e}") end end end diff --git a/lib/vmpooler/metrics/promstats.rb b/lib/vmpooler/metrics/promstats.rb index b5bd657..86ca64c 100644 --- a/lib/vmpooler/metrics/promstats.rb +++ b/lib/vmpooler/metrics/promstats.rb @@ -20,13 +20,11 @@ module Vmpooler @p_metrics = {} - def initialize(params = {}) + def initialize(logger, params = {}) @prefix = params['prefix'] || 'vmpooler' @metrics_prefix = params['metrics_prefix'] || 'vmpooler' @endpoint = params['endpoint'] || '/prometheus' - - # Hmm - think this is breaking the logger ..... - @logger = params.delete('logger') || Logger.new(STDOUT) + @logger = logger # Setup up prometheus registry and data structures @prometheus = Prometheus::Client.registry diff --git a/lib/vmpooler/metrics/statsd.rb b/lib/vmpooler/metrics/statsd.rb index a942e42..def7b58 100644 --- a/lib/vmpooler/metrics/statsd.rb +++ b/lib/vmpooler/metrics/statsd.rb @@ -7,31 +7,32 @@ module Vmpooler class Statsd < Metrics attr_reader :server, :port, :prefix - def initialize(params = {}) + def initialize(logger, params = {}) raise ArgumentError, "Statsd server is required. Config: #{params.inspect}" if params['server'].nil? || params['server'].empty? host = params['server'] @port = params['port'] || 8125 @prefix = params['prefix'] || 'vmpooler' @server = ::Statsd.new(host, @port) + @logger = logger end def increment(label) server.increment(prefix + '.' + label) rescue StandardError => e - warn "Failure incrementing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{e}" + @logger.log('s', "[!] Failure incrementing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{e}") end def gauge(label, value) server.gauge(prefix + '.' + label, value) rescue StandardError => e - warn "Failure updating gauge #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{e}" + @logger.log('s', "[!] Failure updating gauge #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{e}") end def timing(label, duration) server.timing(prefix + '.' + label, duration) rescue StandardError => e - warn "Failure updating timing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{e}" + @logger.log('s', "[!] Failure updating timing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{e}") end end end