Merge branch 'bvl-rename-sli-metrics' into 'master'

Start emitting gitlab_sli metrics separated with '_'

See merge request gitlab-org/gitlab!90977

config/feature_flags/development/gitlab_sli_new_counters.yml

+---
+name: gitlab_sli_new_counters
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/90977
+rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1760
+milestone: '15.2'
+type: development
+group: group::scalability
+default_enabled: false

lib/gitlab/metrics/sli.rb

@@ -48,14 +48,24 @@ def initialize_counters(possible_label_combinations)
         # This module is effectively an abstract class
         @initialized_with_combinations = possible_label_combinations.any? # rubocop:disable Gitlab/ModuleWithInstanceVariables
         possible_label_combinations.each do |label_combination|
-          total_counter.get(label_combination)
-          numerator_counter.get(label_combination)
+          legacy_total_counter.get(label_combination)
+          legacy_numerator_counter.get(label_combination)
+
+          if ::Feature.enabled?(:gitlab_sli_new_counters)
+            total_counter.get(label_combination)
+            numerator_counter.get(label_combination)
+          end
         end
       end
 
       def increment(labels:, increment_numerator:)
-        total_counter.increment(labels)
-        numerator_counter.increment(labels) if increment_numerator
+        legacy_total_counter.increment(labels)
+        legacy_numerator_counter.increment(labels) if increment_numerator
+
+        if ::Feature.enabled?(:gitlab_sli_new_counters)
+          total_counter.increment(labels)
+          numerator_counter.increment(labels) if increment_numerator
+        end
       end
 
       def initialized?
@@ -65,7 +75,11 @@ def initialized?
       private
 
       def total_counter
-        prometheus.counter(counter_name('total'), "Total number of measurements for #{name}")
+        prometheus.counter(counter_name('total', '_'), "Total number of measurements for #{name}")
+      end
+
+      def legacy_total_counter
+        prometheus.counter(counter_name('total', ':'), "Total number of measurements for #{name}")
       end
 
       def prometheus
@@ -81,12 +95,16 @@ def increment(labels:, success:)
 
         private
 
-        def counter_name(suffix)
-          :"#{COUNTER_PREFIX}:#{name}_apdex:#{suffix}"
+        def counter_name(suffix, separator)
+          [COUNTER_PREFIX, "#{name}_apdex", suffix].join(separator).to_sym
         end
 
         def numerator_counter
-          prometheus.counter(counter_name('success_total'), "Number of successful measurements for #{name}")
+          prometheus.counter(counter_name('success_total', '_'), "Number of successful measurements for #{name}")
+        end
+
+        def legacy_numerator_counter
+          prometheus.counter(counter_name('success_total', ':'), "Legacy number of successful measurements for #{name}")
         end
       end
 
@@ -99,12 +117,16 @@ def increment(labels:, error:)
 
         private
 
-        def counter_name(suffix)
-          :"#{COUNTER_PREFIX}:#{name}:#{suffix}"
+        def counter_name(suffix, separator)
+          [COUNTER_PREFIX, name, suffix].join(separator).to_sym
         end
 
         def numerator_counter
-          prometheus.counter(counter_name('error_total'), "Number of error measurements for #{name}")
+          prometheus.counter(counter_name('error_total', '_'), "Number of error measurements for #{name}")
+        end
+
+        def legacy_numerator_counter
+          prometheus.counter(counter_name('error_total', ':'), "Number of error measurements for #{name}")
         end
       end
     end

spec/lib/gitlab/metrics/sli_spec.rb

 # frozen_string_literal: true
 
-require 'fast_spec_helper'
+# Switch this back to `fast_spec_helper` when removing the `gitlab_sli_new_counters`
+# feature flag
+require 'spec_helper'
 
 RSpec.describe Gitlab::Metrics::Sli do
   let(:prometheus) { double("prometheus") }
@@ -18,12 +20,16 @@
     it 'allows different SLIs to be defined on each subclass' do
       apdex_counters = [
         fake_total_counter('foo_apdex'),
-        fake_numerator_counter('foo_apdex', 'success')
+        fake_numerator_counter('foo_apdex', 'success'),
+        fake_total_counter('foo_apdex', ':'),
+        fake_numerator_counter('foo_apdex', 'success', ':')
       ]
 
       error_rate_counters = [
         fake_total_counter('foo'),
-        fake_numerator_counter('foo', 'error')
+        fake_numerator_counter('foo', 'error'),
+        fake_total_counter('foo', ':'),
+        fake_numerator_counter('foo', 'error', ':')
       ]
 
       apdex = described_class::Apdex.initialize_sli(:foo, [{ hello: :world }])
@@ -78,7 +84,9 @@
           it 'returns and stores a new initialized SLI' do
             counters = [
               fake_total_counter("bar#{subclass_info[:suffix]}"),
-              fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator])
+              fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator]),
+              fake_total_counter("bar#{subclass_info[:suffix]}", ':'),
+              fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator], ':')
             ]
 
             sli = described_class.initialize_sli(:bar, [{ hello: :world }])
@@ -91,7 +99,9 @@
           it 'does not change labels for an already-initialized SLI' do
             counters = [
               fake_total_counter("bar#{subclass_info[:suffix]}"),
-              fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator])
+              fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator]),
+              fake_total_counter("bar#{subclass_info[:suffix]}", ':'),
+              fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator], ':')
             ]
 
             sli = described_class.initialize_sli(:bar, [{ hello: :world }])
@@ -112,6 +122,8 @@
           before do
             fake_total_counter("boom#{subclass_info[:suffix]}")
             fake_numerator_counter("boom#{subclass_info[:suffix]}", subclass_info[:numerator])
+            fake_total_counter("boom#{subclass_info[:suffix]}", ':')
+            fake_numerator_counter("boom#{subclass_info[:suffix]}", subclass_info[:numerator], ':')
           end
 
           it 'is true when an SLI was initialized with labels' do
@@ -130,7 +142,9 @@
         it 'initializes counters for the passed label combinations' do
           counters = [
             fake_total_counter("hey#{subclass_info[:suffix]}"),
-            fake_numerator_counter("hey#{subclass_info[:suffix]}", subclass_info[:numerator])
+            fake_numerator_counter("hey#{subclass_info[:suffix]}", subclass_info[:numerator]),
+            fake_total_counter("hey#{subclass_info[:suffix]}", ':'),
+            fake_numerator_counter("hey#{subclass_info[:suffix]}", subclass_info[:numerator], ':')
           ]
 
           described_class.new(:hey).initialize_counters([{ foo: 'bar' }, { foo: 'baz' }])
@@ -138,18 +152,43 @@
           expect(counters).to all(have_received(:get).with({ foo: 'bar' }))
           expect(counters).to all(have_received(:get).with({ foo: 'baz' }))
         end
+
+        context 'when `gitlab_sli_new_counters` is disabled' do
+          before do
+            stub_feature_flags(gitlab_sli_new_counters: false)
+          end
+
+          it 'does not initialize the new counters', :aggregate_failures do
+            new_total_counter = fake_total_counter("bar#{subclass_info[:suffix]}")
+            new_numerator_counter = fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator])
+
+            fake_total_counter("bar#{subclass_info[:suffix]}", ':')
+            fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator], ':')
+
+            described_class.new(:bar).initialize_counters([{ hello: :world }])
+
+            expect(new_total_counter).not_to have_received(:get)
+            expect(new_numerator_counter).not_to have_received(:get)
+          end
+        end
       end
 
       describe "#increment" do
         let!(:sli) { described_class.new(:heyo) }
         let!(:total_counter) { fake_total_counter("heyo#{subclass_info[:suffix]}") }
         let!(:numerator_counter) { fake_numerator_counter("heyo#{subclass_info[:suffix]}", subclass_info[:numerator]) }
+        let!(:legacy_total_counter) { fake_total_counter("heyo#{subclass_info[:suffix]}", ':') }
+        let!(:legacy_numerator_counter) do
+          fake_numerator_counter("heyo#{subclass_info[:suffix]}", subclass_info[:numerator], ':')
+        end
 
         it "increments both counters for labels when #{subclass_info[:numerator]} is true" do
           sli.increment(labels: { hello: "world" }, subclass_info[:numerator] => true)
 
           expect(total_counter).to have_received(:increment).with({ hello: 'world' })
           expect(numerator_counter).to have_received(:increment).with({ hello: 'world' })
+          expect(legacy_total_counter).to have_received(:increment).with({ hello: 'world' })
+          expect(legacy_numerator_counter).to have_received(:increment).with({ hello: 'world' })
         end
 
         it "only increments the total counters for labels when #{subclass_info[:numerator]} is false" do
@@ -157,6 +196,31 @@
 
           expect(total_counter).to have_received(:increment).with({ hello: 'world' })
           expect(numerator_counter).not_to have_received(:increment).with({ hello: 'world' })
+          expect(legacy_total_counter).to have_received(:increment).with({ hello: 'world' })
+          expect(legacy_numerator_counter).not_to have_received(:increment).with({ hello: 'world' })
+        end
+
+        context 'when `gitlab_sli_new_counters` is disabled' do
+          before do
+            stub_feature_flags(gitlab_sli_new_counters: false)
+          end
+
+          it 'does increment new counters', :aggregate_failures do
+            new_total_counter = fake_total_counter("bar#{subclass_info[:suffix]}")
+            new_numerator_counter = fake_numerator_counter("bar#{subclass_info[:suffix]}", subclass_info[:numerator])
+
+            legacy_total_counter = fake_total_counter("bar#{subclass_info[:suffix]}", ':')
+            legacy_numerator_counter = fake_numerator_counter(
+              "bar#{subclass_info[:suffix]}", subclass_info[:numerator], ':'
+            )
+
+            described_class.new(:bar).increment(labels: { hello: 'world' }, subclass_info[:numerator] => true)
+
+            expect(new_total_counter).not_to have_received(:increment)
+            expect(new_numerator_counter).not_to have_received(:increment)
+            expect(legacy_total_counter).to have_received(:increment)
+            expect(legacy_numerator_counter).to have_received(:increment)
+          end
         end
       end
     end
@@ -172,11 +236,11 @@ def fake_prometheus_counter(name)
     fake_counter
   end
 
-  def fake_total_counter(name)
-    fake_prometheus_counter("gitlab_sli:#{name}:total")
+  def fake_total_counter(name, separator = '_')
+    fake_prometheus_counter(['gitlab_sli', name, 'total'].join(separator))
   end
 
-  def fake_numerator_counter(name, numerator_name)
-    fake_prometheus_counter("gitlab_sli:#{name}:#{numerator_name}_total")
+  def fake_numerator_counter(name, numerator_name, separator = '_')
+    fake_prometheus_counter(["gitlab_sli", name, "#{numerator_name}_total"].join(separator))
   end
 end