Add latest changes from gitlab-org/gitlab@13-5-stable-eev13.5.0-rc42

2 files changed, 132 insertions, 71 deletions

diff --git a/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb b/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb
index 09d5e048f6a..ff8f5797f9d 100644
--- a/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb
+++ b/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb
@@ -8,9 +8,16 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do
   describe '#execute' do
     let(:project) { create(:project) }
     let(:dashboard_path) { 'path/to/dashboard.yml' }
+    let(:prometheus_adapter) { double('adapter', clear_prometheus_reactive_cache!: nil) }
 
     subject { described_class.new(dashboard_hash, project: project, dashboard_path: dashboard_path) }
 
+    before do
+      allow_next_instance_of(::Clusters::Applications::ScheduleUpdateService) do |update_service|
+        allow(update_service).to receive(:execute)
+      end
+    end
+
     context 'valid dashboard' do
       let(:dashboard_hash) { load_sample_dashboard }
 
@@ -21,20 +28,32 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do
       end
 
       context 'with existing metrics' do
+        let(:existing_metric_attributes) do
+          {
+            project:        project,
+            identifier:     'metric_b',
+            title:          'overwrite',
+            y_label:        'overwrite',
+            query:          'overwrite',
+            unit:           'overwrite',
+            legend:         'overwrite',
+            dashboard_path: dashboard_path
+          }
+        end
+
         let!(:existing_metric) do
-          create(:prometheus_metric, {
-            project:    project,
-            identifier: 'metric_b',
-            title:      'overwrite',
-            y_label:    'overwrite',
-            query:      'overwrite',
-            unit:       'overwrite',
-            legend:     'overwrite'
-          })
+          create(:prometheus_metric, existing_metric_attributes)
+        end
+
+        let!(:existing_alert) do
+          alert = create(:prometheus_alert, project: project, prometheus_metric: existing_metric)
+          existing_metric.prometheus_alerts << alert
+
+          alert
         end
 
         it 'updates existing PrometheusMetrics' do
-          described_class.new(dashboard_hash, project: project, dashboard_path: dashboard_path).execute
+          subject.execute
 
           expect(existing_metric.reload.attributes.with_indifferent_access).to include({
             title:   'Super Chart B',
@@ -49,6 +68,15 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do
           expect { subject.execute }.to change { PrometheusMetric.count }.by(2)
         end
 
+        it 'updates affected environments' do
+          expect(::Clusters::Applications::ScheduleUpdateService).to receive(:new).with(
+            existing_alert.environment.cluster_prometheus_adapter,
+            project
+          ).and_return(double('ScheduleUpdateService', execute: true))
+
+          subject.execute
+        end
+
         context 'with stale metrics' do
           let!(:stale_metric) do
             create(:prometheus_metric,
@@ -59,11 +87,45 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do
             )
           end
 
+          let!(:stale_alert) do
+            alert = create(:prometheus_alert, project: project, prometheus_metric: stale_metric)
+            stale_metric.prometheus_alerts << alert
+
+            alert
+          end
+
+          it 'updates existing PrometheusMetrics' do
+            subject.execute
+
+            expect(existing_metric.reload.attributes.with_indifferent_access).to include({
+              title:   'Super Chart B',
+              y_label: 'y_label',
+              query:   'query',
+              unit:    'unit',
+              legend:  'Legend Label'
+            })
+          end
+
           it 'deletes stale metrics' do
             subject.execute
 
             expect { stale_metric.reload }.to raise_error(ActiveRecord::RecordNotFound)
           end
+
+          it 'deletes stale alert' do
+            subject.execute
+
+            expect { stale_alert.reload }.to raise_error(ActiveRecord::RecordNotFound)
+          end
+
+          it 'updates affected environments' do
+            expect(::Clusters::Applications::ScheduleUpdateService).to receive(:new).with(
+              existing_alert.environment.cluster_prometheus_adapter,
+              project
+            ).and_return(double('ScheduleUpdateService', execute: true))
+
+            subject.execute
+          end
         end
       end
     end
diff --git a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb
index 69b779d36eb..631325402d9 100644
--- a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb
+++ b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb
@@ -22,7 +22,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
       end
 
       it 'increments requests count' do
-        expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get')
+        expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get', status: 200, feature_category: 'unknown')
 
         subject.call(env)
       end
@@ -32,75 +32,55 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
       end
 
       it 'measures execution time' do
-        expect(described_class).to receive_message_chain(:http_request_duration_seconds, :observe).with({ status: '200', method: 'get' }, a_positive_execution_time)
+        expect(described_class).to receive_message_chain(:http_request_duration_seconds, :observe).with({ method: 'get' }, a_positive_execution_time)
 
         Timecop.scale(3600) { subject.call(env) }
       end
 
       context 'request is a health check endpoint' do
-        it 'increments health endpoint counter' do
-          env['PATH_INFO'] = '/-/liveness'
+        ['/-/liveness', '/-/liveness/', '/-/%6D%65%74%72%69%63%73'].each do |path|
+          context "when path is #{path}" do
+            before do
+              env['PATH_INFO'] = path
+            end
 
-          expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get')
+            it 'increments health endpoint counter rather than overall counter' do
+              expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get', status: 200)
+              expect(described_class).not_to receive(:http_request_total)
 
-          subject.call(env)
-        end
-
-        context 'with trailing slash' do
-          before do
-            env['PATH_INFO'] = '/-/liveness/'
-          end
-
-          it 'increments health endpoint counter' do
-            expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get')
-
-            subject.call(env)
-          end
-        end
-
-        context 'with percent encoded values' do
-          before do
-            env['PATH_INFO'] = '/-/%6D%65%74%72%69%63%73' # /-/metrics
-          end
+              subject.call(env)
+            end
 
-          it 'increments health endpoint counter' do
-            expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get')
+            it 'does not record the request duration' do
+              expect(described_class).not_to receive(:http_request_duration_seconds)
 
-            subject.call(env)
+              subject.call(env)
+            end
           end
         end
       end
 
       context 'request is not a health check endpoint' do
-        it 'does not increment health endpoint counter' do
-          env['PATH_INFO'] = '/-/ordinary-requests'
-
-          expect(described_class).not_to receive(:http_health_requests_total)
-
-          subject.call(env)
-        end
-
-        context 'path info is a root path' do
-          before do
-            env['PATH_INFO'] = '/-/'
-          end
-
-          it 'does not increment health endpoint counter' do
-            expect(described_class).not_to receive(:http_health_requests_total)
-
-            subject.call(env)
-          end
-        end
-
-        context 'path info is a subpath' do
-          before do
-            env['PATH_INFO'] = '/-/health/subpath'
-          end
-
-          it 'does not increment health endpoint counter' do
-            expect(described_class).not_to receive(:http_health_requests_total)
-
-            subject.call(env)
+        ['/-/ordinary-requests', '/-/', '/-/health/subpath'].each do |path|
+          context "when path is #{path}" do
+            before do
+              env['PATH_INFO'] = path
+            end
+
+            it 'increments overall counter rather than health endpoint counter' do
+              expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get', status: 200, feature_category: 'unknown')
+              expect(described_class).not_to receive(:http_health_requests_total)
+
+              subject.call(env)
+            end
+
+            it 'records the request duration' do
+              expect(described_class)
+                .to receive_message_chain(:http_request_duration_seconds, :observe)
+                      .with({ method: 'get' }, a_positive_execution_time)
+
+              subject.call(env)
+            end
           end
         end
       end
@@ -121,7 +101,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
       end
 
       it 'increments requests count' do
-        expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get')
+        expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get', status: 'undefined', feature_category: 'unknown')
 
         expect { subject.call(env) }.to raise_error(StandardError)
       end
@@ -133,13 +113,32 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
       end
     end
 
+    context 'when a feature category header is present' do
+      before do
+        allow(app).to receive(:call).and_return([200, { described_class::FEATURE_CATEGORY_HEADER => 'issue_tracking' }, nil])
+      end
+
+      it 'adds the feature category to the labels for http_request_total' do
+        expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get', status: 200, feature_category: 'issue_tracking')
+
+        subject.call(env)
+      end
+
+      it 'does not record a feature category for health check endpoints' do
+        env['PATH_INFO'] = '/-/liveness'
+
+        expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get', status: 200)
+        expect(described_class).not_to receive(:http_request_total)
+
+        subject.call(env)
+      end
+    end
+
     describe '.initialize_http_request_duration_seconds' do
       it "sets labels" do
         expected_labels = []
-        described_class::HTTP_METHODS.each do |method, statuses|
-          statuses.each do |status|
-            expected_labels << { method: method, status: status.to_s }
-          end
+        described_class::HTTP_METHODS.each do |method|
+          expected_labels << { method: method }
         end
 
         described_class.initialize_http_request_duration_seconds