On 27/02/11 20:44, Markus Roberts wrote:
> B --
>
>> + # Triggers an instrumentation
>
> + #
> + # Call this method around the instrumentation point
> + # Puppet::Util::Instrumentation.instrument(:my_long_computation) do
> + # ... a long computation
> + # end
> + #
> + # This will send an event to all the listeners of
> "my_long_computation".
> + # The same can be achieved faster than using a block by calling
> start and stop.
> + # around the code to instrument.
> + def self.instrument(label, data = {})
> + self.start(label, data)
> + yield
> + ensure
> + self.stop(label)
> + end
> +
> + def self.start(label, data = {})
> + data[:started] = Time.now
> + Thread.current[label] = data
> + publish(label, :start, data)
> + end
> +
> + def self.stop(label)
> + if data = Thread.current[label]
> + Thread.current[label] = nil
> + data[:finished] = Time.now
> + publish(label, :stop, data)
> + else
> + raise "No matching start event for #{label}"
> + end
> + end
>
>
> It appears on first reading that this may fail on if start/stop pairs
> are used in recursive code; specifically, in something like this:
>
> def functastic_method
> Puppet::Util::Instrumentation.instrument(:my_long_computation) do
> # ... a long computation that calls functastic_method
> somewhere deep inside
> end
> end
Yes, I thought about it when I started the code back 3 weeks ago, but
completely forget about it :(
> that when the next to the last call tries to leave the instrument block
> it will raise "No matching start event..."
>
> If we care, this could probably be dealt with by replacing
> Thread.current[label] with something like:
>
> Thread.current[:instrumentation] = Hash.new([])
>
> and then doing:
>
> Thread.current[:instrumentation][label].push data
>
> and the corresponding pop/empty?/etc.
When I first wrote this, the probe would have been put manually around
body of methods, but now that we meta-program the probes around function
call, we can leverage the new scope we created and, I'm not sure we
really need this trick.
We can rewrite the probe to do something like:
call start(label, data)
call original method
call stop(label, data)
If needed, we can stuff into the data hash an unique id (or return it
from start to send it back to stop) to make sure we can pair the start
and stop event later on.
> This would also be a good idea to prevent namespace collisions. If
> someone where to choose a label that we're already using internally
> (e.g. :known_resource_types, environment, declarations, etc.) the
> present system would break things in a rather interesting way.
You're correct, the namespace issue is a problem.
> Finally, I've got an uneasy feeling (but nothing I can stand up and
> explicate) about linking thread data and (global) metaprogramming in
> this way. It's probably fine, but I haven't yet gotten my head around
> all the corners.
>
> + def self.publish(label, event, data)
> + listeners_of(label).each do |k,l|
> + l.notify(label, event, data)
> + end
> + end
>
>
>
> + def self.listeners_of(label)
> + synchronize {
> + @listeners_of[label] ||= @listeners.select do |k,l|
> + l.listen_to?(label)
> + end
> + }
> + end
>
>
> Does this (due to memoization) omit listeners who are added after the
> first publish?
Each time we enable/disable, we call rehash whose job is to client
listeners_of, so we should be safe here.
--
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.
