+1, with a couple of cosmetic comments:
The following is used a lot throughout the patch:
+ if Puppet.features.rrd_legacy? && ! Puppet.features.rrd?
This could be extracted in a method:
def use_legacy_rrd?
Puppet.features.rrd_legacy? && ! Puppet.features.rrd?
end
small note: I would have used "and not" instead of "&& !" because I find
it more readable, but it's fine like this.
The conditionals can also be removed easily by using polymorphism, if we
want to go this route :)
On 25/09/10 02:36, Jesse Wolfe wrote:
> The rrd project has been shipping ruby bindings since 1.3.0, and the old
> rrdtool ruby library is no longer being maintained.
>
> This patch is based upon Davor Ocelic's submitted code, with the
> addition that I've added conditionals so it can still call the old
> rrdtool library if a modern rrd is not installed.
>
> Signed-off-by: Jesse Wolfe <[email protected]>
> ---
> lib/puppet/feature/base.rb | 3 +-
> lib/puppet/util/metric.rb | 47 +++++++++++++++++++++++++++++++++++--------
> 2 files changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/lib/puppet/feature/base.rb b/lib/puppet/feature/base.rb
> index c153fba..c983f5c 100644
> --- a/lib/puppet/feature/base.rb
> +++ b/lib/puppet/feature/base.rb
> @@ -27,7 +27,8 @@ Puppet.features.add :diff, :libs => %w{diff/lcs
> diff/lcs/hunk}
> Puppet.features.add(:augeas, :libs => ["augeas"])
>
> # We have RRD available
> -Puppet.features.add(:rrd, :libs => ["RRDtool"])
> +Puppet.features.add(:rrd_legacy, :libs => ["RRDtool"])
> +Puppet.features.add(:rrd, :libs => ["RRD"])
>
> # We have OpenSSL
> Puppet.features.add(:openssl, :libs => ["openssl"])
> diff --git a/lib/puppet/util/metric.rb b/lib/puppet/util/metric.rb
> index 7e14a5f..0089847 100644
> --- a/lib/puppet/util/metric.rb
> +++ b/lib/puppet/util/metric.rb
> @@ -31,9 +31,12 @@ class Puppet::Util::Metric
>
> start ||= Time.now.to_i - 5
>
> - @rrd = RRDtool.new(self.path)
> args = []
>
> + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd?
> + @rrd = RRDtool.new(self.path)
> + end
> +
> values.each { |value|
> # the 7200 is the heartbeat -- this means that any data that isn't
> # more frequently than every two hours gets thrown away
> @@ -42,14 +45,22 @@ class Puppet::Util::Metric
> args.push "RRA:AVERAGE:0.5:1:300"
>
> begin
> - @rrd.create( Puppet[:rrdinterval].to_i, start, args)
> + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd?
> + @rrd.create( Puppet[:rrdinterval].to_i, start, args)
> + else
> + RRD.create( self.path, '-s', Puppet[:rrdinterval].to_i.to_s, '-b',
> start.to_i.to_s, *args)
> + end
> rescue => detail
> raise "Could not create RRD file #{path}: #{detail}"
> end
> end
>
> def dump
> - puts @rrd.info
> + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd?
> + puts @rrd.info
> + else
> + puts RRD.info self.path
> + end
> end
>
> def graph(range = nil)
> @@ -82,14 +93,26 @@ class Puppet::Util::Metric
> args << lines
> args.flatten!
> if range
> - args.push("--start",range[0],"--end",range[1])
> + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd?
> + args.push("--start",range[0],"--end",range[1])
> + else
> + args.push("--start",range[0].to_i.to_s,"--end",range[1].to_i.to_s)
> + end
> else
> - args.push("--start", Time.now.to_i - time, "--end", Time.now.to_i)
> + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd?
> + args.push("--start", Time.now.to_i - time, "--end", Time.now.to_i)
> + else
> + args.push("--start", (Time.now.to_i - time).to_s, "--end",
> Time.now.to_i.to_s)
> + end
> end
>
> begin
> #Puppet.warning "args = #{args}"
> - RRDtool.graph( args )
> + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd?
> + RRDtool.graph( args )
> + else
> + RRD.graph( *args )
> + end
> rescue => detail
> Puppet.err "Failed to graph #{self.name}: #{detail}"
> end
> @@ -114,13 +137,15 @@ class Puppet::Util::Metric
> end
>
> def store(time)
> - unless Puppet.features.rrd?
> + unless Puppet.features.rrd? || Puppet.features.rrd_legacy?
> Puppet.warning "RRD library is missing; cannot store metrics"
> return
> end
> self.create(time - 5) unless FileTest.exists?(self.path)
>
> - @rrd ||= RRDtool.new(self.path)
> + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd?
> + @rrd ||= RRDtool.new(self.path)
> + end
>
> # XXX this is not terribly error-resistant
> args = [time]
> @@ -133,7 +158,11 @@ class Puppet::Util::Metric
> arg = args.join(":")
> template = temps.join(":")
> begin
> - @rrd.update( template, [ arg ] )
> + if Puppet.features.rrd_legacy? && ! Puppet.features.rrd?
> + @rrd.update( template, [ arg ] )
> + else
> + RRD.update( self.path, '-t', template, arg )
> + end
> #system("rrdtool updatev #{self.path} '#{arg}'")
> rescue => detail
> raise Puppet::Error, "Failed to update #{self.name}: #{detail}"
--
Brice Figureau
My Blog: http://www.masterzen.fr/
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
