Merge branch '219029-refactor-metrics-tranasction' into 'master'

Make Metrics::Transaction more generic

Closes #219029

See merge request gitlab-org/gitlab!32980

changelogs/unreleased/219029-refactor-metrics-tranasction.yml

+---
+title: Unify Prometheus metric initialization by always using inline transaction metrics
+merge_request: 32980
+author:
+type: other

config/initializers/zz_metrics.rb

@@ -199,7 +199,7 @@ if Gitlab::Metrics.enabled? && !Rails.env.test? && !(Rails.env.development? && d
       val = super
 
       if current_transaction = ::Gitlab::Metrics::Transaction.current
-        current_transaction.increment(:new_redis_connections, 1)
+        current_transaction.increment(:gitlab_transaction_new_redis_connections_total, 1)
       end
 
       val

lib/gitlab/database.rb

@@ -2,8 +2,6 @@
 
 module Gitlab
   module Database
-    include Gitlab::Metrics::Methods
-
     # Minimum PostgreSQL version requirement per documentation:
     # https://docs.gitlab.com/ee/install/requirements.html#postgresql-requirements
     MINIMUM_POSTGRES_VERSION = 11
@@ -50,10 +48,6 @@ module Gitlab
     # It does not include the default public schema
     EXTRA_SCHEMAS = [DYNAMIC_PARTITIONS_SCHEMA, STATIC_PARTITIONS_SCHEMA].freeze
 
-    define_histogram :gitlab_database_transaction_seconds do
-      docstring "Time spent in database transactions, in seconds"
-    end
-
     def self.config
       ActiveRecord::Base.configurations[Rails.env]
     end
@@ -363,8 +357,11 @@ module Gitlab
     # observe_transaction_duration is called from ActiveRecordBaseTransactionMetrics.transaction and used to
     # record transaction durations.
     def self.observe_transaction_duration(duration_seconds)
-      labels = Gitlab::Metrics::Transaction.current&.labels || {}
-      gitlab_database_transaction_seconds.observe(labels, duration_seconds)
+      if current_transaction = ::Gitlab::Metrics::Transaction.current
+        current_transaction.observe(:gitlab_database_transaction_seconds, duration_seconds) do
+          docstring "Time spent in database transactions, in seconds"
+        end
+      end
     rescue Prometheus::Client::LabelSetValidator::LabelSetError => err
       # Ensure that errors in recording these metrics don't affect the operation of the application
       Rails.logger.error("Unable to observe database transaction duration: #{err}") # rubocop:disable Gitlab/RailsLogger

lib/gitlab/diff/highlight_cache.rb

@@ -3,7 +3,6 @@
 module Gitlab
   module Diff
     class HighlightCache
-      include Gitlab::Metrics::Methods
       include Gitlab::Utils::StrongMemoize
 
       EXPIRATION = 1.week
@@ -12,19 +11,6 @@ module Gitlab
       delegate :diffable,     to: :@diff_collection
       delegate :diff_options, to: :@diff_collection
 
-      define_histogram :gitlab_redis_diff_caching_memory_usage_bytes do
-        docstring 'Redis diff caching memory usage by key'
-        buckets [100, 1_000, 10_000, 100_000, 1_000_000, 10_000_000]
-      end
-
-      define_counter :gitlab_redis_diff_caching_hit do
-        docstring 'Redis diff caching hits'
-      end
-
-      define_counter :gitlab_redis_diff_caching_miss do
-        docstring 'Redis diff caching misses'
-      end
-
       def initialize(diff_collection)
         @diff_collection = diff_collection
       end
@@ -117,7 +103,10 @@ module Gitlab
 
       def record_memory_usage(memory_usage)
         if memory_usage
-          self.class.gitlab_redis_diff_caching_memory_usage_bytes.observe({}, memory_usage)
+          current_transaction&.observe(:gitlab_redis_diff_caching_memory_usage_bytes, memory_usage) do
+            docstring 'Redis diff caching memory usage by key'
+            buckets [100, 1_000, 10_000, 100_000, 1_000_000, 10_000_000]
+          end
         end
       end
 
@@ -206,6 +195,10 @@ module Gitlab
         #
         @diff_collection.raw_diff_files
       end
+
+      def current_transaction
+        ::Gitlab::Metrics::Transaction.current
+      end
     end
   end
 end

lib/gitlab/diff/stats_cache.rb

@@ -3,7 +3,6 @@
 module Gitlab
   module Diff
     class StatsCache
-      include Gitlab::Metrics::Methods
       include Gitlab::Utils::StrongMemoize
 
       EXPIRATION = 1.week

lib/gitlab/git/blob.rb

@@ -5,7 +5,6 @@ module Gitlab
     class Blob
       include Gitlab::BlobHelper
       include Gitlab::EncodingHelper
-      include Gitlab::Metrics::Methods
       extend Gitlab::Git::WrapsGitalyErrors
 
       # This number is the maximum amount of data that we want to display to
@@ -28,17 +27,21 @@ module Gitlab
       attr_accessor :size, :mode, :id, :commit_id, :loaded_size, :binary
       attr_writer :name, :path, :data
 
-      define_counter :gitlab_blob_truncated_true do
-        docstring 'blob.truncated? == true'
+      def self.gitlab_blob_truncated_true
+        @gitlab_blob_truncated_true ||= ::Gitlab::Metrics.counter(:gitlab_blob_truncated_true, 'blob.truncated? == true')
       end
 
-      define_counter :gitlab_blob_truncated_false do
-        docstring 'blob.truncated? == false'
+      def self.gitlab_blob_truncated_false
+        @gitlab_blob_truncated_false ||= ::Gitlab::Metrics.counter(:gitlab_blob_truncated_false, 'blob.truncated? == false')
       end
 
-      define_histogram :gitlab_blob_size do
-        docstring 'Gitlab::Git::Blob size'
-        buckets [1_000, 5_000, 10_000, 50_000, 100_000, 500_000, 1_000_000]
+      def self.gitlab_blob_size
+        @gitlab_blob_size ||= ::Gitlab::Metrics.histogram(
+          :gitlab_blob_size,
+          'Gitlab::Git::Blob size',
+          {},
+          [1_000, 5_000, 10_000, 50_000, 100_000, 500_000, 1_000_000]
+        )
       end
 
       class << self

lib/gitlab/gitaly_client.rb

@@ -8,8 +8,6 @@ require 'grpc/health/v1/health_services_pb'
 
 module Gitlab
   module GitalyClient
-    include Gitlab::Metrics::Methods
-
     class TooManyInvocationsError < StandardError
       attr_reader :call_site, :invocation_count, :max_call_stack
 
@@ -191,11 +189,6 @@ module Gitlab
       Gitlab::SafeRequestStore[:gitaly_query_time] += duration
     end
 
-    def self.current_transaction_labels
-      Gitlab::Metrics::Transaction.current&.labels || {}
-    end
-    private_class_method :current_transaction_labels
-
     # For some time related tasks we can't rely on `Time.now` since it will be
     # affected by Timecop in some tests, and the clock of some gitaly-related
     # components (grpc's c-core and gitaly server) use system time instead of

lib/gitlab/metrics.rb

@@ -3,7 +3,6 @@
 module Gitlab
   module Metrics
     include Gitlab::Metrics::Prometheus
-    include Gitlab::Metrics::Methods
 
     EXECUTION_MEASUREMENT_BUCKETS = [0.001, 0.01, 0.1, 1].freeze
 
@@ -81,25 +80,16 @@ module Gitlab
       real_time = (real_stop - real_start)
       cpu_time = cpu_stop - cpu_start
 
-      real_duration_seconds = fetch_histogram("gitlab_#{name}_real_duration_seconds".to_sym) do
+      trans.observe("gitlab_#{name}_real_duration_seconds".to_sym, real_time) do
         docstring "Measure #{name}"
-        base_labels Transaction::BASE_LABELS
         buckets EXECUTION_MEASUREMENT_BUCKETS
       end
 
-      real_duration_seconds.observe(trans.labels, real_time)
-
-      cpu_duration_seconds = fetch_histogram("gitlab_#{name}_cpu_duration_seconds".to_sym) do
+      trans.observe("gitlab_#{name}_cpu_duration_seconds".to_sym, cpu_time) do
         docstring "Measure #{name}"
-        base_labels Transaction::BASE_LABELS
         buckets EXECUTION_MEASUREMENT_BUCKETS
         with_feature "prometheus_metrics_measure_#{name}_cpu_duration"
       end
-      cpu_duration_seconds.observe(trans.labels, cpu_time)
-
-      trans.increment("#{name}_real_time", real_time.in_milliseconds, false)
-      trans.increment("#{name}_cpu_time", cpu_time.in_milliseconds, false)
-      trans.increment("#{name}_call_count", 1, false)
 
       retval
     end

lib/gitlab/metrics/elasticsearch_rack_middleware.rb

@@ -8,14 +8,6 @@ module Gitlab
 
       def initialize(app)
         @app = app
-
-        @requests_total_counter = Gitlab::Metrics.counter(:http_elasticsearch_requests_total,
-                                                         'Amount of calls to Elasticsearch servers during web requests',
-                                                         Gitlab::Metrics::Transaction::BASE_LABELS)
-        @requests_duration_histogram = Gitlab::Metrics.histogram(:http_elasticsearch_requests_duration_seconds,
-                                                                 'Query time for Elasticsearch servers during web requests',
-                                                                 Gitlab::Metrics::Transaction::BASE_LABELS,
-                                                                 HISTOGRAM_BUCKETS)
       end
 
       def call(env)
@@ -29,12 +21,17 @@ module Gitlab
       private
 
       def record_metrics(transaction)
-        labels = transaction.labels
         query_time = ::Gitlab::Instrumentation::ElasticsearchTransport.query_time
         request_count = ::Gitlab::Instrumentation::ElasticsearchTransport.get_request_count
 
-        @requests_total_counter.increment(labels, request_count)
-        @requests_duration_histogram.observe(labels, query_time)
+        transaction.increment(:http_elasticsearch_requests_total, request_count) do
+          docstring 'Amount of calls to Elasticsearch servers during web requests'
+        end
+
+        transaction.observe(:http_elasticsearch_requests_duration_seconds, query_time) do
+          docstring 'Query time for Elasticsearch servers during web requests'
+          buckets HISTOGRAM_BUCKETS
+        end
       end
     end
   end

lib/gitlab/metrics/method_call.rb

@@ -4,16 +4,7 @@ module Gitlab
   module Metrics
     # Class for tracking timing information about method calls
     class MethodCall
-      include Gitlab::Metrics::Methods
-      BASE_LABELS = { module: nil, method: nil }.freeze
-      attr_reader :real_time, :cpu_time, :call_count, :labels
-
-      define_histogram :gitlab_method_call_duration_seconds do
-        docstring 'Method calls real duration'
-        base_labels Transaction::BASE_LABELS.merge(BASE_LABELS)
-        buckets [0.01, 0.05, 0.1, 0.5, 1]
-        with_feature :prometheus_metrics_method_instrumentation
-      end
+      attr_reader :real_time, :cpu_time, :call_count
 
       # name - The full name of the method (including namespace) such as
       #        `User#sign_in`.
@@ -42,8 +33,14 @@ module Gitlab
         @cpu_time += cpu_time
         @call_count += 1
 
-        if above_threshold?
-          self.class.gitlab_method_call_duration_seconds.observe(@transaction.labels.merge(labels), real_time)
+        if above_threshold? && transaction
+          label_keys = labels.keys
+          transaction.observe(:gitlab_method_call_duration_seconds, real_time, labels) do
+            docstring 'Method calls real duration'
+            label_keys label_keys
+            buckets [0.01, 0.05, 0.1, 0.5, 1]
+            with_feature :prometheus_metrics_method_instrumentation
+          end
         end
 
         retval
@@ -54,6 +51,10 @@ module Gitlab
       def above_threshold?
         real_time.in_milliseconds >= ::Gitlab::Metrics.method_call_threshold
       end
+
+      private
+
+      attr_reader :labels, :transaction
     end
   end
 end

lib/gitlab/metrics/methods.rb

@@ -69,62 +69,6 @@ module Gitlab
             raise ArgumentError, "uknown metric type #{type}"
           end
         end
-
-        # Fetch and/or initialize counter metric
-        # @param [Symbol] name
-        # @param [Hash] opts
-        def fetch_counter(name, opts = {}, &block)
-          fetch_metric(:counter, name, opts, &block)
-        end
-
-        # Fetch and/or initialize gauge metric
-        # @param [Symbol] name
-        # @param [Hash] opts
-        def fetch_gauge(name, opts = {}, &block)
-          fetch_metric(:gauge, name, opts, &block)
-        end
-
-        # Fetch and/or initialize histogram metric
-        # @param [Symbol] name
-        # @param [Hash] opts
-        def fetch_histogram(name, opts = {}, &block)
-          fetch_metric(:histogram, name, opts, &block)
-        end
-
-        # Fetch and/or initialize summary metric
-        # @param [Symbol] name
-        # @param [Hash] opts
-        def fetch_summary(name, opts = {}, &block)
-          fetch_metric(:summary, name, opts, &block)
-        end
-
-        # Define metric accessor method for a Counter
-        # @param [Symbol] name
-        # @param [Hash] opts
-        def define_counter(name, opts = {}, &block)
-          define_metric(:counter, name, opts, &block)
-        end
-
-        # Define metric accessor method for a Gauge
-        # @param [Symbol] name
-        # @param [Hash] opts
-        def define_gauge(name, opts = {}, &block)
-          define_metric(:gauge, name, opts, &block)
-        end
-
-        # Define metric accessor method for a Histogram
-        # @param [Symbol] name
-        # @param [Hash] opts
-        def define_histogram(name, opts = {}, &block)
-          define_metric(:histogram, name, opts, &block)
-        end
-
-        # Define metric accessor method for a Summary
-        # @param [Symbol] name
-        # @param [Hash] opts
-        def define_summary(name, opts = {}, &block)
-          define_metric(:summary, name, opts, &block)
-        end
       end
     end
   end

lib/gitlab/metrics/methods/metric_options.rb

@@ -4,14 +4,12 @@ module Gitlab
   module Metrics
     module Methods
       class MetricOptions
-        SMALL_NETWORK_BUCKETS = [0.005, 0.01, 0.1, 1, 10].freeze
-
         def initialize(options = {})
           @multiprocess_mode = options[:multiprocess_mode] || :all
-          @buckets = options[:buckets] || SMALL_NETWORK_BUCKETS
-          @base_labels = options[:base_labels] || {}
+          @buckets = options[:buckets] || ::Prometheus::Client::Histogram::DEFAULT_BUCKETS
           @docstring = options[:docstring]
           @with_feature = options[:with_feature]
+          @label_keys = options[:label_keys] || []
         end
 
         # Documentation describing metric in metrics endpoint '/-/metrics'
@@ -40,12 +38,21 @@ module Gitlab
         end
 
         # Base labels are merged with per metric labels
-        def base_labels(base_labels = nil)
-          @base_labels = base_labels unless base_labels.nil?
+        def base_labels
+          @base_labels ||= @label_keys.product([nil]).to_h
 
           @base_labels
         end
 
+        def label_keys(label_keys = nil)
+          unless label_keys.nil?
+            @label_keys = label_keys
+            @base_labels = nil
+          end
+
+          @label_keys
+        end
+
         # Use feature toggle to control whether certain metric is enabled/disabled
         def with_feature(name = nil)
           @with_feature = name unless name.nil?
@@ -55,6 +62,7 @@ module Gitlab
 
         def evaluate(&block)
           instance_eval(&block) if block_given?
+
           self
         end
       end

lib/gitlab/metrics/rack_middleware.rb

@@ -10,8 +10,7 @@ module Gitlab
 
       # env - A Hash containing Rack environment details.
       def call(env)
-        trans  = transaction_from_env(env)
-        retval = nil
+        trans = WebTransaction.new(env)
 
         begin
           retval = trans.run { @app.call(env) }
@@ -24,21 +23,6 @@ module Gitlab
 
         retval
       end
-
-      def transaction_from_env(env)
-        trans = WebTransaction.new(env)
-
-        trans.set(:request_uri, filtered_path(env), false)
-        trans.set(:request_method, env['REQUEST_METHOD'], false)
-
-        trans
-      end
-
-      private
-
-      def filtered_path(env)
-        ActionDispatch::Request.new(env).filtered_path.presence || env['REQUEST_URI']
-      end
     end
   end
 end

lib/gitlab/metrics/redis_rack_middleware.rb

@@ -6,14 +6,6 @@ module Gitlab
     class RedisRackMiddleware
       def initialize(app)
         @app = app
-
-        @requests_total_counter = Gitlab::Metrics.counter(:http_redis_requests_total,
-                                                          'Amount of calls to Redis servers during web requests',
-                                                          Gitlab::Metrics::Transaction::BASE_LABELS)
-        @requests_duration_histogram = Gitlab::Metrics.histogram(:http_redis_requests_duration_seconds,
-                                                                 'Query time for Redis servers during web requests',
-                                                                 Gitlab::Metrics::Transaction::BASE_LABELS,
-                                                                 Gitlab::Instrumentation::Redis::QUERY_TIME_BUCKETS)
       end
 
       def call(env)
@@ -27,12 +19,17 @@ module Gitlab
       private
 
       def record_metrics(transaction)
-        labels = transaction.labels
         query_time = Gitlab::Instrumentation::Redis.query_time
         request_count = Gitlab::Instrumentation::Redis.get_request_count
 
-        @requests_total_counter.increment(labels, request_count)
-        @requests_duration_histogram.observe(labels, query_time)
+        transaction.increment(:http_redis_requests_total, request_count) do
+          docstring 'Amount of calls to Redis servers during web requests'
+        end
+
+        transaction.observe(:http_redis_requests_duration_seconds, query_time) do
+          docstring 'Query time for Redis servers during web requests'
+          buckets Gitlab::Instrumentation::Redis::QUERY_TIME_BUCKETS
+        end
       end
     end
   end

lib/gitlab/metrics/sidekiq_middleware.rb

@@ -12,7 +12,9 @@ module Gitlab
         begin
           # Old gitlad-shell messages don't provide enqueued_at/created_at attributes
           enqueued_at = payload['enqueued_at'] || payload['created_at'] || 0
-          trans.set(:sidekiq_queue_duration, Time.current.to_f - enqueued_at)
+          trans.set(:gitlab_transaction_sidekiq_queue_duration_total, Time.current.to_f - enqueued_at) do
+            multiprocess_mode :livesum
+          end
           trans.run { yield }
         rescue Exception => error # rubocop: disable Lint/RescueException
           trans.add_event(:sidekiq_exception)

lib/gitlab/metrics/subscribers/action_view.rb

@@ -5,14 +5,6 @@ module Gitlab
     module Subscribers
       # Class for tracking the rendering timings of views.
       class ActionView < ActiveSupport::Subscriber
-        include Gitlab::Metrics::Methods
-        define_histogram :gitlab_view_rendering_duration_seconds do
-          docstring 'View rendering time'
-          base_labels Transaction::BASE_LABELS.merge({ path: nil })
-          buckets [0.001, 0.01, 0.1, 1, 10.0]
-          with_feature :prometheus_metrics_view_instrumentation
-        end
-
         attach_to :action_view
 
         SERIES = 'views'
@@ -27,10 +19,14 @@ module Gitlab
 
         def track(event)
           tags = tags_for(event)
+          current_transaction.observe(:gitlab_view_rendering_duration_seconds, event.duration, tags) do
+            docstring 'View rendering time'
+            label_keys %i(view)
+            buckets [0.001, 0.01, 0.1, 1, 10.0]
+            with_feature :prometheus_metrics_view_instrumentation
+          end
 
-          self.class.gitlab_view_rendering_duration_seconds.observe(current_transaction.labels.merge(tags), event.duration)
-
-          current_transaction.increment(:view_duration, event.duration)
+          current_transaction.increment(:gitlab_transaction_view_duration_total, event.duration)
         end
 
         def relative_path(path)

lib/gitlab/metrics/subscribers/active_record.rb

@@ -5,7 +5,6 @@ module Gitlab
     module Subscribers
       # Class for tracking the total query duration of a transaction.
       class ActiveRecord < ActiveSupport::Subscriber
-        include Gitlab::Metrics::Methods
         attach_to :active_record
 
         IGNORABLE_SQL = %w{BEGIN COMMIT}.freeze
@@ -22,7 +21,9 @@ module Gitlab
           payload = event.payload
           return if payload[:name] == 'SCHEMA' || IGNORABLE_SQL.include?(payload[:sql])
 
-          self.class.gitlab_sql_duration_seconds.observe(current_transaction.labels, event.duration / 1000.0)
+          current_transaction.observe(:gitlab_sql_duration_seconds, event.duration / 1000.0) do
+            buckets [0.01, 0.05, 0.1, 0.25, 0.5, 1.0, 2.5, 5.0]
+          end
 
           increment_db_counters(payload)
         end
@@ -37,12 +38,6 @@ module Gitlab
 
         private
 
-        define_histogram :gitlab_sql_duration_seconds do
-          docstring 'SQL time'
-          base_labels Transaction::BASE_LABELS
-          buckets [0.01, 0.05, 0.1, 0.25, 0.5, 1.0, 2.5, 5.0]
-        end
-
         def select_sql_command?(payload)
           payload[:sql].match(/\A((?!(.*[^\w'"](DELETE|UPDATE|INSERT INTO)[^\w'"])))(WITH.*)?(SELECT)((?!(FOR UPDATE|FOR SHARE)).)*$/i)
         end
@@ -58,7 +53,7 @@ module Gitlab
         end
 
         def increment(counter)
-          current_transaction.increment(counter, 1)
+          current_transaction.increment("gitlab_transaction_#{counter}_total".to_sym, 1)
 
           if Gitlab::SafeRequestStore.active?
             Gitlab::SafeRequestStore[counter] = Gitlab::SafeRequestStore[counter].to_i + 1

lib/gitlab/metrics/subscribers/rails_cache.rb

@@ -14,11 +14,10 @@ module Gitlab
           return unless current_transaction
           return if event.payload[:super_operation] == :fetch
 
-          if event.payload[:hit]
-            current_transaction.increment(:cache_read_hit_count, 1, false)
-          else
-            metric_cache_misses_total.increment(current_transaction.labels)
-            current_transaction.increment(:cache_read_miss_count, 1, false)
+          unless event.payload[:hit]
+            current_transaction.increment(:gitlab_cache_misses_total, 1) do
+              docstring 'Cache read miss'
+            end
           end
         end
 
@@ -37,25 +36,30 @@ module Gitlab
         def cache_fetch_hit(event)
           return unless current_transaction
 
-          current_transaction.increment(:cache_read_hit_count, 1)
+          current_transaction.increment(:gitlab_transaction_cache_read_hit_count_total, 1)
         end
 
         def cache_generate(event)
           return unless current_transaction
 
-          metric_cache_misses_total.increment(current_transaction.labels)
-          current_transaction.increment(:cache_read_miss_count, 1)
+          current_transaction.increment(:gitlab_cache_misses_total, 1) do
+            docstring 'Cache read miss'
+          end
+
+          current_transaction.increment(:gitlab_transaction_cache_read_miss_count_total, 1)
         end
 
         def observe(key, duration)
           return unless current_transaction
 
-          metric_cache_operations_total.increment(current_transaction.labels.merge({ operation: key }))
-          metric_cache_operation_duration_seconds.observe({ operation: key }, duration / 1000.0)
-          current_transaction.increment(:cache_duration, duration, false)
-          current_transaction.increment(:cache_count, 1, false)
-          current_transaction.increment("cache_#{key}_duration".to_sym, duration, false)
-          current_transaction.increment("cache_#{key}_count".to_sym, 1, false)
+          labels = { operation: key }
+
+          current_transaction.increment(:gitlab_cache_operations_total, 1, labels) do
+            docstring 'Cache operations'
+            label_keys labels.keys
+          end
+
+          metric_cache_operation_duration_seconds.observe(labels, duration / 1000.0)
         end
 
         private
@@ -64,14 +68,6 @@ module Gitlab
           Transaction.current
         end
 
-        def metric_cache_operations_total
-          @metric_cache_operations_total ||= ::Gitlab::Metrics.counter(
-            :gitlab_cache_operations_total,
-            'Cache operations',
-            Transaction::BASE_LABELS
-          )
-        end
-
         def metric_cache_operation_duration_seconds
           @metric_cache_operation_duration_seconds ||= ::Gitlab::Metrics.histogram(
             :gitlab_cache_operation_duration_seconds,
@@ -80,14 +76,6 @@ module Gitlab
             [0.00001, 0.0001, 0.001, 0.01, 0.1, 1.0]
           )
         end
-
-        def metric_cache_misses_total
-          @metric_cache_misses_total ||= ::Gitlab::Metrics.counter(
-            :gitlab_cache_misses_total,
-            'Cache read miss',
-            Transaction::BASE_LABELS
-          )
-        end
       end
     end
   end

lib/gitlab/metrics/transaction.rb

@@ -6,22 +6,38 @@ module Gitlab
     class Transaction
       include Gitlab::Metrics::Methods
 
-      # base labels shared among all transactions
-      BASE_LABELS = { controller: nil, action: nil, feature_category: nil }.freeze
+      # base label keys shared among all transactions
+      BASE_LABEL_KEYS = %i(controller action feature_category).freeze
       # labels that potentially contain sensitive information and will be filtered
-      FILTERED_LABELS = [:branch, :path].freeze
+      FILTERED_LABEL_KEYS = %i(branch path).freeze
 
       THREAD_KEY = :_gitlab_metrics_transaction
 
+      SMALL_BUCKETS = [0.1, 0.25, 0.5, 1.0, 2.5, 5.0].freeze
+      BIG_BUCKETS = [100, 1000, 10000, 100000, 1000000, 10000000].freeze
+
       # The series to store events (e.g. Git pushes) in.
       EVENT_SERIES = 'events'
 
       attr_reader :method
 
-      def self.current
+      class << self
+        def current
           Thread.current[THREAD_KEY]
         end
 
+        def prometheus_metric(name, type, &block)
+          fetch_metric(type, name) do
+            # set default metric options
+            docstring "#{name.to_s.humanize} #{type}"
+
+            evaluate(&block)
+            # always filter sensitive labels and merge with base ones
+            label_keys BASE_LABEL_KEYS | (label_keys - FILTERED_LABEL_KEYS)
+          end
+        end
+      end
+
       def initialize
         @methods = {}
 
@@ -56,9 +72,15 @@ module Gitlab
         @memory_after = System.memory_usage_rss
         @finished_at = System.monotonic_time
 
-        self.class.gitlab_transaction_cputime_seconds.observe(labels, thread_cpu_duration)
-        self.class.gitlab_transaction_duration_seconds.observe(labels, duration)
-        self.class.gitlab_transaction_allocated_memory_bytes.observe(labels, allocated_memory * 1024.0)
+        observe(:gitlab_transaction_cputime_seconds, thread_cpu_duration) do
+          buckets SMALL_BUCKETS
+        end
+        observe(:gitlab_transaction_duration_seconds, duration) do
+          buckets SMALL_BUCKETS
+        end
+        observe(:gitlab_transaction_allocated_memory_bytes, allocated_memory * 1024.0) do
+          buckets BIG_BUCKETS
+        end
 
         Thread.current[THREAD_KEY] = nil
       end
@@ -71,8 +93,12 @@ module Gitlab
       # 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 = {})
-        filtered_tags = filter_tags(tags)
-        self.class.transaction_metric(event_name, :counter, prefix: 'event_', tags: filtered_tags).increment(filtered_tags.merge(labels))
+        event_name = "gitlab_transaction_event_#{event_name}_total".to_sym
+        metric = self.class.prometheus_metric(event_name, :counter) do
+          label_keys tags.keys
+        end
+
+        metric.increment(filter_labels(tags))
       end
 
       # Returns a MethodCall object for the given name.
@@ -84,52 +110,70 @@ module Gitlab
         method
       end
 
-      def increment(name, value, use_prometheus = true)
-        self.class.transaction_metric(name, :counter).increment(labels, value) if use_prometheus
-      end
+      # Increment counter metric
+      #
+      # It will initialize the metric if metric is not found
+      #
+      # block - if provided can be used to initialize metric with custom options (docstring, labels, with_feature)
+      #
+      # Example:
+      # ```
+      # transaction.increment(:mestric_name, 1, { docstring: 'Custom title', base_labels: {sane: 'yes'} } ) do
+      #
+      # transaction.increment(:mestric_name, 1) do
+      #   docstring 'Custom title'
+      #   label_keys %i(sane)
+      # end
+      # ```
+      def increment(name, value = 1, labels = {}, &block)
+        counter = self.class.prometheus_metric(name, :counter, &block)
 
-      def set(name, value, use_prometheus = true)
-        self.class.transaction_metric(name, :gauge).set(labels, value) if use_prometheus
+        counter.increment(filter_labels(labels), value)
       end
 
-      def labels
-        BASE_LABELS
-      end
+      # Set gauge metric
+      #
+      # It will initialize the metric if metric is not found
+      #
+      # block - if provided, it can be used to initialize metric with custom options (docstring, labels, with_feature, multiprocess_mode)
+      # - multiprocess_mode is :all by default
+      #
+      # Example:
+      # ```
+      # transaction.set(:mestric_name, 1) do
+      #   multiprocess_mode :livesum
+      # end
+      # ```
+      def set(name, value, labels = {}, &block)
+        gauge = self.class.prometheus_metric(name, :gauge, &block)
 
-      define_histogram :gitlab_transaction_cputime_seconds do
-        docstring 'Transaction thread cputime'
-        base_labels BASE_LABELS
-        buckets [0.1, 0.25, 0.5, 1.0, 2.5, 5.0]
+        gauge.set(filter_labels(labels), value)
       end
 
-      define_histogram :gitlab_transaction_duration_seconds do
-        docstring 'Transaction duration'
-        base_labels BASE_LABELS
-        buckets [0.1, 0.25, 0.5, 1.0, 2.5, 5.0]
-      end
+      # Observe histogram metric
+      #
+      # It will initialize the metric if metric is not found
+      #
+      # block - if provided, it can be used to initialize metric with custom options (docstring, labels, with_feature, buckets)
+      #
+      # Example:
+      # ```
+      # transaction.observe(:mestric_name, 1) do
+      #   buckets [100, 1000, 10000, 100000, 1000000, 10000000]
+      # end
+      # ```
+      def observe(name, value, labels = {}, &block)
+        histogram = self.class.prometheus_metric(name, :histogram, &block)
 
-      define_histogram :gitlab_transaction_allocated_memory_bytes do
-        docstring 'Transaction allocated memory bytes'
-        base_labels BASE_LABELS
-        buckets [100, 1000, 10000, 100000, 1000000, 10000000]
+        histogram.observe(filter_labels(labels), value)
       end
 
-      def self.transaction_metric(name, type, prefix: nil, tags: {})
-        metric_name = "gitlab_transaction_#{prefix}#{name}_total".to_sym
-        fetch_metric(type, metric_name) do
-          docstring "Transaction #{prefix}#{name} #{type}"
-          base_labels tags.merge(BASE_LABELS)
-
-          if type == :gauge
-            multiprocess_mode :livesum
-          end
-        end
+      def labels
+        BASE_LABEL_KEYS.product([nil]).to_h
       end
 
-      private
-
-      def filter_tags(tags)
-        tags.without(*FILTERED_LABELS)
+      def filter_labels(labels)
+        labels.empty? ? self.labels : labels.without(*FILTERED_LABEL_KEYS).merge(self.labels)
       end
     end
   end

lib/gitlab/middleware/rails_queue_duration.rb

@@ -19,25 +19,19 @@ module Gitlab
         if trans && proxy_start
           # Time in milliseconds since gitlab-workhorse started the request
           duration = Time.now.to_f * 1_000 - proxy_start.to_f / 1_000_000
-          trans.set(:rails_queue_duration, duration)
+          trans.set(:gitlab_transaction_rails_queue_duration_total, duration) do
+            multiprocess_mode :livesum
+          end
 
           duration_s = Gitlab::Utils.ms_to_round_sec(duration)
-          metric_rails_queue_duration_seconds.observe(trans.labels, duration_s)
+          trans.observe(:gitlab_rails_queue_duration_seconds, duration_s) do
+            docstring 'Measures latency between GitLab Workhorse forwarding a request to Rails'
+          end
           env[GITLAB_RAILS_QUEUE_DURATION_KEY] = duration_s
         end
 
         @app.call(env)
       end
-
-      private
-
-      def metric_rails_queue_duration_seconds
-        @metric_rails_queue_duration_seconds ||= Gitlab::Metrics.histogram(
-          :gitlab_rails_queue_duration_seconds,
-          'Measures latency between GitLab Workhorse forwarding a request to Rails',
-          Gitlab::Metrics::Transaction::BASE_LABELS
-        )
-      end
     end
   end
 end

spec/lib/gitlab/diff/highlight_cache_spec.rb

@@ -214,16 +214,17 @@ RSpec.describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do
   end
 
   describe 'metrics' do
-    it 'defines :gitlab_redis_diff_caching_memory_usage_bytes histogram' do
-      expect(described_class).to respond_to(:gitlab_redis_diff_caching_memory_usage_bytes)
-    end
+    let(:transaction) { Gitlab::Metrics::WebTransaction.new({} ) }
 
-    it 'defines :gitlab_redis_diff_caching_hit' do
-      expect(described_class).to respond_to(:gitlab_redis_diff_caching_hit)
+    before do
+      allow(cache).to receive(:current_transaction).and_return(transaction)
     end
 
-    it 'defines :gitlab_redis_diff_caching_miss' do
-      expect(described_class).to respond_to(:gitlab_redis_diff_caching_miss)
+    it 'observes :gitlab_redis_diff_caching_memory_usage_bytes' do
+      expect(transaction)
+        .to receive(:observe).with(:gitlab_redis_diff_caching_memory_usage_bytes, a_kind_of(Numeric))
+
+      cache.write_if_empty
     end
   end
 end

spec/lib/gitlab/metrics/background_transaction_spec.rb

@@ -4,16 +4,30 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::Metrics::BackgroundTransaction do
   let(:test_worker_class) { double(:class, name: 'TestWorker') }
+  let(:prometheus_metric) { instance_double(Prometheus::Client::Metric, base_labels: {}) }
+
+  before do
+    allow(described_class).to receive(:prometheus_metric).and_return(prometheus_metric)
+  end
 
   subject { described_class.new(test_worker_class) }
 
+  RSpec.shared_examples 'metric with worker labels' do |metric_method|
+    it 'measures with correct labels and value' do
+      value = 1
+      expect(prometheus_metric).to receive(metric_method).with({ controller: 'TestWorker', action: 'perform', feature_category: '' }, value)
+
+      subject.send(metric_method, :bau, value)
+    end
+  end
+
   describe '#label' do
     it 'returns labels based on class name' do
       expect(subject.labels).to eq(controller: 'TestWorker', action: 'perform', feature_category: '')
     end
 
     it 'contains only the labels defined for metrics' do
-      expect(subject.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABELS.keys)
+      expect(subject.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABEL_KEYS)
     end
 
     it 'includes the feature category if there is one' do
@@ -21,4 +35,22 @@ RSpec.describe Gitlab::Metrics::BackgroundTransaction do
       expect(subject.labels).to include(feature_category: 'source_code_management')
     end
   end
+
+  describe '#increment' do
+    let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, :increment, base_labels: {}) }
+
+    it_behaves_like 'metric with worker labels', :increment
+  end
+
+  describe '#set' do
+    let(:prometheus_metric) { instance_double(Prometheus::Client::Gauge, :set, base_labels: {}) }
+
+    it_behaves_like 'metric with worker labels', :set
+  end
+
+  describe '#observe' do
+    let(:prometheus_metric) { instance_double(Prometheus::Client::Histogram, :observe, base_labels: {}) }
+
+    it_behaves_like 'metric with worker labels', :observe
+  end
 end

spec/lib/gitlab/metrics/elasticsearch_rack_middleware_spec.rb

@@ -9,8 +9,6 @@ RSpec.describe Gitlab::Metrics::ElasticsearchRackMiddleware do
   let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) }
 
   describe '#call' do
-    let(:counter) { instance_double(Prometheus::Client::Counter, increment: nil) }
-    let(:histogram) { instance_double(Prometheus::Client::Histogram, observe: nil) }
     let(:elasticsearch_query_time) { 0.1 }
     let(:elasticsearch_requests_count) { 2 }
 
@@ -18,19 +16,6 @@ RSpec.describe Gitlab::Metrics::ElasticsearchRackMiddleware do
       allow(Gitlab::Instrumentation::ElasticsearchTransport).to receive(:query_time) { elasticsearch_query_time }
       allow(Gitlab::Instrumentation::ElasticsearchTransport).to receive(:get_request_count) { elasticsearch_requests_count }
 
-      allow(Gitlab::Metrics).to receive(:counter)
-        .with(:http_elasticsearch_requests_total,
-              an_instance_of(String),
-              Gitlab::Metrics::Transaction::BASE_LABELS)
-        .and_return(counter)
-
-      allow(Gitlab::Metrics).to receive(:histogram)
-        .with(:http_elasticsearch_requests_duration_seconds,
-              an_instance_of(String),
-              Gitlab::Metrics::Transaction::BASE_LABELS,
-              described_class::HISTOGRAM_BUCKETS)
-        .and_return(histogram)
-
       allow(Gitlab::Metrics).to receive(:current_transaction).and_return(transaction)
     end
 
@@ -39,15 +24,15 @@ RSpec.describe Gitlab::Metrics::ElasticsearchRackMiddleware do
     end
 
     it 'records elasticsearch metrics' do
-      expect(counter).to receive(:increment).with(transaction.labels, elasticsearch_requests_count)
-      expect(histogram).to receive(:observe).with(transaction.labels, elasticsearch_query_time)
+      expect(transaction).to receive(:increment).with(:http_elasticsearch_requests_total, elasticsearch_requests_count)
+      expect(transaction).to receive(:observe).with(:http_elasticsearch_requests_duration_seconds, elasticsearch_query_time)
 
       middleware.call(env)
     end
 
     it 'records elasticsearch metrics if an error is raised' do
-      expect(counter).to receive(:increment).with(transaction.labels, elasticsearch_requests_count)
-      expect(histogram).to receive(:observe).with(transaction.labels, elasticsearch_query_time)
+      expect(transaction).to receive(:increment).with(:http_elasticsearch_requests_total, elasticsearch_requests_count)
+      expect(transaction).to receive(:observe).with(:http_elasticsearch_requests_duration_seconds, elasticsearch_query_time)
 
       allow(app).to receive(:call).with(env).and_raise(StandardError)
 

spec/lib/gitlab/metrics/method_call_spec.rb

@@ -3,12 +3,12 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Metrics::MethodCall do
-  let(:transaction) { double(:transaction, labels: {}) }
+  let(:transaction) { Gitlab::Metrics::WebTransaction.new({}) }
   let(:method_call) { described_class.new('Foo#bar', :Foo, '#bar', transaction) }
 
   describe '#measure' do
     after do
-      described_class.reload_metric!(:gitlab_method_call_duration_seconds)
+      ::Gitlab::Metrics::Transaction.reload_metric!(:gitlab_method_call_duration_seconds)
     end
 
     it 'measures the performance of the supplied block' do
@@ -36,13 +36,13 @@ RSpec.describe Gitlab::Metrics::MethodCall do
         end
 
         it 'metric is not a NullMetric' do
-          expect(described_class).not_to be_instance_of(Gitlab::Metrics::NullMetric)
+          method_call.measure { 'foo' }
+          expect(::Gitlab::Metrics::Transaction.prometheus_metric(:gitlab_method_call_duration_seconds, :histogram)).not_to be_instance_of(Gitlab::Metrics::NullMetric)
         end
 
         it 'observes the performance of the supplied block' do
-          expect(described_class.gitlab_method_call_duration_seconds)
-            .to receive(:observe)
-                  .with({ module: :Foo, method: '#bar' }, be_a_kind_of(Numeric))
+          expect(transaction)
+            .to receive(:observe).with(:gitlab_method_call_duration_seconds, be_a_kind_of(Numeric), { method: "#bar", module: :Foo })
 
           method_call.measure { 'foo' }
         end
@@ -53,11 +53,17 @@ RSpec.describe Gitlab::Metrics::MethodCall do
           stub_feature_flags(prometheus_metrics_method_instrumentation: false)
         end
 
-        it 'observes using NullMetric' do
-          expect(described_class.gitlab_method_call_duration_seconds).to be_instance_of(Gitlab::Metrics::NullMetric)
-          expect(described_class.gitlab_method_call_duration_seconds).to receive(:observe)
+        it 'observes the performance of the supplied block' do
+          expect(transaction)
+            .to receive(:observe).with(:gitlab_method_call_duration_seconds, be_a_kind_of(Numeric), { method: "#bar", module: :Foo })
+
+          method_call.measure { 'foo' }
+        end
 
+        it 'observes using NullMetric' do
           method_call.measure { 'foo' }
+
+          expect(::Gitlab::Metrics::Transaction.prometheus_metric(:gitlab_method_call_duration_seconds, :histogram)).to be_instance_of(Gitlab::Metrics::NullMetric)
         end
       end
     end
@@ -68,8 +74,9 @@ RSpec.describe Gitlab::Metrics::MethodCall do
       end
 
       it 'does not observe the performance' do
-        expect(described_class.gitlab_method_call_duration_seconds)
+        expect(transaction)
           .not_to receive(:observe)
+                .with(:gitlab_method_call_duration_seconds, be_a_kind_of(Numeric))
 
         method_call.measure { 'foo' }
       end

spec/lib/gitlab/metrics/methods_spec.rb

@@ -9,9 +9,9 @@ RSpec.describe Gitlab::Metrics::Methods do
     let(:docstring) { 'description' }
     let(:metric_name) { :sample_metric }
 
-    describe "#define_#{metric_type}" do
+    describe "#define_metrics" do
       define_method(:call_define_metric_method) do |**args|
-        subject.__send__("define_#{metric_type}", metric_name, **args)
+        subject.__send__(:define_metric, metric_type, metric_name, **args)
       end
 
       context 'metrics access method not defined' do
@@ -55,11 +55,11 @@ RSpec.describe Gitlab::Metrics::Methods do
       end
     end
 
-    describe "#fetch_#{metric_type}" do
+    describe "#fetch_metric" do
       let(:null_metric) { Gitlab::Metrics::NullMetric.instance }
 
       define_method(:call_fetch_metric_method) do |**args|
-        subject.__send__("fetch_#{metric_type}", metric_name, **args)
+        subject.__send__(:fetch_metric, metric_type, metric_name, **args)
       end
 
       context "when #{metric_type} is not cached" do
@@ -135,5 +135,5 @@ RSpec.describe Gitlab::Metrics::Methods do
 
   include_examples 'metric', :counter, {}
   include_examples 'metric', :gauge, {}, :all
-  include_examples 'metric', :histogram, {}, [0.005, 0.01, 0.1, 1, 10]
+  include_examples 'metric', :histogram, {}, ::Prometheus::Client::Histogram::DEFAULT_BUCKETS
 end

spec/lib/gitlab/metrics/rack_middleware_spec.rb

@@ -25,12 +25,4 @@ RSpec.describe Gitlab::Metrics::RackMiddleware do
       expect { middleware.call(env) }.to raise_error(RuntimeError)
     end
   end
-
-  describe '#transaction_from_env' do
-    let(:transaction) { middleware.transaction_from_env(env) }
-
-    it 'returns a Transaction' do
-      expect(transaction).to be_an_instance_of(Gitlab::Metrics::WebTransaction)
-    end
-  end
 end

spec/lib/gitlab/metrics/redis_rack_middleware_spec.rb

@@ -13,28 +13,12 @@ RSpec.describe Gitlab::Metrics::RedisRackMiddleware do
   end
 
   describe '#call' do
-    let(:counter) { double(Prometheus::Client::Counter, increment: nil) }
-    let(:histogram) { double(Prometheus::Client::Histogram, observe: nil) }
     let(:redis_query_time) { 0.1 }
     let(:redis_requests_count) { 2 }
 
     before do
       allow(Gitlab::Instrumentation::Redis).to receive(:query_time) { redis_query_time }
       allow(Gitlab::Instrumentation::Redis).to receive(:get_request_count) { redis_requests_count }
-
-      allow(Gitlab::Metrics).to receive(:counter)
-        .with(:http_redis_requests_total,
-              an_instance_of(String),
-              Gitlab::Metrics::Transaction::BASE_LABELS)
-        .and_return(counter)
-
-      allow(Gitlab::Metrics).to receive(:histogram)
-        .with(:http_redis_requests_duration_seconds,
-              an_instance_of(String),
-              Gitlab::Metrics::Transaction::BASE_LABELS,
-              Gitlab::Instrumentation::Redis::QUERY_TIME_BUCKETS)
-        .and_return(histogram)
-
       allow(Gitlab::Metrics).to receive(:current_transaction).and_return(transaction)
     end
 
@@ -43,15 +27,15 @@ RSpec.describe Gitlab::Metrics::RedisRackMiddleware do
     end
 
     it 'records redis metrics' do
-      expect(counter).to receive(:increment).with(transaction.labels, redis_requests_count)
-      expect(histogram).to receive(:observe).with(transaction.labels, redis_query_time)
+      expect(transaction).to receive(:increment).with(:http_redis_requests_total, redis_requests_count)
+      expect(transaction).to receive(:observe).with(:http_redis_requests_duration_seconds, redis_query_time)
 
       middleware.call(env)
     end
 
     it 'records redis metrics if an error is raised' do
-      expect(counter).to receive(:increment).with(transaction.labels, redis_requests_count)
-      expect(histogram).to receive(:observe).with(transaction.labels, redis_query_time)
+      expect(transaction).to receive(:increment).with(:http_redis_requests_total, redis_requests_count)
+      expect(transaction).to receive(:observe).with(:http_redis_requests_duration_seconds, redis_query_time)
 
       allow(app).to receive(:call).with(env).and_raise(StandardError)
 

spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb

@@ -11,8 +11,8 @@ RSpec.describe Gitlab::Metrics::SidekiqMiddleware do
       worker = double(:worker, class: double(:class, name: 'TestWorker'))
 
       expect_next_instance_of(Gitlab::Metrics::BackgroundTransaction) do |transaction|
-        expect(transaction).to receive(:set).with(:sidekiq_queue_duration, instance_of(Float))
-        expect(transaction).to receive(:increment).with(:db_count, 1)
+        expect(transaction).to receive(:set).with(:gitlab_transaction_sidekiq_queue_duration_total, instance_of(Float))
+        expect(transaction).to receive(:increment).with(:gitlab_transaction_db_count_total, 1)
       end
 
       middleware.call(worker, message, :test) do
@@ -42,7 +42,7 @@ RSpec.describe Gitlab::Metrics::SidekiqMiddleware do
         .and_call_original
 
       expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:set)
-        .with(:sidekiq_queue_duration, instance_of(Float))
+        .with(:gitlab_transaction_sidekiq_queue_duration_total, instance_of(Float))
 
       middleware.call(worker, {}, :test) { nil }
     end

spec/lib/gitlab/metrics/subscribers/action_view_spec.rb

@@ -22,15 +22,15 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActionView do
   describe '#render_template' do
     it 'tracks rendering of a template' do
       expect(transaction).to receive(:increment)
-        .with(:view_duration, 2.1)
+        .with(:gitlab_transaction_view_duration_total, 2.1)
 
       subscriber.render_template(event)
     end
 
     it 'observes view rendering time' do
-      expect(described_class.gitlab_view_rendering_duration_seconds)
+      expect(transaction)
         .to receive(:observe)
-              .with({ view: 'app/views/x.html.haml' }, 2.1)
+        .with(:gitlab_view_rendering_duration_seconds, 2.1, { view: "app/views/x.html.haml" })
 
       subscriber.render_template(event)
     end

spec/lib/gitlab/metrics/subscribers/active_record_spec.rb

@@ -37,10 +37,11 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do
 
         it 'increments only db count value' do
           described_class::DB_COUNTERS.each do |counter|
+            prometheus_counter = "gitlab_transaction_#{counter}_total".to_sym
             if expected_counters[counter] > 0
-              expect(transaction).to receive(:increment).with(counter, 1)
+              expect(transaction).to receive(:increment).with(prometheus_counter, 1)
             else
-              expect(transaction).not_to receive(:increment).with(counter, 1)
+              expect(transaction).not_to receive(:increment).with(prometheus_counter, 1)
             end
           end
 
@@ -74,7 +75,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do
         expect(subscriber).to receive(:current_transaction)
                                 .at_least(:once)
                                 .and_return(transaction)
-        expect(described_class.send(:gitlab_sql_duration_seconds)).to receive(:observe).with({}, 0.002)
+        expect(transaction).to receive(:observe).with(:gitlab_sql_duration_seconds, 0.002)
+
         subscriber.sql(event)
       end
 

spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb

@@ -26,21 +26,12 @@ RSpec.describe Gitlab::Metrics::Subscribers::RailsCache do
       context 'with hit event' do
         let(:event) { double(:event, duration: 15.2, payload: { hit: true }) }
 
-        it 'increments the cache_read_hit count' do
-          expect(transaction).to receive(:increment)
-                                   .with(:cache_read_hit_count, 1, false)
-          expect(transaction).to receive(:increment)
-                                   .with(any_args).at_least(1) # Other calls
-
-          subscriber.cache_read(event)
-        end
-
         context 'when super operation is fetch' do
           let(:event) { double(:event, duration: 15.2, payload: { hit: true, super_operation: :fetch }) }
 
-          it 'does not increment cache read miss' do
+          it 'does not increment cache read miss total' do
             expect(transaction).not_to receive(:increment)
-                                         .with(:cache_read_hit_count, 1)
+                                         .with(:gitlab_cache_misses_total, 1)
 
             subscriber.cache_read(event)
           end
@@ -50,33 +41,21 @@ RSpec.describe Gitlab::Metrics::Subscribers::RailsCache do
       context 'with miss event' do
         let(:event) { double(:event, duration: 15.2, payload: { hit: false }) }
 
-        it 'increments the cache_read_miss count' do
+        it 'increments the cache_read_miss total' do
           expect(transaction).to receive(:increment)
-                                   .with(:cache_read_miss_count, 1, false)
+                                   .with(:gitlab_cache_misses_total, 1)
           expect(transaction).to receive(:increment)
                                    .with(any_args).at_least(1) # Other calls
 
           subscriber.cache_read(event)
         end
 
-        it 'increments the cache_read_miss total' do
-          expect(subscriber.send(:metric_cache_misses_total)).to receive(:increment).with({})
-
-          subscriber.cache_read(event)
-        end
-
         context 'when super operation is fetch' do
           let(:event) { double(:event, duration: 15.2, payload: { hit: false, super_operation: :fetch }) }
 
-          it 'does not increment cache read miss' do
+          it 'does not increment cache read miss total' do
             expect(transaction).not_to receive(:increment)
-                                         .with(:cache_read_miss_count, 1)
-
-            subscriber.cache_read(event)
-          end
-
-          it 'does not increment cache_read_miss total' do
-            expect(subscriber.send(:metric_cache_misses_total)).not_to receive(:increment).with({})
+                                         .with(:gitlab_cache_misses_total, 1)
 
             subscriber.cache_read(event)
           end
@@ -129,7 +108,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::RailsCache do
 
       it 'increments the cache_read_hit count' do
         expect(transaction).to receive(:increment)
-                                 .with(:cache_read_hit_count, 1)
+                                 .with(:gitlab_transaction_cache_read_hit_count_total, 1)
 
         subscriber.cache_fetch_hit(event)
       end
@@ -146,26 +125,18 @@ RSpec.describe Gitlab::Metrics::Subscribers::RailsCache do
     end
 
     context 'with a transaction' do
-      let(:metric_cache_misses_total) { double('metric_cache_misses_total', increment: nil) }
-
       before do
-        allow(subscriber).to receive(:metric_cache_misses_total).and_return(metric_cache_misses_total)
         allow(subscriber).to receive(:current_transaction)
                                .and_return(transaction)
       end
 
-      it 'increments the cache_fetch_miss count' do
+      it 'increments the cache_fetch_miss count and cache_read_miss total' do
+        expect(transaction).to receive(:increment).with(:gitlab_cache_misses_total, 1)
         expect(transaction).to receive(:increment)
-                                 .with(:cache_read_miss_count, 1)
+                                 .with(:gitlab_transaction_cache_read_miss_count_total, 1)
 
         subscriber.cache_generate(event)
       end
-
-      it 'increments the cache_read_miss total' do
-        subscriber.cache_generate(event)
-
-        expect(metric_cache_misses_total).to have_received(:increment).with({})
-      end
     end
   end
 
@@ -184,22 +155,6 @@ RSpec.describe Gitlab::Metrics::Subscribers::RailsCache do
                                .and_return(transaction)
       end
 
-      it 'increments the total and specific cache duration' do
-        expect(transaction).to receive(:increment)
-                                 .with(:cache_duration, event.duration, false)
-
-        expect(transaction).to receive(:increment)
-                                 .with(:cache_count, 1, false)
-
-        expect(transaction).to receive(:increment)
-                                 .with(:cache_delete_duration, event.duration, false)
-
-        expect(transaction).to receive(:increment)
-                                 .with(:cache_delete_count, 1, false)
-
-        subscriber.observe(:delete, event.duration)
-      end
-
       it 'observes cache metric' do
         expect(subscriber.send(:metric_cache_operation_duration_seconds))
           .to receive(:observe)
@@ -209,9 +164,9 @@ RSpec.describe Gitlab::Metrics::Subscribers::RailsCache do
       end
 
       it 'increments the operations total' do
-        expect(subscriber.send(:metric_cache_operations_total))
+        expect(transaction)
           .to receive(:increment)
-          .with(transaction.labels.merge(operation: :delete))
+          .with(:gitlab_cache_operations_total, 1, { operation: :delete })
 
         subscriber.observe(:delete, event.duration)
       end

spec/lib/gitlab/metrics/transaction_spec.rb

@@ -63,7 +63,7 @@ RSpec.describe Gitlab::Metrics::Transaction do
   end
 
   describe '#add_event' do
-    let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, increment: nil) }
+    let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, increment: nil, base_labels: {}) }
 
     it 'adds a metric' do
       expect(prometheus_metric).to receive(:increment)
@@ -82,7 +82,7 @@ RSpec.describe Gitlab::Metrics::Transaction do
     context 'with sensitive tags' do
       before do
         transaction.add_event(:baubau, **sensitive_tags.merge(sane: 'yes'))
-        allow(described_class).to receive(:transaction_metric).and_return(prometheus_metric)
+        allow(described_class).to receive(:prometheus_metric).and_return(prometheus_metric)
       end
 
       it 'filters tags' do
@@ -94,24 +94,119 @@ RSpec.describe Gitlab::Metrics::Transaction do
   end
 
   describe '#increment' do
-    let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, increment: nil) }
+    let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, increment: nil, base_labels: {}) }
 
     it 'adds a metric' do
-      expect(prometheus_metric).to receive(:increment).with(hash_including(:action, :controller), 1)
-      expect(described_class).to receive(:fetch_metric).with(:counter, :gitlab_transaction_meow_total).and_return(prometheus_metric)
+      expect(prometheus_metric).to receive(:increment)
+      expect(::Gitlab::Metrics).to receive(:counter).with(:meow, 'Meow counter', hash_including(:controller, :action)).and_return(prometheus_metric)
 
       transaction.increment(:meow, 1)
     end
+
+    context 'with block' do
+      it 'overrides docstring' do
+        expect(::Gitlab::Metrics).to receive(:counter).with(:block_docstring, 'test', hash_including(:controller, :action)).and_return(prometheus_metric)
+
+        transaction.increment(:block_docstring, 1) do
+          docstring 'test'
+        end
+      end
+
+      it 'overrides labels' do
+        expect(::Gitlab::Metrics).to receive(:counter).with(:block_labels, 'Block labels counter', hash_including(:controller, :action, :sane)).and_return(prometheus_metric)
+
+        labels = { sane: 'yes' }
+        transaction.increment(:block_labels, 1, labels) do
+          label_keys %i(sane)
+        end
+      end
+
+      it 'filters sensitive tags' do
+        expect(::Gitlab::Metrics).to receive(:counter).with(:metric_with_sensitive_block, 'Metric with sensitive block counter', hash_excluding(sensitive_tags)).and_return(prometheus_metric)
+
+        labels_keys = sensitive_tags.keys
+        transaction.increment(:metric_with_sensitive_block, 1, sensitive_tags) do
+          label_keys labels_keys
+        end
+      end
+    end
   end
 
   describe '#set' do
-    let(:prometheus_metric) { instance_double(Prometheus::Client::Gauge, set: nil) }
+    let(:prometheus_metric) { instance_double(Prometheus::Client::Gauge, set: nil, base_labels: {}) }
+
+    it 'adds a metric' do
+      expect(prometheus_metric).to receive(:set)
+      expect(::Gitlab::Metrics).to receive(:gauge).with(:meow_set, 'Meow set gauge', hash_including(:controller, :action), :all).and_return(prometheus_metric)
+
+      transaction.set(:meow_set, 1)
+    end
+
+    context 'with block' do
+      it 'overrides docstring' do
+        expect(::Gitlab::Metrics).to receive(:gauge).with(:block_docstring_set, 'test', hash_including(:controller, :action), :all).and_return(prometheus_metric)
+
+        transaction.set(:block_docstring_set, 1) do
+          docstring 'test'
+        end
+      end
+
+      it 'overrides labels' do
+        expect(::Gitlab::Metrics).to receive(:gauge).with(:block_labels_set, 'Block labels set gauge', hash_including(:controller, :action, :sane), :all).and_return(prometheus_metric)
+
+        labels = { sane: 'yes' }
+        transaction.set(:block_labels_set, 1, labels) do
+          label_keys %i(sane)
+        end
+      end
+
+      it 'filters sensitive tags' do
+        expect(::Gitlab::Metrics).to receive(:gauge).with(:metric_set_with_sensitive_block, 'Metric set with sensitive block gauge', hash_excluding(sensitive_tags), :all).and_return(prometheus_metric)
+
+        label_keys = sensitive_tags.keys
+        transaction.set(:metric_set_with_sensitive_block, 1, sensitive_tags) do
+          label_keys label_keys
+        end
+      end
+    end
+  end
+
+  describe '#observe' do
+    let(:prometheus_metric) { instance_double(Prometheus::Client::Histogram, observe: nil, base_labels: {}) }
 
     it 'adds a metric' do
-      expect(prometheus_metric).to receive(:set).with(hash_including(:action, :controller), 1)
-      expect(described_class).to receive(:fetch_metric).with(:gauge, :gitlab_transaction_meow_total).and_return(prometheus_metric)
+      expect(prometheus_metric).to receive(:observe)
+      expect(::Gitlab::Metrics).to receive(:histogram).with(:meow_observe, 'Meow observe histogram', hash_including(:controller, :action), kind_of(Array)).and_return(prometheus_metric)
+
+      transaction.observe(:meow_observe, 1)
+    end
 
-      transaction.set(:meow, 1)
+    context 'with block' do
+      it 'overrides docstring' do
+        expect(::Gitlab::Metrics).to receive(:histogram).with(:block_docstring_observe, 'test', hash_including(:controller, :action), kind_of(Array)).and_return(prometheus_metric)
+
+        transaction.observe(:block_docstring_observe, 1) do
+          docstring 'test'
+        end
+      end
+
+      it 'overrides labels' do
+        expect(::Gitlab::Metrics).to receive(:histogram).with(:block_labels_observe, 'Block labels observe histogram', hash_including(:controller, :action, :sane), kind_of(Array)).and_return(prometheus_metric)
+
+        labels = { sane: 'yes' }
+        transaction.observe(:block_labels_observe, 1, labels) do
+          label_keys %i(sane)
+        end
+      end
+
+      it 'filters sensitive tags' do
+        expect(::Gitlab::Metrics).to receive(:histogram).with(:metric_observe_with_sensitive_block, 'Metric observe with sensitive block histogram', hash_excluding(sensitive_tags), kind_of(Array)).and_return(prometheus_metric)
+
+        label_keys = sensitive_tags.keys
+        transaction.observe(:metric_observe_with_sensitive_block, 1, sensitive_tags) do
+          label_keys label_keys
+        end
+      end
     end
   end
 end

spec/lib/gitlab/metrics/web_transaction_spec.rb

@@ -5,13 +5,42 @@ require 'spec_helper'
 RSpec.describe Gitlab::Metrics::WebTransaction do
   let(:env) { {} }
   let(:transaction) { described_class.new(env) }
-  let(:prometheus_metric) { double("prometheus metric") }
+  let(:prometheus_metric) { instance_double(Prometheus::Client::Metric, base_labels: {}) }
 
   before do
-    allow(described_class).to receive(:transaction_metric).and_return(prometheus_metric)
+    allow(described_class).to receive(:prometheus_metric).and_return(prometheus_metric)
+  end
+
+  RSpec.shared_context 'ActionController request' do
+    let(:request) { double(:request, format: double(:format, ref: :html)) }
+    let(:controller_class) { double(:controller_class, name: 'TestController') }
+
+    before do
+      controller = double(:controller, class: controller_class, action_name: 'show', request: request)
+      env['action_controller.instance'] = controller
+    end
+  end
+
+  RSpec.shared_context 'transaction observe metrics' do
+    before do
+      allow(transaction).to receive(:observe)
+    end
+  end
+
+  RSpec.shared_examples 'metric with labels' do |metric_method|
+    include_context 'ActionController request'
+
+    it 'measures with correct labels and value' do
+      value = 1
+      expect(prometheus_metric).to receive(metric_method).with({ controller: 'TestController', action: 'show', feature_category: '' }, value)
+
+      transaction.send(metric_method, :bau, value)
+    end
   end
 
   describe '#duration' do
+    include_context 'transaction observe metrics'
+
     it 'returns the duration of a transaction in seconds' do
       transaction.run { sleep(0.5) }
 
@@ -20,6 +49,8 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
   end
 
   describe '#allocated_memory' do
+    include_context 'transaction observe metrics'
+
     it 'returns the allocated memory in bytes' do
       transaction.run { 'a' * 32 }
 
@@ -28,6 +59,8 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
   end
 
   describe '#run' do
+    include_context 'transaction observe metrics'
+
     it 'yields the supplied block' do
       expect { |b| transaction.run(&b) }.to yield_control
     end
@@ -53,26 +86,7 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
     end
   end
 
-  describe '#increment' do
-    it 'increments a counter' do
-      expect(prometheus_metric).to receive(:increment).with({}, 1)
-
-      transaction.increment(:time, 1)
-    end
-  end
-
-  describe '#set' do
-    it 'sets a value' do
-      expect(prometheus_metric).to receive(:set).with({}, 10)
-
-      transaction.set(:number, 10)
-    end
-  end
-
   describe '#labels' do
-    let(:request) { double(:request, format: double(:format, ref: :html)) }
-    let(:controller_class) { double(:controller_class, name: 'TestController') }
-
     context 'when request goes to Grape endpoint' do
       before do
         route = double(:route, request_method: 'GET', path: '/:version/projects/:id/archive(.:format)')
@@ -86,7 +100,7 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
       end
 
       it 'contains only the labels defined for transactions' do
-        expect(transaction.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABELS.keys)
+        expect(transaction.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABEL_KEYS)
       end
 
       it 'does not provide labels if route infos are missing' do
@@ -100,18 +114,14 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
     end
 
     context 'when request goes to ActionController' do
-      before do
-        controller = double(:controller, class: controller_class, action_name: 'show', request: request)
-
-        env['action_controller.instance'] = controller
-      end
+      include_context 'ActionController request'
 
       it 'tags a transaction with the name and action of a controller' do
         expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: '' })
       end
 
       it 'contains only the labels defined for transactions' do
-        expect(transaction.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABELS.keys)
+        expect(transaction.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABEL_KEYS)
       end
 
       context 'when the request content type is not :html' do
@@ -144,6 +154,8 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
   end
 
   describe '#add_event' do
+    let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, :increment, base_labels: {}) }
+
     it 'adds a metric' do
       expect(prometheus_metric).to receive(:increment)
 
@@ -156,4 +168,22 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
       transaction.add_event(:bau, animal: 'dog')
     end
   end
+
+  describe '#increment' do
+    let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, :increment, base_labels: {}) }
+
+    it_behaves_like 'metric with labels', :increment
+  end
+
+  describe '#set' do
+    let(:prometheus_metric) { instance_double(Prometheus::Client::Gauge, :set, base_labels: {}) }
+
+    it_behaves_like 'metric with labels', :set
+  end
+
+  describe '#observe' do
+    let(:prometheus_metric) { instance_double(Prometheus::Client::Histogram, :observe, base_labels: {}) }
+
+    it_behaves_like 'metric with labels', :observe
+  end
 end

spec/lib/gitlab/metrics_spec.rb

@@ -71,14 +71,9 @@ RSpec.describe Gitlab::Metrics do
       end
 
       it 'adds a metric to the current transaction' do
-        expect(transaction).to receive(:increment)
-                                 .with('foo_real_time', a_kind_of(Numeric), false)
+        expect(transaction).to receive(:observe).with(:gitlab_foo_real_duration_seconds, a_kind_of(Numeric))
 
-        expect(transaction).to receive(:increment)
-                                 .with('foo_cpu_time', a_kind_of(Numeric), false)
-
-        expect(transaction).to receive(:increment)
-                                 .with('foo_call_count', 1, false)
+        expect(transaction).to receive(:observe).with(:gitlab_foo_cpu_duration_seconds, a_kind_of(Numeric))
 
         described_class.measure(:foo) { 10 }
       end

spec/lib/gitlab/middleware/rails_queue_duration_spec.rb

@@ -29,26 +29,19 @@ RSpec.describe Gitlab::Middleware::RailsQueueDuration do
 
       it 'sets proxy_flight_time and calls the app when the header is present' do
         env['HTTP_GITLAB_WORKHORSE_PROXY_START'] = '123'
-        expect(transaction).to receive(:set).with(:rails_queue_duration, an_instance_of(Float))
+        expect(transaction).to receive(:set).with(:gitlab_transaction_rails_queue_duration_total, an_instance_of(Float))
         expect(middleware.call(env)).to eq('yay')
       end
 
       it 'observes rails queue duration metrics and calls the app when the header is present' do
         env['HTTP_GITLAB_WORKHORSE_PROXY_START'] = '2000000000'
 
-        expect(middleware.send(:metric_rails_queue_duration_seconds)).to receive(:observe).with(transaction.labels, 1)
+        expect(transaction).to receive(:observe).with(:gitlab_rails_queue_duration_seconds, 1)
 
         Timecop.freeze(Time.at(3)) do
           expect(middleware.call(env)).to eq('yay')
         end
       end
-
-      it 'creates a metric with a docstring' do
-        metric = middleware.send(:metric_rails_queue_duration_seconds)
-
-        expect(metric).to be_instance_of(Prometheus::Client::Histogram)
-        expect(metric.docstring).to eq('Measures latency between GitLab Workhorse forwarding a request to Rails')
-      end
     end
   end
 end