Add configurable span filters. (#220)

* add trace_ignore_span callback

When using the collector, sometimes there is a lot of tracing noise from
things like selenium or other framework noise. This adds a seam to allow
users to have control of filtering out uninteresting trace spans.

* alternative more composable approach

lib/buildkite/test_collector.rb

@@ -28,7 +28,6 @@ module Buildkite
   module TestCollector
     DEFAULT_URL = "https://analytics-api.buildkite.com/v1/uploads"
     DEFAULT_UPLOAD_BATCH_SIZE = 500
-
     class << self
       attr_accessor :api_token
       attr_accessor :url
@@ -39,6 +38,7 @@ class << self
       attr_accessor :env
       attr_accessor :batch_size
       attr_accessor :trace_min_duration
+      attr_accessor :span_filters
     end
 
     def self.configure(hook:, token: nil, url: nil, tracing_enabled: true, artifact_path: nil, env: {})
@@ -54,6 +54,11 @@ def self.configure(hook:, token: nil, url: nil, tracing_enabled: true, artifact_
         Float(trace_min_ms_string) / 1000
       end
 
+      self.span_filters = []
+      unless self.trace_min_duration.nil?
+        self.span_filters << MinDurationSpanFilter.new(self.trace_min_duration)
+      end
+
       self.hook_into(hook)
     end
 
@@ -84,5 +89,15 @@ def self.enable_tracing!
         Buildkite::TestCollector::Uploader.tracer&.backfill(:sql, finish - start, **{ query: payload[:sql] })
       end
     end
+
+    class MinDurationSpanFilter
+      def initialize(min_duration)
+        @min_duration = min_duration
+      end
+
+      def call(span)
+        span.duration > @min_duration
+      end
+    end
   end
 end

lib/buildkite/test_collector/tracer.rb

@@ -58,7 +58,6 @@ class IncompleteSpan < StandardError; end
     def initialize(min_duration: nil)
       @top = Span.new(:top, MonotonicTime.call, nil, {})
       @stack = [@top]
-      @min_duration = min_duration
     end
 
     def enter(section, **detail)
@@ -69,17 +68,17 @@ def enter(section, **detail)
 
     def leave
       current_span.end_at = MonotonicTime.call
-      duration = current_span.duration
       @stack.pop
-      current_span.children.pop if @min_duration && duration < @min_duration
+
+      current_span.children.pop unless retain_span?(current_span.children.last)
+
       nil # avoid ambiguous return type/value
     end
 
     def backfill(section, duration, **detail)
-      return if @min_duration && duration < @min_duration
       now = MonotonicTime.call
       new_entry = Span.new(section, now - duration, now, detail)
-      current_span.children << new_entry
+      current_span.children << new_entry if retain_span?(new_entry)
     end
 
     def current_span
@@ -95,5 +94,15 @@ def finalize
     def history
       @top.as_hash
     end
+
+    private
+
+    def retain_span?(span)
+      return true unless Buildkite::TestCollector.span_filters
+
+      Buildkite::TestCollector.span_filters.all? do |filter|
+        filter.call(span)
+      end
+    end
   end
 end

spec/test_collector/tracer_spec.rb

@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 
 RSpec.describe Buildkite::TestCollector::Tracer do
-  subject(:tracer) { Buildkite::TestCollector::Tracer.new(min_duration: min_duration) }
-  let(:min_duration) { nil }
+  subject(:tracer) { Buildkite::TestCollector::Tracer.new }
+  before { Buildkite::TestCollector.span_filters = [] }
 
   it "can produce an empty :top span" do
     history = tracer.finalize.history
@@ -69,6 +69,49 @@
     })
   end
 
+  context "span filtering" do
+    let(:filter) { ->(span) { span.section != :ignore_me } }
+    before { Buildkite::TestCollector.span_filters << filter }
+    after { Buildkite::TestCollector.span_filters.delete(filter) }
+
+    it "can filter out spans using provided ignore_span_callback" do
+      tracer.backfill(:sql, 12.34, query: "SELECT hello FROM world")
+
+      begin
+        tracer.enter(:ignore_me, i_am_very: "unexciting")
+      ensure
+        tracer.leave
+      end
+
+      begin
+        tracer.enter(:http, url: "https://example.com/")
+      ensure
+        tracer.leave
+      end
+
+      history = tracer.finalize.history
+
+      expect(history[:children]).to match([
+        {
+          section: :sql,
+          start_at: kind_of(Float),
+          end_at: kind_of(Float),
+          duration: kind_of(Float),
+          detail: {query: "SELECT hello FROM world"},
+          children: []
+        },
+        {
+          section: :http,
+          start_at: kind_of(Float),
+          end_at: kind_of(Float),
+          duration: kind_of(Float),
+          detail: {url: "https://example.com/"},
+          children: []
+        },
+      ])
+    end
+  end
+
   context "with mocked MonotonicTime" do
     before do
       allow(Buildkite::TestCollector::Tracer::MonotonicTime).to receive(:call) do
@@ -95,6 +138,11 @@
 
     describe "filtering traces by min_duration" do
       let(:min_duration) { 2.0 }
+      before do
+        fake_env("BUILDKITE_ANALYTICS_TOKEN", "token")
+        fake_env("BUILDKITE_ANALYTICS_TRACE_MIN_MS", (min_duration * 1000).to_s)
+        Buildkite::TestCollector.configure(hook: :minitest)
+      end
 
       it "can filter traces by duration" do
         monotonic_time_queue << 10.0
@@ -110,7 +158,7 @@
         monotonic_time_queue << 25.0
         tracer.leave
 
-        # skipped by #backfill: monotonic_time_queue << 30.2
+        monotonic_time_queue << 30.2
         tracer.backfill(:fast_backfill, 0.2)
 
         monotonic_time_queue << 43.5