Commit 28e7a1de authored by Vitali Tatarintev's avatar Vitali Tatarintev Committed by Mayra Cabrera

Add missing feature flag definition files

Add missing definition files for some feature flags
parent 8816566a
---
name: prometheus_metrics_method_instrumentation
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/4304
rollout_issue_url:
milestone: '10.5'
type: ops
group:
default_enabled: false
---
name: prometheus_metrics_view_instrumentation
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/4304
rollout_issue_url:
milestone: '10.5'
type: ops
group:
default_enabled: false
...@@ -57,7 +57,7 @@ class Feature ...@@ -57,7 +57,7 @@ class Feature
# use `default_enabled: true` to default the flag to being `enabled` # use `default_enabled: true` to default the flag to being `enabled`
# unless set explicitly. The default is `disabled` # unless set explicitly. The default is `disabled`
# TODO: remove the `default_enabled:` and read it from the `defintion_yaml` # TODO: remove the `default_enabled:` and read it from the `defintion_yaml`
# check: https://gitlab.com/gitlab-org/gitlab/-/issues/30228 # check: https://gitlab.com/gitlab-org/gitlab/-/issues/271275
def enabled?(key, thing = nil, type: :development, default_enabled: false) def enabled?(key, thing = nil, type: :development, default_enabled: false)
if check_feature_flags_definition? if check_feature_flags_definition?
if thing && !thing.respond_to?(:flipper_id) if thing && !thing.respond_to?(:flipper_id)
...@@ -65,11 +65,11 @@ class Feature ...@@ -65,11 +65,11 @@ class Feature
"The thing '#{thing.class.name}' for feature flag '#{key}' needs to include `FeatureGate` or implement `flipper_id`" "The thing '#{thing.class.name}' for feature flag '#{key}' needs to include `FeatureGate` or implement `flipper_id`"
end end
Feature::Definition.valid_usage!(key, type: type, default_enabled: default_enabled) Feature::Definition.valid_usage!(key, type: type, default_enabled: :yaml)
end end
# If `default_enabled: :yaml` we fetch the value from the YAML definition instead. # TODO: Remove rubocop disable comment once `default_enabled` argument is removed https://gitlab.com/gitlab-org/gitlab/-/issues/271275
default_enabled = Feature::Definition.default_enabled?(key) if default_enabled == :yaml default_enabled = Feature::Definition.default_enabled?(key) # rubocop:disable Lint/ShadowedArgument
# During setup the database does not exist yet. So we haven't stored a value # During setup the database does not exist yet. So we haven't stored a value
# for the feature yet and return the default. # for the feature yet and return the default.
......
...@@ -15,6 +15,7 @@ RSpec.describe 'Graphql Field feature flags' do ...@@ -15,6 +15,7 @@ RSpec.describe 'Graphql Field feature flags' do
before do before do
skip_feature_flags_yaml_validation skip_feature_flags_yaml_validation
skip_default_enabled_yaml_check
end end
subject { result } subject { result }
......
...@@ -128,6 +128,7 @@ RSpec.describe Types::BaseField do ...@@ -128,6 +128,7 @@ RSpec.describe Types::BaseField do
before do before do
skip_feature_flags_yaml_validation skip_feature_flags_yaml_validation
skip_default_enabled_yaml_check
end end
it 'returns false if the feature is not enabled' do it 'returns false if the feature is not enabled' do
......
...@@ -183,6 +183,7 @@ RSpec.describe API::Helpers do ...@@ -183,6 +183,7 @@ RSpec.describe API::Helpers do
before do before do
skip_feature_flags_yaml_validation skip_feature_flags_yaml_validation
skip_default_enabled_yaml_check
end end
context 'with feature enabled' do context 'with feature enabled' do
......
...@@ -8,6 +8,7 @@ RSpec.describe Feature::Gitaly do ...@@ -8,6 +8,7 @@ RSpec.describe Feature::Gitaly do
before do before do
skip_feature_flags_yaml_validation skip_feature_flags_yaml_validation
skip_default_enabled_yaml_check
end end
describe ".enabled?" do describe ".enabled?" do
......
...@@ -123,12 +123,35 @@ RSpec.describe Feature, stub_feature_flags: false do ...@@ -123,12 +123,35 @@ RSpec.describe Feature, stub_feature_flags: false do
end end
describe '.enabled?' do describe '.enabled?' do
it 'returns false for undefined feature' do let(:disabled_ff_definition) do
expect(described_class.enabled?(:some_random_feature_flag)).to be_falsey Feature::Definition.new(
'development/disabled_feature_flag.yml',
name: 'disabled_feature_flag',
type: 'development',
default_enabled: false
)
end end
it 'returns true for undefined feature with default_enabled' do let(:enabled_ff_definition) do
expect(described_class.enabled?(:some_random_feature_flag, default_enabled: true)).to be_truthy Feature::Definition.new(
'development/enabled_feature_flag.yml',
name: 'enabled_feature_flag',
type: 'development',
default_enabled: true
)
end
before do
allow(Feature::Definition).to receive(:definitions) do
{
disabled_ff_definition.key => disabled_ff_definition,
enabled_ff_definition.key => enabled_ff_definition
}
end
end
it 'raises an exception for undefined feature' do
expect { described_class.enabled?(:some_random_feature_flag) }.to raise_error Feature::InvalidFeatureFlagError
end end
it 'returns false for existing disabled feature in the database' do it 'returns false for existing disabled feature in the database' do
...@@ -146,39 +169,58 @@ RSpec.describe Feature, stub_feature_flags: false do ...@@ -146,39 +169,58 @@ RSpec.describe Feature, stub_feature_flags: false do
it { expect(described_class.send(:l1_cache_backend)).to eq(Gitlab::ProcessMemoryCache.cache_backend) } it { expect(described_class.send(:l1_cache_backend)).to eq(Gitlab::ProcessMemoryCache.cache_backend) }
it { expect(described_class.send(:l2_cache_backend)).to eq(Rails.cache) } it { expect(described_class.send(:l2_cache_backend)).to eq(Rails.cache) }
it 'caches the status in L1 and L2 caches', it 'caches the status in L1 and L2 caches', :request_store, :use_clean_rails_memory_store_caching, :aggregate_failures do
:request_store, :use_clean_rails_memory_store_caching do
described_class.enable(:enabled_feature_flag) described_class.enable(:enabled_feature_flag)
flipper_key = "flipper/v1/feature/enabled_feature_flag" flipper_features_key = 'flipper/v1/features'
flipper_feature_key = 'flipper/v1/feature/enabled_feature_flag'
expect(described_class.send(:l2_cache_backend))
.to receive(:fetch)
.once
.with(flipper_key, expires_in: 1.hour)
.and_call_original
expect(described_class.send(:l1_cache_backend)) allow(described_class.send(:l2_cache_backend)).to receive(:fetch).twice.and_call_original
.to receive(:fetch) allow(described_class.send(:l1_cache_backend)).to receive(:fetch).twice.and_call_original
.once
.with(flipper_key, expires_in: 1.minute)
.and_call_original
2.times do 2.times do
expect(described_class.enabled?(:enabled_feature_flag)).to be_truthy expect(described_class.enabled?(:enabled_feature_flag)).to be_truthy
end end
expect(described_class.send(:l2_cache_backend)).to have_received(:fetch).with(flipper_features_key, expires_in: 1.hour)
expect(described_class.send(:l2_cache_backend)).to have_received(:fetch).with(flipper_feature_key, expires_in: 1.hour)
expect(described_class.send(:l1_cache_backend)).to have_received(:fetch).with(flipper_features_key, expires_in: 1.minute)
expect(described_class.send(:l1_cache_backend)).to have_received(:fetch).with(flipper_feature_key, expires_in: 1.minute)
end
it 'returns the default value when the database does not exist', :aggregate_falures do
a_feature = Feature::Definition.new(
'development/a_feature.yml',
name: 'a_feature',
type: 'development',
default_enabled: true
)
allow(Feature::Definition).to receive(:definitions) do
{ a_feature.key => a_feature }
end end
it 'returns the default value when the database does not exist' do
fake_default = double('fake default')
expect(ActiveRecord::Base).to receive(:connection) { raise ActiveRecord::NoDatabaseError, "No database" } expect(ActiveRecord::Base).to receive(:connection) { raise ActiveRecord::NoDatabaseError, "No database" }
expect(described_class.enabled?(:a_feature, default_enabled: fake_default)).to eq(fake_default) expect(described_class.enabled?(:a_feature)).to eq(true)
end end
context 'cached feature flag', :request_store do context 'cached feature flag', :request_store, :use_clean_rails_memory_store_caching, :aggregate_failures do
let(:flag) { :some_feature_flag } let(:flag) { :some_feature_flag }
let(:some_feature_flag) do
Feature::Definition.new(
"development/#{flag}.yml",
name: flag.to_s,
type: 'development',
default_enabled: true
)
end
before do before do
allow(Feature::Definition).to receive(:definitions) do
{ some_feature_flag.key => some_feature_flag }
end
described_class.send(:flipper).memoize = false described_class.send(:flipper).memoize = false
described_class.enabled?(flag) described_class.enabled?(flag)
end end
...@@ -273,22 +315,8 @@ RSpec.describe Feature, stub_feature_flags: false do ...@@ -273,22 +315,8 @@ RSpec.describe Feature, stub_feature_flags: false do
.to raise_error(/The `type:` of/) .to raise_error(/The `type:` of/)
end end
it 'when invalid default_enabled is used' do
expect { described_class.enabled?(:my_feature_flag, default_enabled: true) }
.to raise_error(/The `default_enabled:` of/)
end
context 'when `default_enabled: :yaml` is used in code' do
it 'reads the default from the YAML definition' do
expect(described_class.enabled?(:my_feature_flag, default_enabled: :yaml)).to eq(false)
end
context 'when default_enabled is true in the YAML definition' do
let(:default_enabled) { true }
it 'reads the default from the YAML definition' do it 'reads the default from the YAML definition' do
expect(described_class.enabled?(:my_feature_flag, default_enabled: :yaml)).to eq(true) expect(described_class.enabled?(:my_feature_flag)).to eq(false)
end
end end
context 'when YAML definition does not exist for an optional type' do context 'when YAML definition does not exist for an optional type' do
...@@ -296,7 +324,7 @@ RSpec.describe Feature, stub_feature_flags: false do ...@@ -296,7 +324,7 @@ RSpec.describe Feature, stub_feature_flags: false do
context 'when in dev or test environment' do context 'when in dev or test environment' do
it 'raises an error for dev' do it 'raises an error for dev' do
expect { described_class.enabled?(:non_existent_flag, type: optional_type, default_enabled: :yaml) } expect { described_class.enabled?(:non_existent_flag, type: optional_type) }
.to raise_error( .to raise_error(
Feature::InvalidFeatureFlagError, Feature::InvalidFeatureFlagError,
"The feature flag YAML definition for 'non_existent_flag' does not exist") "The feature flag YAML definition for 'non_existent_flag' does not exist")
...@@ -316,7 +344,7 @@ RSpec.describe Feature, stub_feature_flags: false do ...@@ -316,7 +344,7 @@ RSpec.describe Feature, stub_feature_flags: false do
it 'checks the persisted status and returns false' do it 'checks the persisted status and returns false' do
expect(described_class).to receive(:get).with(:non_existent_flag).and_call_original expect(described_class).to receive(:get).with(:non_existent_flag).and_call_original
expect(described_class.enabled?(:non_existent_flag, type: optional_type, default_enabled: :yaml)).to eq(false) expect(described_class.enabled?(:non_existent_flag, type: optional_type)).to eq(false)
end end
end end
...@@ -328,22 +356,44 @@ RSpec.describe Feature, stub_feature_flags: false do ...@@ -328,22 +356,44 @@ RSpec.describe Feature, stub_feature_flags: false do
it 'returns false without checking the status in the database' do it 'returns false without checking the status in the database' do
expect(described_class).not_to receive(:get) expect(described_class).not_to receive(:get)
expect(described_class.enabled?(:non_existent_flag, type: optional_type, default_enabled: :yaml)).to eq(false) expect(described_class.enabled?(:non_existent_flag, type: optional_type)).to eq(false)
end end
end end
end end
end end
end end
end end
describe '.disabled?' do
let(:disabled_ff_definition) do
Feature::Definition.new(
'development/disabled_feature_flag.yml',
name: 'disabled_feature_flag',
type: 'development',
default_enabled: false
)
end end
describe '.disable?' do let(:enabled_ff_definition) do
it 'returns true for undefined feature' do Feature::Definition.new(
expect(described_class.disabled?(:some_random_feature_flag)).to be_truthy 'development/enabled_feature_flag.yml',
name: 'enabled_feature_flag',
type: 'development',
default_enabled: true
)
end
before do
allow(Feature::Definition).to receive(:definitions) do
{
disabled_ff_definition.key => disabled_ff_definition,
enabled_ff_definition.key => enabled_ff_definition
}
end
end end
it 'returns false for undefined feature with default_enabled' do it 'raises an exception for undefined feature' do
expect(described_class.disabled?(:some_random_feature_flag, default_enabled: true)).to be_falsey expect { described_class.disabled?(:some_random_feature_flag) }.to raise_error Feature::InvalidFeatureFlagError
end end
it 'returns true for existing disabled feature in the database' do it 'returns true for existing disabled feature in the database' do
......
...@@ -12,6 +12,7 @@ RSpec.describe Gitlab::GonHelper do ...@@ -12,6 +12,7 @@ RSpec.describe Gitlab::GonHelper do
describe '#push_frontend_feature_flag' do describe '#push_frontend_feature_flag' do
before do before do
skip_feature_flags_yaml_validation skip_feature_flags_yaml_validation
skip_default_enabled_yaml_check
end end
it 'pushes a feature flag to the frontend' do it 'pushes a feature flag to the frontend' do
......
...@@ -102,6 +102,11 @@ RSpec.describe Gitlab::Metrics::Methods do ...@@ -102,6 +102,11 @@ RSpec.describe Gitlab::Metrics::Methods do
let(:feature_name) { :some_metric_feature } let(:feature_name) { :some_metric_feature }
let(:metric) { call_fetch_metric_method(docstring: docstring, with_feature: feature_name) } let(:metric) { call_fetch_metric_method(docstring: docstring, with_feature: feature_name) }
before do
skip_feature_flags_yaml_validation
skip_default_enabled_yaml_check
end
context 'when feature is enabled' do context 'when feature is enabled' do
before do before do
stub_feature_flags(feature_name => true) stub_feature_flags(feature_name => true)
......
...@@ -54,6 +54,11 @@ RSpec.describe Gitlab::Metrics do ...@@ -54,6 +54,11 @@ RSpec.describe Gitlab::Metrics do
end end
describe '.measure' do describe '.measure' do
before do
skip_feature_flags_yaml_validation
skip_default_enabled_yaml_check
end
context 'without a transaction' do context 'without a transaction' do
it 'returns the return value of the block' do it 'returns the return value of the block' do
val = described_class.measure(:foo) { 10 } val = described_class.measure(:foo) { 10 }
......
...@@ -438,6 +438,8 @@ RSpec.describe API::Features, stub_feature_flags: false do ...@@ -438,6 +438,8 @@ RSpec.describe API::Features, stub_feature_flags: false do
context 'when the gate value was set' do context 'when the gate value was set' do
before do before do
skip_feature_flags_yaml_validation
skip_default_enabled_yaml_check
Feature.enable(feature_name) Feature.enable(feature_name)
end end
......
...@@ -73,6 +73,7 @@ RSpec.describe API::UsageData do ...@@ -73,6 +73,7 @@ RSpec.describe API::UsageData do
context 'with unknown event' do context 'with unknown event' do
before do before do
skip_feature_flags_yaml_validation skip_feature_flags_yaml_validation
skip_default_enabled_yaml_check
end end
it 'returns status ok' do it 'returns status ok' do
...@@ -149,6 +150,7 @@ RSpec.describe API::UsageData do ...@@ -149,6 +150,7 @@ RSpec.describe API::UsageData do
context 'with unknown event' do context 'with unknown event' do
before do before do
skip_feature_flags_yaml_validation skip_feature_flags_yaml_validation
skip_default_enabled_yaml_check
end end
it 'returns status ok' do it 'returns status ok' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment