diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-10-21 10:08:36 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-10-21 10:08:36 +0300 |
commit | 48aff82709769b098321c738f3444b9bdaa694c6 (patch) | |
tree | e00c7c43e2d9b603a5a6af576b1685e400410dee /spec/lib/gitlab/metrics | |
parent | 879f5329ee916a948223f8f43d77fba4da6cd028 (diff) |
Add latest changes from gitlab-org/gitlab@13-5-stable-eev13.5.0-rc42
Diffstat (limited to 'spec/lib/gitlab/metrics')
-rw-r--r-- | spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb | 82 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb | 121 |
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 |