From 9be0d30af4f6024984c1880a785d02c6db136469 Mon Sep 17 00:00:00 2001 From: Luke Lu Date: Wed, 10 Apr 2013 08:34:29 +0000 Subject: [PATCH] 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-2@1466381 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 3 + .../apache/hadoop/metrics2/MetricsFilter.java | 2 +- .../metrics2/filter/TestPatternFilter.java | 63 +++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 3b944494b08..064c5a967c9 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -122,6 +122,9 @@ Release 2.0.4-alpha - UNRELEASED BUG FIXES + HADOOP-9467. Metrics2 record filter should check name as well as tags. + (Ganeshan Iyler via llu) + HADOOP-9406. hadoop-client leaks dependency on JDK tools jar. (tucu) HADOOP-9301. hadoop client servlet/jsp/jetty/tomcat JARs creating diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/MetricsFilter.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/MetricsFilter.java index 7baf66f717f..f35ffdf27ad 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/MetricsFilter.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/MetricsFilter.java @@ -55,7 +55,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()); } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/filter/TestPatternFilter.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/filter/TestPatternFilter.java index 29849605e0f..2bdfdb978a9 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/filter/TestPatternFilter.java +++ b/hadoop-common-project/hadoop-common/src/test/java/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; import static org.apache.hadoop.metrics2.lib.Interns.*; @@ -38,6 +40,8 @@ public class TestPatternFilter { SubsetConfiguration empty = new ConfigBuilder().subset(""); shouldAccept(empty, "anything"); shouldAccept(empty, Arrays.asList(tag("key", "desc", "value"))); + shouldAccept(empty, mockMetricsRecord("anything", Arrays.asList( + tag("key", "desc", "value")))); } /** @@ -50,9 +54,15 @@ public class TestPatternFilter { shouldAccept(wl, "foo"); shouldAccept(wl, Arrays.asList(tag("bar", "", ""), tag("foo", "", "f"))); + shouldAccept(wl, mockMetricsRecord("foo", Arrays.asList( + tag("bar", "", ""), tag("foo", "", "f")))); shouldReject(wl, "bar"); shouldReject(wl, Arrays.asList(tag("bar", "", ""))); shouldReject(wl, Arrays.asList(tag("foo", "", "boo"))); + shouldReject(wl, mockMetricsRecord("bar", Arrays.asList( + tag("foo", "", "f")))); + shouldReject(wl, mockMetricsRecord("foo", Arrays.asList( + tag("bar", "", "")))); } /** @@ -64,9 +74,15 @@ public class TestPatternFilter { .add("p.exclude.tags", "foo:f").subset("p"); shouldAccept(bl, "bar"); shouldAccept(bl, Arrays.asList(tag("bar", "", ""))); + shouldAccept(bl, mockMetricsRecord("bar", Arrays.asList( + tag("bar", "", "")))); shouldReject(bl, "foo"); shouldReject(bl, Arrays.asList(tag("bar", "", ""), tag("foo", "", "f"))); + shouldReject(bl, mockMetricsRecord("foo", Arrays.asList( + tag("bar", "", "")))); + shouldReject(bl, mockMetricsRecord("bar", Arrays.asList( + tag("bar", "", ""), tag("foo", "", "f")))); } /** @@ -81,10 +97,18 @@ public class TestPatternFilter { .add("p.exclude.tags", "bar:b").subset("p"); shouldAccept(c, "foo"); shouldAccept(c, Arrays.asList(tag("foo", "", "f"))); + shouldAccept(c, mockMetricsRecord("foo", Arrays.asList( + tag("foo", "", "f")))); shouldReject(c, "bar"); shouldReject(c, Arrays.asList(tag("bar", "", "b"))); + shouldReject(c, mockMetricsRecord("bar", Arrays.asList( + tag("foo", "", "f")))); + shouldReject(c, mockMetricsRecord("foo", Arrays.asList( + tag("bar", "", "b")))); shouldAccept(c, "foobar"); shouldAccept(c, Arrays.asList(tag("foobar", "", ""))); + shouldAccept(c, mockMetricsRecord("foobar", Arrays.asList( + tag("foobar", "", "")))); } /** @@ -98,6 +122,8 @@ public class TestPatternFilter { .add("p.exclude.tags", "foo:f").subset("p"); shouldAccept(c, "foo"); shouldAccept(c, Arrays.asList(tag("foo", "", "f"))); + shouldAccept(c, mockMetricsRecord("foo", Arrays.asList( + tag("foo", "", "f")))); } static void shouldAccept(SubsetConfiguration conf, String s) { @@ -110,6 +136,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)); @@ -120,6 +157,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 @@ -141,4 +189,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 tags + * @return MetricsRecord newly created mock + */ + private static MetricsRecord mockMetricsRecord(String name, + List tags) { + MetricsRecord record = mock(MetricsRecord.class); + when(record.name()).thenReturn(name); + when(record.tags()).thenReturn(tags); + return record; + } }