B --
Thoughts:
1) Wow.
> + def enable
> + raise "Probe already enabled" if enabled?
> + method = @method
> + label = @label
> + data = @data
> + klass.class_eval {
> + alias_method("instrumented_#{method}", method)
> + define_method(method) do |*args|
> + begin
> + instrumentation_label = label.respond_to?(:call) ?
> label.call(self, args) : label
> + instrumentation_data = data.respond_to?(:call) ?
> data.call(self, args) : data
> + Puppet::Util::Instrumentation.start(instrumentation_label,
> instrumentation_data)
> + send("instrumented_#{method}".to_sym, *args)
> + rescue => e
> + raise
> + ensure
> + Puppet::Util::Instrumentation.stop(instrumentation_label)
> + end
> + end
> + }
> + @enabled = true
> + end
>
2) Would you get a more useful stack-trace without the rescue?
3) And is there a race condition on @enabled?
> + def disable
> + raise "Probe is not enabled" unless enabled?
> + method = @method
> + label = @label
> + data = @data
> + klass.class_eval do
> + alias_method("instrumented_#{method}", method)
> + remove_method("instrumented_#{method}".to_sym)
> + end
> + end
>
4) Should you set @enabled = false somewhere in here?
> + def self.clear_probes
> + INSTRUMENTED_CLASSES.synchronize {
> + INSTRUMENTED_CLASSES.clear
> + }
> + end
>
5) Do you want a:
nil # do not leak our probes to the exterior world
on clear_probes as well as on each_probes?
-- M
-----------------------------------------------------------
When in trouble or in doubt, run in circles,
scream and shout. -- 1920's parody of the
maritime general prudential rule
------------------------------------------------------------
--
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.
