From 7d51abc5f99f5b9d68d25f51f68b1d908926973f Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Thu, 20 Feb 2020 16:24:06 +0100 Subject: [PATCH] Move metric check for name length (#893) --- .../opentelemetry/internal/StringUtils.java | 4 ++- .../opentelemetry/metrics/DefaultMeter.java | 27 +++++-------------- .../metrics/DoubleCounterTest.java | 3 ++- .../metrics/DoubleObserverTest.java | 3 ++- .../metrics/LongCounterTest.java | 3 ++- .../metrics/LongObserverTest.java | 3 ++- 6 files changed, 18 insertions(+), 25 deletions(-) diff --git a/api/src/main/java/io/opentelemetry/internal/StringUtils.java b/api/src/main/java/io/opentelemetry/internal/StringUtils.java index 8679f55ecf..2ef6acb2ce 100644 --- a/api/src/main/java/io/opentelemetry/internal/StringUtils.java +++ b/api/src/main/java/io/opentelemetry/internal/StringUtils.java @@ -22,6 +22,8 @@ import javax.annotation.concurrent.Immutable; @Immutable public final class StringUtils { + public static final int NAME_MAX_LENGTH = 255; + /** * Determines whether the {@code String} contains only printable characters. * @@ -52,7 +54,7 @@ public final class StringUtils { * @return whether the metricName contains a valid name. */ public static boolean isValidMetricName(String metricName) { - if (metricName.isEmpty()) { + if (metricName.isEmpty() || metricName.length() > NAME_MAX_LENGTH) { return false; } String pattern = "[aA-zZ][aA-zZ0-9_\\-.]*"; diff --git a/api/src/main/java/io/opentelemetry/metrics/DefaultMeter.java b/api/src/main/java/io/opentelemetry/metrics/DefaultMeter.java index aa494861ec..634f699bc8 100644 --- a/api/src/main/java/io/opentelemetry/metrics/DefaultMeter.java +++ b/api/src/main/java/io/opentelemetry/metrics/DefaultMeter.java @@ -31,10 +31,9 @@ public final class DefaultMeter implements Meter { private static final DefaultMeter INSTANCE = new DefaultMeter(); - /* VisibleForTesting */ static final int NAME_MAX_LENGTH = 255; /* VisibleForTesting */ static final String ERROR_MESSAGE_INVALID_NAME = "Name should be a ASCII string with a length no greater than " - + NAME_MAX_LENGTH + + StringUtils.NAME_MAX_LENGTH + " characters."; /** @@ -50,54 +49,42 @@ public final class DefaultMeter implements Meter { @Override public DoubleCounter.Builder doubleCounterBuilder(String name) { Utils.checkNotNull(name, "name"); - Utils.checkArgument( - StringUtils.isValidMetricName(name) && name.length() <= NAME_MAX_LENGTH, - ERROR_MESSAGE_INVALID_NAME); + Utils.checkArgument(StringUtils.isValidMetricName(name), ERROR_MESSAGE_INVALID_NAME); return new NoopDoubleCounter.NoopBuilder(); } @Override public LongCounter.Builder longCounterBuilder(String name) { Utils.checkNotNull(name, "name"); - Utils.checkArgument( - StringUtils.isValidMetricName(name) && name.length() <= NAME_MAX_LENGTH, - ERROR_MESSAGE_INVALID_NAME); + Utils.checkArgument(StringUtils.isValidMetricName(name), ERROR_MESSAGE_INVALID_NAME); return new NoopLongCounter.NoopBuilder(); } @Override public DoubleMeasure.Builder doubleMeasureBuilder(String name) { Utils.checkNotNull(name, "name"); - Utils.checkArgument( - StringUtils.isValidMetricName(name) && name.length() <= NAME_MAX_LENGTH, - ERROR_MESSAGE_INVALID_NAME); + Utils.checkArgument(StringUtils.isValidMetricName(name), ERROR_MESSAGE_INVALID_NAME); return new NoopDoubleMeasure.NoopBuilder(); } @Override public LongMeasure.Builder longMeasureBuilder(String name) { Utils.checkNotNull(name, "name"); - Utils.checkArgument( - StringUtils.isValidMetricName(name) && name.length() <= NAME_MAX_LENGTH, - ERROR_MESSAGE_INVALID_NAME); + Utils.checkArgument(StringUtils.isValidMetricName(name), ERROR_MESSAGE_INVALID_NAME); return new NoopLongMeasure.NoopBuilder(); } @Override public DoubleObserver.Builder doubleObserverBuilder(String name) { Utils.checkNotNull(name, "name"); - Utils.checkArgument( - StringUtils.isValidMetricName(name) && name.length() <= NAME_MAX_LENGTH, - ERROR_MESSAGE_INVALID_NAME); + Utils.checkArgument(StringUtils.isValidMetricName(name), ERROR_MESSAGE_INVALID_NAME); return new NoopDoubleObserver.NoopBuilder(); } @Override public LongObserver.Builder longObserverBuilder(String name) { Utils.checkNotNull(name, "name"); - Utils.checkArgument( - StringUtils.isValidMetricName(name) && name.length() <= NAME_MAX_LENGTH, - ERROR_MESSAGE_INVALID_NAME); + Utils.checkArgument(StringUtils.isValidMetricName(name), ERROR_MESSAGE_INVALID_NAME); return new NoopLongObserver.NoopBuilder(); } diff --git a/api/src/test/java/io/opentelemetry/metrics/DoubleCounterTest.java b/api/src/test/java/io/opentelemetry/metrics/DoubleCounterTest.java index 58842f2e66..9635893884 100644 --- a/api/src/test/java/io/opentelemetry/metrics/DoubleCounterTest.java +++ b/api/src/test/java/io/opentelemetry/metrics/DoubleCounterTest.java @@ -17,6 +17,7 @@ package io.opentelemetry.metrics; import io.opentelemetry.OpenTelemetry; +import io.opentelemetry.internal.StringUtils; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -47,7 +48,7 @@ public class DoubleCounterTest { @Test public void preventTooLongName() { - char[] chars = new char[DefaultMeter.NAME_MAX_LENGTH + 1]; + char[] chars = new char[StringUtils.NAME_MAX_LENGTH + 1]; Arrays.fill(chars, 'a'); String longName = String.valueOf(chars); thrown.expect(IllegalArgumentException.class); diff --git a/api/src/test/java/io/opentelemetry/metrics/DoubleObserverTest.java b/api/src/test/java/io/opentelemetry/metrics/DoubleObserverTest.java index d6e64e3355..88485c4657 100644 --- a/api/src/test/java/io/opentelemetry/metrics/DoubleObserverTest.java +++ b/api/src/test/java/io/opentelemetry/metrics/DoubleObserverTest.java @@ -17,6 +17,7 @@ package io.opentelemetry.metrics; import io.opentelemetry.OpenTelemetry; +import io.opentelemetry.internal.StringUtils; import java.util.Arrays; import java.util.Collections; import org.junit.Rule; @@ -40,7 +41,7 @@ public class DoubleObserverTest { @Test public void preventTooLongName() { - char[] chars = new char[DefaultMeter.NAME_MAX_LENGTH + 1]; + char[] chars = new char[StringUtils.NAME_MAX_LENGTH + 1]; Arrays.fill(chars, 'a'); String longName = String.valueOf(chars); thrown.expect(IllegalArgumentException.class); diff --git a/api/src/test/java/io/opentelemetry/metrics/LongCounterTest.java b/api/src/test/java/io/opentelemetry/metrics/LongCounterTest.java index dc4d9528b6..fec7a00efb 100644 --- a/api/src/test/java/io/opentelemetry/metrics/LongCounterTest.java +++ b/api/src/test/java/io/opentelemetry/metrics/LongCounterTest.java @@ -17,6 +17,7 @@ package io.opentelemetry.metrics; import io.opentelemetry.OpenTelemetry; +import io.opentelemetry.internal.StringUtils; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -46,7 +47,7 @@ public class LongCounterTest { @Test public void preventTooLongName() { - char[] chars = new char[DefaultMeter.NAME_MAX_LENGTH + 1]; + char[] chars = new char[StringUtils.NAME_MAX_LENGTH + 1]; Arrays.fill(chars, 'a'); String longName = String.valueOf(chars); thrown.expect(IllegalArgumentException.class); diff --git a/api/src/test/java/io/opentelemetry/metrics/LongObserverTest.java b/api/src/test/java/io/opentelemetry/metrics/LongObserverTest.java index 532cbfebcf..13866f4449 100644 --- a/api/src/test/java/io/opentelemetry/metrics/LongObserverTest.java +++ b/api/src/test/java/io/opentelemetry/metrics/LongObserverTest.java @@ -17,6 +17,7 @@ package io.opentelemetry.metrics; import io.opentelemetry.OpenTelemetry; +import io.opentelemetry.internal.StringUtils; import java.util.Arrays; import java.util.Collections; import org.junit.Rule; @@ -40,7 +41,7 @@ public class LongObserverTest { @Test public void preventTooLongName() { - char[] chars = new char[DefaultMeter.NAME_MAX_LENGTH + 1]; + char[] chars = new char[StringUtils.NAME_MAX_LENGTH + 1]; Arrays.fill(chars, 'a'); String longName = String.valueOf(chars); thrown.expect(IllegalArgumentException.class);