More report and metrics manipulations. This should be the last of it.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1351 980ebf18-57e1-0310-9a29-db15c13687c0

4 files changed, 53 insertions, 17 deletions

diff --git a/lib/puppet/client/master.rb b/lib/puppet/client/master.rb
index 2544d3aef..0a3419ea4 100644
--- a/lib/puppet/client/master.rb
+++ b/lib/puppet/client/master.rb
@@ -86,6 +86,7 @@ class Puppet::Client::MasterClient < Puppet::Client
             transaction.ignoreschedules = true
         end
 
+        transaction.addtimes :config_retrieval => @configtime
         begin
             transaction.evaluate
         rescue Puppet::Error => detail
@@ -101,11 +102,11 @@ class Puppet::Client::MasterClient < Puppet::Client
         end
 
         if Puppet[:report]
-            report = transaction.report()
-            if Puppet[:rrdgraph] == true
-                report.graph()
-            end
             begin
+                report = transaction.report()
+                if Puppet[:rrdgraph] == true
+                    report.graph()
+                end
                 reportclient().report(report)
             rescue => detail
                 Puppet.err "Reporting failed: %s" % detail
@@ -401,7 +402,9 @@ class Puppet::Client::MasterClient < Puppet::Client
         else
             lock do
                 @running = true
-                self.getconfig
+                @configtime = thinmark do
+                    self.getconfig
+                end
 
                 if defined? @objects and @objects
                     unless @local
diff --git a/lib/puppet/metric.rb b/lib/puppet/metric.rb
index 1370a5297..ca6f106cc 100644
--- a/lib/puppet/metric.rb
+++ b/lib/puppet/metric.rb
@@ -43,7 +43,7 @@ module Puppet
             if label
                 @label = label
             else
-                @label = name.to_s.capitalize
+                @label = name.to_s.capitalize.gsub("_", " ")
             end
 
             @values = []
@@ -51,7 +51,7 @@ module Puppet
 
         def newvalue(name,value,label = nil)
             unless label
-                label = name.to_s.capitalize
+                label = name.to_s.capitalize.gsub("_", " ")
             end
             @values.push [name,label,value]
         end
@@ -67,7 +67,7 @@ module Puppet
             args.push("--title",self.label)
             args.push("--imgformat","PNG")
             args.push("--interlace")
-            colorstack = %w{#ff0000 #00ff00 #0000ff #099000 #000990 #f00990}
+            colorstack = %w{#ff0000 #00ff00 #0000ff #099000 #000990 #f00990 #0f0f0f}
             i = 0
             defs = []
             lines = []
@@ -93,7 +93,7 @@ module Puppet
 
         def store(time)
             require 'RRD'
-            unless FileTest.exists?(File.join(Puppet[:rrddir],@name + ".rrd"))
+            unless FileTest.exists?(self.path)
                 self.create
             end
 
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 03183e90f..7a3b761b7 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -16,6 +16,13 @@ class Transaction
             Values must be comma-separated."]
     )
 
+    # Add some additional times for reporting
+    def addtimes(hash)
+        hash.each do |name, num|
+            @timemetrics[name] = num
+        end
+    end
+
     # Apply all changes for a child, returning a list of the events
     # generated.
     def apply(child)
@@ -55,6 +62,10 @@ class Transaction
             changes = [changes]
         end
 
+        if changes.length > 0
+            @objectmetrics[:out_of_sync] += 1
+        end
+
         childevents = changes.collect { |change|
             @changes << change
             @count += 1
@@ -78,13 +89,13 @@ class Transaction
             # if we ever get to that point
             unless events.nil? or (events.is_a?(Array) and events.empty?)
                 change.changed = true
+                @objectmetrics[:applied] += 1
             end
 
             events
         }.flatten.reject { |e| e.nil? }
 
         unless changes.empty?
-            @objectmetrics[:changed] += 1
             # Record when we last synced
             child.cache(:synced, Time.now)
         end
@@ -143,6 +154,7 @@ class Transaction
                 events = nil
                 if (tags.nil? or child.tagged?(tags))
                     if self.ignoreschedules or child.scheduled?
+                        @objectmetrics[:scheduled] += 1
                         # Perform the actual changes
 
                         seconds = thinmark do
@@ -190,11 +202,12 @@ class Transaction
 
         @objectmetrics = {
             :total => @objects.length,
-            :outofsync => 0,    # The number of objects that had changes
-            :changed => 0,        # The number of objects fixed
+            :out_of_sync => 0,    # The number of objects that had changes
+            :applied => 0,        # The number of objects fixed
             :skipped => 0,      # The number of objects skipped
             :restarted => 0,    # The number of objects triggered
-            :failedrestarts => 0 # The number of objects that fail a trigger
+            :failed_restarts => 0, # The number of objects that fail a trigger
+            :scheduled => 0     # The number of objects scheduled
         }
 
         # Metrics for distributing times across the different types.
@@ -224,19 +237,39 @@ class Transaction
 
     # Generate a transaction report.
     def report
-        @objectmetrics[:failed] = @failures.length
+        @objectmetrics[:failed] = @failures.find_all do |name, num|
+            num > 0
+        end.length
+
+        # Get the total time spent
+        @timemetrics[:total] = @timemetrics.inject(0) do |total, vals|
+            total += vals[1]
+            total
+        end
+
+        # Unfortunately, RRD does not deal well with changing lists of values,
+        # so we have to pick a list of values and stick with it.  In this case,
+        # that means we record the total time, the config time, and that's about
+        # it.  We should probably send each type's time as a separate metric.
+        @timemetrics.dup.each do |name, value|
+            if Puppet::Type.type(name)
+                @timemetrics.delete(name)
+            end
+        end
 
         # Add all of the metrics related to object count and status
         @report.newmetric(:objects, @objectmetrics)
 
         # Record the relative time spent in each object.
-        @report.newmetric(:percentage_time, @timemetrics)
+        @report.newmetric(:time, @timemetrics)
 
         # Then all of the change-related metrics
         @report.newmetric(:changes,
             :total => @changes.length
         )
 
+        @report.time = Time.now
+
         return @report
     end
 
@@ -315,7 +348,7 @@ class Transaction
                         obj.err "Failed to call %s on %s: %s" %
                             [callback, obj, detail]
 
-                        @objectmetrics[:failedrestarts] += 1
+                        @objectmetrics[:failed_restarts] += 1
 
                         if Puppet[:debug]
                             puts detail.backtrace
diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index 93afa284a..cca1a90f2 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -5,7 +5,7 @@ require 'puppet'
 # change produces, and Metrics are all of the numerical data involved
 # in the transaction.
 class Puppet::Transaction::Report
-    attr_accessor :logs, :metrics
+    attr_accessor :logs, :metrics, :time
 
     def initialize
         @metrics = {}