Merge pull request #470 from shaylang/pr_155_take2

backends/graphite.js

@@ -97,15 +97,23 @@ var flush_stats = function graphite_flush(ts, metrics) {
   var timer_data = metrics.timer_data;
   var statsd_metrics = metrics.statsd_metrics;
 
+  // Sanitize key for graphite if not done globally
+  function sk(key) {
+      return key.replace(/\s+/g, '_')
+                .replace(/\//g, '-')
+                .replace(/[^a-zA-Z_\-0-9\.]/g, '');
+  };
+
   for (key in counters) {
-    var namespace = counterNamespace.concat(key);
     var value = counters[key];
     var valuePerSecond = counter_rates[key]; // pre-calculated "per second" rate
+    var keyName = sk(key);
+    var namespace = counterNamespace.concat(keyName);
 
     if (legacyNamespace === true) {
       statString += namespace.join(".")   + globalSuffix + valuePerSecond + ts_suffix;
       if (flush_counts) {
-        statString += 'stats_counts.' + key + globalSuffix + value + ts_suffix;
+        statString += 'stats_counts.' + keyName + globalSuffix + value + ts_suffix;
       }
     } else {
       statString += namespace.concat('rate').join(".")  + globalSuffix + valuePerSecond + ts_suffix;
@@ -118,8 +126,9 @@ var flush_stats = function graphite_flush(ts, metrics) {
   }
 
   for (key in timer_data) {
-    var namespace = timerNamespace.concat(key);
+    var namespace = timerNamespace.concat(sk(key));
     var the_key = namespace.join(".");
+
     for (timer_data_key in timer_data[key]) {
       if (typeof(timer_data[key][timer_data_key]) === 'number') {
         statString += the_key + '.' + timer_data_key + globalSuffix + timer_data[key][timer_data_key] + ts_suffix;
@@ -137,13 +146,13 @@ var flush_stats = function graphite_flush(ts, metrics) {
   }
 
   for (key in gauges) {
-    var namespace = gaugesNamespace.concat(key);
+    var namespace = gaugesNamespace.concat(sk(key));
     statString += namespace.join(".") + globalSuffix + gauges[key] + ts_suffix;
     numStats += 1;
   }
 
   for (key in sets) {
-    var namespace = setsNamespace.concat(key);
+    var namespace = setsNamespace.concat(sk(key));
     statString += namespace.join(".") + '.count' + globalSuffix + sets[key].values().length + ts_suffix;
     numStats += 1;
   }

stats.js

@@ -184,8 +184,7 @@ config.configFile(process.argv[2], function (config) {
   counters[packets_received] = 0;
   counters[metrics_received] = 0;
 
-  if (!serverLoaded) {
-
+    if (!serverLoaded) {
     // key counting
     var keyFlushInterval = Number((config.keyFlush && config.keyFlush.interval) || 0);
 
@@ -211,10 +210,7 @@ config.configFile(process.argv[2], function (config) {
           l.log(metrics[midx].toString());
         }
         var bits = metrics[midx].toString().split(':');
-        var key = bits.shift()
-                      .replace(/\s+/g, '_')
-                      .replace(/\//g, '-')
-                      .replace(/[^a-zA-Z_\-0-9\.]/g, '');
+        var key = bits.shift();
 
         if (keyFlushInterval > 0) {
           if (! keyCounter[key]) {

test/graphite_tests.js

@@ -358,5 +358,24 @@ module.exports = {
         });
       });
     });
+  },
+
+  metric_names_are_sanitized: function(test) {
+    var me = this;
+    this.acceptor.once('connection', function(c) {
+      statsd_send('fo/o:250|c',me.sock,'127.0.0.1',8125,function(){
+        statsd_send('b ar:250|c',me.sock,'127.0.0.1',8125,function(){
+          statsd_send('foo+bar:250|c',me.sock,'127.0.0.1',8125,function(){
+            collect_for(me.acceptor, me.myflush * 2, function(strings){
+              var str = strings.join();
+              test.ok(str.indexOf('fo-o') !== -1, "Did not map 'fo/o' => 'fo-o'");
+              test.ok(str.indexOf('b_ar') !== -1, "Did not map 'b ar' => 'b_ar'");
+              test.ok(str.indexOf('foobar') !== -1, "Did not map 'foo+bar' => 'foobar'");
+              test.done();
+            });
+          });
+        });
+      });
+    });
   }
 }