From b6dcd772282045633efe223b7d0932937c13c1f7 Mon Sep 17 00:00:00 2001 From: John O'Connor Date: Tue, 16 Jun 2020 17:11:33 +0100 Subject: [PATCH] (POOLER-170) Revise vmpooler usage stats Break down the usage stats into smaller groups so as to manage the number of stat lines collected for Prometheus. This may need some further revision to filter out Litmus stats, or otherwise collect litmus usage information. --- lib/vmpooler/metrics/promstats.rb | 30 ++++++---- lib/vmpooler/pool_manager.rb | 34 ++++------- spec/unit/pool_manager_spec.rb | 93 ++++++++++++++++++++++--------- spec/unit/promstats_spec.rb | 36 +++++++++--- 4 files changed, 126 insertions(+), 67 deletions(-) diff --git a/lib/vmpooler/metrics/promstats.rb b/lib/vmpooler/metrics/promstats.rb index 86ca64c..9a9e7a6 100644 --- a/lib/vmpooler/metrics/promstats.rb +++ b/lib/vmpooler/metrics/promstats.rb @@ -43,21 +43,29 @@ module Vmpooler }, param_labels: %i[template_name] }, - usage: { + user: { mtype: M_COUNTER, - docstring: 'Number of Pool Instances of this created', - prom_metric_prefix: "#{@metrics_prefix}_usage", + docstring: 'Number of Pool Instances this user created created', + prom_metric_prefix: "#{@metrics_prefix}_user", param_labels: %i[user poolname] }, - user: { - # This metrics is leads to a lot of label values which is likely to challenge data storage - # on prometheus - see Best Practices: https://prometheus.io/docs/practices/naming/#labels - # So it is likely that this metric may need to be simplified or broken into a number - # of smaller metrics to capture the detail without challenging prometheus + usage_jenkins_instance: { mtype: M_COUNTER, - docstring: 'vmpooler user counters', - prom_metric_prefix: "#{@metrics_prefix}_user", - param_labels: %i[user instancex value_stream branch project job_name component_to_test poolname] + docstring: 'Pools by Jenkins Instance usage', + prom_metric_prefix: "#{@metrics_prefix}_usage_jenkins_instance", + param_labels: %i[jenkins_instance value_stream poolname] + }, + usage_branch_project: { + mtype: M_COUNTER, + docstring: 'Pools by branch/project usage', + prom_metric_prefix: "#{@metrics_prefix}_usage_branch_project", + param_labels: %i[branch project poolname] + }, + usage_job_component: { + mtype: M_COUNTER, + docstring: 'Pools by job/component usage', + prom_metric_prefix: "#{@metrics_prefix}_usage_job_component", + param_labels: %i[job_name component_to_test poolname] }, checkout: { mtype: M_COUNTER, diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index bed6a48..cdd6f63 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -491,15 +491,16 @@ module Vmpooler return if checkout.nil? user ||= 'unauthenticated' - unless jenkins_build_url - user = user.gsub('.', '_') - $metrics.increment("usage.#{user}.#{poolname}") - return - end + user = user.gsub('.', '_') + $metrics.increment("user.#{user}.#{poolname}") + return unless jenkins_build_url + + # TBD - Add Filter for Litmus here as well - to ignore for the moment. url_parts = jenkins_build_url.split('/')[2..-1] - instance = url_parts[0] + jenkins_instance = url_parts[0].gsub('.', '_') value_stream_parts = url_parts[2].split('_') + value_stream_parts = value_stream_parts.map { |s| s.gsub('.', '_') } value_stream = value_stream_parts.shift branch = value_stream_parts.pop project = value_stream_parts.shift @@ -507,22 +508,9 @@ module Vmpooler build_metadata_parts = url_parts[3] component_to_test = component_to_test('RMM_COMPONENT_TO_TEST_NAME', build_metadata_parts) - metric_parts = [ - 'usage', - user, - instance, - value_stream, - branch, - project, - job_name, - component_to_test, - poolname - ] - - metric_parts = metric_parts.reject(&:nil?) - metric_parts = metric_parts.map { |s| s.gsub('.', '_') } - - $metrics.increment(metric_parts.join('.')) + $metrics.increment("usage_jenkins_instance.#{jenkins_instance}.#{value_stream}.#{poolname}") + $metrics.increment("usage_branch_project.#{branch}.#{project}.#{poolname}") + $metrics.increment("usage_job_component.#{job_name}.#{component_to_test}.#{poolname}") rescue StandardError => e $logger.log('d', "[!] [#{poolname}] failed while evaluating usage labels on '#{vm}' with an error: #{e}") raise @@ -537,7 +525,7 @@ module Vmpooler next if value.nil? return value if key == match end - nil + 'none' end def purge_unused_vms_and_folders diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index efe6b0e..ed7d0ae 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -1110,7 +1110,7 @@ EOT it 'should emit a metric' do redis_connection_pool.with do |redis| - expect(metrics).to receive(:increment).with("usage.unauthenticated.#{template}") + expect(metrics).to receive(:increment).with("user.unauthenticated.#{template}") subject.get_vm_usage_labels(vm, redis) end @@ -1126,7 +1126,8 @@ EOT end it 'should emit a metric' do - expect(metrics).to receive(:increment).with("usage.#{user}.#{template}") + expect(metrics).to receive(:increment).with("user.#{user}.#{template}") + expect(metrics).not_to receive(:increment) redis_connection_pool.with do |redis| subject.get_vm_usage_labels(vm, redis) @@ -1135,7 +1136,7 @@ EOT context 'with a user with period in name' do let(:user) { 'test.user'.gsub('.', '_') } - let(:metric_string) { "usage.#{user}.#{template}" } + let(:metric_string) { "user.#{user}.#{template}" } let(:metric_nodes) { metric_string.split('.') } before(:each) do @@ -1146,6 +1147,7 @@ EOT it 'should emit a metric with the character replaced' do expect(metrics).to receive(:increment).with(metric_string) + expect(metrics).not_to receive(:increment) redis_connection_pool.with do |redis| subject.get_vm_usage_labels(vm, redis) @@ -1155,7 +1157,6 @@ EOT it 'should include three nodes' do expect(metric_nodes.count).to eq(3) end - end context 'with a jenkins_build_url label' do @@ -1167,16 +1168,11 @@ EOT let(:branch) { value_stream_parts.pop } let(:project) { value_stream_parts.shift } let(:job_name) { value_stream_parts.join('_') } - let(:metric_string_nodes) { - [ - 'usage', user, instance, value_stream, branch, project, job_name, template - ] - } - let(:metric_string_sub) { - metric_string_nodes.map { |s| s.gsub('.', '_') - } - } - let(:metric_string) { metric_string_sub.join('.') } + + let(:metric_string_1) { "user.#{user}.#{template}" } + let(:metric_string_2) { "usage_jenkins_instance.#{instance.gsub('.', '_')}.#{value_stream.gsub('.', '_')}.#{template}" } + let(:metric_string_3) { "usage_branch_project.#{branch.gsub('.', '_')}.#{project.gsub('.', '_')}.#{template}" } + let(:metric_string_4) { "usage_job_component.#{job_name.gsub('.', '_')}.none.#{template}" } before(:each) do redis_connection_pool.with do |redis| @@ -1184,8 +1180,12 @@ EOT end end - it 'should emit a metric with information from the URL' do - expect(metrics).to receive(:increment).with(metric_string) + it 'should emit 4 metric withs information from the URL' do + expect(metrics).to receive(:increment).with(metric_string_1) + expect(metrics).to receive(:increment).with(metric_string_2) + expect(metrics).to receive(:increment).with(metric_string_3) + expect(metrics).to receive(:increment).with(metric_string_4) + expect(metrics).not_to receive(:increment) redis_connection_pool.with do |redis| subject.get_vm_usage_labels(vm, redis) @@ -1207,14 +1207,23 @@ EOT let(:expected_string) { "usage.#{user}.#{instance}.#{value_stream}.#{branch}.#{project}.#{job_name}.#{build_component}.#{template}" } let(:metric_nodes) { expected_string.split('.') } + let(:metric_string_1) { "user.#{user}.#{template}" } + let(:metric_string_2) { "usage_jenkins_instance.#{instance.gsub('.', '_')}.#{value_stream.gsub('.', '_')}.#{template}" } + let(:metric_string_3) { "usage_branch_project.#{branch.gsub('.', '_')}.#{project.gsub('.', '_')}.#{template}" } + let(:metric_string_4) { "usage_job_component.#{job_name.gsub('.', '_')}.#{build_component}.#{template}" } + before(:each) do redis_connection_pool.with do |redis| create_tag(vm, 'jenkins_build_url', jenkins_build_url, redis) end end - it 'should emit a metric with information from the URL' do - expect(metrics).to receive(:increment).with(expected_string) + it 'should emit 4 metrics with information from the URL' do + expect(metrics).to receive(:increment).with(metric_string_1) + expect(metrics).to receive(:increment).with(metric_string_2) + expect(metrics).to receive(:increment).with(metric_string_3) + expect(metrics).to receive(:increment).with(metric_string_4) + expect(metrics).not_to receive(:increment) redis_connection_pool.with do |redis| subject.get_vm_usage_labels(vm, redis) @@ -1236,20 +1245,54 @@ EOT let(:project) { value_stream_parts.shift } let(:job_name) { value_stream_parts.join('_') } + let(:metric_string_1) { "user.#{user}.#{template}" } + let(:metric_string_2) { "usage_jenkins_instance.#{instance.gsub('.', '_')}.#{value_stream.gsub('.', '_')}.#{template}" } + let(:metric_string_3) { "usage_branch_project.#{branch.gsub('.', '_')}.#{project.gsub('.', '_')}.#{template}" } + let(:metric_string_4) { "usage_job_component.#{job_name.gsub('.', '_')}.none.#{template}" } + before(:each) do redis_connection_pool.with do |redis| create_tag(vm, 'jenkins_build_url', jenkins_build_url, redis) end end - it 'should emit a metric with information from the URL without a build_component' do - expect(metrics).to receive(:increment).with("usage.#{user}.#{instance}.#{value_stream}.#{branch}.#{project}.#{job_name}.#{template}") - + it 'should emit 4 metrics with information from the URL without a build_component' do + expect(metrics).to receive(:increment).with(metric_string_1) + expect(metrics).to receive(:increment).with(metric_string_2) + expect(metrics).to receive(:increment).with(metric_string_3) + expect(metrics).to receive(:increment).with(metric_string_4) + expect(metrics).not_to receive(:increment) + redis_connection_pool.with do |redis| subject.get_vm_usage_labels(vm, redis) end end end + + + end + + context 'with a litmus job' do + let(:jenkins_build_url) { 'https://litmus_manual' } + + let(:metric_string_1) { "user.#{user}.#{template}" } + let(:metric_string_2) { "usage_litmus.#{user}.#{template}" } + + before(:each) do + redis_connection_pool.with do |redis| + create_tag(vm, 'jenkins_build_url', jenkins_build_url, redis) + end + end + + it 'should emit 2 metrics with the second indicating a litmus job' do + expect(metrics).to receive(:increment).with(metric_string_1) + expect(metrics).to receive(:increment).with(metric_string_2) + expect(metrics).not_to receive(:increment) + + redis_connection_pool.with do |redis| + subject.get_vm_usage_labels(vm, redis) + end + end end end @@ -1269,21 +1312,21 @@ EOT end context 'when match contains no value' do - it 'should return nil' do - expect(subject.component_to_test(matching_key, matching_key)).to be nil + it 'should return none' do + expect(subject.component_to_test(matching_key, matching_key)).to eq('none') end end end context 'when string contains no key value pairs' do it 'should return' do - expect(subject.component_to_test(matching_key, nonmatrix_string)).to be nil + expect(subject.component_to_test(matching_key, nonmatrix_string)).to eq('none') end end context 'when labels_string is a job number' do it 'should return nil' do - expect(subject.component_to_test(matching_key, '25')).to be nil + expect(subject.component_to_test(matching_key, '25')).to eq('none') end end diff --git a/spec/unit/promstats_spec.rb b/spec/unit/promstats_spec.rb index 5db495d..d721cce 100644 --- a/spec/unit/promstats_spec.rb +++ b/spec/unit/promstats_spec.rb @@ -142,22 +142,42 @@ describe 'prometheus' do po.get(labels: metric[:labels]) }.by(1) end - it 'Increments usage.#{user}.#{poolname}' do + it 'Increments user.#{user}.#{poolname}' do user = 'myuser' poolname = 'test-pool' - expect { subject.increment("usage.#{user}.#{poolname}") }.to change { - metric, po = subject.get("usage.#{user}.#{poolname}") + expect { subject.increment("user.#{user}.#{poolname}") }.to change { + metric, po = subject.get("user.#{user}.#{poolname}") po.get(labels: metric[:labels]) }.by(1) end - it 'Increments label :user' do - # subject.increment(:user, :instance, :value_stream, :branch, :project, :job_name, :component_to_test, :poolname) - showing labels here - pending 'increment only supports a string containing a dot separator' - expect { subject.increment(:user) }.to change { - metric, po = subject.get(:user) + it 'Increments label usage_jenkins_instance.#{jenkins_instance}.#{value_stream}.#{poolname}' do + jenkins_instance = 'jenkins_test_instance' + value_stream = 'notional_value' + poolname = 'test-pool' + expect { subject.increment("usage_jenkins_instance.#{jenkins_instance}.#{value_stream}.#{poolname}") }.to change { + metric, po = subject.get("usage_jenkins_instance.#{jenkins_instance}.#{value_stream}.#{poolname}") po.get(labels: metric[:labels]) }.by(1) end + it 'Increments label usage_branch_project.#{branch}.#{project}.#{poolname}' do + branch = 'treetop' + project = 'test-project' + poolname = 'test-pool' + expect { subject.increment("usage_branch_project.#{branch}.#{project}.#{poolname}") }.to change { + metric, po = subject.get("usage_branch_project.#{branch}.#{project}.#{poolname}") + po.get(labels: metric[:labels]) + }.by(1) + end + it 'Increments label usage_job_component.#{job_name}.#{component_to_test}.#{poolname}' do + job_name = 'a-job' + component_to_test = 'component-name' + poolname = 'test-pool' + expect { subject.increment("usage_job_component.#{job_name}.#{component_to_test}.#{poolname}") }.to change { + metric, po = subject.get("usage_job_component.#{job_name}.#{component_to_test}.#{poolname}") + po.get(labels: metric[:labels]) + }.by(1) + end + it 'Increments connect.open' do expect { subject.increment('connect.open') }.to change { metric, po = subject.get('connect.open')