Merge pull request #308 from mattkirby/control_your_layers

Ensure nodes are consistent for usage stats

CHANGELOG.md

@@ -12,6 +12,9 @@ If you're looking for changes from before this, refer to the project's
 git logs & PR history.
 # [Unreleased](https://github.com/puppetlabs/vmpooler/compare/0.2.3...master)
 
+### Fixed
+- Ensure that metric nodes for vm usage stats are consistent
+
 # [0.2.3](https://github.com/puppetlabs/vmpooler/compare/0.2.2...0.2.3)
 
 ### Fixed

lib/vmpooler/pool_manager.rb

@@ -333,12 +333,13 @@ module Vmpooler
       poolname = $redis.hget("vmpooler__vm__#{vm}", "template")
 
       unless jenkins_build_url
+        user = user.gsub('.', '_')
         $metrics.increment("usage.#{user}.#{poolname}")
         return
       end
 
       url_parts = jenkins_build_url.split('/')[2..-1]
-      instance = url_parts[0].gsub('.', '_')
+      instance = url_parts[0]
       value_stream_parts = url_parts[2].split('_')
       value_stream = value_stream_parts.shift
       branch = value_stream_parts.pop
@@ -360,6 +361,7 @@ module Vmpooler
       ]
 
       metric_parts = metric_parts.reject { |s| s.nil? }
+      metric_parts = metric_parts.map { |s| s.gsub('.', '_') }
 
       $metrics.increment(metric_parts.join('.'))
     rescue => err

spec/unit/pool_manager_spec.rb

@@ -867,22 +867,53 @@ EOT
             subject.get_vm_usage_labels(vm)
           end
 
+          context 'with a user with period in name' do
+            let(:user) { 'test.user'.gsub('.', '_') }
+            let(:metric_string) { "usage.#{user}.#{template}" }
+            let(:metric_nodes) { metric_string.split('.') }
+
+            before(:each) do
+              create_running_vm(template, vm)
+            end
+
+            it 'should emit a metric with the character replaced' do
+              expect(metrics).to receive(:increment).with(metric_string)
+
+              subject.get_vm_usage_labels(vm)
+            end
+
+            it 'should include three nodes' do
+              expect(metric_nodes.count).to eq(3)
+            end
+
+          end
+
           context 'with a jenkins_build_url label' do
             let(:jenkins_build_url) { 'https://jenkins.example.com/job/enterprise_pe-acceptance-tests_integration-system_pe_full-agent-upgrade_weekend_2018.1.x/LAYOUT=centos6-64mcd-ubuntu1404-32f-64f,LEGACY_AGENT_VERSION=NONE,PLATFORM=NOTUSED,SCM_BRANCH=2018.1.x,UPGRADE_FROM=2018.1.0,UPGRADE_TO_VERSION=NONE,label=beaker/222/' }
             let(:url_parts) { jenkins_build_url.split('/')[2..-1] }
-            let(:instance) { url_parts[0].gsub('.', '_') }
+            let(:instance) { url_parts[0] }
             let(:value_stream_parts) { url_parts[2].split('_') }
             let(:value_stream) { value_stream_parts.shift }
             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('.') }
 
             before(:each) do
               create_tag(vm, 'jenkins_build_url', jenkins_build_url)
             end
 
             it 'should emit a metric with information from the URL' do
-              expect(metrics).to receive(:increment).with("usage.#{user}.#{instance}.#{value_stream}.#{branch}.#{project}.#{job_name}.#{template}")
+              expect(metrics).to receive(:increment).with(metric_string)
 
               subject.get_vm_usage_labels(vm)
             end
@@ -899,17 +930,23 @@ EOT
             let(:job_name) { value_stream_parts.join('_') }
             let(:build_metadata) { url_parts[3] }
             let(:build_component) { subject.component_to_test('RMM_COMPONENT_TO_TEST_NAME', build_metadata) }
+            let(:expected_string) { "usage.#{user}.#{instance}.#{value_stream}.#{branch}.#{project}.#{job_name}.#{build_component}.#{template}" }
+            let(:metric_nodes) { expected_string.split('.') }
 
             before(:each) do
               create_tag(vm, 'jenkins_build_url', jenkins_build_url)
             end
 
             it 'should emit a metric with information from the URL' do
-              expect(metrics).to receive(:increment).with("usage.#{user}.#{instance}.#{value_stream}.#{branch}.#{project}.#{job_name}.#{build_component}.#{template}")
+              expect(metrics).to receive(:increment).with(expected_string)
 
               subject.get_vm_usage_labels(vm)
             end
 
+            it 'should contain exactly nine nodes' do
+              expect(metric_nodes.count).to eq(9)
+            end
+
             context 'when there is no matrix job information' do
 
               let(:jenkins_build_url) { 'https://jenkins.example.com/job/platform_puppet-agent-extra_puppet-agent-integration-suite_pr/824/' }