From a2e759f814dc0c6de6c25a5f25c7e0c8c0e662e7 Mon Sep 17 00:00:00 2001 From: Liam Clarke Date: Mon, 23 Dec 2019 12:03:00 +1300 Subject: [PATCH] Removed metric name validation when using a CustomSampleMappingBuilder to allow usage on arbitrarily named metrics Signed-off-by: Liam Clarke --- .../samplebuilder/GraphiteNamePattern.java | 7 +- .../samplebuilder/MapperConfig.java | 16 +--- .../GraphiteNamePatternTest.java | 92 ++++++++++++------- .../samplebuilder/MapperConfigTest.java | 6 -- 4 files changed, 59 insertions(+), 62 deletions(-) diff --git a/simpleclient_dropwizard/src/main/java/io/prometheus/client/dropwizard/samplebuilder/GraphiteNamePattern.java b/simpleclient_dropwizard/src/main/java/io/prometheus/client/dropwizard/samplebuilder/GraphiteNamePattern.java index 45767e3fc..2fe49c8b4 100644 --- a/simpleclient_dropwizard/src/main/java/io/prometheus/client/dropwizard/samplebuilder/GraphiteNamePattern.java +++ b/simpleclient_dropwizard/src/main/java/io/prometheus/client/dropwizard/samplebuilder/GraphiteNamePattern.java @@ -5,7 +5,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import static io.prometheus.client.dropwizard.samplebuilder.MapperConfig.METRIC_GLOB_REGEX; /** * GraphiteNamePattern is initialised with a simplified glob pattern that only allows '*' as special character. @@ -20,7 +19,6 @@ * It contains logic to match a metric name and to extract named parameters from it. */ class GraphiteNamePattern { - private static final Pattern VALIDATION_PATTERN = Pattern.compile(METRIC_GLOB_REGEX); private Pattern pattern; private String patternStr; @@ -30,10 +28,7 @@ class GraphiteNamePattern { * * @param pattern The glob style pattern to be used. */ - GraphiteNamePattern(final String pattern) throws IllegalArgumentException { - if (!VALIDATION_PATTERN.matcher(pattern).matches()) { - throw new IllegalArgumentException(String.format("Provided pattern [%s] does not matches [%s]", pattern, METRIC_GLOB_REGEX)); - } + GraphiteNamePattern(final String pattern) { initializePattern(pattern); } diff --git a/simpleclient_dropwizard/src/main/java/io/prometheus/client/dropwizard/samplebuilder/MapperConfig.java b/simpleclient_dropwizard/src/main/java/io/prometheus/client/dropwizard/samplebuilder/MapperConfig.java index 887226635..ae7e3fc0f 100644 --- a/simpleclient_dropwizard/src/main/java/io/prometheus/client/dropwizard/samplebuilder/MapperConfig.java +++ b/simpleclient_dropwizard/src/main/java/io/prometheus/client/dropwizard/samplebuilder/MapperConfig.java @@ -19,13 +19,9 @@ * Dropwizard metrics that match the "match" pattern will be further processed to have a new name and new labels based on this config. */ public class MapperConfig { - // each part of the metric name between dots - private static final String METRIC_PART_REGEX = "[a-zA-Z_0-9](-?[a-zA-Z0-9_])+"; - // Simplified GLOB: we can have "*." at the beginning and "*" only at the end - static final String METRIC_GLOB_REGEX = "^(\\*\\.|" + METRIC_PART_REGEX + "\\.)+(\\*|" + METRIC_PART_REGEX + ")$"; + // Labels validation. private static final String LABEL_REGEX = "^[a-zA-Z_][a-zA-Z0-9_]+$"; - private static final Pattern MATCH_EXPRESSION_PATTERN = Pattern.compile(METRIC_GLOB_REGEX); private static final Pattern LABEL_PATTERN = Pattern.compile(LABEL_REGEX); /** @@ -74,13 +70,11 @@ public MapperConfig() { // for tests MapperConfig(final String match) { - validateMatch(match); this.match = match; } public MapperConfig(final String match, final String name, final Map labels) { this.name = name; - validateMatch(match); this.match = match; validateLabels(labels); this.labels = labels; @@ -96,7 +90,6 @@ public String getMatch() { } public void setMatch(final String match) { - validateMatch(match); this.match = match; } @@ -118,13 +111,6 @@ public void setLabels(final Map labels) { this.labels = labels; } - private void validateMatch(final String match) - { - if (!MATCH_EXPRESSION_PATTERN.matcher(match).matches()) { - throw new IllegalArgumentException(String.format("Match expression [%s] does not match required pattern %s", match, MATCH_EXPRESSION_PATTERN)); - } - } - private void validateLabels(final Map labels) { if (labels != null) { diff --git a/simpleclient_dropwizard/src/test/java/io/prometheus/client/dropwizard/samplebuilder/GraphiteNamePatternTest.java b/simpleclient_dropwizard/src/test/java/io/prometheus/client/dropwizard/samplebuilder/GraphiteNamePatternTest.java index 8b95e8e3e..3d8d38ae0 100644 --- a/simpleclient_dropwizard/src/test/java/io/prometheus/client/dropwizard/samplebuilder/GraphiteNamePatternTest.java +++ b/simpleclient_dropwizard/src/test/java/io/prometheus/client/dropwizard/samplebuilder/GraphiteNamePatternTest.java @@ -8,40 +8,10 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Map.Entry; public class GraphiteNamePatternTest { - @Test(expected = IllegalArgumentException.class) - public void createNew_WHEN_InvalidPattern_THEN_ShouldThrowException() { - final List invalidPatterns = Arrays.asList( - "", - "a", - "1org", - "1org.", - "org.", - "org.**", - "org.**", - "org.company-", - "org.company-.", - "org.company-*", - "org.company.**", - "org.company.**-", - "org.com*pany.*", - "org.test.contr.oller.gather.status..400", - "org.test.controller.gather.status..400" - ); - final GraphiteNamePattern graphiteNamePattern = new GraphiteNamePattern(""); - for (String pattern : invalidPatterns) { - try { - new GraphiteNamePattern(pattern); - - Assertions.failBecauseExceptionWasNotThrown(IllegalArgumentException.class); - } catch (IllegalArgumentException e) { - Assertions.assertThat(e).hasMessageContaining(pattern); - } - } - } - @Test public void createNew_WHEN_ValidPattern_THEN_ShouldCreateThePatternSuccessfully() { final List validPatterns = Arrays.asList( @@ -58,6 +28,7 @@ public void createNew_WHEN_ValidPattern_THEN_ShouldCreateThePatternSuccessfully( } } + @Test public void createNew_WHEN_ValidPattern_THEN_ShouldInitInternalPatternSuccessfully() { final Map validPatterns = new HashMap(); @@ -72,6 +43,7 @@ public void createNew_WHEN_ValidPattern_THEN_ShouldInitInternalPatternSuccessful } } + @Test public void match_WHEN_NotMatchingMetricNameProvided_THEN_ShouldNotMatch() { final GraphiteNamePattern pattern = new GraphiteNamePattern("org.test.controller.*.status.*"); @@ -86,6 +58,7 @@ public void match_WHEN_NotMatchingMetricNameProvided_THEN_ShouldNotMatch() { } } + @Test public void match_WHEN_MatchingMetricNameProvided_THEN_ShouldMatch() { final GraphiteNamePattern pattern = new GraphiteNamePattern("org.test.controller.*.status.*"); @@ -102,6 +75,28 @@ public void match_WHEN_MatchingMetricNameProvided_THEN_ShouldMatch() { } } + + @Test + public void match_WHEN_onlyGlob_THEN_ShouldMatchAny() { + GraphiteNamePattern pattern = new GraphiteNamePattern("*"); + Assertions.assertThat(pattern.matches("foo")).isTrue(); + Assertions.assertThat(pattern.matches("bar")).isTrue(); + } + + + @Test + public void match_WHEN_varyingFormats_THEN_ShouldMatchRegardless() { + Map metricsToPatterns = new HashMap(); + metricsToPatterns.put("snake_case_example_metric", "snake_case_*_metric"); + metricsToPatterns.put("CamelCasedExampleMetric", "CamelCased*Metric"); + metricsToPatterns.put("Weird.Mixture_Of_Formats.Example_metric", "Weird.Mixture_Of_Formats.*_metric"); + + for (Entry metricToPattern : metricsToPatterns.entrySet()) { + Assertions.assertThat(new GraphiteNamePattern(metricToPattern.getValue()).matches(metricToPattern.getKey())).isTrue(); + } + } + + @Test public void extractParameters() { GraphiteNamePattern pattern; @@ -110,7 +105,7 @@ public void extractParameters() { expected.put("${1}", "400"); pattern = new GraphiteNamePattern("org.test.controller.*.status.*"); Assertions.assertThat(pattern.extractParameters("org.test.controller.gather.status.400")) - .isEqualTo(expected); + .isEqualTo(expected); expected = new HashMap(); expected.put("${0}", "org"); @@ -118,9 +113,10 @@ public void extractParameters() { expected.put("${2}", "400"); pattern = new GraphiteNamePattern("*.test.controller.*.status.*"); Assertions.assertThat(pattern.extractParameters("org.test.controller.gather.status.400")) - .isEqualTo(expected); + .isEqualTo(expected); } + @Test public void extractParameters_WHEN_emptyStringInDottedMetricsName_THEN_ShouldReturnEmptyString() { GraphiteNamePattern pattern; @@ -129,16 +125,42 @@ public void extractParameters_WHEN_emptyStringInDottedMetricsName_THEN_ShouldRet expected.put("${1}", "400"); pattern = new GraphiteNamePattern("org.test.controller.*.status.*"); Assertions.assertThat(pattern.extractParameters("org.test.controller..status.400")) - .isEqualTo(expected); + .isEqualTo(expected); } + @Test public void extractParameters_WHEN_moreDots_THEN_ShouldReturnNoMatches() { GraphiteNamePattern pattern; pattern = new GraphiteNamePattern("org.test.controller.*.status.*"); Assertions.assertThat(pattern.extractParameters("org.test.controller...status.400")) - .isEqualTo(Collections.emptyMap()); + .isEqualTo(Collections.emptyMap()); + + } + @Test + public void extractParameters_WHEN_onlyGlob_THEN_ShouldExtractEntireMetric() { + String metric = "http_requests"; + GraphiteNamePattern pattern = new GraphiteNamePattern("*"); + Map expected = new HashMap(); + expected.put("${0}", metric); + Assertions.assertThat(pattern.extractParameters(metric)).isEqualTo(expected); + } + + + @Test + public void extractParameters_WHEN_varyingFormats_THEN_ShouldExtractRegardless() { + Map metricsToPatterns = new HashMap(); + metricsToPatterns.put("snake_case_example_metric", "snake_case_*_metric"); + metricsToPatterns.put("CamelCasedExampleMetric", "CamelCased*Metric"); + metricsToPatterns.put("Weird.Mixture_Of_Formats.Example_metric", "Weird.Mixture_Of_Formats.*_metric"); + + for (Entry metricToPattern : metricsToPatterns.entrySet()) { + GraphiteNamePattern graphiteNamePattern = new GraphiteNamePattern(metricToPattern.getValue()); + Entry actual = graphiteNamePattern.extractParameters(metricToPattern.getKey()).entrySet().iterator().next(); + Assertions.assertThat(actual.getKey()).isEqualTo("${0}"); + Assertions.assertThat(actual.getValue()).isEqualToIgnoringCase("example"); + } } } \ No newline at end of file diff --git a/simpleclient_dropwizard/src/test/java/io/prometheus/client/dropwizard/samplebuilder/MapperConfigTest.java b/simpleclient_dropwizard/src/test/java/io/prometheus/client/dropwizard/samplebuilder/MapperConfigTest.java index e0ded6978..9f58513ae 100644 --- a/simpleclient_dropwizard/src/test/java/io/prometheus/client/dropwizard/samplebuilder/MapperConfigTest.java +++ b/simpleclient_dropwizard/src/test/java/io/prometheus/client/dropwizard/samplebuilder/MapperConfigTest.java @@ -17,12 +17,6 @@ public void setMatch_WHEN_ExpressionMatchesPattern_AllGood() { assertEquals("com.company.meter.*", mapperConfig.getMatch()); } - @Test(expected = IllegalArgumentException.class) - public void setMatch_WHEN_ExpressionDoesnNotMatchPattern_ThrowException() { - final MapperConfig mapperConfig = new MapperConfig(); - mapperConfig.setMatch("com.company.meter.**.yay"); - } - @Test public void setLabels_WHEN_ExpressionMatchesPattern_AllGood() { final MapperConfig mapperConfig = new MapperConfig();