Merge pull request gjtorikian#46 from mtodd/instrument-doc

Include output and results to filter, pipeline event payload

Author: jch

README.md

@@ -198,9 +198,10 @@ Pipeline.new [ RootRelativeFilter ], { :base_url => 'http://somehost.com' }
 
 ## Instrumenting
 
-To instrument each filter and a full pipeline call, set an
-[ActiveSupport::Notifications](http://api.rubyonrails.org/classes/ActiveSupport/Notifications.html)
-service object on a pipeline object. New pipeline objects will default to the
+Filters and Pipelines can be set up to be instrumented when called. The pipeline
+must be setup with an [ActiveSupport::Notifications]
+(http://api.rubyonrails.org/classes/ActiveSupport/Notifications.html)
+compatible service object and a name. New pipeline objects will default to the
 `HTML::Pipeline.default_instrumentation_service` object.
 
 ``` ruby
@@ -209,10 +210,12 @@ service = ActiveSupport::Notifications
 
 # instrument a specific pipeline
 pipeline = HTML::Pipeline.new [MarkdownFilter], context
-pipeline.instrumentation_service = service
+pipeline.setup_instrumentation "MarkdownPipeline", service
 
-# or instrument all new pipelines
+# or set default instrumentation service for all new pipelines
 HTML::Pipeline.default_instrumentation_service = service
+pipeline = HTML::Pipeline.new [MarkdownFilter], context
+pipeline.setup_instrumentation "MarkdownPipeline"
 ```
 
 Filters are instrumented when they are run through the pipeline. A
@@ -222,15 +225,24 @@ instrumentation call.
 
 ``` ruby
 service.subscribe "call_filter.html_pipeline" do |event, start, ending, transaction_id, payload|
+  payload[:pipeline] #=> "MarkdownPipeline", set with `setup_instrumentation`
   payload[:filter] #=> "MarkdownFilter"
+  payload[:context] #=> context Hash
+  payload[:result] #=> instance of result class
+  payload[:result][:output] #=> output HTML String or Nokogiri::DocumentFragment
 end
 ```
 
 The full pipeline is also instrumented:
 
 ``` ruby
 service.subscribe "call_pipeline.html_pipeline" do |event, start, ending, transaction_id, payload|
+  payload[:pipeline] #=> "MarkdownPipeline", set with `setup_instrumentation`
   payload[:filters] #=> ["MarkdownFilter"]
+  payload[:doc] #=> HTML String or Nokogiri::DocumentFragment
+  payload[:context] #=> context Hash
+  payload[:result] #=> instance of result class
+  payload[:result][:output] #=> output HTML String or Nokogiri::DocumentFragment
 end
 ```
 

lib/html/pipeline.rb

@@ -25,7 +25,6 @@ module HTML
   #                   some semblance of type safety.
   class Pipeline
     autoload :VERSION,               'html/pipeline/version'
-    autoload :Pipeline,              'html/pipeline/pipeline'
     autoload :Filter,                'html/pipeline/filter'
     autoload :AbsoluteSourceFilter,  'html/pipeline/absolute_source_filter'
     autoload :BodyContent,           'html/pipeline/body_content'
@@ -65,6 +64,12 @@ def self.parse(document_or_html)
     # Set an ActiveSupport::Notifications compatible object to enable.
     attr_accessor :instrumentation_service
 
+    # Public: String name for this Pipeline. Defaults to Class name.
+    attr_writer :instrumentation_name
+    def instrumentation_name
+      @instrumentation_name || self.class.name
+    end
+
     class << self
       # Public: Default instrumentation service for new pipeline objects.
       attr_accessor :default_instrumentation_service
@@ -94,7 +99,9 @@ def call(html, context = {}, result = nil)
       context = @default_context.merge(context)
       context = context.freeze
       result ||= @result_class.new
-      instrument "call_pipeline.html_pipeline", :filters => @filters.map(&:name) do
+      payload = default_payload :filters => @filters.map(&:name),
+        :context => context, :result => result
+      instrument "call_pipeline.html_pipeline", payload do
         result[:output] =
           @filters.inject(html) do |doc, filter|
             perform_filter(filter, doc, context, result)
@@ -109,22 +116,13 @@ def call(html, context = {}, result = nil)
     #
     # Returns the result of the filter.
     def perform_filter(filter, doc, context, result)
-      instrument "call_filter.html_pipeline", :filter => filter.name do
+      payload = default_payload :filter => filter.name,
+        :context => context, :result => result
+      instrument "call_filter.html_pipeline", payload do
         filter.call(doc, context, result)
       end
     end
 
-    # Internal: if the `instrumentation_service` object is set, instruments the
-    # block, otherwise the block is ran without instrumentation.
-    #
-    # Returns the result of the provided block.
-    def instrument(event, payload = nil)
-      return yield unless instrumentation_service
-      instrumentation_service.instrument event, payload do
-        yield
-      end
-    end
-
     # Like call but guarantee the value returned is a DocumentFragment.
     # Pipelines may return a DocumentFragment or a String. Callers that need a
     # DocumentFragment should use this method.
@@ -143,6 +141,36 @@ def to_html(input, context = {}, result = nil)
         output.to_s
       end
     end
+
+    # Public: setup instrumentation for this pipeline.
+    #
+    # Returns nothing.
+    def setup_instrumentation(name = nil, service = nil)
+      self.instrumentation_name = name
+      self.instrumentation_service =
+        service || self.class.default_instrumentation_service
+    end
+
+    # Internal: if the `instrumentation_service` object is set, instruments the
+    # block, otherwise the block is ran without instrumentation.
+    #
+    # Returns the result of the provided block.
+    def instrument(event, payload = nil)
+      payload ||= default_payload
+      return yield(payload) unless instrumentation_service
+      instrumentation_service.instrument event, payload do |payload|
+        yield payload
+      end
+    end
+
+    # Internal: Default payload for instrumentation.
+    #
+    # Accepts a Hash of additional payload data to be merged.
+    #
+    # Returns a Hash.
+    def default_payload(payload = {})
+      {:pipeline => instrumentation_name}.merge(payload)
+    end
   end
 end
 

test/helpers/mocked_instrumentation_service.rb

@@ -5,11 +5,13 @@ def initialize(event = nil, events = [])
     subscribe event
   end
   def instrument(event, payload = nil)
-    res = yield
+    payload ||= {}
+    res = yield payload
     events << [event, payload, res] if @subscribe == event
     res
   end
   def subscribe(event)
     @subscribe = event
+    @events
   end
 end

test/html/pipeline_test.rb

@@ -5,7 +5,7 @@ class HTML::PipelineTest < Test::Unit::TestCase
   Pipeline = HTML::Pipeline
   class TestFilter
     def self.call(input, context, result)
-      input
+      input.reverse
     end
   end
 
@@ -17,24 +17,28 @@ def setup
 
   def test_filter_instrumentation
     service = MockedInstrumentationService.new
-    service.subscribe "call_filter.html_pipeline"
+    events = service.subscribe "call_filter.html_pipeline"
     @pipeline.instrumentation_service = service
-    filter("hello")
-    event, payload, res = service.events.pop
+    filter(body = "hello")
+    event, payload, res = events.pop
     assert event, "event expected"
     assert_equal "call_filter.html_pipeline", event
     assert_equal TestFilter.name, payload[:filter]
+    assert_equal @pipeline.class.name, payload[:pipeline]
+    assert_equal body.reverse, payload[:result][:output]
   end
 
   def test_pipeline_instrumentation
     service = MockedInstrumentationService.new
-    service.subscribe "call_pipeline.html_pipeline"
+    events = service.subscribe "call_pipeline.html_pipeline"
     @pipeline.instrumentation_service = service
-    filter("hello")
-    event, payload, res = service.events.pop
+    filter(body = "hello")
+    event, payload, res = events.pop
     assert event, "event expected"
     assert_equal "call_pipeline.html_pipeline", event
     assert_equal @pipeline.filters.map(&:name), payload[:filters]
+    assert_equal @pipeline.class.name, payload[:pipeline]
+    assert_equal body.reverse, payload[:result][:output]
   end
 
   def test_default_instrumentation_service
@@ -46,6 +50,24 @@ def test_default_instrumentation_service
     Pipeline.default_instrumentation_service = nil
   end
 
+  def test_setup_instrumentation
+    assert_nil @pipeline.instrumentation_service
+
+    service = MockedInstrumentationService.new
+    events = service.subscribe "call_pipeline.html_pipeline"
+    @pipeline.setup_instrumentation name = 'foo', service
+
+    assert_equal service, @pipeline.instrumentation_service
+    assert_equal name, @pipeline.instrumentation_name
+
+    filter(body = 'foo')
+
+    event, payload, res = events.pop
+    assert event, "expected event"
+    assert_equal name, payload[:pipeline]
+    assert_equal body.reverse, payload[:result][:output]
+  end
+
   def filter(input)
     @pipeline.call(input)
   end