Merge branch 'add-action-to-all-metrics' into 'master'

See merge request !2370

6 files changed, 41 insertions, 25 deletions

diff --git a/lib/gitlab/metrics/rack_middleware.rb b/lib/gitlab/metrics/rack_middleware.rb
index e7a2f26d48b..6f179789d3e 100644
--- a/lib/gitlab/metrics/rack_middleware.rb
+++ b/lib/gitlab/metrics/rack_middleware.rb
@@ -39,10 +39,8 @@ module Gitlab
       end
 
       def tag_controller(trans, env)
-        controller = env[CONTROLLER_KEY]
-        label      = "#{controller.class.name}##{controller.action_name}"
-
-        trans.add_tag(:action, label)
+        controller   = env[CONTROLLER_KEY]
+        trans.action = "#{controller.class.name}##{controller.action_name}"
       end
     end
   end
diff --git a/lib/gitlab/metrics/sidekiq_middleware.rb b/lib/gitlab/metrics/sidekiq_middleware.rb
index ad441decfa2..fd98aa3412e 100644
--- a/lib/gitlab/metrics/sidekiq_middleware.rb
+++ b/lib/gitlab/metrics/sidekiq_middleware.rb
@@ -5,19 +5,14 @@ module Gitlab
     # This middleware is intended to be used as a server-side middleware.
     class SidekiqMiddleware
       def call(worker, message, queue)
-        trans = Transaction.new
+        trans = Transaction.new("#{worker.class.name}#perform")
 
         begin
           trans.run { yield }
         ensure
-          tag_worker(trans, worker)
           trans.finish
         end
       end
-
-      def tag_worker(trans, worker)
-        trans.add_tag(:action, "#{worker.class.name}#perform")
-      end
     end
   end
 end
diff --git a/lib/gitlab/metrics/transaction.rb b/lib/gitlab/metrics/transaction.rb
index 73131cc6ef2..86606b1c6d6 100644
--- a/lib/gitlab/metrics/transaction.rb
+++ b/lib/gitlab/metrics/transaction.rb
@@ -6,11 +6,15 @@ module Gitlab
 
       attr_reader :tags, :values
 
+      attr_accessor :action
+
       def self.current
         Thread.current[THREAD_KEY]
       end
 
-      def initialize
+      # action - A String describing the action performed, usually the class
+      #          plus method name.
+      def initialize(action = nil)
         @metrics = []
 
         @started_at  = nil
@@ -18,6 +22,7 @@ module Gitlab
 
         @values = Hash.new(0)
         @tags   = {}
+        @action = action
       end
 
       def duration
@@ -70,7 +75,15 @@ module Gitlab
       end
 
       def submit
-        Metrics.submit_metrics(@metrics.map(&:to_hash))
+        metrics = @metrics.map do |metric|
+          hash = metric.to_hash
+
+          hash[:tags][:action] ||= @action if @action
+
+          hash
+        end
+
+        Metrics.submit_metrics(metrics)
       end
 
       def sidekiq?
diff --git a/spec/lib/gitlab/metrics/rack_middleware_spec.rb b/spec/lib/gitlab/metrics/rack_middleware_spec.rb
index 4e6dfc73df2..b99be4e1060 100644
--- a/spec/lib/gitlab/metrics/rack_middleware_spec.rb
+++ b/spec/lib/gitlab/metrics/rack_middleware_spec.rb
@@ -57,7 +57,7 @@ describe Gitlab::Metrics::RackMiddleware do
 
       middleware.tag_controller(transaction, env)
 
-      expect(transaction.tags[:action]).to eq('TestController#show')
+      expect(transaction.action).to eq('TestController#show')
     end
   end
 end
diff --git a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb
index 5882e7d81c7..e520a968999 100644
--- a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb
+++ b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb
@@ -5,22 +5,15 @@ describe Gitlab::Metrics::SidekiqMiddleware do
 
   describe '#call' do
     it 'tracks the transaction' do
-      worker = Class.new.new
+      worker = double(:worker, class: double(:class, name: 'TestWorker'))
+
+      expect(Gitlab::Metrics::Transaction).to receive(:new).
+        with('TestWorker#perform').
+        and_call_original
 
       expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:finish)
 
       middleware.call(worker, 'test', :test) { nil }
     end
   end
-
-  describe '#tag_worker' do
-    it 'adds the worker class and action to the transaction' do
-      trans  = Gitlab::Metrics::Transaction.new
-      worker = double(:worker, class: double(:class, name: 'TestWorker'))
-
-      expect(trans).to receive(:add_tag).with(:action, 'TestWorker#perform')
-
-      middleware.tag_worker(trans, worker)
-    end
-  end
 end
diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb
index 3a27f897735..6bdeb719491 100644
--- a/spec/lib/gitlab/metrics/transaction_spec.rb
+++ b/spec/lib/gitlab/metrics/transaction_spec.rb
@@ -96,5 +96,22 @@ describe Gitlab::Metrics::Transaction do
 
       transaction.submit
     end
+
+    it 'adds the action as a tag for every metric' do
+      transaction.action = 'Foo#bar'
+      transaction.track_self
+
+      hash = {
+        series:    'rails_transactions',
+        tags:      { action: 'Foo#bar' },
+        values:    { duration: 0.0 },
+        timestamp: an_instance_of(Fixnum)
+      }
+
+      expect(Gitlab::Metrics).to receive(:submit_metrics).
+        with([hash])
+
+      transaction.submit
+    end
   end
 end