HADOOP-9467. Metrics2 record filter should check name as well as tags. (Ganeshan Iyler via llu)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-1.0@1466388 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -8,6 +8,9 @@ Release 1.0.5 - unreleased
 
   BUG FIXES
 
+    HADOOP-9467. Metrics2 record filter should check name as well as tags.
+    (Ganeshan Iyler via llu)
+
     HADOOP-8419. Fixed GzipCode NPE reset for IBM JDK. (Yu Li via eyang)
 
     MAPREDUCE-4272. SortedRanges.Range#compareTo is not spec compliant.

src/core/org/apache/hadoop/metrics2/MetricsFilter.java

@@ -54,7 +54,7 @@ public abstract class MetricsFilter implements MetricsPlugin {
    * @return  true to accept; false otherwise.
    */
   public boolean accepts(MetricsRecord record) {
-    return accepts(record.tags());
+    return accepts(record.name()) && accepts(record.tags());
   }
 
 }

src/test/org/apache/hadoop/metrics2/filter/TestPatternFilter.java

@@ -24,7 +24,9 @@ import java.util.List;
 import org.apache.commons.configuration.SubsetConfiguration;
 import org.junit.Test;
 import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
 
+import org.apache.hadoop.metrics2.MetricsRecord;
 import org.apache.hadoop.metrics2.MetricsTag;
 import org.apache.hadoop.metrics2.impl.ConfigBuilder;
 
@@ -37,6 +39,8 @@ public class TestPatternFilter {
     SubsetConfiguration empty = new ConfigBuilder().subset("");
     shouldAccept(empty, "anything");
     shouldAccept(empty, Arrays.asList(new MetricsTag("key", "desc", "value")));
+    shouldAccept(empty, mockMetricsRecord("anything", Arrays.asList(
+      new MetricsTag("key", "desc", "value"))));
   }
 
   /**
@@ -49,9 +53,16 @@ public class TestPatternFilter {
     shouldAccept(wl, "foo");
     shouldAccept(wl, Arrays.asList(new MetricsTag("bar", "", ""),
                                    new MetricsTag("foo", "", "f")));
+    shouldAccept(wl, mockMetricsRecord("foo", Arrays.asList(
+      new MetricsTag("bar", "", ""),
+      new MetricsTag("foo", "", "f"))));
     shouldReject(wl, "bar");
     shouldReject(wl, Arrays.asList(new MetricsTag("bar", "", "")));
     shouldReject(wl, Arrays.asList(new MetricsTag("foo", "", "boo")));
+    shouldReject(wl, mockMetricsRecord("bar", Arrays.asList(
+      new MetricsTag("foo", "", "f"))));
+    shouldReject(wl, mockMetricsRecord("foo", Arrays.asList(
+      new MetricsTag("bar", "", ""))));
   }
 
   /**
@@ -63,9 +74,16 @@ public class TestPatternFilter {
         .add("p.exclude.tags", "foo:f").subset("p");
     shouldAccept(bl, "bar");
     shouldAccept(bl, Arrays.asList(new MetricsTag("bar", "", "")));
+    shouldAccept(bl, mockMetricsRecord("bar", Arrays.asList(
+      new MetricsTag("bar", "", ""))));
     shouldReject(bl, "foo");
     shouldReject(bl, Arrays.asList(new MetricsTag("bar", "", ""),
                                    new MetricsTag("foo", "", "f")));
+    shouldReject(bl, mockMetricsRecord("foo", Arrays.asList(
+      new MetricsTag("bar", "", ""))));
+    shouldReject(bl, mockMetricsRecord("bar", Arrays.asList(
+      new MetricsTag("bar", "", ""),
+      new MetricsTag("foo", "", "f"))));
   }
 
   /**
@@ -80,10 +98,18 @@ public class TestPatternFilter {
         .add("p.exclude.tags", "bar:b").subset("p");
     shouldAccept(c, "foo");
     shouldAccept(c, Arrays.asList(new MetricsTag("foo", "", "f")));
+    shouldAccept(c, mockMetricsRecord("foo", Arrays.asList(
+      new MetricsTag("foo", "", "f"))));
     shouldReject(c, "bar");
     shouldReject(c, Arrays.asList(new MetricsTag("bar", "", "b")));
+    shouldReject(c, mockMetricsRecord("bar", Arrays.asList(
+      new MetricsTag("foo", "", "f"))));
+    shouldReject(c, mockMetricsRecord("foo", Arrays.asList(
+      new MetricsTag("bar", "", "b"))));
     shouldAccept(c, "foobar");
     shouldAccept(c, Arrays.asList(new MetricsTag("foobar", "", "")));
+    shouldAccept(c, mockMetricsRecord("foobar", Arrays.asList(
+      new MetricsTag("foobar", "", ""))));
   }
 
   /**
@@ -97,6 +123,8 @@ public class TestPatternFilter {
         .add("p.exclude.tags", "foo:f").subset("p");
     shouldAccept(c, "foo");
     shouldAccept(c, Arrays.asList(new MetricsTag("foo", "", "f")));
+    shouldAccept(c, mockMetricsRecord("foo", Arrays.asList(
+      new MetricsTag("foo", "", "f"))));
   }
 
   static void shouldAccept(SubsetConfiguration conf, String s) {
@@ -109,6 +137,17 @@ public class TestPatternFilter {
     assertTrue("accepts "+ tags, newRegexFilter(conf).accepts(tags));
   }
 
+  /**
+   * Asserts that filters with the given configuration accept the given record.
+   * 
+   * @param conf SubsetConfiguration containing filter configuration
+   * @param record MetricsRecord to check
+   */
+  static void shouldAccept(SubsetConfiguration conf, MetricsRecord record) {
+    assertTrue("accepts " + record, newGlobFilter(conf).accepts(record));
+    assertTrue("accepts " + record, newRegexFilter(conf).accepts(record));
+  }
+
   static void shouldReject(SubsetConfiguration conf, String s) {
     assertTrue("rejects "+ s, !newGlobFilter(conf).accepts(s));
     assertTrue("rejects "+ s, !newRegexFilter(conf).accepts(s));
@@ -119,6 +158,17 @@ public class TestPatternFilter {
     assertTrue("rejects "+ tags, !newRegexFilter(conf).accepts(tags));
   }
 
+  /**
+   * Asserts that filters with the given configuration reject the given record.
+   * 
+   * @param conf SubsetConfiguration containing filter configuration
+   * @param record MetricsRecord to check
+   */
+  static void shouldReject(SubsetConfiguration conf, MetricsRecord record) {
+    assertTrue("rejects " + record, !newGlobFilter(conf).accepts(record));
+    assertTrue("rejects " + record, !newRegexFilter(conf).accepts(record));
+  }
+
   /**
    * Create a new glob filter with a config object
    * @param conf  the config object
@@ -140,4 +190,19 @@ public class TestPatternFilter {
     f.init(conf);
     return f;
   }
+
+  /**
+   * Creates a mock MetricsRecord with the given name and tags.
+   * 
+   * @param name String name
+   * @param tags List<MetricsTag> tags
+   * @return MetricsRecord newly created mock
+   */
+  private static MetricsRecord mockMetricsRecord(String name,
+      List<MetricsTag> tags) {
+    MetricsRecord record = mock(MetricsRecord.class);
+    when(record.name()).thenReturn(name);
+    when(record.tags()).thenReturn(tags);
+    return record;
+  }
 }