(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
This commit is contained in:
John O'Connor 2020-05-22 17:07:33 +01:00
parent bbd76bde4c
commit 5c38ba240a
6 changed files with 17 additions and 26 deletions

View file

@ -11,7 +11,8 @@ redis_connection_pool_size = config[:redis]['connection_pool_size']
redis_connection_pool_timeout = config[:redis]['connection_pool_timeout'] redis_connection_pool_timeout = config[:redis]['connection_pool_timeout']
logger_file = config[:config]['logfile'] logger_file = config[:config]['logfile']
metrics = Vmpooler::Metrics.init(config) logger = Vmpooler::Logger.new logger_file
metrics = Vmpooler::Metrics.init(logger, config)
torun_threads = [] torun_threads = []
if ARGV.count == 0 if ARGV.count == 0
@ -41,7 +42,7 @@ if torun.include? 'manager'
manager = Thread.new do manager = Thread.new do
Vmpooler::PoolManager.new( Vmpooler::PoolManager.new(
config, 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), Vmpooler.redis_connection_pool(redis_host, redis_port, redis_password, redis_connection_pool_size, redis_connection_pool_timeout, metrics),
metrics metrics
).execute! ).execute!

View file

@ -106,12 +106,6 @@ module Vmpooler
parsed_config[:graphite]['prefix'] = ENV['GRAPHITE_PREFIX'] if ENV['GRAPHITE_PREFIX'] 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'] 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'] parsed_config[:auth] = parsed_config[:auth] || {} if ENV['AUTH_PROVIDER']
if parsed_config.key? :auth if parsed_config.key? :auth
parsed_config[:auth]['provider'] = ENV['AUTH_PROVIDER'] if ENV['AUTH_PROVIDER'] 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) Redis.new(host: host, port: port, password: password)
end end
def self.new_logger(logfile)
Vmpooler::Logger.new logfile
end
def self.pools(conf) def self.pools(conf)
conf[:pools] conf[:pools]
end end

View file

@ -3,13 +3,13 @@
module Vmpooler module Vmpooler
class Metrics class Metrics
# static class instantiate appropriate metrics object. # static class instantiate appropriate metrics object.
def self.init(params) def self.init(logger, params)
if params[:statsd] if params[:statsd]
metrics = Vmpooler::Statsd.new(params[:statsd]) metrics = Vmpooler::Statsd.new(logger, params[:statsd])
elsif params[:graphite] elsif params[:graphite]
metrics = Vmpooler::Graphite.new(params[:graphite]) metrics = Vmpooler::Graphite.new(logger, params[:graphite])
elsif params[:prometheus] elsif params[:prometheus]
metrics = Vmpooler::Promstats.new(params[:prometheus]) metrics = Vmpooler::Promstats.new(logger, params[:prometheus])
else else
metrics = Vmpooler::DummyStatsd.new metrics = Vmpooler::DummyStatsd.new
end end

View file

@ -6,12 +6,13 @@ module Vmpooler
class Graphite < Metrics class Graphite < Metrics
attr_reader :server, :port, :prefix 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? raise ArgumentError, "Graphite server is required. Config: #{params.inspect}" if params['server'].nil? || params['server'].empty?
@server = params['server'] @server = params['server']
@port = params['port'] || 2003 @port = params['port'] || 2003
@prefix = params['prefix'] || 'vmpooler' @prefix = params['prefix'] || 'vmpooler'
@logger = logger
end end
def increment(label) def increment(label)
@ -38,7 +39,7 @@ module Vmpooler
rescue Errno::EADDRNOTAVAIL => e rescue Errno::EADDRNOTAVAIL => e
warn "Could not assign address to graphite server #{server}: #{e}" warn "Could not assign address to graphite server #{server}: #{e}"
rescue StandardError => 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 end
end end

View file

@ -20,13 +20,11 @@ module Vmpooler
@p_metrics = {} @p_metrics = {}
def initialize(params = {}) def initialize(logger, params = {})
@prefix = params['prefix'] || 'vmpooler' @prefix = params['prefix'] || 'vmpooler'
@metrics_prefix = params['metrics_prefix'] || 'vmpooler' @metrics_prefix = params['metrics_prefix'] || 'vmpooler'
@endpoint = params['endpoint'] || '/prometheus' @endpoint = params['endpoint'] || '/prometheus'
@logger = logger
# Hmm - think this is breaking the logger .....
@logger = params.delete('logger') || Logger.new(STDOUT)
# Setup up prometheus registry and data structures # Setup up prometheus registry and data structures
@prometheus = Prometheus::Client.registry @prometheus = Prometheus::Client.registry

View file

@ -7,31 +7,32 @@ module Vmpooler
class Statsd < Metrics class Statsd < Metrics
attr_reader :server, :port, :prefix 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? raise ArgumentError, "Statsd server is required. Config: #{params.inspect}" if params['server'].nil? || params['server'].empty?
host = params['server'] host = params['server']
@port = params['port'] || 8125 @port = params['port'] || 8125
@prefix = params['prefix'] || 'vmpooler' @prefix = params['prefix'] || 'vmpooler'
@server = ::Statsd.new(host, @port) @server = ::Statsd.new(host, @port)
@logger = logger
end end
def increment(label) def increment(label)
server.increment(prefix + '.' + label) server.increment(prefix + '.' + label)
rescue StandardError => e 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 end
def gauge(label, value) def gauge(label, value)
server.gauge(prefix + '.' + label, value) server.gauge(prefix + '.' + label, value)
rescue StandardError => e 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 end
def timing(label, duration) def timing(label, duration)
server.timing(prefix + '.' + label, duration) server.timing(prefix + '.' + label, duration)
rescue StandardError => e 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 end
end end