Add latest changes from gitlab-org/gitlab@13-1-stable-ee

12 files changed, 349 insertions, 33 deletions

diff --git a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb
index d772b0c7a5f..2703339d89c 100644
--- a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb
+++ b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb
@@ -142,7 +142,7 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi
 
   describe '.find_all_paths' do
     let(:all_dashboard_paths) { described_class.find_all_paths(project) }
-    let(:system_dashboard) { { path: system_dashboard_path, display_name: 'Default', default: true, system_dashboard: true } }
+    let(:system_dashboard) { { path: system_dashboard_path, display_name: 'Default dashboard', default: true, system_dashboard: true } }
 
     it 'includes only the system dashboard by default' do
       expect(all_dashboard_paths).to eq([system_dashboard])
@@ -163,7 +163,7 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi
       let(:self_monitoring_dashboard) do
         {
           path: self_monitoring_dashboard_path,
-          display_name: 'Default',
+          display_name: 'Default dashboard',
           default: true,
           system_dashboard: false
         }
diff --git a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb
index b2fca0b5954..7250cefb9ff 100644
--- a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb
+++ b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb
@@ -16,7 +16,8 @@ describe Gitlab::Metrics::Dashboard::Processor do
         Gitlab::Metrics::Dashboard::Stages::EndpointInserter,
         Gitlab::Metrics::Dashboard::Stages::Sorter,
         Gitlab::Metrics::Dashboard::Stages::AlertsInserter,
-        Gitlab::Metrics::Dashboard::Stages::PanelIdsInserter
+        Gitlab::Metrics::Dashboard::Stages::PanelIdsInserter,
+        Gitlab::Metrics::Dashboard::Stages::UrlValidator
       ]
     end
 
@@ -201,6 +202,27 @@ describe Gitlab::Metrics::Dashboard::Processor do
 
       it_behaves_like 'errors with message', 'Each "metric" must define one of :query or :query_range'
     end
+
+    describe 'validating links' do
+      context 'when the links contain a blocked url' do
+        let(:dashboard_yml_links) do
+          [{ 'url' => 'http://1.1.1.1.1' }, { 'url' => 'https://gitlab.com' }]
+        end
+
+        let(:expected) do
+          [{ url: '' }, { url: 'https://gitlab.com' }]
+        end
+
+        before do
+          stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
+          dashboard_yml['links'] = dashboard_yml_links
+        end
+
+        it 'replaces the blocked url with an empty string' do
+          expect(dashboard[:links]).to eq(expected)
+        end
+      end
+    end
   end
 
   private
diff --git a/spec/lib/gitlab/metrics/elasticsearch_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/elasticsearch_rack_middleware_spec.rb
new file mode 100644
index 00000000000..305768ef060
--- /dev/null
+++ b/spec/lib/gitlab/metrics/elasticsearch_rack_middleware_spec.rb
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Metrics::ElasticsearchRackMiddleware do
+  let(:app) { double(:app, call: 'app call result') }
+  let(:middleware) { described_class.new(app) }
+  let(:env) { {} }
+  let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) }
+
+  describe '#call' do
+    let(:counter) { instance_double(Prometheus::Client::Counter, increment: nil) }
+    let(:histogram) { instance_double(Prometheus::Client::Histogram, observe: nil) }
+    let(:elasticsearch_query_time) { 0.1 }
+    let(:elasticsearch_requests_count) { 2 }
+
+    before do
+      allow(Gitlab::Instrumentation::ElasticsearchTransport).to receive(:query_time) { elasticsearch_query_time }
+      allow(Gitlab::Instrumentation::ElasticsearchTransport).to receive(:get_request_count) { elasticsearch_requests_count }
+
+      allow(Gitlab::Metrics).to receive(:counter)
+        .with(:http_elasticsearch_requests_total,
+              an_instance_of(String),
+              Gitlab::Metrics::Transaction::BASE_LABELS)
+        .and_return(counter)
+
+      allow(Gitlab::Metrics).to receive(:histogram)
+        .with(:http_elasticsearch_requests_duration_seconds,
+              an_instance_of(String),
+              Gitlab::Metrics::Transaction::BASE_LABELS,
+              described_class::HISTOGRAM_BUCKETS)
+        .and_return(histogram)
+
+      allow(Gitlab::Metrics).to receive(:current_transaction).and_return(transaction)
+    end
+
+    it 'calls the app' do
+      expect(middleware.call(env)).to eq('app call result')
+    end
+
+    it 'records elasticsearch metrics' do
+      expect(counter).to receive(:increment).with(transaction.labels, elasticsearch_requests_count)
+      expect(histogram).to receive(:observe).with(transaction.labels, elasticsearch_query_time)
+
+      middleware.call(env)
+    end
+
+    it 'records elasticsearch metrics if an error is raised' do
+      expect(counter).to receive(:increment).with(transaction.labels, elasticsearch_requests_count)
+      expect(histogram).to receive(:observe).with(transaction.labels, elasticsearch_query_time)
+
+      allow(app).to receive(:call).with(env).and_raise(StandardError)
+
+      expect { middleware.call(env) }.to raise_error(StandardError)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb
index 229db67ec88..035d875258c 100644
--- a/spec/lib/gitlab/metrics/method_call_spec.rb
+++ b/spec/lib/gitlab/metrics/method_call_spec.rb
@@ -26,7 +26,7 @@ describe Gitlab::Metrics::MethodCall do
 
       context 'prometheus instrumentation is enabled' do
         before do
-          Feature.get(:prometheus_metrics_method_instrumentation).enable
+          stub_feature_flags(prometheus_metrics_method_instrumentation: true)
         end
 
         around do |example|
@@ -50,7 +50,7 @@ describe Gitlab::Metrics::MethodCall do
 
       context 'prometheus instrumentation is disabled' do
         before do
-          Feature.get(:prometheus_metrics_method_instrumentation).disable
+          stub_feature_flags(prometheus_metrics_method_instrumentation: false)
         end
 
         it 'observes using NullMetric' do
diff --git a/spec/lib/gitlab/metrics/methods_spec.rb b/spec/lib/gitlab/metrics/methods_spec.rb
index bca94deb1d8..5cf8db55142 100644
--- a/spec/lib/gitlab/metrics/methods_spec.rb
+++ b/spec/lib/gitlab/metrics/methods_spec.rb
@@ -104,7 +104,7 @@ describe Gitlab::Metrics::Methods do
 
         context 'when feature is enabled' do
           before do
-            Feature.get(feature_name).enable
+            stub_feature_flags(feature_name => true)
           end
 
           it "initializes #{metric_type} metric" do
@@ -118,7 +118,7 @@ describe Gitlab::Metrics::Methods do
 
         context 'when feature is disabled' do
           before do
-            Feature.get(feature_name).disable
+            stub_feature_flags(feature_name => false)
           end
 
           it "returns NullMetric" do
diff --git a/spec/lib/gitlab/metrics/redis_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/redis_rack_middleware_spec.rb
new file mode 100644
index 00000000000..f2f36ccad20
--- /dev/null
+++ b/spec/lib/gitlab/metrics/redis_rack_middleware_spec.rb
@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Metrics::RedisRackMiddleware do
+  let(:app) { double(:app) }
+  let(:middleware) { described_class.new(app) }
+  let(:env) { {} }
+  let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) }
+
+  before do
+    allow(app).to receive(:call).with(env).and_return('wub wub')
+  end
+
+  describe '#call' do
+    let(:counter) { double(Prometheus::Client::Counter, increment: nil) }
+    let(:histogram) { double(Prometheus::Client::Histogram, observe: nil) }
+    let(:redis_query_time) { 0.1 }
+    let(:redis_requests_count) { 2 }
+
+    before do
+      allow(Gitlab::Instrumentation::Redis).to receive(:query_time) { redis_query_time }
+      allow(Gitlab::Instrumentation::Redis).to receive(:get_request_count) { redis_requests_count }
+
+      allow(Gitlab::Metrics).to receive(:counter)
+        .with(:http_redis_requests_total,
+              an_instance_of(String),
+              Gitlab::Metrics::Transaction::BASE_LABELS)
+        .and_return(counter)
+
+      allow(Gitlab::Metrics).to receive(:histogram)
+        .with(:http_redis_requests_duration_seconds,
+              an_instance_of(String),
+              Gitlab::Metrics::Transaction::BASE_LABELS,
+              Gitlab::Instrumentation::Redis::QUERY_TIME_BUCKETS)
+        .and_return(histogram)
+
+      allow(Gitlab::Metrics).to receive(:current_transaction).and_return(transaction)
+    end
+
+    it 'calls the app' do
+      expect(middleware.call(env)).to eq('wub wub')
+    end
+
+    it 'records redis metrics' do
+      expect(counter).to receive(:increment).with(transaction.labels, redis_requests_count)
+      expect(histogram).to receive(:observe).with(transaction.labels, redis_query_time)
+
+      middleware.call(env)
+    end
+
+    it 'records redis metrics if an error is raised' do
+      expect(counter).to receive(:increment).with(transaction.labels, redis_requests_count)
+      expect(histogram).to receive(:observe).with(transaction.labels, redis_query_time)
+
+      allow(app).to receive(:call).with(env).and_raise(StandardError)
+
+      expect { middleware.call(env) }.to raise_error(StandardError)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb
index fdf3b5bd045..087a0bfbac5 100644
--- a/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb
+++ b/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb
@@ -3,7 +3,17 @@
 require 'spec_helper'
 
 describe Gitlab::Metrics::Samplers::DatabaseSampler do
-  subject { described_class.new(described_class::SAMPLING_INTERVAL_SECONDS) }
+  subject { described_class.new }
+
+  describe '#interval' do
+    it 'samples every five seconds by default' do
+      expect(subject.interval).to eq(5)
+    end
+
+    it 'samples at other intervals if requested' do
+      expect(described_class.new(11).interval).to eq(11)
+    end
+  end
 
   describe '#sample' do
     before do
diff --git a/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb
index 1097d26c320..df63f2ebe28 100644
--- a/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb
+++ b/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb
@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe Gitlab::Metrics::Samplers::PumaSampler do
-  subject { described_class.new(5) }
+  subject { described_class.new }
 
   let(:null_metric) { double('null_metric', set: nil, observe: nil) }
 
@@ -11,6 +11,16 @@ describe Gitlab::Metrics::Samplers::PumaSampler do
     allow(Gitlab::Metrics::NullMetric).to receive(:instance).and_return(null_metric)
   end
 
+  describe '#interval' do
+    it 'samples every five seconds by default' do
+      expect(subject.interval).to eq(5)
+    end
+
+    it 'samples at other intervals if requested' do
+      expect(described_class.new(11).interval).to eq(11)
+    end
+  end
+
   describe '#sample' do
     before do
       expect(subject).to receive(:puma_stats).and_return(puma_stats)
diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb
index ead650a27f0..9fc8dd10922 100644
--- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb
+++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb
@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe Gitlab::Metrics::Samplers::RubySampler do
-  let(:sampler) { described_class.new(5) }
+  let(:sampler) { described_class.new }
   let(:null_metric) { double('null_metric', set: nil, observe: nil) }
 
   before do
@@ -18,6 +18,16 @@ describe Gitlab::Metrics::Samplers::RubySampler do
     end
   end
 
+  describe '#interval' do
+    it 'samples every sixty seconds by default' do
+      expect(subject.interval).to eq(60)
+    end
+
+    it 'samples at other intervals if requested' do
+      expect(described_class.new(11).interval).to eq(11)
+    end
+  end
+
   describe '#sample' do
     it 'adds a metric containing the process resident memory bytes' do
       expect(Gitlab::Metrics::System).to receive(:memory_usage_rss).and_return(9000)
diff --git a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb
index 67336cf83e6..ea9e8fa6795 100644
--- a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb
+++ b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb
@@ -10,17 +10,19 @@ describe Gitlab::Metrics::SidekiqMiddleware do
     it 'tracks the transaction' do
       worker = double(:worker, class: double(:class, name: 'TestWorker'))
 
-      expect(Gitlab::Metrics::BackgroundTransaction).to receive(:new)
-        .with(worker.class)
-        .and_call_original
+      expect_next_instance_of(Gitlab::Metrics::BackgroundTransaction) do |transaction|
+        expect(transaction).to receive(:set).with(:sidekiq_queue_duration, instance_of(Float))
+        expect(transaction).to receive(:increment).with(:db_count, 1)
+      end
 
-      expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:set)
-        .with(:sidekiq_queue_duration, instance_of(Float))
+      middleware.call(worker, message, :test) do
+        ActiveRecord::Base.connection.execute('SELECT pg_sleep(0.1);')
+      end
 
-      middleware.call(worker, message, :test) { nil }
+      expect(message).to include(:db_count, :db_write_count, :db_cached_count)
     end
 
-    it 'tracks the transaction (for messages without `enqueued_at`)' do
+    it 'tracks the transaction (for messages without `enqueued_at`)', :aggregate_failures do
       worker = double(:worker, class: double(:class, name: 'TestWorker'))
 
       expect(Gitlab::Metrics::BackgroundTransaction).to receive(:new)
@@ -33,7 +35,7 @@ describe Gitlab::Metrics::SidekiqMiddleware do
       middleware.call(worker, {}, :test) { nil }
     end
 
-    it 'tracks any raised exceptions' do
+    it 'tracks any raised exceptions', :aggregate_failures do
       worker = double(:worker, class: double(:class, name: 'TestWorker'))
 
       expect_any_instance_of(Gitlab::Metrics::Transaction)
@@ -44,6 +46,8 @@ describe Gitlab::Metrics::SidekiqMiddleware do
 
       expect { middleware.call(worker, message, :test) }
         .to raise_error(RuntimeError)
+
+      expect(message).to include(:db_count, :db_write_count, :db_cached_count)
     end
   end
 end
diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb
index 1624cea8bda..a78d048908d 100644
--- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb
+++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb
@@ -6,10 +6,15 @@ describe Gitlab::Metrics::Subscribers::ActiveRecord do
   let(:env) { {} }
   let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) }
   let(:subscriber)  { described_class.new }
+  let(:payload) { { sql: 'SELECT * FROM users WHERE id = 10' } }
 
   let(:event) do
-    double(:event, duration: 2,
-                   payload:  { sql: 'SELECT * FROM users WHERE id = 10' })
+    double(
+      :event,
+      name: 'sql.active_record',
+      duration: 2,
+      payload:  payload
+    )
   end
 
   describe '#sql' do
@@ -23,6 +28,63 @@ describe Gitlab::Metrics::Subscribers::ActiveRecord do
     end
 
     describe 'with a current transaction' do
+      shared_examples 'read only query' do
+        it 'increments only db count value' do
+          allow(subscriber).to receive(:current_transaction)
+                                  .at_least(:once)
+                                  .and_return(transaction)
+
+          expect(transaction).to receive(:increment)
+                                   .with(:db_count, 1)
+
+          expect(transaction).not_to receive(:increment)
+                                       .with(:db_cached_count, 1)
+
+          expect(transaction).not_to receive(:increment)
+                                       .with(:db_write_count, 1)
+
+          subscriber.sql(event)
+        end
+      end
+
+      shared_examples 'write query' do
+        it 'increments db_write_count and db_count value' do
+          expect(subscriber).to receive(:current_transaction)
+                                  .at_least(:once)
+                                  .and_return(transaction)
+
+          expect(transaction).to receive(:increment)
+                                   .with(:db_count, 1)
+
+          expect(transaction).not_to receive(:increment)
+                                       .with(:db_cached_count, 1)
+
+          expect(transaction).to receive(:increment)
+                                       .with(:db_write_count, 1)
+
+          subscriber.sql(event)
+        end
+      end
+
+      shared_examples 'cached query' do
+        it 'increments db_cached_count and db_count value' do
+          expect(subscriber).to receive(:current_transaction)
+                                  .at_least(:once)
+                                  .and_return(transaction)
+
+          expect(transaction).to receive(:increment)
+                                   .with(:db_count, 1)
+
+          expect(transaction).to receive(:increment)
+                                       .with(:db_cached_count, 1)
+
+          expect(transaction).not_to receive(:increment)
+                                   .with(:db_write_count, 1)
+
+          subscriber.sql(event)
+        end
+      end
+
       it 'observes sql_duration metric' do
         expect(subscriber).to receive(:current_transaction)
                                 .at_least(:once)
@@ -31,18 +93,66 @@ describe Gitlab::Metrics::Subscribers::ActiveRecord do
         subscriber.sql(event)
       end
 
-      it 'increments the :sql_duration value' do
-        expect(subscriber).to receive(:current_transaction)
-          .at_least(:once)
-          .and_return(transaction)
+      it_behaves_like 'read only query'
+
+      context 'with select for update sql event' do
+        let(:payload) { { sql: 'SELECT * FROM users WHERE id = 10 FOR UPDATE' } }
 
-        expect(transaction).to receive(:increment)
-          .with(:sql_duration, 2, false)
+        it_behaves_like 'write query'
+      end
 
-        expect(transaction).to receive(:increment)
-          .with(:sql_count, 1, false)
+      context 'with common table expression' do
+        context 'with insert' do
+          let(:payload) { { sql: 'WITH archived_rows AS (SELECT * FROM users WHERE archived = true) INSERT INTO products_log SELECT * FROM archived_rows' } }
 
-        subscriber.sql(event)
+          it_behaves_like 'write query'
+        end
+
+        context 'with only select' do
+          let(:payload) { { sql: 'WITH active_milestones AS (SELECT COUNT(*), state FROM milestones GROUP BY state) SELECT * FROM active_milestones' } }
+
+          it_behaves_like 'read only query'
+        end
+      end
+
+      context 'with delete sql event' do
+        let(:payload) { { sql: 'DELETE FROM users where id = 10' } }
+
+        it_behaves_like 'write query'
+      end
+
+      context 'with insert sql event' do
+        let(:payload) { { sql: 'INSERT INTO project_ci_cd_settings (project_id) SELECT id FROM projects' } }
+
+        it_behaves_like 'write query'
+      end
+
+      context 'with update sql event' do
+        let(:payload) { { sql: 'UPDATE users SET admin = true WHERE id = 10' } }
+
+        it_behaves_like 'write query'
+      end
+
+      context 'with cached payload ' do
+        let(:payload) do
+          {
+            sql: 'SELECT * FROM users WHERE id = 10',
+            cached: true
+          }
+        end
+
+        it_behaves_like 'cached query'
+      end
+
+      context 'with cached payload name' do
+        let(:payload) do
+          {
+           sql: 'SELECT * FROM users WHERE id = 10',
+           name: 'CACHE'
+          }
+        end
+
+        it_behaves_like 'cached query'
       end
 
       context 'events are internal to Rails or irrelevant' do
diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb
index cf46fa3e91c..693ec3cb7e7 100644
--- a/spec/lib/gitlab/metrics/transaction_spec.rb
+++ b/spec/lib/gitlab/metrics/transaction_spec.rb
@@ -65,18 +65,16 @@ describe Gitlab::Metrics::Transaction do
   describe '#add_event' do
     let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, increment: nil) }
 
-    before do
-      allow(described_class).to receive(:transaction_metric).and_return(prometheus_metric)
-    end
-
     it 'adds a metric' do
       expect(prometheus_metric).to receive(:increment)
+      expect(described_class).to receive(:fetch_metric).with(:counter, :gitlab_transaction_event_meow_total).and_return(prometheus_metric)
 
       transaction.add_event(:meow)
     end
 
     it 'allows tracking of custom tags' do
       expect(prometheus_metric).to receive(:increment).with(hash_including(animal: "dog"))
+      expect(described_class).to receive(:fetch_metric).with(:counter, :gitlab_transaction_event_bau_total).and_return(prometheus_metric)
 
       transaction.add_event(:bau, animal: 'dog')
     end
@@ -84,6 +82,7 @@ describe Gitlab::Metrics::Transaction do
     context 'with sensitive tags' do
       before do
         transaction.add_event(:baubau, **sensitive_tags.merge(sane: 'yes'))
+        allow(described_class).to receive(:transaction_metric).and_return(prometheus_metric)
       end
 
       it 'filters tags' do
@@ -93,4 +92,37 @@ describe Gitlab::Metrics::Transaction do
       end
     end
   end
+
+  describe '#increment' do
+    let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, increment: nil) }
+
+    it 'adds a metric' do
+      expect(prometheus_metric).to receive(:increment).with(hash_including(:action, :controller), 1)
+      expect(described_class).to receive(:fetch_metric).with(:counter, :gitlab_transaction_meow_total).and_return(prometheus_metric)
+
+      transaction.increment(:meow, 1)
+    end
+  end
+
+  describe '#set' do
+    let(:prometheus_metric) { instance_double(Prometheus::Client::Gauge, set: nil) }
+
+    it 'adds a metric' do
+      expect(prometheus_metric).to receive(:set).with(hash_including(:action, :controller), 1)
+      expect(described_class).to receive(:fetch_metric).with(:gauge, :gitlab_transaction_meow_total).and_return(prometheus_metric)
+
+      transaction.set(:meow, 1)
+    end
+  end
+
+  describe '#get' do
+    let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, get: nil) }
+
+    it 'gets a metric' do
+      expect(described_class).to receive(:fetch_metric).with(:counter, :gitlab_transaction_meow_total).and_return(prometheus_metric)
+      expect(prometheus_metric).to receive(:get)
+
+      transaction.get(:meow, :counter)
+    end
+  end
 end