diff --git a/config/feature_flags/development/observability_tracing.yml b/config/feature_flags/development/observability_tracing.yml deleted file mode 100644 index a32d84096088d09b1c528e554b7172ccb1d015a4..0000000000000000000000000000000000000000 --- a/config/feature_flags/development/observability_tracing.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: observability_tracing -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/124966 -rollout_issue_url: https://gitlab.com/gitlab-org/opstrace/opstrace/-/issues/2252 -milestone: '16.2' -type: development -group: group::observability -default_enabled: false diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 12946c191f3ae839b2c17bf371742da191222a75..8aefe4ca77cd9e605aec6f83c0b2b15aaeee2e18 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -254,8 +254,7 @@ module ProjectPolicy end condition(:observability_enabled) do - # temporarily checking :observability_tracing for backward compability until all existing users have the new FF enabled - (::Feature.enabled?(:observability_tracing, @subject.root_namespace) || ::Feature.enabled?(:observability_features, @subject.root_namespace)) && + ::Feature.enabled?(:observability_features, @subject.root_namespace) && @subject.licensed_feature_available?(:observability) end diff --git a/ee/spec/lib/ee/sidebars/projects/menus/monitor_menu_spec.rb b/ee/spec/lib/ee/sidebars/projects/menus/monitor_menu_spec.rb index c5396e1988bdcb5a41c6ba06a5e9d982619ed322..380c55453e5559a084492d7b21ee406b04689d9a 100644 --- a/ee/spec/lib/ee/sidebars/projects/menus/monitor_menu_spec.rb +++ b/ee/spec/lib/ee/sidebars/projects/menus/monitor_menu_spec.rb @@ -57,7 +57,6 @@ describe 'when feature flag is disabled' do before do stub_feature_flags(observability_features: false) - stub_feature_flags(observability_tracing: false) end it { is_expected.to be_nil } @@ -93,7 +92,6 @@ describe 'when feature flag is disabled' do before do stub_feature_flags(observability_features: false) - stub_feature_flags(observability_tracing: false) end it { is_expected.to be_nil } @@ -129,7 +127,6 @@ describe 'when feature flag is disabled' do before do stub_feature_flags(observability_features: false) - stub_feature_flags(observability_tracing: false) end it { is_expected.to be_nil } diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 1c3d16433105491aff8d6bcbd91646b0c5649726..6f71f01e0717f06d99eb39e7b429b5b09659aed9 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -3374,10 +3374,9 @@ def create_member_role(member, abilities = member_role_abilities) stub_licensed_features(observability: true) end - describe 'when observability and observability_tracing feature flags are disabled' do + describe 'when observability_features is disabled' do before do stub_feature_flags(observability_features: false) - stub_feature_flags(observability_tracing: false) end it { is_expected.to be_disallowed(:read_observability) } @@ -3391,14 +3390,6 @@ def create_member_role(member, abilities = member_role_abilities) it { is_expected.to be_allowed(:read_observability) } end - describe 'when observability_tracing feature flag is enabled for root namespace' do - before do - stub_feature_flags(observability_tracing: project.root_namespace) - end - - it { is_expected.to be_allowed(:read_observability) } - end - describe 'when the project does not have the correct license' do before do stub_feature_flags(observability_features: true) diff --git a/ee/spec/requests/projects/logs_controller_spec.rb b/ee/spec/requests/projects/logs_controller_spec.rb index 452a3c7ec1d15fb2cd9f1dacc869c56cff2fddac..e39c9b205dc0d973e0a6355672a40ac7a73d9a4d 100644 --- a/ee/spec/requests/projects/logs_controller_spec.rb +++ b/ee/spec/requests/projects/logs_controller_spec.rb @@ -33,7 +33,6 @@ before do stub_licensed_features(observability: true) stub_feature_flags(observability_features: observability_ff) - stub_feature_flags(observability_tracing: observability_ff) sign_in(user) end diff --git a/ee/spec/requests/projects/metrics_controller_spec.rb b/ee/spec/requests/projects/metrics_controller_spec.rb index ed25e5bea565fa52b98a6b9593ec2ca11a42c702..9b7e7922b4bc284589891d3c35edb4cdaa0c2458 100644 --- a/ee/spec/requests/projects/metrics_controller_spec.rb +++ b/ee/spec/requests/projects/metrics_controller_spec.rb @@ -32,7 +32,6 @@ before do stub_licensed_features(observability: true) - stub_feature_flags(observability_tracing: observability_ff) stub_feature_flags(observability_features: observability_ff) sign_in(user) end diff --git a/ee/spec/requests/projects/tracing_controller_spec.rb b/ee/spec/requests/projects/tracing_controller_spec.rb index 280b86d46035059d0b5a9078fbcc73a7e774f687..64794a4e8be50a8f97d8e355c845df27624b69dc 100644 --- a/ee/spec/requests/projects/tracing_controller_spec.rb +++ b/ee/spec/requests/projects/tracing_controller_spec.rb @@ -32,7 +32,6 @@ before do stub_licensed_features(observability: true) - stub_feature_flags(observability_tracing: observability_ff) stub_feature_flags(observability_features: observability_ff) sign_in(user) end diff --git a/lib/gitlab/observability.rb b/lib/gitlab/observability.rb index d1e6fad6e9eeae4bc7088f8b223b01ed656b5844..e3af81efa71d98858038247dcf2d29682c270261 100644 --- a/lib/gitlab/observability.rb +++ b/lib/gitlab/observability.rb @@ -25,8 +25,7 @@ def provisioning_url(project) def should_enable_observability_auth_scopes?(resource) # Enable the needed oauth scopes if tracing is enabled. if resource.is_a?(Group) || resource.is_a?(Project) - return Feature.enabled?(:observability_tracing, resource.root_ancestor) || - Feature.enabled?(:observability_features, resource.root_ancestor) + return Feature.enabled?(:observability_features, resource.root_ancestor) end false diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 551197add7f4f689c75ff0af4544f082713d751e..c1dba7e19ac3db635ce8c734118458495f5355d9 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -95,11 +95,9 @@ end context 'with observability feature flags' do - feature_flags = [:observability_tracing, :observability_features] - context 'when all disabled' do before do - stub_feature_flags(feature_flags.index_with { false }) + stub_feature_flags(observability_features: false) end it 'contains for group all resource bot scopes without observability scopes' do @@ -126,70 +124,64 @@ end end - flag_states = [true, false].repeated_permutation(feature_flags.length) - flag_tests = flag_states.filter(&:any?).map { |flags| Hash[feature_flags.zip(flags)] } + context "with feature flag enabled for specific root group" do + let(:parent) { build_stubbed(:group) } + let(:group) do + build_stubbed(:group, parent: parent).tap { |g| g.namespace_settings = build_stubbed(:namespace_settings, namespace: g) } + end - flag_tests.each do |flags| - context "with flags #{flags} enabled for specific root group" do - let(:parent) { build_stubbed(:group) } - let(:group) do - build_stubbed(:group, parent: parent).tap { |g| g.namespace_settings = build_stubbed(:namespace_settings, namespace: g) } - end + let(:project) { build_stubbed(:project, namespace: group) } - let(:project) { build_stubbed(:project, namespace: group) } + before do + stub_feature_flags(observability_features: parent) + end - before do - flags.transform_values! { |v| v ? parent : false } - stub_feature_flags(flags) - end + it 'contains for group all resource bot scopes including observability scopes' do + expect(subject.available_scopes_for(group)).to match_array %i[ + api read_api read_repository write_repository read_registry write_registry + read_observability write_observability create_runner manage_runner k8s_proxy ai_features + ] + end - it 'contains for group all resource bot scopes including observability scopes' do - expect(subject.available_scopes_for(group)).to match_array %i[ - api read_api read_repository write_repository read_registry write_registry - read_observability write_observability create_runner manage_runner k8s_proxy ai_features - ] - end + it 'contains for admin user all non-default scopes with ADMIN access and without observability scopes' do + user = build_stubbed(:user, admin: true) - it 'contains for admin user all non-default scopes with ADMIN access and without observability scopes' do - user = build_stubbed(:user, admin: true) + expect(subject.available_scopes_for(user)).to match_array %i[ + api read_user read_api read_repository write_repository read_registry write_registry read_service_ping + sudo admin_mode create_runner manage_runner k8s_proxy ai_features + ] + end - expect(subject.available_scopes_for(user)).to match_array %i[ - api read_user read_api read_repository write_repository read_registry write_registry read_service_ping - sudo admin_mode create_runner manage_runner k8s_proxy ai_features - ] - end + it 'contains for project all resource bot scopes including observability scopes' do + expect(subject.available_scopes_for(project)).to match_array %i[ + api read_api read_repository write_repository read_registry write_registry + read_observability write_observability create_runner manage_runner k8s_proxy ai_features + ] + end - it 'contains for project all resource bot scopes including observability scopes' do - expect(subject.available_scopes_for(project)).to match_array %i[ - api read_api read_repository write_repository read_registry write_registry - read_observability write_observability create_runner manage_runner k8s_proxy ai_features - ] + it 'contains for other group all resource bot scopes without observability scopes' do + other_parent = build_stubbed(:group) + other_group = build_stubbed(:group, parent: other_parent).tap do |g| + g.namespace_settings = build_stubbed(:namespace_settings, namespace: g) end - it 'contains for other group all resource bot scopes without observability scopes' do - other_parent = build_stubbed(:group) - other_group = build_stubbed(:group, parent: other_parent).tap do |g| - g.namespace_settings = build_stubbed(:namespace_settings, namespace: g) - end + expect(subject.available_scopes_for(other_group)).to match_array %i[ + api read_api read_repository write_repository read_registry write_registry + create_runner manage_runner k8s_proxy ai_features + ] + end - expect(subject.available_scopes_for(other_group)).to match_array %i[ - api read_api read_repository write_repository read_registry write_registry - create_runner manage_runner k8s_proxy ai_features - ] + it 'contains for other project all resource bot scopes without observability scopes' do + other_parent = build_stubbed(:group) + other_group = build_stubbed(:group, parent: other_parent).tap do |g| + g.namespace_settings = build_stubbed(:namespace_settings, namespace: g) end + other_project = build_stubbed(:project, namespace: other_group) - it 'contains for other project all resource bot scopes without observability scopes' do - other_parent = build_stubbed(:group) - other_group = build_stubbed(:group, parent: other_parent).tap do |g| - g.namespace_settings = build_stubbed(:namespace_settings, namespace: g) - end - other_project = build_stubbed(:project, namespace: other_group) - - expect(subject.available_scopes_for(other_project)).to match_array %i[ - api read_api read_repository write_repository read_registry write_registry - create_runner manage_runner k8s_proxy ai_features - ] - end + expect(subject.available_scopes_for(other_project)).to match_array %i[ + api read_api read_repository write_repository read_registry write_registry + create_runner manage_runner k8s_proxy ai_features + ] end end end diff --git a/spec/lib/gitlab/observability_spec.rb b/spec/lib/gitlab/observability_spec.rb index f8fa67735ebf3eee6d400a90364ab4848a55be73..8853198bb92fc2137735c0cb1781b760b08aee2d 100644 --- a/spec/lib/gitlab/observability_spec.rb +++ b/spec/lib/gitlab/observability_spec.rb @@ -56,34 +56,47 @@ end end - feature_flags = [:observability_tracing, :observability_features] - flag_states = [true, false].repeated_permutation(feature_flags.length) - flag_tests = flag_states.map { |flags| Hash[feature_flags.zip(flags)] } + context "when feature flag is enabled" do + before do + stub_feature_flags(observability_features: true) + end + + describe 'when resource is group' do + it { is_expected.to be true } + end + + describe 'when resource is project' do + let(:resource) { build_stubbed(:project, namespace: parent) } + + it { is_expected.to be true } + end - flag_tests.each do |flags| - context "with feature flag state #{flags}" do - before do - flags.transform_values! { |v| v ? parent : false } - stub_feature_flags(flags) - end + describe 'when resource is not a group or project' do + let(:resource) { build_stubbed(:user) } - let(:expected_enabled) { flags.values.any? } + it { is_expected.to be false } + end + end - describe 'when resource is group' do - it { is_expected.to be expected_enabled } - end + context "when feature flag is disabled" do + before do + stub_feature_flags(observability_features: false) + end - describe 'when resource is project' do - let(:resource) { build_stubbed(:project, namespace: parent) } + describe 'when resource is group' do + it { is_expected.to be false } + end + + describe 'when resource is project' do + let(:resource) { build_stubbed(:project, namespace: parent) } - it { is_expected.to be expected_enabled } - end + it { is_expected.to be false } + end - describe 'when resource is not a group or project' do - let(:resource) { build_stubbed(:user) } + describe 'when resource is not a group or project' do + let(:resource) { build_stubbed(:user) } - it { is_expected.to be false } - end + it { is_expected.to be false } end end end