diff --git a/config/feature_flags/development/gitlab_sli_new_counters.yml b/config/feature_flags/development/gitlab_sli_new_counters.yml
new file mode 100644
index 0000000000000000000000000000000000000000..62d6b82b0dbd8ee679b8aeab6e5aca2583380939
--- /dev/null
+++ b/config/feature_flags/development/gitlab_sli_new_counters.yml
@@ -0,0 +1,8 @@
+---
+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
diff --git a/lib/gitlab/metrics/sli.rb b/lib/gitlab/metrics/sli.rb
index 2de1951435497ec340e010f1b064d7ae7456b649..75734c792d508585a2561d26dd81087ca62f2644 100644
--- a/lib/gitlab/metrics/sli.rb
+++ b/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
diff --git a/spec/lib/gitlab/metrics/sli_spec.rb b/spec/lib/gitlab/metrics/sli_spec.rb
index 102ea442b3a767f6d84af2e4fa895820e58e0c80..983b90b9f25756300a55cf0bc067131e15fc2fe4 100644
--- a/spec/lib/gitlab/metrics/sli_spec.rb
+++ b/spec/lib/gitlab/metrics/sli_spec.rb
@@ -1,6 +1,8 @@
# 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