From fa72bf51cfb0f223325de5dc8a04670f3a1bc98d Mon Sep 17 00:00:00 2001
From: Daniele Rossetti <drossetti@gitlab.com>
Date: Mon, 22 Jul 2024 15:07:30 +0000
Subject: [PATCH] Remove deprecated observability_tracing feature flag
Changelog: changed
---
.../development/observability_tracing.yml | 8 --
ee/app/policies/ee/project_policy.rb | 3 +-
.../projects/menus/monitor_menu_spec.rb | 3 -
ee/spec/policies/project_policy_spec.rb | 11 +-
.../requests/projects/logs_controller_spec.rb | 1 -
.../projects/metrics_controller_spec.rb | 1 -
.../projects/tracing_controller_spec.rb | 1 -
lib/gitlab/observability.rb | 3 +-
spec/lib/gitlab/auth_spec.rb | 102 ++++++++----------
spec/lib/gitlab/observability_spec.rb | 55 ++++++----
10 files changed, 84 insertions(+), 104 deletions(-)
delete mode 100644 config/feature_flags/development/observability_tracing.yml
diff --git a/config/feature_flags/development/observability_tracing.yml b/config/feature_flags/development/observability_tracing.yml
deleted file mode 100644
index a32d84096088d..0000000000000
--- 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 12946c191f3ae..8aefe4ca77cd9 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 c5396e1988bdc..380c55453e555 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 1c3d164331054..6f71f01e0717f 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 452a3c7ec1d15..e39c9b205dc0d 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 ed25e5bea565f..9b7e7922b4bc2 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 280b86d460350..64794a4e8be50 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 d1e6fad6e9eea..e3af81efa71d9 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 551197add7f4f..c1dba7e19ac3d 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 f8fa67735ebf3..8853198bb92fc 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
--
GitLab