Ensure nodes are consistent for usage stats

This commit updates vm usage stats collection to replace all instances of '.' characters within node strings. Without this change the node string containing a '.' character causes the metric to be interpreted as containing another node.

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/' }