diff options
Diffstat (limited to 'spec/lib/gitlab/metrics')
33 files changed, 497 insertions, 173 deletions
diff --git a/spec/lib/gitlab/metrics/background_transaction_spec.rb b/spec/lib/gitlab/metrics/background_transaction_spec.rb index 84f405d7369..640bbebf0da 100644 --- a/spec/lib/gitlab/metrics/background_transaction_spec.rb +++ b/spec/lib/gitlab/metrics/background_transaction_spec.rb @@ -2,14 +2,23 @@ require 'spec_helper' -describe Gitlab::Metrics::BackgroundTransaction do +RSpec.describe Gitlab::Metrics::BackgroundTransaction do let(:test_worker_class) { double(:class, name: 'TestWorker') } subject { described_class.new(test_worker_class) } describe '#label' do it 'returns labels based on class name' do - expect(subject.labels).to eq(controller: 'TestWorker', action: 'perform') + expect(subject.labels).to eq(controller: 'TestWorker', action: 'perform', feature_category: '') + end + + it 'contains only the labels defined for metrics' do + expect(subject.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABELS.keys) + end + + it 'includes the feature category if there is one' do + expect(test_worker_class).to receive(:get_feature_category).and_return('source_code_management') + expect(subject.labels).to include(feature_category: 'source_code_management') end end end diff --git a/spec/lib/gitlab/metrics/dashboard/defaults_spec.rb b/spec/lib/gitlab/metrics/dashboard/defaults_spec.rb index 420b246b3f5..dd61f8ebc4d 100644 --- a/spec/lib/gitlab/metrics/dashboard/defaults_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/defaults_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Dashboard::Defaults do +RSpec.describe Gitlab::Metrics::Dashboard::Defaults do it { is_expected.to be_const_defined(:DEFAULT_PANEL_TYPE) } it { is_expected.to be_const_defined(:DEFAULT_PANEL_WEIGHT) } end diff --git a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb index 2703339d89c..60e1e29d4c5 100644 --- a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_caching do +RSpec.describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_caching do include MetricsDashboardHelpers let_it_be(:project) { create(:project) } @@ -118,7 +118,7 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi end describe '.find_raw' do - let(:dashboard) { YAML.load_file(Rails.root.join('config', 'prometheus', 'common_metrics.yml')) } + let(:dashboard) { load_dashboard_yaml(File.read(Rails.root.join('config', 'prometheus', 'common_metrics.yml'))) } let(:params) { {} } subject { described_class.find_raw(project, **params) } @@ -132,7 +132,7 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi end context 'when an existing project dashboard is specified' do - let(:dashboard) { YAML.safe_load(fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml')) } + let(:dashboard) { load_sample_dashboard } let(:params) { { dashboard_path: '.gitlab/dashboards/test.yml' } } let(:project) { project_with_dashboard(params[:dashboard_path]) } @@ -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 dashboard', default: true, system_dashboard: true } } + let(:system_dashboard) { { path: system_dashboard_path, display_name: 'Default dashboard', default: true, system_dashboard: true, out_of_the_box_dashboard: true } } it 'includes only the system dashboard by default' do expect(all_dashboard_paths).to eq([system_dashboard]) @@ -153,7 +153,7 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi let(:project) { project_with_dashboard(dashboard_path) } it 'includes system and project dashboards' do - project_dashboard = { path: dashboard_path, display_name: 'test.yml', default: false, system_dashboard: false } + project_dashboard = { path: dashboard_path, display_name: 'test.yml', default: false, system_dashboard: false, out_of_the_box_dashboard: false } expect(all_dashboard_paths).to contain_exactly(system_dashboard, project_dashboard) end @@ -165,7 +165,8 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi path: self_monitoring_dashboard_path, display_name: 'Default dashboard', default: true, - system_dashboard: false + system_dashboard: false, + out_of_the_box_dashboard: true } end let(:dashboard_path) { '.gitlab/dashboards/test.yml' } @@ -180,7 +181,8 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi path: dashboard_path, display_name: 'test.yml', default: false, - system_dashboard: false + system_dashboard: false, + out_of_the_box_dashboard: false } expect(all_dashboard_paths).to contain_exactly(self_monitoring_dashboard, project_dashboard) diff --git a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb index 7250cefb9ff..7f7070dfafb 100644 --- a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb @@ -2,10 +2,12 @@ require 'spec_helper' -describe Gitlab::Metrics::Dashboard::Processor do +RSpec.describe Gitlab::Metrics::Dashboard::Processor do + include MetricsDashboardHelpers + let(:project) { build(:project) } let(:environment) { create(:environment, project: project) } - let(:dashboard_yml) { YAML.load_file('spec/fixtures/lib/gitlab/metrics/dashboard/sample_dashboard.yml') } + let(:dashboard_yml) { load_sample_dashboard } describe 'process' do let(:sequence) do @@ -13,7 +15,7 @@ describe Gitlab::Metrics::Dashboard::Processor do Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter, Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter, Gitlab::Metrics::Dashboard::Stages::CustomMetricsDetailsInserter, - Gitlab::Metrics::Dashboard::Stages::EndpointInserter, + Gitlab::Metrics::Dashboard::Stages::MetricEndpointInserter, Gitlab::Metrics::Dashboard::Stages::Sorter, Gitlab::Metrics::Dashboard::Stages::AlertsInserter, Gitlab::Metrics::Dashboard::Stages::PanelIdsInserter, @@ -98,7 +100,7 @@ describe Gitlab::Metrics::Dashboard::Processor do let(:sequence) do [ Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter, - Gitlab::Metrics::Dashboard::Stages::EndpointInserter, + Gitlab::Metrics::Dashboard::Stages::MetricEndpointInserter, Gitlab::Metrics::Dashboard::Stages::Sorter ] end @@ -202,27 +204,6 @@ 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/dashboard/service_selector_spec.rb b/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb index 245c98cdd00..f3c8209e0b6 100644 --- a/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Dashboard::ServiceSelector do +RSpec.describe Gitlab::Metrics::Dashboard::ServiceSelector do include MetricsDashboardHelpers describe '#call' do @@ -109,6 +109,46 @@ describe Gitlab::Metrics::Dashboard::ServiceSelector do it { is_expected.to be Metrics::Dashboard::TransientEmbedService } end + + context 'when cluster is provided' do + let(:arguments) { { cluster: "some cluster" } } + + it { is_expected.to be Metrics::Dashboard::ClusterDashboardService } + end + + context 'when cluster is provided and embedded is not true' do + let(:arguments) { { cluster: "some cluster", embedded: 'false' } } + + it { is_expected.to be Metrics::Dashboard::ClusterDashboardService } + end + + context 'when cluster dashboard_path is provided' do + let(:arguments) { { dashboard_path: ::Metrics::Dashboard::ClusterDashboardService::DASHBOARD_PATH } } + + it { is_expected.to be Metrics::Dashboard::ClusterDashboardService } + end + + context 'when cluster is provided and embed params' do + let(:arguments) do + { + cluster: "some cluster", + embedded: 'true', + cluster_type: 'project', + format: :json, + group: 'Food metrics', + title: 'Pizza Consumption', + y_label: 'Slice Count' + } + end + + it { is_expected.to be Metrics::Dashboard::ClusterMetricsEmbedService } + end + + context 'when metrics embed is for an alert' do + let(:arguments) { { embedded: true, prometheus_alert_id: 5 } } + + it { is_expected.to be Metrics::Dashboard::GitlabAlertEmbedService } + end end end end diff --git a/spec/lib/gitlab/metrics/dashboard/stages/grafana_formatter_spec.rb b/spec/lib/gitlab/metrics/dashboard/stages/grafana_formatter_spec.rb index 5d4bd4512e3..8a236f72a60 100644 --- a/spec/lib/gitlab/metrics/dashboard/stages/grafana_formatter_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/stages/grafana_formatter_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Dashboard::Stages::GrafanaFormatter do +RSpec.describe Gitlab::Metrics::Dashboard::Stages::GrafanaFormatter do include GrafanaApiHelpers let_it_be(:namespace) { create(:namespace, name: 'foo') } diff --git a/spec/lib/gitlab/metrics/dashboard/stages/panel_ids_inserter_spec.rb b/spec/lib/gitlab/metrics/dashboard/stages/panel_ids_inserter_spec.rb index 6124f471e39..7a3a9021f86 100644 --- a/spec/lib/gitlab/metrics/dashboard/stages/panel_ids_inserter_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/stages/panel_ids_inserter_spec.rb @@ -2,7 +2,9 @@ require 'spec_helper' -describe Gitlab::Metrics::Dashboard::Stages::PanelIdsInserter do +RSpec.describe Gitlab::Metrics::Dashboard::Stages::PanelIdsInserter do + include MetricsDashboardHelpers + let(:project) { build_stubbed(:project) } def fetch_panel_ids(dashboard_hash) @@ -12,7 +14,7 @@ describe Gitlab::Metrics::Dashboard::Stages::PanelIdsInserter do describe '#transform!' do subject(:transform!) { described_class.new(project, dashboard, nil).transform! } - let(:dashboard) { YAML.safe_load(fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml')).deep_symbolize_keys } + let(:dashboard) { load_sample_dashboard.deep_symbolize_keys } context 'when dashboard panels are present' do it 'assigns unique ids to each panel using PerformanceMonitoring::PrometheusPanel', :aggregate_failures do diff --git a/spec/lib/gitlab/metrics/dashboard/stages/url_validator_spec.rb b/spec/lib/gitlab/metrics/dashboard/stages/url_validator_spec.rb new file mode 100644 index 00000000000..83cf161c4e2 --- /dev/null +++ b/spec/lib/gitlab/metrics/dashboard/stages/url_validator_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Metrics::Dashboard::Stages::UrlValidator do + let(:project) { build_stubbed(:project) } + + describe '#transform!' do + context 'when the links contain a blocked url' do + let(:dashboard) do + { + dashboard: "Test Dashboard", + links: [ + { url: "http://1.1.1.1.1" }, + { url: "https://gitlab.com" }, + { url: "http://0.0.0.0" } + ], + panel_groups: [ + { + group: "Group A", + panels: [ + { + title: "Super Chart A1", + type: "area-chart", + y_label: "y_label", + metrics: [ + { + id: "metric_a1", + query_range: "query", + unit: "unit", + label: "Legend Label" + } + ], + links: [ + { url: "http://1.1.1.1.1" }, + { url: "https://gitlab.com" }, + { url: "http://0.0.0.0" } + ] + } + ] + } + ] + } + end + + let(:expected) do + [{ url: '' }, { url: 'https://gitlab.com' }, { url: 'http://0.0.0.0' }] + end + + let(:transform!) { described_class.new(project, dashboard, nil).transform! } + + before do + stub_env('RSPEC_ALLOW_INVALID_URLS', 'false') + stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) + end + + context 'dashboard related links' do + it 'replaces the blocked url with an empty string' do + transform! + + expect(dashboard[:links]).to eq(expected) + end + end + + context 'chart links' do + it 'replaces the blocked url with an empty string' do + transform! + + result = dashboard.dig(:panel_groups, 0, :panels, 0, :links) + expect(result).to eq(expected) + end + end + + context 'when local requests are not allowed' do + before do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) + end + + let(:expected) do + [{ url: '' }, { url: 'https://gitlab.com' }, { url: '' }] + end + + it 'replaces the blocked url with an empty string' do + transform! + + expect(dashboard[:links]).to eq(expected) + end + end + + context 'when the links are an array of strings instead of hashes' do + before do + dashboard[:links] = dashboard[:links].map(&:values) + end + + it 'prevents an invalid link definition from erroring out' do + expect { transform! }.not_to raise_error + end + end + end + end +end diff --git a/spec/lib/gitlab/metrics/dashboard/stages/variable_endpoint_inserter_spec.rb b/spec/lib/gitlab/metrics/dashboard/stages/variable_endpoint_inserter_spec.rb new file mode 100644 index 00000000000..9303ff981fb --- /dev/null +++ b/spec/lib/gitlab/metrics/dashboard/stages/variable_endpoint_inserter_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Metrics::Dashboard::Stages::VariableEndpointInserter do + include MetricsDashboardHelpers + + let(:project) { build_stubbed(:project) } + let(:environment) { build_stubbed(:environment, project: project) } + + describe '#transform!' do + subject(:transform!) { described_class.new(project, dashboard, environment: environment).transform! } + + let(:dashboard) { load_sample_dashboard.deep_symbolize_keys } + + context 'when dashboard variables are present' do + it 'assigns prometheus_endpoint_path to metric_label_values variable type' do + endpoint_path = Gitlab::Routing.url_helpers.prometheus_api_project_environment_path( + project, + environment, + proxy_path: :series, + match: ['backend:haproxy_backend_availability:ratio{env="{{env}}"}'] + ) + + transform! + + expect( + dashboard.dig(:templating, :variables, :metric_label_values_variable, :options) + ).to include(prometheus_endpoint_path: endpoint_path) + end + + it 'does not modify other variable types' do + original_text_variable = dashboard[:templating][:variables][:text_variable_full_syntax].deep_dup + + transform! + + expect(dashboard[:templating][:variables][:text_variable_full_syntax]).to eq(original_text_variable) + end + + context 'when variable does not have the required series_selector' do + it 'adds prometheus_endpoint_path without match parameter' do + dashboard[:templating][:variables][:metric_label_values_variable][:options].delete(:series_selector) + endpoint_path = Gitlab::Routing.url_helpers.prometheus_api_project_environment_path( + project, + environment, + proxy_path: :series + ) + + transform! + + expect( + dashboard.dig(:templating, :variables, :metric_label_values_variable, :options) + ).to include(prometheus_endpoint_path: endpoint_path) + end + end + end + + context 'when no variables are present' do + it 'does not fail' do + dashboard.delete(:templating) + + expect { transform! }.not_to raise_error + end + end + + context 'with no environment' do + subject(:transform!) { described_class.new(project, dashboard, {}).transform! } + + it 'raises error' do + expect { transform! }.to raise_error( + Gitlab::Metrics::Dashboard::Errors::DashboardProcessingError, + 'Environment is required for Stages::VariableEndpointInserter' + ) + end + end + end +end diff --git a/spec/lib/gitlab/metrics/dashboard/url_spec.rb b/spec/lib/gitlab/metrics/dashboard/url_spec.rb index 75f9f99c8a6..56556423b05 100644 --- a/spec/lib/gitlab/metrics/dashboard/url_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/url_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Dashboard::Url do +RSpec.describe Gitlab::Metrics::Dashboard::Url do include Gitlab::Routing.url_helpers describe '#metrics_regex' do @@ -46,6 +46,35 @@ describe Gitlab::Metrics::Dashboard::Url do end end + describe '#clusters_regex' do + let(:url) do + Gitlab::Routing.url_helpers.namespace_project_cluster_url( + 'foo', + 'bar', + '1', + group: 'Cluster Health', + title: 'Memory Usage', + y_label: 'Memory 20(GiB)', + anchor: 'title' + ) + end + + let(:expected_params) do + { + 'url' => url, + 'namespace' => 'foo', + 'project' => 'bar', + 'cluster_id' => '1', + 'query' => '?group=Cluster+Health&title=Memory+Usage&y_label=Memory+20%28GiB%29', + 'anchor' => '#title' + } + end + + subject { described_class.clusters_regex } + + it_behaves_like 'regex which matches url when expected' + end + describe '#grafana_regex' do let(:url) do namespace_project_grafana_api_metrics_dashboard_url( diff --git a/spec/lib/gitlab/metrics/delta_spec.rb b/spec/lib/gitlab/metrics/delta_spec.rb index 9bb011dc8fc..e768da875c2 100644 --- a/spec/lib/gitlab/metrics/delta_spec.rb +++ b/spec/lib/gitlab/metrics/delta_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Delta do +RSpec.describe Gitlab::Metrics::Delta do let(:delta) { described_class.new } describe '#compared_with' do diff --git a/spec/lib/gitlab/metrics/elasticsearch_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/elasticsearch_rack_middleware_spec.rb index 305768ef060..1fbd41bcc88 100644 --- a/spec/lib/gitlab/metrics/elasticsearch_rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/elasticsearch_rack_middleware_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::ElasticsearchRackMiddleware do +RSpec.describe Gitlab::Metrics::ElasticsearchRackMiddleware do let(:app) { double(:app, call: 'app call result') } let(:middleware) { described_class.new(app) } let(:env) { {} } diff --git a/spec/lib/gitlab/metrics/exporter/base_exporter_spec.rb b/spec/lib/gitlab/metrics/exporter/base_exporter_spec.rb index 47ec69e2f45..e4f85243528 100644 --- a/spec/lib/gitlab/metrics/exporter/base_exporter_spec.rb +++ b/spec/lib/gitlab/metrics/exporter/base_exporter_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Exporter::BaseExporter do +RSpec.describe Gitlab::Metrics::Exporter::BaseExporter do let(:exporter) { described_class.new } let(:log_filename) { File.join(Rails.root, 'log', 'sidekiq_exporter.log') } let(:settings) { double('settings') } diff --git a/spec/lib/gitlab/metrics/exporter/sidekiq_exporter_spec.rb b/spec/lib/gitlab/metrics/exporter/sidekiq_exporter_spec.rb index 0b820fdbde9..2c5ef09f799 100644 --- a/spec/lib/gitlab/metrics/exporter/sidekiq_exporter_spec.rb +++ b/spec/lib/gitlab/metrics/exporter/sidekiq_exporter_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Exporter::SidekiqExporter do +RSpec.describe Gitlab::Metrics::Exporter::SidekiqExporter do let(:exporter) { described_class.new } after do diff --git a/spec/lib/gitlab/metrics/exporter/web_exporter_spec.rb b/spec/lib/gitlab/metrics/exporter/web_exporter_spec.rb index f22993cf057..ce98c807e2e 100644 --- a/spec/lib/gitlab/metrics/exporter/web_exporter_spec.rb +++ b/spec/lib/gitlab/metrics/exporter/web_exporter_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Exporter::WebExporter do +RSpec.describe Gitlab::Metrics::Exporter::WebExporter do let(:exporter) { described_class.new } let(:readiness_probe) { exporter.send(:readiness_probe).execute } diff --git a/spec/lib/gitlab/metrics/instrumentation_spec.rb b/spec/lib/gitlab/metrics/instrumentation_spec.rb index bf84a476df9..2729fbce974 100644 --- a/spec/lib/gitlab/metrics/instrumentation_spec.rb +++ b/spec/lib/gitlab/metrics/instrumentation_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Instrumentation do +RSpec.describe Gitlab::Metrics::Instrumentation do let(:env) { {} } let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) } diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb index 035d875258c..42361cbc36a 100644 --- a/spec/lib/gitlab/metrics/method_call_spec.rb +++ b/spec/lib/gitlab/metrics/method_call_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::MethodCall do +RSpec.describe Gitlab::Metrics::MethodCall do let(:transaction) { double(:transaction, labels: {}) } let(:method_call) { described_class.new('Foo#bar', :Foo, '#bar', transaction) } diff --git a/spec/lib/gitlab/metrics/methods_spec.rb b/spec/lib/gitlab/metrics/methods_spec.rb index 5cf8db55142..3c171680272 100644 --- a/spec/lib/gitlab/metrics/methods_spec.rb +++ b/spec/lib/gitlab/metrics/methods_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Methods do +RSpec.describe Gitlab::Metrics::Methods do subject { Class.new { include Gitlab::Metrics::Methods } } shared_context 'metric' do |metric_type, *args| diff --git a/spec/lib/gitlab/metrics/prometheus_spec.rb b/spec/lib/gitlab/metrics/prometheus_spec.rb index e15a063fc9e..2273987551d 100644 --- a/spec/lib/gitlab/metrics/prometheus_spec.rb +++ b/spec/lib/gitlab/metrics/prometheus_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Prometheus, :prometheus do +RSpec.describe Gitlab::Metrics::Prometheus, :prometheus do let(:all_metrics) { Gitlab::Metrics } let(:registry) { all_metrics.registry } diff --git a/spec/lib/gitlab/metrics/rack_middleware_spec.rb b/spec/lib/gitlab/metrics/rack_middleware_spec.rb index dd1dbf7a1f4..335e5a490a6 100644 --- a/spec/lib/gitlab/metrics/rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/rack_middleware_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::RackMiddleware do +RSpec.describe Gitlab::Metrics::RackMiddleware do let(:app) { double(:app) } let(:middleware) { described_class.new(app) } diff --git a/spec/lib/gitlab/metrics/redis_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/redis_rack_middleware_spec.rb index f2f36ccad20..a85968dbd43 100644 --- a/spec/lib/gitlab/metrics/redis_rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/redis_rack_middleware_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::RedisRackMiddleware do +RSpec.describe Gitlab::Metrics::RedisRackMiddleware do let(:app) { double(:app) } let(:middleware) { described_class.new(app) } let(:env) { {} } diff --git a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb index 6ee8acbf6fd..69b779d36eb 100644 --- a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::RequestsRackMiddleware do +RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do let(:app) { double('app') } subject { described_class.new(app) } diff --git a/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb index 087a0bfbac5..b94d19ff227 100644 --- a/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Samplers::DatabaseSampler do +RSpec.describe Gitlab::Metrics::Samplers::DatabaseSampler do subject { described_class.new } describe '#interval' do diff --git a/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb index df63f2ebe28..214649d3e7e 100644 --- a/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Samplers::PumaSampler do +RSpec.describe Gitlab::Metrics::Samplers::PumaSampler do subject { described_class.new } let(:null_metric) { double('null_metric', set: nil, observe: nil) } diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb index 9fc8dd10922..59a70ac74a5 100644 --- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Samplers::RubySampler do +RSpec.describe Gitlab::Metrics::Samplers::RubySampler do let(:sampler) { described_class.new } let(:null_metric) { double('null_metric', set: nil, observe: nil) } diff --git a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb index a64aae73d43..9f2180c4170 100644 --- a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Samplers::UnicornSampler do +RSpec.describe Gitlab::Metrics::Samplers::UnicornSampler do subject { described_class.new(1.second) } describe '#sample' do diff --git a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb index ea9e8fa6795..c66d8b1075c 100644 --- a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::SidekiqMiddleware do +RSpec.describe Gitlab::Metrics::SidekiqMiddleware do let(:middleware) { described_class.new } let(:message) { { 'args' => ['test'], 'enqueued_at' => Time.new(2016, 6, 23, 6, 59).to_f } } @@ -18,8 +18,20 @@ describe Gitlab::Metrics::SidekiqMiddleware do middleware.call(worker, message, :test) do ActiveRecord::Base.connection.execute('SELECT pg_sleep(0.1);') end + end + + it 'prevents database counters from leaking to the next transaction' do + worker = double(:worker, class: double(:class, name: 'TestWorker')) - expect(message).to include(:db_count, :db_write_count, :db_cached_count) + 2.times do + Gitlab::WithRequestStore.with_request_store do + middleware.call(worker, message, :test) do + ActiveRecord::Base.connection.execute('SELECT pg_sleep(0.1);') + end + end + end + + expect(message).to include(db_count: 1, db_write_count: 0, db_cached_count: 0) end it 'tracks the transaction (for messages without `enqueued_at`)', :aggregate_failures do @@ -35,19 +47,20 @@ describe Gitlab::Metrics::SidekiqMiddleware do middleware.call(worker, {}, :test) { nil } end - it 'tracks any raised exceptions', :aggregate_failures do + it 'tracks any raised exceptions', :aggregate_failures, :request_store do worker = double(:worker, class: double(:class, name: 'TestWorker')) expect_any_instance_of(Gitlab::Metrics::Transaction) - .to receive(:run).and_raise(RuntimeError) - - expect_any_instance_of(Gitlab::Metrics::Transaction) .to receive(:add_event).with(:sidekiq_exception) - expect { middleware.call(worker, message, :test) } - .to raise_error(RuntimeError) + expect do + middleware.call(worker, message, :test) do + ActiveRecord::Base.connection.execute('SELECT pg_sleep(0.1);') + raise RuntimeError + end + end.to raise_error(RuntimeError) - expect(message).to include(:db_count, :db_write_count, :db_cached_count) + expect(message).to include(db_count: 1, db_write_count: 0, db_cached_count: 0) end end end diff --git a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb index 857e54d3432..161527c01aa 100644 --- a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Subscribers::ActionView do +RSpec.describe Gitlab::Metrics::Subscribers::ActionView do let(:env) { {} } let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) } diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb index a78d048908d..2fd5dd1d83b 100644 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Subscribers::ActiveRecord do +RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do let(:env) { {} } let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) } let(:subscriber) { described_class.new } @@ -28,60 +28,45 @@ 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 + shared_examples 'track executed query' do + before 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) + .at_least(:once) + .and_return(transaction) 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) + it 'increments only db count value' do + described_class::DB_COUNTERS.each do |counter| + if expected_counters[counter] > 0 + expect(transaction).to receive(:increment).with(counter, 1) + else + expect(transaction).not_to receive(:increment).with(counter, 1) + end + end 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) + context 'when RequestStore is enabled' do + it 'caches db count value', :request_store, :aggregate_failures do + subscriber.sql(event) + + described_class::DB_COUNTERS.each do |counter| + expect(Gitlab::SafeRequestStore[counter].to_i).to eq expected_counters[counter] + end + end + + it 'prevents db counters from leaking to the next transaction' do + 2.times do + Gitlab::WithRequestStore.with_request_store do + subscriber.sql(event) + + described_class::DB_COUNTERS.each do |counter| + expect(Gitlab::SafeRequestStore[counter].to_i).to eq expected_counters[counter] + end + end + end + end end end @@ -93,66 +78,96 @@ describe Gitlab::Metrics::Subscribers::ActiveRecord do subscriber.sql(event) end - it_behaves_like 'read only query' + context 'with read query' do + let(:expected_counters) do + { + db_count: 1, + db_write_count: 0, + db_cached_count: 0 + } + end + + it_behaves_like 'track executed query' - context 'with select for update sql event' do - let(:payload) { { sql: 'SELECT * FROM users WHERE id = 10 FOR UPDATE' } } + 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 'write query' + it_behaves_like 'track executed query' + end end - 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' } } + context 'write query' do + let(:expected_counters) do + { + db_count: 1, + db_write_count: 1, + db_cached_count: 0 + } + end + + context 'with select for update sql event' do + let(:payload) { { sql: 'SELECT * FROM users WHERE id = 10 FOR UPDATE' } } - it_behaves_like 'write query' + it_behaves_like 'track executed 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' } } + 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' } } - it_behaves_like 'read only query' + it_behaves_like 'track executed query' + end end - end - context 'with delete sql event' do - let(:payload) { { sql: 'DELETE FROM users where id = 10' } } + context 'with delete sql event' do + let(:payload) { { sql: 'DELETE FROM users where id = 10' } } - it_behaves_like 'write query' - end + it_behaves_like 'track executed query' + end - context 'with insert sql event' do - let(:payload) { { sql: 'INSERT INTO project_ci_cd_settings (project_id) SELECT id FROM projects' } } + 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 + it_behaves_like 'track executed query' + end - context 'with update sql event' do - let(:payload) { { sql: 'UPDATE users SET admin = true WHERE id = 10' } } + context 'with update sql event' do + let(:payload) { { sql: 'UPDATE users SET admin = true WHERE id = 10' } } - it_behaves_like 'write query' + it_behaves_like 'track executed query' + end end - context 'with cached payload ' do - let(:payload) do + context 'with cached query' do + let(:expected_counters) do { - sql: 'SELECT * FROM users WHERE id = 10', - cached: true + db_count: 1, + db_write_count: 0, + db_cached_count: 1 } end - it_behaves_like 'cached query' - end + context 'with cached payload ' do + let(:payload) do + { + sql: 'SELECT * FROM users WHERE id = 10', + cached: true + } + end - context 'with cached payload name' do - let(:payload) do - { - sql: 'SELECT * FROM users WHERE id = 10', - name: 'CACHE' - } + it_behaves_like 'track executed query' end - it_behaves_like 'cached query' + context 'with cached payload name' do + let(:payload) do + { + sql: 'SELECT * FROM users WHERE id = 10', + name: 'CACHE' + } + end + + it_behaves_like 'track executed query' + end end context 'events are internal to Rails or irrelevant' do @@ -215,4 +230,54 @@ describe Gitlab::Metrics::Subscribers::ActiveRecord do end end end + + describe 'self.db_counter_payload' do + before do + allow(subscriber).to receive(:current_transaction) + .at_least(:once) + .and_return(transaction) + end + + context 'when RequestStore is enabled', :request_store do + context 'when query is executed' do + let(:expected_payload) do + { + db_count: 1, + db_cached_count: 0, + db_write_count: 0 + } + end + + it 'returns correct payload' do + subscriber.sql(event) + + expect(described_class.db_counter_payload).to eq(expected_payload) + end + end + + context 'when query is not executed' do + let(:expected_payload) do + { + db_count: 0, + db_cached_count: 0, + db_write_count: 0 + } + end + + it 'returns correct payload' do + expect(described_class.db_counter_payload).to eq(expected_payload) + end + end + end + + context 'when RequestStore is disabled' do + let(:expected_payload) { {} } + + it 'returns empty payload' do + subscriber.sql(event) + + expect(described_class.db_counter_payload).to eq(expected_payload) + end + end + end end diff --git a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb index ab0d89b2683..f7ac719c16a 100644 --- a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Subscribers::RailsCache do +RSpec.describe Gitlab::Metrics::Subscribers::RailsCache do let(:env) { {} } let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) } let(:subscriber) { described_class.new } diff --git a/spec/lib/gitlab/metrics/system_spec.rb b/spec/lib/gitlab/metrics/system_spec.rb index abb6a0096d6..720bd5d79b3 100644 --- a/spec/lib/gitlab/metrics/system_spec.rb +++ b/spec/lib/gitlab/metrics/system_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::System do +RSpec.describe Gitlab::Metrics::System do context 'when /proc files exist' do # Fixtures pulled from: # Linux carbon 5.3.0-7648-generic #41~1586789791~19.10~9593806-Ubuntu SMP Mon Apr 13 17:50:40 UTC x86_64 x86_64 x86_64 GNU/Linux diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb index 693ec3cb7e7..e64179bd5c1 100644 --- a/spec/lib/gitlab/metrics/transaction_spec.rb +++ b/spec/lib/gitlab/metrics/transaction_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::Transaction do +RSpec.describe Gitlab::Metrics::Transaction do let(:transaction) { described_class.new } let(:sensitive_tags) do @@ -114,15 +114,4 @@ describe Gitlab::Metrics::Transaction do 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 diff --git a/spec/lib/gitlab/metrics/web_transaction_spec.rb b/spec/lib/gitlab/metrics/web_transaction_spec.rb index 47f1bd3bd10..12e98089066 100644 --- a/spec/lib/gitlab/metrics/web_transaction_spec.rb +++ b/spec/lib/gitlab/metrics/web_transaction_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Metrics::WebTransaction do +RSpec.describe Gitlab::Metrics::WebTransaction do let(:env) { {} } let(:transaction) { described_class.new(env) } let(:prometheus_metric) { double("prometheus metric") } @@ -70,6 +70,9 @@ describe Gitlab::Metrics::WebTransaction do end describe '#labels' do + let(:request) { double(:request, format: double(:format, ref: :html)) } + let(:controller_class) { double(:controller_class, name: 'TestController') } + context 'when request goes to Grape endpoint' do before do route = double(:route, request_method: 'GET', path: '/:version/projects/:id/archive(.:format)') @@ -77,8 +80,13 @@ describe Gitlab::Metrics::WebTransaction do env['api.endpoint'] = endpoint end + it 'provides labels with the method and path of the route in the grape endpoint' do - expect(transaction.labels).to eq({ controller: 'Grape', action: 'GET /projects/:id/archive' }) + expect(transaction.labels).to eq({ controller: 'Grape', action: 'GET /projects/:id/archive', feature_category: '' }) + end + + it 'contains only the labels defined for transactions' do + expect(transaction.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABELS.keys) end it 'does not provide labels if route infos are missing' do @@ -92,24 +100,25 @@ describe Gitlab::Metrics::WebTransaction do end context 'when request goes to ActionController' do - let(:request) { double(:request, format: double(:format, ref: :html)) } - before do - klass = double(:klass, name: 'TestController') - controller = double(:controller, class: klass, action_name: 'show', request: request) + controller = double(:controller, class: controller_class, action_name: 'show', request: request) env['action_controller.instance'] = controller end it 'tags a transaction with the name and action of a controller' do - expect(transaction.labels).to eq({ controller: 'TestController', action: 'show' }) + expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: '' }) + end + + it 'contains only the labels defined for transactions' do + expect(transaction.labels.keys).to contain_exactly(*described_class.superclass::BASE_LABELS.keys) end context 'when the request content type is not :html' do let(:request) { double(:request, format: double(:format, ref: :json)) } it 'appends the mime type to the transaction action' do - expect(transaction.labels).to eq({ controller: 'TestController', action: 'show.json' }) + expect(transaction.labels).to eq({ controller: 'TestController', action: 'show.json', feature_category: '' }) end end @@ -117,7 +126,14 @@ describe Gitlab::Metrics::WebTransaction do let(:request) { double(:request, format: double(:format, ref: 'http://example.com')) } it 'does not append the MIME type to the transaction action' do - expect(transaction.labels).to eq({ controller: 'TestController', action: 'show' }) + expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: '' }) + end + end + + context 'when the feature category is known' do + it 'includes it in the feature category label' do + expect(controller_class).to receive(:feature_category_for_action).with('show').and_return(:source_code_management) + expect(transaction.labels).to eq({ controller: 'TestController', action: 'show', feature_category: "source_code_management" }) end end end |