Merge pull request #176 from m0dular/SUP-3535-sar_metrics

(SUP-3535) Add more sar metrics to collect

Author: MartyEwings

files/metrics_tidy

@@ -45,7 +45,7 @@ done
 
 
 # Guard against deleting or archiving files outside of a Puppet service metrics directory.
-valid_paths=(puppetserver puppetdb orchestrator console ace bolt activemq postgres system_processes system_memory system_cpu vmware)
+valid_paths=(puppetserver puppetdb orchestrator console ace bolt activemq postgres system_processes system_memory system_cpu vmware sar)
 
 # Arguments and defaults.
 metrics_directory="${metrics_directory:-/opt/puppetlabs/puppet-metrics-collector/puppetserver}"

files/system_metrics

@@ -32,7 +32,7 @@ module SystemMetrics
   #
   # @attr [integer] polling_interval Time in seconds between calls to poll the system for data.
   # @attr [integer] file_interval Time in seconds between the creation of each output file.
-  # @attr [string] metric_type system_cpu|system_memory|system_processes
+  # @attr [string] metric_type system_cpu|system_processes
   # @attr [string] metrics_dir The puppet_metrics_collector output directory.
   # @attr [string] process_expression Expression to pass to egrep that matches processes to track
   # @attr [boolean] verbose Verbose output
@@ -47,7 +47,7 @@ module SystemMetrics
     #
     # @param [integer] polling_interval Time in seconds between calls to poll the system for data.
     # @param [integer] file_interval Time in seconds between the creation of each output file.
-    # @param [string] metric_type system_cpu|system_memory|system_processes
+    # @param [string] metric_type system_cpu|system_processes
     # @param [string] process_expression Expression to pass to egrep that matches processes to track
     # @param [string] metrics_dir The puppet_metrics_collector output directory.
     # @param [boolean] verbose Verbose output
@@ -78,7 +78,7 @@ module SystemMetrics
     def run_sar
       times_to_poll = (@file_interval / @polling_interval).round
       # sar inputs are polling interval and how many times to poll
-      comm_flags = ' -r' if %r{system_memory}.match?(@metric_type)
+      comm_flags = ' -u -r -w -W -B'
       comm = "sar #{comm_flags} #{@polling_interval} #{times_to_poll}"
       puts "sar command is: #{comm}" if @verbose
       begin
@@ -97,41 +97,25 @@ module SystemMetrics
     #
     # @return [hash] The metrics data
     def parse_sar_output(sar_output)
-      sar_output_arr = sar_output.split(%r{\n+|\r+}).reject(&:empty?).map { |line| line.split }
-
-      unique_header_str = if @metric_type == 'system_memory'
-                            '%memused'
-                          else
-                            '%user'
-                          end
-      headers_line = sar_output_arr.find { |e| e.include? unique_header_str }
-      sar_error_missing_headers = <<-EOF
-        sar output invalid or missing headers. Failed to find line with #{unique_header_str}.
-        Full output:
-        #{sar_output}
-      EOF
-      send_error_to_output_file_and_exit(sar_error_missing_headers) if headers_line.nil?
-
-      averages_line = sar_output_arr.find { |e| e.include? 'Average:' }
-      sar_error_missing_averages = <<-EOF
-        sar output missing "Average:"
-        Full output:
-        #{sar_output}"
-      EOF
-      send_error_to_output_file_and_exit(sar_error_missing_averages) if averages_line.nil?
-
-      Hash[headers_line.reverse.zip(averages_line.reverse).reverse]
-
-      puts "sar headers and averages:\n#{headers_line.join(',')}\n#{averages_line.join(',')}" if @verbose
-
-      # example of array data
-      # 04:59:13,PM,CPU,%user,%nice,%system,%iowait,%steal,%idle
-      # Average:,all,0.58,0.00,0.08,0.00,0.00,99.33
-      # combine the arrays into a hash starting from the deal with the unmatched columns in the front
-      data_hash = Hash[headers_line.reverse.zip(averages_line.reverse).reverse]
-      # remove anything that doesn't have a number for an average like "Average:" or "all"
-      data_hash.select! { |_k, v| v =~ %r{\A[-+]?[0-9]*\.?[0-9]+\Z} }
-      data_hash.transform_values!(&:to_f)
+      results = []
+      # Split the output on newlines and select the "Average" metrics that sar calculates over the interval
+      sar_averages = sar_output.split(%r{\n+|\r+}).select { |line| line.include? 'Average' }
+
+      # Each average will consist of a header line and a values line, e.g.
+      # Average:        CPU     %user     %nice   %system   %iowait    %steal     %idle
+      # Average:        all      0.18      0.00      0.10      0.00      0.03     99.70
+      # So, step over the array in increments of two to process each average
+      0.step(sar_averages.size - 1, 2) do |i|
+        # Split each line on spaces and join them together, e.g.
+        #   [["Average:", "Average:"], ["CPU", "all"], ["%user", "0.38"], ["%nice", "0.00"], ["%system", "0.08"], ["%iowait", "0.00"], ["%steal", "0.03"], ["%idle", "99.52"]]
+        merged_sar = sar_averages[i].split.zip(sar_averages[i + 1].split)
+        # Filter non-numeric values, then turn them into key/value pairs, e.g.
+        #   {"name": "%user", "value": 0.45}
+        merged_sar.select! { |_k, v| v =~ %r{\A[-+]?[0-9]*\.?[0-9]+\Z} }.each do |header, val|
+          results.append({ name: header, value: val.to_f })
+        end
+      end
+      results
     end
 
     # Run pidstat to collect process specific data
@@ -407,7 +391,7 @@ end
 
 if $PROGRAM_NAME == __FILE__
 
-  VALID_METRIC_TYPES = ['system_cpu', 'system_memory', 'system_processes'].freeze
+  VALID_METRIC_TYPES = ['system_cpu', 'system_processes'].freeze
   FILE_INTERVAL_DEFAULT = 60 * 5
   POLLING_INTERVAL_DEFAULT = 1
   METRIC_TYPE_DEFAULT = 'system_cpu'

manifests/collect.pp

@@ -3,60 +3,87 @@
 #   Creates systemd units for collecting a given metric
 # 
 define puppet_metrics_collector::collect (
+  Enum['present', 'absent'] $ensure          = 'present',
   String $metrics_type    = $title,
   String $metrics_command = undef,
   String $tidy_command    = undef,
+  Optional[Hash] $env_vars   = undef,
   String $metric_ensure   = 'present',
   String $minute          = '5',
 ) {
-  $service_ensure = $metric_ensure ? {
-    'present' => running,
-    'absent'  => stopped,
-  }
+  if $ensure == 'absent' {
+    $services = [
+      "puppet_${metrics_type}-metrics.service",
+      "puppet_${metrics_type}-metrics.timer",
+      "puppet_${metrics_type}-tidy.service",
+      "puppet_${metrics_type}-tidy.timer",
+    ]
+    service { $services:
+      ensure => stopped,
+      enable => false,
+    }
 
-  $service_enable = $metric_ensure ? {
-    'present' => true,
-    'absent'  => false,
-  }
+    $files = [
+      "/etc/systemd/system/puppet_${metrics_type}-metrics.service",
+      "/etc/systemd/system/puppet_${metrics_type}-metrics.timer",
+      "/etc/systemd/system/puppet_${metrics_type}-tidy.service",
+      "/etc/systemd/system/puppet_${metrics_type}-tidy.timer",
+    ]
 
-  file { "/etc/systemd/system/puppet_${metrics_type}-metrics.service":
-    ensure  => $metric_ensure,
-    content => epp('puppet_metrics_collector/service.epp',
-      { 'service' => "puppet_${metrics_type}", 'metrics_command' => $metrics_command }
-    ),
-  }
-  file { "/etc/systemd/system/puppet_${metrics_type}-metrics.timer":
-    ensure  => $metric_ensure,
-    content => epp('puppet_metrics_collector/timer.epp',
-      { 'service' => "puppet_${metrics_type}", 'minute' => $minute },
-    ),
+    file { $files:
+      ensure => absent,
+    }
   }
+  else {
+    $service_ensure = $metric_ensure ? {
+      'present' => running,
+      'absent'  => stopped,
+    }
 
-  file { "/etc/systemd/system/puppet_${metrics_type}-tidy.service":
-    ensure  => $metric_ensure,
-    content => epp('puppet_metrics_collector/tidy.epp',
-      { 'service' => "puppet_${metrics_type}", 'tidy_command' => $tidy_command }
-    ),
-  }
-  file { "/etc/systemd/system/puppet_${metrics_type}-tidy.timer":
-    ensure  => $metric_ensure,
-    content => epp('puppet_metrics_collector/tidy_timer.epp',
-      { 'service' => "puppet_${metrics_type}" }
-    ),
-  }
+    $service_enable = $metric_ensure ? {
+      'present' => true,
+      'absent'  => false,
+    }
 
-  service { "puppet_${metrics_type}-metrics.service":
-  }
-  service { "puppet_${metrics_type}-metrics.timer":
-    ensure    => $service_ensure,
-    enable    => $service_enable,
-    subscribe => File["/etc/systemd/system/puppet_${metrics_type}-metrics.timer"],
-  }
+    file { "/etc/systemd/system/puppet_${metrics_type}-metrics.service":
+      ensure  => $metric_ensure,
+      content => epp('puppet_metrics_collector/service.epp',
+        { 'service' => "puppet_${metrics_type}", 'metrics_command' => $metrics_command, 'env_vars' => $env_vars }
+      ),
+    }
+    file { "/etc/systemd/system/puppet_${metrics_type}-metrics.timer":
+      ensure  => $metric_ensure,
+      content => epp('puppet_metrics_collector/timer.epp',
+        { 'service' => "puppet_${metrics_type}", 'minute' => $minute },
+      ),
+    }
+
+    file { "/etc/systemd/system/puppet_${metrics_type}-tidy.service":
+      ensure  => $metric_ensure,
+      content => epp('puppet_metrics_collector/tidy.epp',
+        { 'service' => "puppet_${metrics_type}", 'tidy_command' => $tidy_command }
+      ),
+    }
+    file { "/etc/systemd/system/puppet_${metrics_type}-tidy.timer":
+      ensure  => $metric_ensure,
+      content => epp('puppet_metrics_collector/tidy_timer.epp',
+        { 'service' => "puppet_${metrics_type}" }
+      ),
+    }
+
+    service { "puppet_${metrics_type}-metrics.service":
+    }
+    service { "puppet_${metrics_type}-metrics.timer":
+      ensure    => $service_ensure,
+      enable    => $service_enable,
+      subscribe => File["/etc/systemd/system/puppet_${metrics_type}-metrics.timer"],
+    }
 
-  service { "puppet_${metrics_type}-tidy.service": }
-  service { "puppet_${metrics_type}-tidy.timer":
-    ensure    => $service_ensure,
-    enable    => $service_enable,
-    subscribe => File["/etc/systemd/system/puppet_${metrics_type}-tidy.timer"],
+    service { "puppet_${metrics_type}-tidy.service": }
+    service { "puppet_${metrics_type}-tidy.timer":
+      ensure    => $service_ensure,
+      enable    => $service_enable,
+      subscribe => File["/etc/systemd/system/puppet_${metrics_type}-tidy.timer"],
+    }
   }
 }

manifests/pe_metric.pp

@@ -16,6 +16,7 @@
   Optional[String]          $metrics_server_hostname  = undef,
   Optional[Integer]         $metrics_server_port      = undef,
   Optional[String]          $metrics_server_db_name   = undef,
+  Optional[Hash]            $env_vars                 = undef,
 ) {
   $metrics_output_dir = "${puppet_metrics_collector::output_dir}/${metrics_type}"
 
@@ -84,10 +85,12 @@
   $tidy_command = "${puppet_metrics_collector::scripts_dir}/metrics_tidy -d ${metrics_output_dir} -r ${retention_days}"
 
   puppet_metrics_collector::collect { $metrics_type:
+    ensure          => $metric_ensure,
     metrics_command => $metrics_command,
     tidy_command    => $tidy_command,
     metric_ensure   => $metric_ensure,
     minute          => $cron_minute,
+    env_vars        => $env_vars,
     notify          => Exec['puppet_metrics_collector_daemon_reload'],
   }
 

manifests/sar_metric.pp

@@ -33,6 +33,7 @@
   Integer                   $polling_frequency_seconds = 1,
   String                    $metric_script_file        = 'system_metrics',
   String                    $metrics_shipping_command  = $puppet_metrics_collector::system::metrics_shipping_command,
+  Optional[Hash]     $env_vars                  = undef,
 ) {
   $metrics_output_dir = "${puppet_metrics_collector::system::output_dir}/${metrics_type}"
 
@@ -66,10 +67,12 @@
   $tidy_command = "${puppet_metrics_collector::system::scripts_dir}/metrics_tidy -d ${metrics_output_dir} -r ${retention_days}"
 
   puppet_metrics_collector::collect { $metrics_type:
+    ensure          => $metric_ensure,
     metrics_command => $metrics_command,
     tidy_command    => $tidy_command,
     metric_ensure   => $metric_ensure,
     minute          => $cron_minute,
+    env_vars        => $env_vars,
     notify          => Exec['puppet_metrics_collector_system_daemon_reload'],
   }
 

manifests/system.pp

@@ -93,8 +93,7 @@
       source => 'puppet:///modules/puppet_metrics_collector/system_metrics',
     }
 
-    contain puppet_metrics_collector::system::cpu
-    contain puppet_metrics_collector::system::memory
+    contain puppet_metrics_collector::system::sar
     contain puppet_metrics_collector::system::processes
   }
 
@@ -133,4 +132,14 @@
   file { $metric_legacy_files :
     ensure => absent,
   }
+
+  # Legacy sar cleanup
+  puppet_metrics_collector::sar_metric { 'system_memory' :
+    metric_ensure             => 'absent',
+    cron_minute               => '0',
+    retention_days            => 0,
+    collection_frequency      => 0,
+    polling_frequency_seconds => 0,
+    metrics_shipping_command  => 'foo',
+  }
 }

manifests/system/memory.pp

@@ -2,19 +2,22 @@
 #
 # @api private
 #
-class puppet_metrics_collector::system::cpu (
+class puppet_metrics_collector::system::sar (
   String  $metrics_ensure            = $puppet_metrics_collector::system::system_metrics_ensure,
   Integer $collection_frequency      = $puppet_metrics_collector::system::collection_frequency,
   Integer $retention_days            = $puppet_metrics_collector::system::retention_days,
   Integer $polling_frequency_seconds = $puppet_metrics_collector::system::polling_frequency_seconds,
   Optional[String] $metrics_shipping_command  = undef,
 ) {
+  # This is to ensure that files are written to a directory that the sup script will pick up
   puppet_metrics_collector::sar_metric { 'system_cpu' :
     metric_ensure             => $metrics_ensure,
     cron_minute               => "0/${collection_frequency}",
     retention_days            => $retention_days,
     collection_frequency      => $collection_frequency,
     polling_frequency_seconds => $polling_frequency_seconds,
     metrics_shipping_command  => $metrics_shipping_command,
+    # This ensures that sar reports the time field as one 24 hour field, instead of a 12 hour format with spaces
+    env_vars                  => { 'LC_TIME' => 'POSIX' },
   }
 }

manifests/system/cpu.pp renamed to manifests/system/sar.pp

@@ -49,15 +49,13 @@
     context 'system timers are running' do
       it { expect(service('puppet_system_cpu-metrics.timer')).to be_running }
       it { expect(service('puppet_system_cpu-tidy.timer')).to be_running }
-      it { expect(service('puppet_system_memory-metrics.timer')).to be_running }
-      it { expect(service('puppet_system_memory-tidy.timer')).to be_running }
       it { expect(service('puppet_system_processes-metrics.timer')).to be_running }
       it { expect(service('puppet_system_processes-tidy.timer')).to be_running }
     end
 
     it 'creates system tidy services files' do
-      files = run_shell('ls /etc/systemd/system/puppet_system*-tidy.service').stdout
-      expect(files.split("\n").count).to eq(3)
+      expect(run_shell('ls /etc/systemd/system/puppet_system_cpu-tidy.service').exit_code).to eq(0)
+      expect(run_shell('ls /etc/systemd/system/puppet_system_processes-tidy.service').exit_code).to eq(0)
     end
   end
 
@@ -79,15 +77,13 @@ class { 'puppet_metrics_collector::system':
     context 'system timers are running' do
       it { expect(service('puppet_system_cpu-metrics.timer')).to be_running }
       it { expect(service('puppet_system_cpu-tidy.timer')).to be_running }
-      it { expect(service('puppet_system_memory-metrics.timer')).to be_running }
-      it { expect(service('puppet_system_memory-tidy.timer')).to be_running }
       it { expect(service('puppet_system_processes-metrics.timer')).to be_running }
       it { expect(service('puppet_system_processes-tidy.timer')).to be_running }
     end
 
     it 'creates system tidy services files' do
-      files = run_shell('ls /etc/systemd/system/puppet_system*-tidy.service').stdout
-      expect(files.split("\n").count).to eq(3)
+      expect(run_shell('ls /etc/systemd/system/puppet_system_cpu-tidy.service').exit_code).to eq(0)
+      expect(run_shell('ls /etc/systemd/system/puppet_system_processes-tidy.service').exit_code).to eq(0)
     end
   end
 end