buildkite / test-collector-ruby

Buildkite Test Analytics collector for Ruby test frameworks
http://buildkite.com/test-analytics
MIT License
15 stars 26 forks source link

Add configurable span filters. #220

Closed catkins closed 2 days ago

catkins commented 4 months ago

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

Usage

ignore_selenium_requests = ->(span) do
  return true unless span[:section] == "http"

  !span.dig(:detail, :url) =~ /^http://selenium:4444//
end

Buildkite::TestCollector.configure(hook: :rspec)
Buildkite::TestCollector.span_filters << ignore_selenium_requests

This doesn't need to be a lambda, any #call-able will work, as long as it returns a truthy value from the callback when we want to retain the span.

Multiple span_filters can be provided, and as long they all return a truthy value, then the span will be retained, this allows composing multiple filters together.

This would be a public interface to the library, so I'm happy to workshop the naming/semantics.

wooly commented 4 months ago

My gut says that the use of the word 'filter' in the option is throwing me since I'm associating it with Array#Filter, which I think has the opposite semantics of this: 'keep the stuff for which the block returns true'. I know that it's inverted internally but it just feels ever so slightly off.

What if the option was just span_filter and it filtered out everything that returned false?

catkins commented 4 months ago

Yeah that's what I started with too actually and I went back and forth a bit.

Perhaps I'll make a more interesting filter that rejects a few other types of span to see how that would look.

catkins commented 4 months ago

It would be great if there was a block form of Buildkite::TestCollector.configure similar to how rspec or vcr get setup

Buildkite::TestCollector.configure(hook: :rspec) do |config|
  config.batch_size = 200
  config.span_filters << ->(span) do
    # ... filtering things
  end
end
wooly commented 4 months ago

Yeah that feels like a much more expressive API for sure.

catkins commented 4 months ago

I shuffled things around slightly to be composable, and re-expressed the minimum duration filtering @pda added in #215 to use the new construct.

I also updated the PR description to show the new syntax.

wooly commented 4 months ago

Looks neat!

catkins commented 2 days ago

@zhming0 fancy taking over the PR? I haven't had my head in the TA space just at the moment, and it could be good for you to have a play with doing a collector release