diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index 0995a5553ee7f22bdb568f2c9848d0701d376e6c..5f6fa7b8fa0294ac70e8a09b4b38fdd18d96ccf2 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -181,6 +181,7 @@ The following metrics are available: | `gitlab_connection_pool_size` | Gauge | 16.7 | Size of connection pool | | | `gitlab_connection_pool_available_count` | Gauge | 16.7 | Number of available connections in the pool | | | `gitlab_security_policies_scan_result_process_duration_seconds` | Histogram | 16.7 | The amount of time to process merge request approval policies | | +| `gitlab_security_policies_scan_execution_configuration_rendering_seconds` | Histogram | 17.3 | The amount of time to render scan execution policy CI configurations | | | `gitlab_highlight_usage` | Counter | 16.8 | The number of times `Gitlab::Highlight` is used | `used_on` | | `dependency_linker_usage` | Counter | 16.8 | The number of times dependency linker is used | `used_on` | | `gitlab_keeparound_refs_requested_total` | Counter | 16.10 | Counts the number of keep-around refs requested to be created | `source` | diff --git a/ee/app/services/security/security_orchestration_policies/observe_histograms_service.rb b/ee/app/services/security/security_orchestration_policies/observe_histograms_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..c44f27971baa4e88c0ea63dd5aa7229c0833da38 --- /dev/null +++ b/ee/app/services/security/security_orchestration_policies/observe_histograms_service.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module Security + module SecurityOrchestrationPolicies + class ObserveHistogramsService + HISTOGRAMS = { + gitlab_security_policies_scan_execution_configuration_rendering_seconds: { + description: 'The amount of time to render scan execution policy CI configurations', + buckets: (0..10).to_a.freeze + }, + gitlab_security_policies_scan_result_process_duration_seconds: { + description: 'The amount of time to process scan result policies', + buckets: [120, 240, 360, 480, 600, 720, 840, 960].freeze + } + }.freeze + + class << self + def measure(name, **labels) + lo = ::Gitlab::Metrics::System.monotonic_time + ret_val = yield + hi = ::Gitlab::Metrics::System.monotonic_time + + histogram(name).observe(labels, hi - lo) + + ret_val + end + + def histogram(name) + histograms[name] ||= begin + config = HISTOGRAMS[name] || raise(ArgumentError, "unsupported histogram: #{name}") + + Gitlab::Metrics.histogram(name, config[:description], {}, config[:buckets]) + end + end + + def histograms + @histograms ||= {} + end + end + end + end +end diff --git a/ee/app/services/security/security_orchestration_policies/scan_pipeline_service.rb b/ee/app/services/security/security_orchestration_policies/scan_pipeline_service.rb index 009aea0d1d28a04ba76497e925f5d48b4845442c..c81c7b68d160c6d8b9a746703a69dd130a58765e 100644 --- a/ee/app/services/security/security_orchestration_policies/scan_pipeline_service.rb +++ b/ee/app/services/security/security_orchestration_policies/scan_pipeline_service.rb @@ -3,6 +3,8 @@ module Security module SecurityOrchestrationPolicies class ScanPipelineService + HISTOGRAM = :gitlab_security_policies_scan_execution_configuration_rendering_seconds + SCAN_VARIABLES = { secret_detection: { 'SECRET_DETECTION_HISTORIC_SCAN' => 'false' @@ -36,27 +38,29 @@ def initialize(context, base_variables: {}) end def execute(actions) - actions = actions.select do |action| - valid_scan_type?(action[:scan]) && pipeline_scan_type?(action[:scan].to_s) - end + measure(HISTOGRAM, project_id: project.id, action_count: actions.size) do + actions = actions.select do |action| + valid_scan_type?(action[:scan]) && pipeline_scan_type?(action[:scan].to_s) + end - on_demand_scan_actions, other_actions = actions.partition do |action| - on_demand_scan_type?(action[:scan].to_s) - end + on_demand_scan_actions, other_actions = actions.partition do |action| + on_demand_scan_type?(action[:scan].to_s) + end - pipeline_scan_configs = other_actions.map.with_index do |action, index| - prepare_policy_configuration(action, index) - end + pipeline_scan_configs = other_actions.map.with_index do |action, index| + prepare_policy_configuration(action, index) + end - on_demand_configs = prepare_on_demand_policy_configuration(on_demand_scan_actions) + on_demand_configs = prepare_on_demand_policy_configuration(on_demand_scan_actions) - pipeline_variables = collect_config_variables(other_actions, pipeline_scan_configs) - on_demand_variables = collect_config_variables(on_demand_scan_actions, on_demand_configs) - variables = pipeline_variables.merge(on_demand_variables) + pipeline_variables = collect_config_variables(other_actions, pipeline_scan_configs) + on_demand_variables = collect_config_variables(on_demand_scan_actions, on_demand_configs) + variables = pipeline_variables.merge(on_demand_variables) - { pipeline_scan: pipeline_scan_configs.reduce({}, :merge), - on_demand: on_demand_configs.reduce({}, :merge), - variables: variables } + { pipeline_scan: pipeline_scan_configs.reduce({}, :merge), + on_demand: on_demand_configs.reduce({}, :merge), + variables: variables } + end end private @@ -119,6 +123,8 @@ def scan_variables_with_action_variables(action, fallback: {}) def allow_restricted_variables? Feature.enabled?(:allow_restricted_variables_at_policy_level, project, type: :beta) end + + delegate :measure, to: Security::SecurityOrchestrationPolicies::ObserveHistogramsService end end end diff --git a/ee/app/workers/security/process_scan_result_policy_worker.rb b/ee/app/workers/security/process_scan_result_policy_worker.rb index a58fcc9029077683a52c6cc04e0d09aba62ca3c6..47feea919ebb1e89145828e5d1742afb9bc5db61 100644 --- a/ee/app/workers/security/process_scan_result_policy_worker.rb +++ b/ee/app/workers/security/process_scan_result_policy_worker.rb @@ -11,10 +11,10 @@ class ProcessScanResultPolicyWorker sidekiq_options retry: true feature_category :security_policy_management - HISTOGRAM_BUCKETS = [120, 240, 360, 480, 600, 720, 840, 960].freeze + HISTOGRAM = :gitlab_security_policies_scan_result_process_duration_seconds def perform(project_id, configuration_id) - measure(project_id, configuration_id) do + measure(HISTOGRAM, project_id: project_id, configuration_id: configuration_id) do project = Project.find_by_id(project_id) configuration = Security::OrchestrationPolicyConfiguration.find_by_id(configuration_id) break unless project && configuration @@ -50,20 +50,6 @@ def sync_policies(project, configuration, active_scan_result_policies) end end - def measure(project_id, configuration_id) - lo = ::Gitlab::Metrics::System.monotonic_time - yield - hi = ::Gitlab::Metrics::System.monotonic_time - - histogram.observe({ project_id: project_id, configuration_id: configuration_id }, hi - lo) - end - - def histogram - Gitlab::Metrics.histogram( - :gitlab_security_policies_scan_result_process_duration_seconds, - 'The amount of time to process scan result policies', - {}, - HISTOGRAM_BUCKETS) - end + delegate :measure, to: Security::SecurityOrchestrationPolicies::ObserveHistogramsService end end diff --git a/ee/spec/services/security/security_orchestration_policies/observe_histograms_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/observe_histograms_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..5fbe9d6350baf28b1f04260de8f727f94b416d9e --- /dev/null +++ b/ee/spec/services/security/security_orchestration_policies/observe_histograms_service_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::SecurityOrchestrationPolicies::ObserveHistogramsService, feature_category: :security_policy_management do + let(:name) { :gitlab_security_policies_scan_execution_configuration_rendering_seconds } + let(:histogram) { described_class.histogram(name) } + + describe '.histogram' do + let(:description) { described_class::HISTOGRAMS.dig(name, :description) } + let(:buckets) { described_class::HISTOGRAMS.dig(name, :buckets) } + + it 'returns the expected histogram', :aggregate_failures do + expect(histogram.name).to be(name) + expect(histogram.docstring).to eq(description) + expect(histogram.instance_variable_get(:@buckets)).to eq(buckets) + end + end + + describe '.measure' do + let(:labels) { { foo: "bar" } } + let(:return_value) { Object.new } + + subject(:measure) { described_class.measure(name, **labels) { return_value } } + + before do + allow(Gitlab::Metrics::System).to receive(:monotonic_time).twice.and_return(1, 2) + end + + it 'observes' do + expect(histogram).to receive(:observe).with(labels, 1.0) + + measure + end + + it 'returns the block return value' do + allow(histogram).to receive(:observe).with(labels, 1.0) + expect(measure).to be(return_value) + end + end +end diff --git a/ee/spec/services/security/security_orchestration_policies/scan_pipeline_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/scan_pipeline_service_spec.rb index bd33355c159c1dedcbec60c96610d4ca924acbef..f15d23387d37ce8cb336acb99371180e335edf3a 100644 --- a/ee/spec/services/security/security_orchestration_policies/scan_pipeline_service_spec.rb +++ b/ee/spec/services/security/security_orchestration_policies/scan_pipeline_service_spec.rb @@ -189,5 +189,19 @@ it_behaves_like 'creates scan jobs', pipeline_scan_job_templates: %w[Jobs/Secret-Detection], variables: { 'secret-detection-0': { 'SECRET_DETECTION_HISTORIC_SCAN' => 'false', 'SECRET_DETECTION_EXCLUDED_PATHS' => '' } } end end + + describe 'metrics' do + let(:actions) { [{ scan: 'secret_detection' }, { scan: 'container_scanning' }] } + let(:description) { histograms.dig(described_class::HISTOGRAM, :description) } + let(:labels) { { project_id: project.id, action_count: actions.count } } + + specify do + hist = Security::SecurityOrchestrationPolicies::ObserveHistogramsService.histogram(described_class::HISTOGRAM) + + expect(hist).to receive(:observe).with(labels, kind_of(Float)).and_call_original + + subject + end + end end end diff --git a/ee/spec/workers/security/process_scan_result_policy_worker_spec.rb b/ee/spec/workers/security/process_scan_result_policy_worker_spec.rb index aa7c0cf02efd042bb3c9f047b52f8adabb300b3d..b46a1c38840c2ef7776bf965709323880a7cb0fa 100644 --- a/ee/spec/workers/security/process_scan_result_policy_worker_spec.rb +++ b/ee/spec/workers/security/process_scan_result_policy_worker_spec.rb @@ -60,24 +60,15 @@ subject(:worker) { described_class.new } describe 'metrics' do - specify do - expect(Gitlab::Metrics).to receive(:histogram).with( - :gitlab_security_policies_scan_result_process_duration_seconds, - 'The amount of time to process scan result policies', - {}, - described_class::HISTOGRAM_BUCKETS - ).and_call_original - - worker.perform(configuration.project_id, configuration.id) - end + let(:histograms) { Security::SecurityOrchestrationPolicies::ObserveHistogramsService::HISTOGRAMS } + let(:description) { histograms.dig(described_class::HISTOGRAM, :description) } + let(:labels) { { project_id: configuration.project_id, configuration_id: configuration.id } } specify do - histogram = instance_double('Prometheus::Client::Histogram') - expect(histogram).to receive(:observe).with( - { project_id: configuration.project_id, - configuration_id: configuration.id }, an_instance_of(Float)) + hist = Security::SecurityOrchestrationPolicies::ObserveHistogramsService.histogram(described_class::HISTOGRAM) + + expect(hist).to receive(:observe).with(labels, kind_of(Float)).and_call_original - allow(worker).to receive(:histogram).and_return(histogram) worker.perform(configuration.project_id, configuration.id) end end