Remove support for metrics-type legacy config file

The script must now be invoked with the --config option to specify a configuration file, OR all required configuration parameters must be passed as command-line flags. Because this involves a change in the API to interact with the script, the amq_metrics script is now in scope also, and has had the same option parsing interface applied to it.
npwalker · Jun 15, 2017 · 9d228ab · 9d228ab
1 parent 0a606cb
commit 9d228ab
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 53 deletions.
diff --git a/files/amq_metrics b/files/amq_metrics
@@ -7,31 +7,70 @@ require 'time'
 require 'optparse'
 require 'yaml'
 
-options = {}
-OptionParser.new do |opts|
-  opts.banner = "Usage: amq_metrics [options]"
+#===========================================================================#
+# BUILD CONFIGURATION OPTIONS                                               #
+#===========================================================================#
+
+settings = { }
+config = { }
+from_cli = { }
+from_file = { }
+
+OptionParser.new do |parser|
+  parser.banner = "Usage: tk_metrics [options]"
+
+  save_ident = lambda { |name,arg| from_cli[name] = arg }
+  save_list  = lambda { |name,arg| from_cli[name] = arg.split(',') }
+
+  opt_bool  = lambda { |arg| "--[no-]#{arg}" }
+  opt_value = lambda { |arg| "--#{arg} ARG" }
+
+  option = lambda do |name,desc,fopt,fsave,default=nil|
+    settings[name] = { default: default }
+    parser.on(fopt.call(name.to_s), desc) { |arg| fsave.call(name, arg) }
+  end
+
+  # Each possible configuration option is defined here
+  option.call(:config, 'Path to configuration file', opt_value, save_ident, false)
+  option.call(:"output-dir", 'Directory to save output to', opt_value, save_ident, false)
+  option.call(:hosts, 'Hosts to collect metrics from (comma-separated)', opt_value, save_list, ['localhost'])
+  option.call(:"additional-metrics", 'Additional metrics to collect (comma-separated)', opt_value, save_list, [])
+  option.call(:"metrics-port", 'The port the metrics service runs on', opt_value, save_ident)
+  option.call(:clientcert, 'Not used', opt_value, save_ident)
+  option.call(:"pe-version", 'The version of PE in use', opt_value, save_ident)
+  option.call(:print, 'Print to stdout', opt_bool, save_ident, true)
+  option.call(:ssl, 'Whether or not to use SSL when gather metrics', opt_bool, save_ident, true)
+  option.call(:"metrics-type", 'Type of metric to collect', opt_value, save_ident)
+
+  # Kinda pointless to include descriptions unless there's a help flag
+  parser.on("-h", "--help", "Prints this help") { puts parser; exit }
 
-  opts.on('-p', '--[no-]print', 'Print to stdout') { |p| options[:print] = p }
-  opts.on('-m [TYPE]', '--metrics_type [TYPE]', 'Type of metric to collect') { |v| options[:metrics_type] = v }
-  opts.on('-o [DIR]', '--output-dir [DIR]', 'Directory to save output to') { |o| options[:output_dir] = o }
 end.parse!
 
-if options[:metrics_type].nil? then
-  STDERR.puts '--metrics_type (-m) is a required argument'
-  exit 1
+# If a configuration file has been specified, read additional configuration
+# from it.
+if from_cli[:config]
+  begin
+    from_file = YAML.load_file(from_cli[:config])
+  rescue Exception => e
+    STDERR.puts "ERROR: unable to read configuration from #{config[:config]}: #{e}"
+    exit 1
+  end
 end
 
-METRICS_TYPE = options[:metrics_type]
-config = YAML.load_file(File.join(File.dirname(File.expand_path(__FILE__)),"#{METRICS_TYPE}_config.yaml"))
-
-OUTPUT_DIR = options[:output_dir]
-HOSTS      = config['hosts']
-PORT       = config['metrics_port']
-METRICS    = config['additional_metrics']
-CLIENTCERT = config['clientcert']
-PE_VERSION = config['pe_version']
+# For the options which MAY be read from a config file, retrieve those values
+# if they have not been specified during invocation. Values specified on the
+# command line are given precedence.
+settings.each do |opt,attrs|
+  possibilities = [from_cli[opt], from_file[opt.to_s], attrs[:default]]
+  if (config[opt] = possibilities.find {|p| !p.nil? }).nil?
+    raise("ERROR: Missing configuration option \"#{opt}\"")
+  end
+end
 
-POST_DATA = METRICS.to_json
+#===========================================================================#
+# MAIN SCRIPT LOGIC                                                         #
+#===========================================================================#
 
 def recurse_merge!(a,b)
   a.merge!(b) do |_,aa,bb|
@@ -70,39 +109,39 @@ end
 
 filename = Time.now.utc.strftime('%Y%m%dT%H%M%SZ') + '.json'
 
-HOSTS.each do |host|
+config[:hosts].each do |host|
   begin
     timestamp = Time.now
     dataset = {'timestamp' => timestamp.utc.iso8601, 'servers' => {}}
     hostkey = host.gsub('.', '-')
-    dataset['servers'][hostkey] = {METRICS_TYPE => {}}
+    dataset['servers'][hostkey] = {config[:"metrics-type"] => {}}
 
-    host_url = "https://#{host}:#{PORT}/api/jolokia"
-    response = get_endpoint(host_url, POST_DATA)
+    host_url = "https://#{host}:#{config[:port]}/api/jolokia"
+    response = get_endpoint(host_url, config[:"additional-metrics"].to_json)
 
     JSON.parse(response.body).each do |element|
       case element['value']
       when Hash
         element['value'].each do |mbean,attributes|
           metrics = treeify(mbean, attributes)
-          recurse_merge!(dataset['servers'][hostkey][METRICS_TYPE], metrics)
+          recurse_merge!(dataset['servers'][hostkey][config[:"metrics-type"]], metrics)
         end
       else
         req = element['request']
         metrics = treeify(req['mbean'], { req['attribute'] => req['value'] })
-        recurse_merge!(dataset['servers'][hostkey][METRICS_TYPE], metrics)
+        recurse_merge!(dataset['servers'][hostkey][config[:"metrics-type"]], metrics)
       end
     end
 
-    dataset['servers'][hostkey][METRICS_TYPE]['error'] = $error_array
-    dataset['servers'][hostkey][METRICS_TYPE]['error_count'] = $error_array.count
-    dataset['servers'][hostkey][METRICS_TYPE]['api-query-start'] = timestamp.utc.iso8601
-    dataset['servers'][hostkey][METRICS_TYPE]['api-query-duration'] = Time.now - timestamp
+    dataset['servers'][hostkey][config[:"metrics-type"]]['error'] = $error_array
+    dataset['servers'][hostkey][config[:"metrics-type"]]['error_count'] = $error_array.count
+    dataset['servers'][hostkey][config[:"metrics-type"]]['api-query-start'] = timestamp.utc.iso8601
+    dataset['servers'][hostkey][config[:"metrics-type"]]['api-query-duration'] = Time.now - timestamp
 
     json_dataset = JSON.pretty_generate(dataset)
 
-    unless OUTPUT_DIR.nil? then
-      Dir.chdir(OUTPUT_DIR) do
+    unless config[:"output-dir"].nil? then
+      Dir.chdir(config[:"output-dir"]) do
         Dir.mkdir(host) unless File.exist?(host)
         File.open(File.join(host, filename), 'w') do |file|
           file.write(json_dataset)
@@ -114,5 +153,6 @@ HOSTS.each do |host|
     end
   rescue Exception => e
     STDERR.puts "Error getting metrics for #{host}: #{e}"
+    STDERR.puts e.backtrace
   end
 end
diff --git a/files/tk_metrics b/files/tk_metrics
@@ -47,27 +47,14 @@ OptionParser.new do |parser|
 
 end.parse!
 
-# This is to support legacy from the time when passing --metrics-type was what
-# determined the filename of the config file, rather than the --config option.
-metrics_type_file = if from_cli[:"metrics-type"]
-  scriptdir = File.dirname(File.expand_path(__FILE__))
-  legacyfile = "#{from_cli[:"metrics-type"]}_config.yaml"
-  File.join(scriptdir, legacyfile)
-end
-
 # If a configuration file has been specified, read additional configuration
-# from it. Some complexity exists due to support for metrics-type filename.
-config[:config] = from_cli[:config] || metrics_type_file
-if config[:config]
+# from it.
+if from_cli[:config]
   begin
-    from_file = YAML.load_file(config[:config])
+    from_file = YAML.load_file(from_cli[:config])
   rescue Exception => e
-    if from_cli[:config] # means we're not using a metrics-type filename
-      STDERR.puts "ERROR: unable to read configuration from #{config[:config]}: #{e}"
-      exit 1
-    else
-      STDERR.puts "WARN: unable to read configuration from #{config[:config]}: #{e}"
-    end
+    STDERR.puts "ERROR: unable to read configuration from #{config[:config]}: #{e}"
+    exit 1
   end
 end
 
@@ -76,8 +63,9 @@ end
 # command line are given precedence.
 settings.each do |opt,attrs|
   possibilities = [from_cli[opt], from_file[opt.to_s], attrs[:default]]
-  config[opt] = possibilities.find {|p| !p.nil? }
-  raise("ERROR: Missing configuration option \"#{opt}\"") if config[opt].nil?
+  if (config[opt] = possibilities.find {|p| !p.nil? }).nil?
+    raise("ERROR: Missing configuration option \"#{opt}\"")
+  end
 end
 
 #===========================================================================#

diff --git a/manifests/pe_metric.pp b/manifests/pe_metric.pp
@@ -31,7 +31,9 @@
     'ssl'                => $ssl,
   }
 
-  file { "${scripts_dir}/${metrics_type}_config.yaml" :
+  $config_file = "${scripts_dir}/${metrics_type}_config.yaml"
+
+  file { $config_file:
     ensure  => $metric_ensure,
     mode    => '0644',
     content => $config_hash.pe_metric_curl_cron_jobs::to_yaml(),
@@ -41,7 +43,7 @@
 
   cron { "${metrics_type}_metrics_collection" :
     ensure  => $metric_ensure,
-    command => "${script_file_name} --metrics_type ${metrics_type} --output-dir ${metrics_output_dir} --no-print",
+    command => "${script_file_name} --config ${config_file} --output-dir ${metrics_output_dir} --no-print",
     user    => 'root',
     minute  => $cron_minute,
   }