Transaction needs to be able to describe controller action by itself

Author: pawelchcki

lib/gitlab/metrics/rack_middleware.rb

@@ -2,20 +2,6 @@ module Gitlab
   module Metrics
     # Rack middleware for tracking Rails and Grape requests.
     class RackMiddleware
-      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
-
       def initialize(app)
         @app = app
       end
@@ -42,49 +28,14 @@ def call(env)
       end
 
       def transaction_from_env(env)
-        trans = Transaction.new
+        trans = Transaction.new(env)
 
         trans.set(:request_uri, filtered_path(env), false)
         trans.set(:request_method, env['REQUEST_METHOD'], false)
 
-        if env[CONTROLLER_KEY]
-          tag_controller(trans, env)
-        elsif env[ENDPOINT_KEY]
-          tag_endpoint(trans, env)
-        end
-
         trans
       end
 
-      def tag_controller(trans, env)
-        controller = env[CONTROLLER_KEY]
-        action = "#{controller.class.name}##{controller.action_name}"
-        suffix = CONTENT_TYPES[controller.content_type]
-
-        if suffix && suffix != :html
-          action += ".#{suffix}"
-        end
-
-        trans.action = action
-      end
-
-      def tag_endpoint(trans, env)
-        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]
-          trans.action = "Grape##{route.request_method} #{path}"
-        end
-      end
-
       private
 
       def filtered_path(env)

lib/gitlab/metrics/transaction.rb

@@ -2,22 +2,35 @@ module Gitlab
   module Metrics
     # Class for storing metrics information of a single transaction.
     class Transaction
+      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
 
-      attr_accessor :action
-
       def self.current
         Thread.current[THREAD_KEY]
       end
 
       # action - A String describing the action performed, usually the class
       #          plus method name.
-      def initialize(action = nil)
+      def initialize(env)
         @metrics = []
         @methods = {}
 
@@ -26,7 +39,7 @@ def initialize(action = nil)
 
         @values = Hash.new(0)
         @tags = {}
-        @action = action
+        @env = env
 
         @memory_before = 0
         @memory_after = 0
@@ -40,22 +53,12 @@ def allocated_memory
         @memory_after - @memory_before
       end
 
-      def self.metric_transaction_duration_seconds
-        @metric_transaction_duration_seconds ||= Gitlab::Metrics.histogram(
-          :gitlab_transaction_duration_seconds,
-          'Transaction duration',
-          { action: nil },
-          [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',
-          { action: nil },
-          [1000, 10000, 20000, 500000, 1000000, 2000000, 5000000, 10000000, 20000000, 100000000]
-        )
+      def action
+        @action ||= if @env[CONTROLLER_KEY]
+                      action_from_controller(@env) || ''
+                    elsif @env[ENDPOINT_KEY]
+                      action_from_endpoint(@env) || ''
+                    end
       end
 
       def run
@@ -135,7 +138,7 @@ def submit
         submit_hashes = submit.map do |metric|
           hash = metric.to_hash
 
-          hash[:tags][:action] ||= @action if @action && !metric.event?
+          hash[:tags][:action] ||= action if action && !metric.event?
 
           hash
         end
@@ -145,6 +148,24 @@ def submit
 
       private
 
+      def self.metric_transaction_duration_seconds
+        @metric_transaction_duration_seconds ||= Gitlab::Metrics.histogram(
+          :gitlab_transaction_duration_seconds,
+          'Transaction duration',
+          { action: nil },
+          [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',
+          { action: nil },
+          [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(
@@ -167,6 +188,36 @@ def self.metric_transaction_gauge(name)
           "gitlab_transaction_#{name}".to_sym, "Transaction gauge #{name}", { action: nil }, :livesum
         )
       end
+
+      def action_from_controller(env)
+        controller = env[CONTROLLER_KEY]
+
+        action = "#{controller.class.name}##{controller.action_name}"
+        suffix = CONTENT_TYPES[controller.content_type]
+
+        if suffix && suffix != :html
+          action += ".#{suffix}"
+        end
+
+        action
+      end
+
+      def action_from_endpoint(env)
+        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]
+          "Grape##{route.request_method} #{path}"
+        end
+      end
     end
   end
 end