Commit 2252cae4 authored by Alex Kalderimis's avatar Alex Kalderimis

Merge branch '346106-log-only-features-on-the-current-and-previous-release' into 'master'

Resolve "Log only features on the current and future release"

See merge request gitlab-org/gitlab!76105
parents 4c2da834 667d68cb
......@@ -18,7 +18,8 @@ RSpec.describe EE::API::Entities::Experiment do
milestone: '13.7',
type: 'experiment',
group: 'group::adoption',
default_enabled: false
default_enabled: false,
log_state_changes: nil
},
current_status: {
state: :off,
......
......@@ -63,7 +63,8 @@ RSpec.describe API::Experiments do
milestone: '13.7',
type: 'experiment',
group: 'group::adoption',
default_enabled: false
default_enabled: false,
log_state_changes: nil
},
current_status: {
state: :off,
......
......@@ -159,7 +159,7 @@ class Feature
end
def log_feature_flag_states?(key)
key != :feature_flag_state_logs && Feature.enabled?(:feature_flag_state_logs, type: :ops)
Feature::Definition.log_states?(key)
end
def log_feature_flag_state(key, feature_value)
......
......@@ -82,6 +82,16 @@ class Feature
attributes
end
def for_upcoming_milestone?
return false unless milestone
Gitlab::VersionInfo.parse(milestone + '.999') >= Gitlab.version_info
end
def force_log_state_changes?
attributes[:log_state_changes]
end
class << self
def paths
@paths ||= [Rails.root.join('config', 'feature_flags', '**', '*.yml')]
......@@ -106,6 +116,14 @@ class Feature
definitions.has_key?(key.to_sym)
end
def log_states?(key)
return false if key == :feature_flag_state_logs
return false if Feature.disabled?(:feature_flag_state_logs, type: :ops)
return false unless (feature = get(key))
feature.force_log_state_changes? || feature.for_upcoming_milestone?
end
def valid_usage!(key, type:, default_enabled:)
if definition = get(key)
definition.valid_usage!(type_in_code: type, default_enabled_in_code: default_enabled)
......
......@@ -58,6 +58,7 @@ class Feature
introduced_by_url
rollout_issue_url
milestone
log_state_changes
type
group
default_enabled
......
......@@ -161,6 +161,41 @@ RSpec.describe Feature::Definition do
end
end
describe '.for_upcoming_milestone?' do
using RSpec::Parameterized::TableSyntax
let(:definition) do
Feature::Definition.new("development/enabled_feature_flag.yml",
name: :enabled_feature_flag,
type: 'development',
milestone: milestone,
default_enabled: false)
end
before do
allow(Feature::Definition).to receive(:definitions) do
{ definition.key => definition }
end
allow(Gitlab).to receive(:version_info).and_return(Gitlab::VersionInfo.parse(current_milestone))
end
subject { definition.for_upcoming_milestone? }
where(:ctx, :milestone, :current_milestone, :expected) do
'no milestone' | nil | '1.0.0' | false
'upcoming milestone - major' | '2.3' | '1.9.999' | true
'upcoming milestone - minor' | '2.3' | '2.2.999' | true
'current milestone' | '2.3' | '2.3.999' | true
'past milestone - major' | '1.9' | '2.3.999' | false
'past milestone - minor' | '2.2' | '2.3.999' | false
end
with_them do
it {is_expected.to be(expected)}
end
end
describe '.valid_usage!' do
before do
allow(described_class).to receive(:definitions) do
......@@ -215,7 +250,42 @@ RSpec.describe Feature::Definition do
end
end
describe '.defaul_enabled?' do
describe '.log_states?' do
using RSpec::Parameterized::TableSyntax
let(:definition) do
Feature::Definition.new("development/enabled_feature_flag.yml",
name: :enabled_feature_flag,
type: 'development',
milestone: milestone,
log_state_changes: log_state_change,
default_enabled: false)
end
before do
allow(Feature::Definition).to receive(:definitions) do
{ definition.key => definition }
end
allow(Gitlab).to receive(:version_info).and_return(Gitlab::VersionInfo.new(10, 0, 0))
end
subject { Feature::Definition.log_states?(key) }
where(:ctx, :key, :milestone, :log_state_change, :expected) do
'When flag does not exist' | :no_flag | "0.0" | true | false
'When flag is old, and logging is not forced' | :enabled_feature_flag | "0.0" | false | false
'When flag is old, but logging is forced' | :enabled_feature_flag | "0.0" | true | true
'When flag is current' | :enabled_feature_flag | "10.0" | true | true
'Flag is upcoming' | :enabled_feature_flag | "10.0" | true | true
end
with_them do
it { is_expected.to be(expected) }
end
end
describe '.default_enabled?' do
subject { described_class.default_enabled?(key) }
context 'when feature flag exist' do
......
......@@ -186,6 +186,17 @@ RSpec.describe Feature, stub_feature_flags: false do
context 'logging is enabled', :request_store do
before do
allow(Feature).to receive(:log_feature_flag_states?).and_call_original
definition = Feature::Definition.new("development/enabled_feature_flag.yml",
name: :enabled_feature_flag,
type: 'development',
log_state_changes: true,
default_enabled: false)
allow(Feature::Definition).to receive(:definitions) do
{ definition.key => definition }
end
described_class.enable(:feature_flag_state_logs)
described_class.enable(:enabled_feature_flag)
described_class.enabled?(:enabled_feature_flag)
......@@ -513,6 +524,82 @@ RSpec.describe Feature, stub_feature_flags: false do
end
end
describe '.log_feature_flag_states?' do
let(:log_state_changes) { false }
let(:milestone) { "0.0" }
let(:flag_name) { :some_flag }
let(:definition) do
Feature::Definition.new("development/#{flag_name}.yml",
name: flag_name,
type: 'development',
milestone: milestone,
log_state_changes: log_state_changes,
default_enabled: false)
end
before do
Feature.enable(:feature_flag_state_logs)
Feature.enable(:some_flag)
allow(Feature).to receive(:log_feature_flag_states?).and_return(false)
allow(Feature).to receive(:log_feature_flag_states?).with(:feature_flag_state_logs).and_call_original
allow(Feature).to receive(:log_feature_flag_states?).with(:some_flag).and_call_original
allow(Feature::Definition).to receive(:definitions) do
{ definition.key => definition }
end
end
subject { described_class.log_feature_flag_states?(flag_name) }
context 'when flag is feature_flag_state_logs' do
let(:milestone) { "14.6" }
let(:flag_name) { :feature_flag_state_logs }
let(:log_state_changes) { true }
it { is_expected.to be_falsey }
end
context 'when flag is old' do
it { is_expected.to be_falsey }
end
context 'when flag is old while log_state_changes is not present ' do
let(:definition) do
Feature::Definition.new("development/#{flag_name}.yml",
name: flag_name,
type: 'development',
milestone: milestone,
default_enabled: false)
end
it { is_expected.to be_falsey }
end
context 'when flag is old but log_state_changes is true' do
let(:log_state_changes) { true }
it { is_expected.to be_truthy }
end
context 'when flag is new and not feature_flag_state_logs' do
let(:milestone) { "14.6" }
it { is_expected.to be_truthy }
end
context 'when milestone is nil' do
let(:definition) do
Feature::Definition.new("development/#{flag_name}.yml",
name: flag_name,
type: 'development',
default_enabled: false)
end
it { is_expected.to be_falsey }
end
end
context 'caching with stale reads from the database', :use_clean_rails_redis_caching, :request_store, :aggregate_failures do
let(:actor) { stub_feature_flag_gate('CustomActor:5') }
let(:another_actor) { stub_feature_flag_gate('CustomActor:10') }
......
......@@ -98,15 +98,23 @@ RSpec.describe Gitlab::Lograge::CustomOptions do
context 'when feature flags are present', :request_store do
before do
allow(Feature::Definition).to receive(:valid_usage!).and_return(true)
allow(Feature).to receive(:log_feature_flag_states?).and_return(false)
definitions = {}
[:enabled_feature, :disabled_feature].each do |flag_name|
definitions[flag_name] = Feature::Definition.new("development/enabled_feature.yml",
name: flag_name,
type: 'development',
log_state_changes: true,
default_enabled: false)
allow(Feature).to receive(:log_feature_flag_states?).with(flag_name).and_call_original
end
allow(Feature::Definition).to receive(:definitions).and_return(definitions)
Feature.enable(:enabled_feature)
Feature.disable(:disabled_feature)
allow(Feature).to receive(:log_feature_flag_states?).with(:enabled_feature).and_call_original
allow(Feature).to receive(:log_feature_flag_states?).with(:disabled_feature).and_call_original
end
context 'and :feature_flag_log_states is enabled' 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