Repository: beam Updated Branches: refs/heads/master 63de41ac2 -> 61777b433
Ensure metric names are not null or empty Project: http://git-wip-us.apache.org/repos/asf/beam/repo Commit: http://git-wip-us.apache.org/repos/asf/beam/commit/b6c838e1 Tree: http://git-wip-us.apache.org/repos/asf/beam/tree/b6c838e1 Diff: http://git-wip-us.apache.org/repos/asf/beam/diff/b6c838e1 Branch: refs/heads/master Commit: b6c838e18f56f4b29eee74d2cc7735a154bf44ae Parents: 63de41a Author: bchambers <bchamb...@google.com> Authored: Wed Sep 27 10:44:48 2017 -0700 Committer: Kenneth Knowles <k...@google.com> Committed: Mon Oct 9 12:47:16 2017 -0700 ---------------------------------------------------------------------- .../org/apache/beam/sdk/metrics/MetricName.java | 7 +++++ .../apache/beam/sdk/metrics/MetricsTest.java | 28 ++++++++++++++++++++ sdks/python/apache_beam/metrics/metric_test.py | 16 +++++++++++ sdks/python/apache_beam/metrics/metricbase.py | 4 +++ 4 files changed, 55 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/beam/blob/b6c838e1/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/MetricName.java ---------------------------------------------------------------------- diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/MetricName.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/MetricName.java index 3c77043..6c88fa2 100644 --- a/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/MetricName.java +++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/MetricName.java @@ -17,7 +17,10 @@ */ package org.apache.beam.sdk.metrics; +import static com.google.common.base.Preconditions.checkArgument; + import com.google.auto.value.AutoValue; +import com.google.common.base.Strings; import java.io.Serializable; import org.apache.beam.sdk.annotations.Experimental; import org.apache.beam.sdk.annotations.Experimental.Kind; @@ -38,10 +41,14 @@ public abstract class MetricName implements Serializable { public abstract String name(); public static MetricName named(String namespace, String name) { + checkArgument(!Strings.isNullOrEmpty(namespace), "Metric namespace must be non-empty"); + checkArgument(!Strings.isNullOrEmpty(name), "Metric name must be non-empty"); return new AutoValue_MetricName(namespace, name); } public static MetricName named(Class<?> namespace, String name) { + checkArgument(namespace != null, "Metric namespace must be non-null"); + checkArgument(!Strings.isNullOrEmpty(name), "Metric name must be non-empty"); return new AutoValue_MetricName(namespace.getName(), name); } } http://git-wip-us.apache.org/repos/asf/beam/blob/b6c838e1/sdks/java/core/src/test/java/org/apache/beam/sdk/metrics/MetricsTest.java ---------------------------------------------------------------------- diff --git a/sdks/java/core/src/test/java/org/apache/beam/sdk/metrics/MetricsTest.java b/sdks/java/core/src/test/java/org/apache/beam/sdk/metrics/MetricsTest.java index bc768f8..bdcf892 100644 --- a/sdks/java/core/src/test/java/org/apache/beam/sdk/metrics/MetricsTest.java +++ b/sdks/java/core/src/test/java/org/apache/beam/sdk/metrics/MetricsTest.java @@ -48,6 +48,7 @@ import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.junit.rules.ExpectedException; import org.mockito.Mockito; /** @@ -71,6 +72,9 @@ public class MetricsTest implements Serializable { @Rule public final transient TestPipeline pipeline = TestPipeline.create(); + @Rule + public final transient ExpectedException thrown = ExpectedException.none(); + @After public void tearDown() { MetricsEnvironment.setCurrentContainer(null); @@ -95,6 +99,30 @@ public class MetricsTest implements Serializable { } @Test + public void testCounterWithEmptyName() { + thrown.expect(IllegalArgumentException.class); + Metrics.counter(NS, ""); + } + + @Test + public void testCounterWithEmptyNamespace() { + thrown.expect(IllegalArgumentException.class); + Metrics.counter("", NAME); + } + + @Test + public void testDistributionWithEmptyName() { + thrown.expect(IllegalArgumentException.class); + Metrics.distribution(NS, ""); + } + + @Test + public void testDistributionWithEmptyNamespace() { + thrown.expect(IllegalArgumentException.class); + Metrics.distribution("", NAME); + } + + @Test public void testDistributionToCell() { MetricsContainer mockContainer = Mockito.mock(MetricsContainer.class); Distribution mockDistribution = Mockito.mock(Distribution.class); http://git-wip-us.apache.org/repos/asf/beam/blob/b6c838e1/sdks/python/apache_beam/metrics/metric_test.py ---------------------------------------------------------------------- diff --git a/sdks/python/apache_beam/metrics/metric_test.py b/sdks/python/apache_beam/metrics/metric_test.py index 75c3aa0..ef98b2d 100644 --- a/sdks/python/apache_beam/metrics/metric_test.py +++ b/sdks/python/apache_beam/metrics/metric_test.py @@ -98,6 +98,22 @@ class MetricsTest(unittest.TestCase): with self.assertRaises(ValueError): Metrics.get_namespace(object()) + def test_counter_empty_name(self): + with self.assertRaises(ValueError): + Metrics.counter("namespace", "") + + def test_counter_empty_namespace(self): + with self.assertRaises(ValueError): + Metrics.counter("", "names") + + def test_distribution_empty_name(self): + with self.assertRaises(ValueError): + Metrics.distribution("namespace", "") + + def test_distribution_empty_namespace(self): + with self.assertRaises(ValueError): + Metrics.distribution("", "names") + def test_create_counter_distribution(self): MetricsEnvironment.set_current_container(MetricsContainer('mystep')) counter_ns = 'aCounterNamespace' http://git-wip-us.apache.org/repos/asf/beam/blob/b6c838e1/sdks/python/apache_beam/metrics/metricbase.py ---------------------------------------------------------------------- diff --git a/sdks/python/apache_beam/metrics/metricbase.py b/sdks/python/apache_beam/metrics/metricbase.py index 699f29c..9b19189 100644 --- a/sdks/python/apache_beam/metrics/metricbase.py +++ b/sdks/python/apache_beam/metrics/metricbase.py @@ -47,6 +47,10 @@ class MetricName(object): namespace: A string with the namespace of a metric. name: A string with the name of a metric. """ + if not namespace: + raise ValueError('Metric namespace must be non-empty') + if not name: + raise ValueError('Metric name must be non-empty') self.namespace = namespace self.name = name
