Merge branch 'pl-tracing-core-5-settings-haml' into 'master'

Move remaining parts of Tracing to Core [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!44574

app/helpers/projects_helper.rb

@@ -301,7 +301,6 @@ module ProjectsHelper
     !disabled && !compact_mode
   end
 
-  # overridden in EE
   def settings_operations_available?
     can?(current_user, :read_environment, @project)
   end
@@ -755,6 +754,7 @@ module ProjectsHelper
       logs
       product_analytics
       metrics_dashboard
+      tracings
     ]
   end
 

app/views/layouts/nav/sidebar/_project.html.haml

@@ -236,7 +236,7 @@
                     = _('Logs')
 
             - if project_nav_tab? :environments
-              = render_if_exists "layouts/nav/sidebar/tracing_link"
+              = render "layouts/nav/sidebar/tracing_link"
 
             - if project_nav_tab?(:error_tracking)
               = nav_link(controller: :error_tracking) do

ee/app/views/layouts/nav/sidebar/_tracing_link.html.haml → app/views/layouts/nav/sidebar/_tracing_link.html.haml

-- return unless @project.feature_available?(:tracing, current_user) && can?(current_user, :read_environment, @project)
+- return unless can?(current_user, :read_environment, @project)
 
 - if project_nav_tab? :settings
   = nav_link(controller: :tracings, action: [:show]) do

ee/app/views/projects/settings/operations/_tracing.html.haml → app/views/projects/settings/operations/_tracing.html.haml

-- return unless @project.feature_available?(:tracing, current_user)
-
 - setting = tracing_setting
 - has_jaeger_url = setting.external_url.present?
 
@@ -11,7 +9,7 @@
       = _('Expand')
     %p
       - if has_jaeger_url
-        - tracing_link = link_to sanitize(@project.tracing_external_url, scrubber: Rails::Html::TextOnlyScrubber.new), target: "_blank", rel: 'noopener noreferrer' do
+        - tracing_link = link_to sanitize(setting.external_url, scrubber: Rails::Html::TextOnlyScrubber.new), target: "_blank", rel: 'noopener noreferrer' do
           %span
             = _('Tracing')
             = sprite_icon('external-link', css_class: 'ml-1 vertical-align-middle')

app/views/projects/settings/operations/show.html.haml

@@ -8,5 +8,5 @@
 = render 'projects/settings/operations/prometheus', service: prometheus_service if Feature.enabled?(:settings_operations_prometheus_service)
 = render 'projects/settings/operations/metrics_dashboard'
 = render 'projects/settings/operations/grafana_integration'
-= render_if_exists 'projects/settings/operations/tracing'
+= render 'projects/settings/operations/tracing'
 = render_if_exists 'projects/settings/operations/status_page'

changelogs/unreleased/pl-tracing-core-5-settings-haml.yml

+---
+title: Move Tracing feature to Core
+merge_request: 44574
+author:
+type: added

ee/app/helpers/ee/projects_helper.rb

@@ -20,7 +20,6 @@ module EE
     override :sidebar_operations_paths
     def sidebar_operations_paths
       super + %w[
-        tracings
         feature_flags
       ]
     end
@@ -272,12 +271,6 @@ module EE
         current_user.ab_feature_enabled?(:discover_security)
     end
 
-    def settings_operations_available?
-      return true if super
-
-      @project.feature_available?(:tracing, current_user) && can?(current_user, :read_environment, @project)
-    end
-
     override :can_import_members?
     def can_import_members?
       super && !membership_locked?

ee/app/models/license.rb

@@ -150,7 +150,6 @@ class License < ApplicationRecord
     status_page
     subepics
     threat_monitoring
-    tracing
     quality_management
   ]
   EEU_FEATURES.freeze

ee/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb

@@ -3,14 +3,12 @@
 require 'spec_helper'
 
 RSpec.describe 'layouts/nav/sidebar/_project' do
-  let(:project) { create(:project, :repository) }
+  let_it_be_with_refind(:project) { create(:project, :repository) }
 
   before do
     assign(:project, project)
     assign(:repository, project.repository)
     allow(view).to receive(:current_ref).and_return('master')
-
-    stub_licensed_features(tracing: true)
   end
 
   describe 'issue boards' do
@@ -55,43 +53,6 @@ RSpec.describe 'layouts/nav/sidebar/_project' do
     end
   end
 
-  describe 'Operations > Tracing' do
-    it 'is not visible when no valid license' do
-      allow(view).to receive(:can?).and_return(true)
-      stub_licensed_features(tracing: false)
-
-      render
-
-      expect(rendered).not_to have_text 'Tracing'
-    end
-
-    it 'is not visible to unauthorized user' do
-      render
-
-      expect(rendered).not_to have_text 'Tracing'
-    end
-
-    it 'links to Tracing page' do
-      allow(view).to receive(:can?).and_return(true)
-
-      render
-
-      expect(rendered).to have_link('Tracing', href: project_tracing_path(project))
-    end
-
-    context 'without project.tracing_external_url' do
-      before do
-        allow(view).to receive(:can?).and_return(true)
-      end
-
-      it 'links to Tracing page' do
-        render
-
-        expect(rendered).to have_link('Tracing', href: project_tracing_path(project))
-      end
-    end
-  end
-
   describe 'Operations > Pod logs' do
     before do
       allow(view).to receive(:can?).with(nil, :read_environment, project).and_return(can_read_environment)
@@ -222,7 +183,6 @@ RSpec.describe 'layouts/nav/sidebar/_project' do
   describe 'Settings > Operations' do
     it 'is not visible when no valid license' do
       allow(view).to receive(:can?).and_return(true)
-      stub_licensed_features(tracing: false)
 
       render
 

ee/spec/views/projects/settings/operations/show.html.haml_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe 'projects/settings/operations/show' do
-  let_it_be(:project, refind: true) { create(:project, :repository) }
-  let_it_be(:error_tracking_setting) { create(:project_error_tracking_setting, project: project) }
-
-  let(:operations_show_locals) do
-    {
-      prometheus_service: project.find_or_initialize_service('prometheus'),
-      alerts_service: project.find_or_initialize_service('alerts')
-    }
-  end
-
-  before do
-    assign(:project, project)
-    assign(:repository, project.repository)
-    allow(view).to receive(:current_ref).and_return('master')
-    allow(view).to receive(:error_tracking_setting).and_return(error_tracking_setting)
-    allow(view).to receive(:incident_management_available?) { false }
-    stub_licensed_features(tracing: true)
-  end
-
-  describe 'Operations > Tracing' do
-    context 'with project.tracing_external_url' do
-      let(:tracing_url) { 'https://tracing.url' }
-      let(:tracing_setting) { create(:project_tracing_setting, project: project, external_url: tracing_url) }
-
-      before do
-        allow(view).to receive(:can?).and_return(true)
-        allow(view).to receive(:tracing_setting).and_return(tracing_setting)
-      end
-
-      it 'links to project.tracing_external_url' do
-        render template: 'projects/settings/operations/show', locals: operations_show_locals
-
-        expect(rendered).to have_link('Tracing', href: tracing_url)
-      end
-
-      context 'with malicious external_url' do
-        let(:malicious_tracing_url) { "https://replaceme.com/'><script>alert(document.cookie)</script>" }
-        let(:cleaned_url) { "https://replaceme.com/'>" }
-
-        before do
-          tracing_setting.update_column(:external_url, malicious_tracing_url)
-        end
-
-        it 'sanitizes external_url' do
-          render template: 'projects/settings/operations/show', locals: operations_show_locals
-
-          expect(tracing_setting.external_url).to eq(malicious_tracing_url)
-          expect(rendered).to have_link('Tracing', href: cleaned_url)
-        end
-      end
-    end
-
-    context 'without project.tracing_external_url' do
-      let(:tracing_setting) { build(:project_tracing_setting, project: project) }
-
-      before do
-        allow(view).to receive(:can?).and_return(true)
-        allow(view).to receive(:tracing_setting).and_return(tracing_setting)
-
-        tracing_setting.external_url = nil
-      end
-
-      it 'links to Tracing page' do
-        render template: 'projects/settings/operations/show', locals: operations_show_locals
-
-        expect(rendered).to have_link('Tracing', href: project_tracing_path(project))
-      end
-    end
-  end
-end

spec/controllers/projects/tracings_controller_spec.rb

@@ -43,9 +43,7 @@ RSpec.describe Projects::TracingsController do
       end
     end
 
-    describe 'with valid license' do
     before do
-        stub_licensed_features(tracing: true)
       sign_in(user)
     end
 
@@ -61,16 +59,4 @@ RSpec.describe Projects::TracingsController do
       it_behaves_like 'user without read access', :private
     end
   end
-
-    context 'with invalid license' do
-      before do
-        stub_licensed_features(tracing: false)
-        sign_in(user)
-      end
-
-      it_behaves_like 'user without read access', :public
-      it_behaves_like 'user without read access', :internal
-      it_behaves_like 'user without read access', :private
-    end
-  end
 end

ee/spec/features/projects/tracings_spec.rb → spec/features/projects/tracings_spec.rb

@@ -3,13 +3,16 @@
 require 'spec_helper'
 
 RSpec.describe 'Tracings Content Security Policy' do
+  let_it_be(:project) { create(:project) }
   let_it_be(:user) { create(:user) }
-  let(:project) { create(:project) }
 
   subject { response_headers['Content-Security-Policy'] }
 
-  before do
+  before_all do
     project.add_maintainer(user)
+  end
+
+  before do
     sign_in(user)
   end
 

spec/support/shared_contexts/navbar_structure_context.rb

@@ -65,6 +65,7 @@ RSpec.shared_context 'project navbar structure' do
         nav_sub_items: [
           _('Metrics'),
           _('Logs'),
+          _('Tracing'),
           _('Error Tracking'),
           _('Alerts'),
           _('Incidents'),

spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe 'layouts/nav/sidebar/_project' do
-  let(:project) { create(:project, :repository) }
+  let_it_be_with_reload(:project) { create(:project, :repository) }
 
   before do
     assign(:project, project)
@@ -246,6 +246,30 @@ RSpec.describe 'layouts/nav/sidebar/_project' do
       end
     end
 
+    describe 'Tracing' do
+      it 'is not visible to unauthorized user' do
+        allow(view).to receive(:can?).and_return(false)
+
+        render
+
+        expect(rendered).not_to have_text 'Tracing'
+      end
+
+      it 'links to Tracing page' do
+        render
+
+        expect(rendered).to have_link('Tracing', href: project_tracing_path(project))
+      end
+
+      context 'without project.tracing_external_url' do
+        it 'links to Tracing page' do
+          render
+
+          expect(rendered).to have_link('Tracing', href: project_tracing_path(project))
+        end
+      end
+    end
+
     describe 'Alert Management' do
       it 'shows the Alerts sidebar entry' do
         render

spec/views/projects/settings/operations/show.html.haml_spec.rb

@@ -6,31 +6,39 @@ RSpec.describe 'projects/settings/operations/show' do
   let_it_be(:project) { create(:project) }
   let_it_be(:user) { create(:user) }
 
+  let_it_be(:error_tracking_setting) do
+    create(:project_error_tracking_setting, project: project)
+  end
+
+  let_it_be_with_reload(:tracing_setting) do
+    create(:project_tracing_setting, project: project)
+  end
+
+  let_it_be(:prometheus_service) { create(:prometheus_service, project: project) }
+  let_it_be(:alerts_service) { create(:alerts_service, project: project) }
+
   let(:operations_show_locals) do
     {
-      prometheus_service: project.find_or_initialize_service('prometheus'),
-      alerts_service: project.find_or_initialize_service('alerts')
+      prometheus_service: prometheus_service,
+      alerts_service: alerts_service
     }
   end
 
-  before do
-    assign :project, project
+  before_all do
+    project.add_reporter(user)
   end
 
-  describe 'Operations > Error Tracking' do
   before do
-      project.add_reporter(user)
+    assign :project, project
 
     allow(view).to receive(:error_tracking_setting)
       .and_return(error_tracking_setting)
+    allow(view).to receive(:tracing_setting)
+      .and_return(tracing_setting)
     allow(view).to receive(:current_user).and_return(user)
-      allow(view).to receive(:incident_management_available?) { false }
-    end
-
-    let_it_be(:error_tracking_setting) do
-      create(:project_error_tracking_setting, project: project)
   end
 
+  describe 'Operations > Error Tracking' do
     context 'Settings page ' do
       it 'renders the Operations Settings page' do
         render template: 'projects/settings/operations/show', locals: operations_show_locals
@@ -40,4 +48,44 @@ RSpec.describe 'projects/settings/operations/show' do
       end
     end
   end
+
+  describe 'Operations > Tracing' do
+    context 'with project.tracing_external_url' do
+      it 'links to project.tracing_external_url' do
+        render template: 'projects/settings/operations/show', locals: operations_show_locals
+
+        expect(rendered).to have_link('Tracing', href: tracing_setting.external_url)
+      end
+
+      context 'with malicious external_url' do
+        let(:malicious_tracing_url) { "https://replaceme.com/'><script>alert(document.cookie)</script>" }
+        let(:cleaned_url) { "https://replaceme.com/'>" }
+
+        before do
+          tracing_setting.update_column(:external_url, malicious_tracing_url)
+        end
+
+        it 'sanitizes external_url' do
+          render template: 'projects/settings/operations/show', locals: operations_show_locals
+
+          expect(tracing_setting.external_url).to eq(malicious_tracing_url)
+          expect(rendered).to have_link('Tracing', href: cleaned_url)
+        end
+      end
+    end
+
+    context 'without project.tracing_external_url' do
+      let(:tracing_setting) { build(:project_tracing_setting, project: project) }
+
+      before do
+        tracing_setting.external_url = nil
+      end
+
+      it 'links to Tracing page' do
+        render template: 'projects/settings/operations/show', locals: operations_show_locals
+
+        expect(rendered).to have_link('Tracing', href: project_tracing_path(project))
+      end
+    end
+  end
 end