Tests for Web transaction and remove simple transacton

11 files changed, 41 insertions, 272 deletions

diff --git a/lib/gitlab/metrics/base_transaction.rb b/lib/gitlab/metrics/base_transaction.rb
index a783bfeccd5..cf616cb13a3 100644
--- a/lib/gitlab/metrics/base_transaction.rb
+++ b/lib/gitlab/metrics/base_transaction.rb
@@ -2,7 +2,8 @@ module Gitlab
   module Metrics
     # Class for storing metrics information of a single transaction.
     class BaseTransaction
-      BASE_LABELS = { }.freeze
+      # base labels shared among all transactions
+      BASE_LABELS = { controller: nil, action: nil }.freeze
 
       THREAD_KEY = :_gitlab_metrics_transaction
 
@@ -15,9 +16,7 @@ module Gitlab
         Thread.current[THREAD_KEY]
       end
 
-      # action - A String describing the action performed, usually the class
-      #          plus method name.
-      def initialize(env)
+      def initialize
         @metrics = []
         @methods = {}
 
@@ -26,7 +25,6 @@ module Gitlab
 
         @values = Hash.new(0)
         @tags = {}
-        @env = env
 
         @memory_before = 0
         @memory_after = 0
@@ -40,10 +38,6 @@ module Gitlab
         @memory_after - @memory_before
       end
 
-      def labels
-        {}
-      end
-
       def run
         Thread.current[THREAD_KEY] = self
 
@@ -120,7 +114,6 @@ module Gitlab
 
         submit_hashes = submit.map do |metric|
           hash = metric.to_hash
-
           hash[:tags][:action] ||= action if action && !metric.event?
 
           hash
@@ -129,7 +122,16 @@ module Gitlab
         Metrics.submit_metrics(submit_hashes)
       end
 
-      private
+      def labels
+        BASE_LABELS
+      end
+
+      # returns string describing the action performed, usually the class plus method name.
+      def action
+        "#{labels[:controller]}##{labels[:action]}" if labels && !labels.empty?
+      end
+
+      protected
 
       def self.metric_transaction_duration_seconds
         @metric_transaction_duration_seconds ||= Gitlab::Metrics.histogram(
diff --git a/lib/gitlab/metrics/sidekiq_middleware.rb b/lib/gitlab/metrics/sidekiq_middleware.rb
index b983a40611f..4dbf404f857 100644
--- a/lib/gitlab/metrics/sidekiq_middleware.rb
+++ b/lib/gitlab/metrics/sidekiq_middleware.rb
@@ -5,7 +5,7 @@ module Gitlab
     # This middleware is intended to be used as a server-side middleware.
     class SidekiqMiddleware
       def call(worker, message, queue)
-        trans = Transaction.new("#{worker.class.name}#perform")
+        trans = SidekiqTransaction.new(worker.class)
 
         begin
           # Old gitlad-shell messages don't provide enqueued_at/created_at attributes
diff --git a/lib/gitlab/metrics/sidekiq_transaction.rb b/lib/gitlab/metrics/sidekiq_transaction.rb
new file mode 100644
index 00000000000..4024b892ecc
--- /dev/null
+++ b/lib/gitlab/metrics/sidekiq_transaction.rb
@@ -0,0 +1,15 @@
+module Gitlab
+  module Metrics
+    class SidekiqTransaction
+      def initialize(worker_class)
+        @worker_class = worker_class
+      end
+
+      protected
+
+      def labels
+        { controller: worker.class.name, action: 'perform' }
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/metrics/transaction.rb b/lib/gitlab/metrics/transaction.rb
deleted file mode 100644
index 5bf8580d91f..00000000000
--- a/lib/gitlab/metrics/transaction.rb
+++ /dev/null
@@ -1,245 +0,0 @@
-module Gitlab
-  module Metrics
-    # Class for storing metrics information of a single transaction.
-    class Transaction
-      BASE_LABELS = { controller: nil, action: nil }.freeze
-      CONTROLLER_KEY = 'action_controller.instance'.freeze
-      ENDPOINT_KEY = 'api.endpoint'.freeze
-
-      CONTENT_TYPES = {
-        'text/html' => :html,
-        'text/plain' => :txt,
-        'application/json' => :json,
-        'text/js' => :js,
-        'application/atom+xml' => :atom,
-        'image/png' => :png,
-        'image/jpeg' => :jpeg,
-        'image/gif' => :gif,
-        'image/svg+xml' => :svg
-      }.freeze
-
-      THREAD_KEY = :_gitlab_metrics_transaction
-
-      # The series to store events (e.g. Git pushes) in.
-      EVENT_SERIES = 'events'.freeze
-
-      attr_reader :tags, :values, :method, :metrics
-
-      def self.current
-        Thread.current[THREAD_KEY]
-      end
-
-      # action - A String describing the action performed, usually the class
-      #          plus method name.
-      def initialize(env)
-        @metrics = []
-        @methods = {}
-
-        @started_at = nil
-        @finished_at = nil
-
-        @values = Hash.new(0)
-        @tags = {}
-        @env = env
-
-        @memory_before = 0
-        @memory_after = 0
-      end
-
-      def duration
-        @finished_at ? (@finished_at - @started_at) : 0.0
-      end
-
-      def allocated_memory
-        @memory_after - @memory_before
-      end
-
-      def action
-        "#{labels[:controller]}##{labels[:action]}" if labels && !labels.empty?
-      end
-
-      def labels
-        return @labels if @labels
-
-        # memoize transaction labels only source env variables were present
-        @labels = if @env[CONTROLLER_KEY]
-                    labels_from_controller || {}
-                  elsif @env[ENDPOINT_KEY]
-                    labels_from_endpoint || {}
-                  end
-
-        @labels || {}
-      end
-
-      def run
-        Thread.current[THREAD_KEY] = self
-
-        @memory_before = System.memory_usage
-        @started_at = System.monotonic_time
-
-        yield
-      ensure
-        @memory_after = System.memory_usage
-        @finished_at = System.monotonic_time
-
-        Transaction.metric_transaction_duration_seconds.observe(labels, duration * 1000)
-        Transaction.metric_transaction_allocated_memory_bytes.observe(labels, allocated_memory * 1024.0)
-
-        Thread.current[THREAD_KEY] = nil
-      end
-
-      def add_metric(series, values, tags = {})
-        @metrics << Metric.new("#{Metrics.series_prefix}#{series}", values, tags)
-      end
-
-      # Tracks a business level event
-      #
-      # Business level events including events such as Git pushes, Emails being
-      # sent, etc.
-      #
-      # event_name - The name of the event (e.g. "git_push").
-      # tags - A set of tags to attach to the event.
-      def add_event(event_name, tags = {})
-        self.class.metric_event_counter(event_name, tags).increment(tags.merge(labels))
-        @metrics << Metric.new(EVENT_SERIES, { count: 1 }, tags.merge(event: event_name), :event)
-      end
-
-      # Returns a MethodCall object for the given name.
-      def method_call_for(name, module_name, method_name)
-        unless method = @methods[name]
-          @methods[name] = method = MethodCall.new(name, module_name, method_name, self)
-        end
-
-        method
-      end
-
-      def increment(name, value, use_prometheus = true)
-        self.class.metric_transaction_counter(name).increment(labels, value) if use_prometheus
-        @values[name] += value
-      end
-
-      def set(name, value, use_prometheus = true)
-        self.class.metric_transaction_gauge(name).set(labels, value) if use_prometheus
-        @values[name] = value
-      end
-
-      def finish
-        track_self
-        submit
-      end
-
-      def track_self
-        values = { duration: duration, allocated_memory: allocated_memory }
-
-        @values.each do |name, value|
-          values[name] = value
-        end
-
-        add_metric('transactions', values, @tags)
-      end
-
-      def submit
-        submit = @metrics.dup
-
-        @methods.each do |name, method|
-          submit << method.to_metric if method.above_threshold?
-        end
-
-        submit_hashes = submit.map do |metric|
-          hash = metric.to_hash
-
-          hash[:tags][:action] ||= action if action && !metric.event?
-
-          hash
-        end
-
-        Metrics.submit_metrics(submit_hashes)
-      end
-
-      private
-
-      def self.metric_transaction_duration_seconds
-        @metric_transaction_duration_seconds ||= Gitlab::Metrics.histogram(
-          :gitlab_transaction_duration_seconds,
-          'Transaction duration',
-          BASE_LABELS,
-          [0.001, 0.002, 0.005, 0.01, 0.02, 0.05, 0.1, 0.500, 2.0, 10.0]
-        )
-      end
-
-      def self.metric_transaction_allocated_memory_bytes
-        @metric_transaction_allocated_memory_bytes ||= Gitlab::Metrics.histogram(
-          :gitlab_transaction_allocated_memory_bytes,
-          'Transaction allocated memory bytes',
-          BASE_LABELS,
-          [1000, 10000, 20000, 500000, 1000000, 2000000, 5000000, 10000000, 20000000, 100000000]
-        )
-      end
-
-      def self.metric_event_counter(event_name, tags)
-        @metric_event_counters ||= {}
-        @metric_event_counters[event_name] ||= Gitlab::Metrics.counter(
-          "gitlab_transaction_event_#{event_name}_total".to_sym,
-          "Transaction event #{event_name} counter",
-          tags.merge(BASE_LABELS)
-        )
-      end
-
-      def self.metric_transaction_counter(name)
-        @metric_transaction_counters ||= {}
-        @metric_transaction_counters[name] ||= Gitlab::Metrics.counter(
-          "gitlab_transaction_#{name}_total".to_sym, "Transaction #{name} counter", BASE_LABELS
-        )
-      end
-
-      def self.metric_transaction_gauge(name)
-        @metric_transaction_gauges ||= {}
-        @metric_transaction_gauges[name] ||= Gitlab::Metrics.gauge(
-          "gitlab_transaction_#{name}".to_sym, "Transaction gauge #{name}", BASE_LABELS, :livesum
-        )
-      end
-
-      def labels_from_controller
-        controller = @env[CONTROLLER_KEY]
-
-        action = "#{controller.action_name}"
-        suffix = CONTENT_TYPES[controller.content_type]
-
-        if suffix && suffix != :html
-          action += ".#{suffix}"
-        end
-
-        { controller: controller.class.name, action: action }
-      end
-
-      def labels_from_endpoint
-        endpoint = @env[ENDPOINT_KEY]
-
-        begin
-          route = endpoint.route
-        rescue
-          # endpoint.route is calling env[Grape::Env::GRAPE_ROUTING_ARGS][:route_info]
-          # but env[Grape::Env::GRAPE_ROUTING_ARGS] is nil in the case of a 405 response
-          # so we're rescuing exceptions and bailing out
-        end
-
-        if route
-          path = endpoint_paths_cache[route.request_method][route.path]
-          { controller: 'Grape', action: "#{route.request_method} #{path}" }
-        end
-      end
-
-      def endpoint_paths_cache
-        @endpoint_paths_cache ||= Hash.new do |hash, http_method|
-          hash[http_method] = Hash.new do |inner_hash, raw_path|
-            inner_hash[raw_path] = endpoint_instrumentable_path(raw_path)
-          end
-        end
-      end
-
-      def endpoint_instrumentable_path(raw_path)
-        raw_path.sub('(.:format)', '').sub('/:version', '')
-      end
-    end
-  end
-end
diff --git a/lib/gitlab/metrics/web_transaction.rb b/lib/gitlab/metrics/web_transaction.rb
index e7ac9e68f9f..f85929b1c4e 100644
--- a/lib/gitlab/metrics/web_transaction.rb
+++ b/lib/gitlab/metrics/web_transaction.rb
@@ -1,8 +1,6 @@
 module Gitlab
   module Metrics
-    # Class for storing metrics information of a single transaction.
-    class WebTransaction
-      BASE_LABELS = { controller: nil, action: nil }.freeze
+    class WebTransaction < BaseTransaction
       CONTROLLER_KEY = 'action_controller.instance'.freeze
       ENDPOINT_KEY = 'api.endpoint'.freeze
 
@@ -18,10 +16,9 @@ module Gitlab
         'image/svg+xml' => :svg
       }.freeze
 
-      attr_reader :tags, :values, :method, :metrics
-
-      def action
-        "#{labels[:controller]}##{labels[:action]}" if labels && !labels.empty?
+      def initialize(env)
+        super()
+        @env = env
       end
 
       def labels
diff --git a/spec/lib/gitlab/metrics/instrumentation_spec.rb b/spec/lib/gitlab/metrics/instrumentation_spec.rb
index 28f8c681263..977bc250049 100644
--- a/spec/lib/gitlab/metrics/instrumentation_spec.rb
+++ b/spec/lib/gitlab/metrics/instrumentation_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Gitlab::Metrics::Instrumentation do
   let(:env) { {} }
-  let(:transaction) { Gitlab::Metrics::Transaction.new(env) }
+  let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) }
 
   before do
     @dummy = Class.new do
diff --git a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb
index 3d811aa267a..b2887911194 100644
--- a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb
+++ b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Gitlab::Metrics::Subscribers::ActionView do
   let(:env) { {} }
-  let(:transaction) { Gitlab::Metrics::Transaction.new(env) }
+  let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) }
 
   let(:subscriber) { described_class.new }
 
diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb
index 157be2578e9..ef2945970c1 100644
--- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb
+++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Gitlab::Metrics::Subscribers::ActiveRecord do
   let(:env) { {} }
-  let(:transaction) { Gitlab::Metrics::Transaction.new(env) }
+  let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) }
   let(:subscriber)  { described_class.new }
 
   let(:event) do
diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/web_transaction_spec.rb
index 19c2d0a2b0b..1d162f53a13 100644
--- a/spec/lib/gitlab/metrics/transaction_spec.rb
+++ b/spec/lib/gitlab/metrics/web_transaction_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::Metrics::Transaction do
+describe Gitlab::Metrics::WebTransaction do
   let(:env) { {} }
   let(:transaction) { described_class.new(env) }
 
diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb
index 906abdac47c..1619fbd88b1 100644
--- a/spec/lib/gitlab/metrics_spec.rb
+++ b/spec/lib/gitlab/metrics_spec.rb
@@ -115,7 +115,7 @@ describe Gitlab::Metrics do
     end
 
     context 'with a transaction' do
-      let(:transaction) { Gitlab::Metrics::Transaction.new({}) }
+      let(:transaction) { Gitlab::Metrics::WebTransaction.new({}) }
 
       before do
         allow(described_class).to receive(:current_transaction)
@@ -155,7 +155,7 @@ describe Gitlab::Metrics do
 
     context 'with a transaction' do
       it 'sets the action of a transaction' do
-        trans = Gitlab::Metrics::Transaction.new({})
+        trans = Gitlab::Metrics::WebTransaction.new({})
 
         expect(described_class).to receive(:current_transaction)
           .and_return(trans)
@@ -185,7 +185,7 @@ describe Gitlab::Metrics do
 
     context 'with a transaction' do
       it 'adds an event' do
-        transaction = Gitlab::Metrics::Transaction.new({})
+        transaction = Gitlab::Metrics::WebTransaction.new({})
 
         expect(transaction).to receive(:add_event).with(:meow)
 
diff --git a/spec/lib/gitlab/middleware/rails_queue_duration_spec.rb b/spec/lib/gitlab/middleware/rails_queue_duration_spec.rb
index b14c4477590..e64c4884fef 100644
--- a/spec/lib/gitlab/middleware/rails_queue_duration_spec.rb
+++ b/spec/lib/gitlab/middleware/rails_queue_duration_spec.rb
@@ -4,7 +4,7 @@ describe Gitlab::Middleware::RailsQueueDuration do
   let(:app) { double(:app) }
   let(:middleware) { described_class.new(app) }
   let(:env) { {} }
-  let(:transaction) { Gitlab::Metrics::Transaction.new(env) }
+  let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) }
 
   before do
     expect(app).to receive(:call).with(env).and_return('yay')