HADOOP-8757. Metrics should disallow names with invalid characters (rchiang via rkanter)

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -483,6 +483,9 @@ Release 2.7.0 - UNRELEASED
     HADOOP-11481. ClassCastException while using a key created by keytool to
     create encryption zone. (Charles Lamb via Colin P. Mccabe)
 
+    HADOOP-8757. Metrics should disallow names with invalid characters
+    (rchiang via rkanter)
+
   OPTIMIZATIONS
 
     HADOOP-11323. WritableComparator#compare keeps reference to byte array.

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MetricsRegistry.java

@@ -363,6 +363,20 @@ Collection<MutableMetric> metrics() {
   }
 
   private void checkMetricName(String name) {
+    // Check for invalid characters in metric name
+    boolean foundWhitespace = false;
+    for (int i = 0; i < name.length(); i++) {
+      char c = name.charAt(i);
+      if (Character.isWhitespace(c)) {
+        foundWhitespace = true;
+        break;
+      }
+    }
+    if (foundWhitespace) {
+      throw new MetricsException("Metric name '"+ name +
+          "' contains illegal whitespace character");
+    }
+    // Check if name has already been registered
     if (metricsMap.containsKey(name)) {
       throw new MetricsException("Metric name "+ name +" already exists!");
     }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/lib/TestMetricsRegistry.java

@@ -18,6 +18,7 @@
 
 package org.apache.hadoop.metrics2.lib;
 
+import org.junit.Ignore;
 import org.junit.Test;
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
@@ -56,6 +57,46 @@ public class TestMetricsRegistry {
     });
   }
 
+  /**
+   * Test adding metrics with whitespace in the name
+   */
+  @Test
+  public void testMetricsRegistryIllegalMetricNames() {
+    final MetricsRegistry r = new MetricsRegistry("test");
+    // Fill up with some basics
+    r.newCounter("c1", "c1 desc", 1);
+    r.newGauge("g1", "g1 desc", 1);
+    r.newQuantiles("q1", "q1 desc", "q1 name", "q1 val type", 1);
+    // Add some illegal names
+    expectMetricsException("Metric name 'badcount 2' contains "+
+        "illegal whitespace character", new Runnable() {
+      @Override
+      public void run() { r.newCounter("badcount 2", "c2 desc", 2); }
+    });
+    expectMetricsException("Metric name 'badcount3  ' contains "+
+        "illegal whitespace character", new Runnable() {
+      @Override
+      public void run() { r.newCounter("badcount3  ", "c3 desc", 3); }
+    });
+    expectMetricsException("Metric name '  badcount4' contains "+
+        "illegal whitespace character", new Runnable() {
+      @Override
+      public void run() { r.newCounter("  badcount4", "c4 desc", 4); }
+    });
+    expectMetricsException("Metric name 'withtab5	' contains "+
+        "illegal whitespace character", new Runnable() {
+      @Override
+      public void run() { r.newCounter("withtab5	", "c5 desc", 5); }
+    });
+    expectMetricsException("Metric name 'withnewline6\n' contains "+
+        "illegal whitespace character", new Runnable() {
+      @Override
+      public void run() { r.newCounter("withnewline6\n", "c6 desc", 6); }
+    });
+    // Final validation
+    assertEquals("num metrics in registry", 3, r.metrics().size());
+  }
+
   /**
    * Test the add by name method
    */
@@ -81,6 +122,7 @@ public class TestMetricsRegistry {
     });
   }
 
+  @Ignore
   private void expectMetricsException(String prefix, Runnable fun) {
     try {
       fun.run();