trigger_statsd_increment does not support multiple calls with different tags

[Noezor]

👋 , thansk for the great library

Issue

My use-case is that I have a method which issues a bunch of statsD calls to the same metric with different tags. I then want to verify that this metric has correctly received the expected amount of calls, each with their respective tags. Here is a test I would expect to pass:

require 'statsd-instrument'

RSpec.configure do |config|
  config.include StatsD::Instrument::Matchers
end

RSpec.describe 'Matchers' do
  context 'trigger_statsd_increment' do
    it 'will pass if every statsD call matches its calls' do
      expect do
        StatsD.increment('counter', tags: ['variation:a'])
        StatsD.increment('counter', tags: ['variation:b'])
      end.to trigger_statsd_increment('counter', times: 2, tags: [['variation:a'], ['variation:b']])
    end
  end
end

However, today this test fails.

https://github.com/Shopify/statsd-instrument/blob/5518d8de15c838b24a987871c7b54ccb90f3a7c3/lib/statsd/instrument/matchers.rb#L59 will test that for each call to increment, all the tags expected are present. We are not allowed to do different tags per calls.

Thus, likewise

      end.to trigger_statsd_increment('counter', times: 2, tags: ['variation:a', 'variation:b'])

would not work either

Rewriting the test as

RSpec.describe 'Matchers' do
  context 'trigger_statsd_increment' do
    it 'will pass if every statsD call matches its calls' do
      expect do
        StatsD.increment('counter', tags: ['variation:a'])
        StatsD.increment('counter', tags: ['variation:b'])
      end.to trigger_statsd_increment('counter', times: 1, tags: ["variation:a"]).and trigger_statsd_increment('counter', times: 1, tags: ["variation:b"])
    end
  end
end

Does not pass either and fails with

The numbers of StatsD calls for metric counter was unexpected. Expected 1, got 2

Indeed https://github.com/Shopify/statsd-instrument/blob/5518d8de15c838b24a987871c7b54ccb90f3a7c3/lib/statsd/instrument/matchers.rb#L45-L53 will first test for the metric name and return the error above as the error has not been matched enough times. The tags are not looked at for this to fail.

Propositions

A: support trigger_statsd_increment('counter', times: 2, tags: [['variation:a'], ['variation:b']])

Here, each trigger_statsd_increment is linked by the metric name.

The tags would have 2 behaviors when present:

• it is a list of lists, and then each element is supposed to match the call to metric_name in the order those calls happened
• it is a list, and then we keep the behavior of today: all the calls for metric_name need to have tags match the unique list of expected tags. This would make the change retrocompatible (I don't expect anyone to use lists of list of tags)

B: support trigger_statsd_increment('counter', times: 1, tags: ["variation:a"]).and trigger_statsd_increment('counter', times: 1, tags: ["variation:b"])

When matching the times, we make sure that both the metric_name and the tags match.

The advantage I find to this is that it matches the API for metric_name matching where each metric_name is supposed to be assessed in different calls :

    it 'will pass if every statsD call matches its calls' do
        expect do
          StatsD.increment('counter')
          StatsD.increment('counter2')
        end.to trigger_statsd_increment('counter', times: 1).and trigger_statsd_increment('counter2', times: 1)
      end  
    end

That change would make it that different ((metric_name,tags) should be assessed in different calls).

Implementation

I'm willing to implement those changes, and favor proposition A. WDYT ?