From bd9f86bb8abb4759a0c72f94fb0492b1ff8619b5 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Thu, 31 Dec 2015 17:51:12 +0100
Subject: [PATCH] Use separate series for Rails/Sidekiq transactions

This removes the need for tagging all metrics with a "process_type" tag.
---
 lib/gitlab/metrics/metric.rb                            | 3 +--
 lib/gitlab/metrics/rack_middleware.rb                   | 4 +++-
 lib/gitlab/metrics/sidekiq_middleware.rb                | 4 +++-
 lib/gitlab/metrics/transaction.rb                       | 7 +++----
 spec/lib/gitlab/metrics/instrumentation_spec.rb         | 2 +-
 spec/lib/gitlab/metrics/metric_spec.rb                  | 1 -
 spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb      | 2 +-
 spec/lib/gitlab/metrics/subscribers/action_view_spec.rb | 2 +-
 spec/lib/gitlab/metrics/transaction_spec.rb             | 4 ++--
 9 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/lib/gitlab/metrics/metric.rb b/lib/gitlab/metrics/metric.rb
index 753008df99a..8319e628a40 100644
--- a/lib/gitlab/metrics/metric.rb
+++ b/lib/gitlab/metrics/metric.rb
@@ -19,8 +19,7 @@ module Gitlab
         {
           series: @series,
           tags:   @tags.merge(
-            hostname:     Metrics.hostname,
-            process_type: Sidekiq.server? ? 'sidekiq' : 'rails'
+            hostname: Metrics.hostname
           ),
           values:    @values,
           timestamp: @created_at.to_i * 1_000_000_000
diff --git a/lib/gitlab/metrics/rack_middleware.rb b/lib/gitlab/metrics/rack_middleware.rb
index 5c0587c4c51..bb9e4fcb918 100644
--- a/lib/gitlab/metrics/rack_middleware.rb
+++ b/lib/gitlab/metrics/rack_middleware.rb
@@ -4,6 +4,8 @@ module Gitlab
     class RackMiddleware
       CONTROLLER_KEY = 'action_controller.instance'
 
+      SERIES = 'rails_transactions'
+
       def initialize(app)
         @app = app
       end
@@ -30,7 +32,7 @@ module Gitlab
       end
 
       def transaction_from_env(env)
-        trans = Transaction.new
+        trans = Transaction.new(SERIES)
 
         trans.add_tag(:request_method, env['REQUEST_METHOD'])
         trans.add_tag(:request_uri, env['REQUEST_URI'])
diff --git a/lib/gitlab/metrics/sidekiq_middleware.rb b/lib/gitlab/metrics/sidekiq_middleware.rb
index ad441decfa2..6e804dd2562 100644
--- a/lib/gitlab/metrics/sidekiq_middleware.rb
+++ b/lib/gitlab/metrics/sidekiq_middleware.rb
@@ -4,8 +4,10 @@ module Gitlab
     #
     # This middleware is intended to be used as a server-side middleware.
     class SidekiqMiddleware
+      SERIES = 'sidekiq_transactions'
+
       def call(worker, message, queue)
-        trans = Transaction.new
+        trans = Transaction.new(SERIES)
 
         begin
           trans.run { yield }
diff --git a/lib/gitlab/metrics/transaction.rb b/lib/gitlab/metrics/transaction.rb
index a61dbd989e7..43a7dab5323 100644
--- a/lib/gitlab/metrics/transaction.rb
+++ b/lib/gitlab/metrics/transaction.rb
@@ -4,8 +4,6 @@ module Gitlab
     class Transaction
       THREAD_KEY = :_gitlab_metrics_transaction
 
-      SERIES = 'transactions'
-
       attr_reader :uuid, :tags
 
       def self.current
@@ -13,7 +11,8 @@ module Gitlab
       end
 
       # name - The name of this transaction as a String.
-      def initialize
+      def initialize(series)
+        @series  = series
         @metrics = []
         @uuid    = SecureRandom.uuid
 
@@ -55,7 +54,7 @@ module Gitlab
       end
 
       def track_self
-        add_metric(SERIES, { duration: duration }, @tags)
+        add_metric(@series, { duration: duration }, @tags)
       end
 
       def submit
diff --git a/spec/lib/gitlab/metrics/instrumentation_spec.rb b/spec/lib/gitlab/metrics/instrumentation_spec.rb
index a7eab9d11cc..a9003d8796b 100644
--- a/spec/lib/gitlab/metrics/instrumentation_spec.rb
+++ b/spec/lib/gitlab/metrics/instrumentation_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Metrics::Instrumentation do
-  let(:transaction) { Gitlab::Metrics::Transaction.new }
+  let(:transaction) { Gitlab::Metrics::Transaction.new('rspec') }
 
   before do
     @dummy = Class.new do
diff --git a/spec/lib/gitlab/metrics/metric_spec.rb b/spec/lib/gitlab/metrics/metric_spec.rb
index aa76315c79c..9b942855140 100644
--- a/spec/lib/gitlab/metrics/metric_spec.rb
+++ b/spec/lib/gitlab/metrics/metric_spec.rb
@@ -39,7 +39,6 @@ describe Gitlab::Metrics::Metric do
         expect(hash[:tags]).to be_an_instance_of(Hash)
 
         expect(hash[:tags][:hostname]).to be_an_instance_of(String)
-        expect(hash[:tags][:process_type]).to be_an_instance_of(String)
       end
 
       it 'includes the values' do
diff --git a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb
index 5882e7d81c7..5fda6de52f4 100644
--- a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb
+++ b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb
@@ -15,7 +15,7 @@ describe Gitlab::Metrics::SidekiqMiddleware do
 
   describe '#tag_worker' do
     it 'adds the worker class and action to the transaction' do
-      trans  = Gitlab::Metrics::Transaction.new
+      trans  = Gitlab::Metrics::Transaction.new('rspec')
       worker = double(:worker, class: double(:class, name: 'TestWorker'))
 
       expect(trans).to receive(:add_tag).with(:action, 'TestWorker#perform')
diff --git a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb
index c6cd584663f..bca76ca5a69 100644
--- a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb
+++ b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Metrics::Subscribers::ActionView do
-  let(:transaction) { Gitlab::Metrics::Transaction.new }
+  let(:transaction) { Gitlab::Metrics::Transaction.new('rspec') }
 
   let(:subscriber) { described_class.new }
 
diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb
index 6862fc9e2d1..345163bfbea 100644
--- a/spec/lib/gitlab/metrics/transaction_spec.rb
+++ b/spec/lib/gitlab/metrics/transaction_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Metrics::Transaction do
-  let(:transaction) { described_class.new }
+  let(:transaction) { described_class.new('rspec') }
 
   describe '#duration' do
     it 'returns the duration of a transaction in seconds' do
@@ -58,7 +58,7 @@ describe Gitlab::Metrics::Transaction do
   describe '#track_self' do
     it 'adds a metric for the transaction itself' do
       expect(transaction).to receive(:add_metric).
-        with(described_class::SERIES, { duration: transaction.duration }, {})
+        with('rspec', { duration: transaction.duration }, {})
 
       transaction.track_self
     end
-- 
GitLab