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

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.

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.

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?

-- 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.

Reply via email to