Fix transaction metrics

- Revert label keys
- Default multiprocess_mode to :all for transaction
- Make sure that existing gauge metrics still use :livesum
- Fix git blob metric scope
- Rename :gitlab_transaction_rails_queue_duration_total
- Rename :sidekiq_queue_duration metric

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

@@ -27,6 +27,24 @@ module Gitlab
       attr_accessor :size, :mode, :id, :commit_id, :loaded_size, :binary
       attr_writer :name, :path, :data
 
+      def self.gitlab_blob_truncated_true
+        @gitlab_blob_truncated_true ||= ::Gitlab::Metrics.counter(:gitlab_blob_truncated_true, 'blob.truncated? == true')
+      end
+
+      def self.gitlab_blob_truncated_false
+        @gitlab_blob_truncated_false ||= ::Gitlab::Metrics.counter(:gitlab_blob_truncated_false, 'blob.truncated? == false')
+      end
+
+      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
         def find(repository, sha, path, limit: MAX_DATA_DISPLAY_SIZE)
           tree_entry(repository, sha, path, limit)
@@ -193,31 +211,20 @@ module Gitlab
       def record_metric_blob_size
         return unless size
 
-        current_transaction&.observe(:gitlab_blob_size, size) do
-          docstring 'Gitlab::Git::Blob size'
-          buckets [1_000, 5_000, 10_000, 50_000, 100_000, 500_000, 1_000_000]
-        end
+        self.class.gitlab_blob_size.observe({}, size)
       end
 
       def record_metric_truncated(bool)
         if bool
-          current_transaction&.increment(:gitlab_blob_truncated_true) do
-            docstring 'blob.truncated? == true'
-          end
+          self.class.gitlab_blob_truncated_true.increment
         else
-          current_transaction&.increment(:gitlab_blob_truncated_false) do
-            docstring 'blob.truncated? == false'
-          end
+          self.class.gitlab_blob_truncated_false.increment
         end
       end
 
       def has_lfs_version_key?
         !empty? && text_in_repo? && data.start_with?("version https://git-lfs.github.com/spec")
       end
-
-      def current_transaction
-        ::Gitlab::Metrics::Transaction.current
-      end
     end
   end
 end

lib/gitlab/metrics/method_call.rb

@@ -34,9 +34,10 @@ module Gitlab
         @call_count += 1
 
         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 labels.keys
+            label_keys label_keys
             buckets [0.01, 0.05, 0.1, 0.5, 1]
             with_feature :prometheus_metrics_method_instrumentation
           end

lib/gitlab/metrics/methods/metric_options.rb

@@ -7,7 +7,6 @@ module Gitlab
         def initialize(options = {})
           @multiprocess_mode = options[:multiprocess_mode] || :all
           @buckets = options[:buckets] || ::Prometheus::Client::Histogram::DEFAULT_BUCKETS
-          @base_labels = options[:base_labels] || nil
           @docstring = options[:docstring]
           @with_feature = options[:with_feature]
           @label_keys = options[:label_keys] || []
@@ -62,17 +61,10 @@ module Gitlab
         end
 
         def evaluate(&block)
-          if block_given?
-            @self_before_instance_eval = eval "self", block.binding, __FILE__, __LINE__
-            instance_eval(&block)
-          end
+          instance_eval(&block)  if block_given?
 
           self
         end
-
-        def method_missing(method, *args, &block)
-          @self_before_instance_eval ? @self_before_instance_eval.send(method, *args, &block) : super # rubocop:disable GitlabSecurity/PublicSend
-        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/transaction.rb

@@ -30,7 +30,6 @@ module Gitlab
           fetch_metric(type, name) do
             # set default metric options
             docstring "#{name.to_s.humanize} #{type}"
-            multiprocess_mode :livesum if type == :gauge
 
             evaluate(&block)
             # always filter sensitive labels and merge with base ones
@@ -73,13 +72,13 @@ module Gitlab
         @memory_after = System.memory_usage_rss
         @finished_at = System.monotonic_time
 
-        observe(:cputime_seconds, thread_cpu_duration) do
+        observe(:gitlab_transaction_cputime_seconds, thread_cpu_duration) do
           buckets SMALL_BUCKETS
         end
-        observe(:duration_seconds, duration) do
+        observe(:gitlab_transaction_duration_seconds, duration) do
           buckets SMALL_BUCKETS
         end
-        observe(:allocated_memory_bytes, allocated_memory * 1024.0) do
+        observe(:gitlab_transaction_allocated_memory_bytes, allocated_memory * 1024.0) do
           buckets BIG_BUCKETS
         end
 
@@ -137,11 +136,12 @@ module Gitlab
       # 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 :all
+      #   multiprocess_mode :livesum
       # end
       # ```
       def set(name, value, labels = {}, &block)

lib/gitlab/middleware/rails_queue_duration.rb

@@ -19,7 +19,9 @@ 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(:gitlab_rails_queue_duration_total, duration)
+          trans.set(:gitlab_transaction_rails_queue_duration_total, duration) do
+            multiprocess_mode :livesum
+          end
 
           duration_s = Gitlab::Utils.ms_to_round_sec(duration)
           trans.observe(:gitlab_rails_queue_duration_seconds, duration_s) do

spec/lib/gitlab/git/blob_spec.rb

@@ -10,64 +10,34 @@ RSpec.describe Gitlab::Git::Blob, :seed_helper do
 
   describe 'initialize' do
     let(:blob) { Gitlab::Git::Blob.new(name: 'test') }
-    let(:transaction) { Gitlab::Metrics::WebTransaction.new( {} ) }
-
-    before do
-      allow(::Gitlab::Metrics::Transaction).to receive(:current).and_return(transaction)
-    end
 
     it 'handles nil data' do
-      expect(blob).not_to receive(:record_metric_blob_size)
+      expect(described_class).not_to receive(:gitlab_blob_size)
 
       expect(blob.name).to eq('test')
       expect(blob.size).to eq(nil)
       expect(blob.loaded_size).to eq(nil)
     end
 
-    context 'with size' do
-      let(:blob1) { Gitlab::Git::Blob.new(name: 'test', size: 4, data: 'abcd') }
+    it 'records blob size' do
+      expect(described_class).to receive(:gitlab_blob_size).and_call_original
 
-      it 'observes gitlab_blob_size' do
-        expect(transaction)
-          .to receive(:observe).with(:gitlab_blob_size, a_kind_of(Numeric))
-
-        blob1
-      end
+      Gitlab::Git::Blob.new(name: 'test', size: 4, data: 'abcd')
     end
 
     context 'when untruncated' do
-      let(:blob) { Gitlab::Git::Blob.new(name: 'test', size: 4, data: 'abcd') }
+      it 'attempts to record gitlab_blob_truncated_false' do
+        expect(described_class).to receive(:gitlab_blob_truncated_false).and_call_original
 
-      it 'doesnt increment :gitlab_blob_truncated_true counter' do
-        expect(transaction)
-          .not_to receive(:increment).with(:gitlab_blob_truncated_true)
-
-        blob
-      end
-
-      it 'increments :gitlab_blob_truncated_false counter' do
-        expect(transaction)
-          .to receive(:increment).with(:gitlab_blob_truncated_false)
-
-        blob
+        Gitlab::Git::Blob.new(name: 'test', size: 4, data: 'abcd')
       end
     end
 
     context 'when truncated' do
-      let(:blob) { Gitlab::Git::Blob.new(name: 'test', size: 40, data: 'abcd') }
-
-      it 'increment :gitlab_blob_truncated_true counter' do
-        expect(transaction)
-          .to receive(:increment).with(:gitlab_blob_truncated_true)
+      it 'attempts to record gitlab_blob_truncated_true' do
+        expect(described_class).to receive(:gitlab_blob_truncated_true).and_call_original
 
-        blob
-      end
-
-      it 'doesnt increment :gitlab_blob_truncated_false counter' do
-        expect(transaction)
-          .not_to receive(:increment).with(:gitlab_blob_truncated_false)
-
-        blob
+        Gitlab::Git::Blob.new(name: 'test', size: 40, data: 'abcd')
       end
     end
   end
@@ -669,6 +639,20 @@ RSpec.describe Gitlab::Git::Blob, :seed_helper do
     end
   end
 
+  describe 'metrics' do
+    it 'defines :gitlab_blob_truncated_true counter' do
+      expect(described_class).to respond_to(:gitlab_blob_truncated_true)
+    end
+
+    it 'defines :gitlab_blob_truncated_false counter' do
+      expect(described_class).to respond_to(:gitlab_blob_truncated_false)
+    end
+
+    it 'defines :gitlab_blob_size histogram' do
+      expect(described_class).to respond_to(:gitlab_blob_size)
+    end
+  end
+
   describe '#lines' do
     context 'when the encoding cannot be detected' do
       it 'successfully splits the data' do

spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb

@@ -11,7 +11,7 @@ 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(:set).with(:gitlab_transaction_sidekiq_queue_duration_total, instance_of(Float))
         expect(transaction).to receive(:increment).with(:gitlab_transaction_db_count_total, 1)
       end
 
@@ -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/middleware/rails_queue_duration_spec.rb

@@ -29,7 +29,7 @@ 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(:gitlab_rails_queue_duration_total, 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