Skip to content

Commit

Permalink
Removed metric name validation when using a CustomSampleMappingBuilde…
Browse files Browse the repository at this point in the history 
…r to allow usage on arbitrarily named metrics

Signed-off-by: Liam Clarke <[email protected]>
  • Loading branch information
Liam Clarke committed Dec 24, 2019
1 parent ef16c43 commit a2e759f
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 62 deletions.
7 changes: 1 addition & 6 deletions ...zard/src/main/java/io/prometheus/client/dropwizard/samplebuilder/GraphiteNamePattern.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand All @@ -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);
}

Expand Down
16 changes: 1 addition & 15 deletions ..._dropwizard/src/main/java/io/prometheus/client/dropwizard/samplebuilder/MapperConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

/**
Expand Down Expand Up @@ -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<String, String> labels) {
this.name = name;
validateMatch(match);
this.match = match;
validateLabels(labels);
this.labels = labels;
Expand All @@ -96,7 +90,6 @@ public String getMatch() {
}

public void setMatch(final String match) {
validateMatch(match);
this.match = match;
}

Expand All @@ -118,13 +111,6 @@ public void setLabels(final Map<String, String> 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<String, String> labels)
{
if (labels != null) {
Expand Down
92 changes: 57 additions & 35 deletions .../src/test/java/io/prometheus/client/dropwizard/samplebuilder/GraphiteNamePatternTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> 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<String> validPatterns = Arrays.asList(
Expand All @@ -58,6 +28,7 @@ public void createNew_WHEN_ValidPattern_THEN_ShouldCreateThePatternSuccessfully(
}
}


@Test
public void createNew_WHEN_ValidPattern_THEN_ShouldInitInternalPatternSuccessfully() {
final Map<String, String> validPatterns = new HashMap<String, String>();
Expand All @@ -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.*");
Expand All @@ -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.*");
Expand All @@ -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<String, String> metricsToPatterns = new HashMap<String, String>();
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<String, String> metricToPattern : metricsToPatterns.entrySet()) {
Assertions.assertThat(new GraphiteNamePattern(metricToPattern.getValue()).matches(metricToPattern.getKey())).isTrue();
}
}


@Test
public void extractParameters() {
GraphiteNamePattern pattern;
Expand All @@ -110,17 +105,18 @@ 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<String, String>();
expected.put("${0}", "org");
expected.put("${1}", "gather");
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;
Expand All @@ -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<String, String> expected = new HashMap<String, String>();
expected.put("${0}", metric);
Assertions.assertThat(pattern.extractParameters(metric)).isEqualTo(expected);
}


@Test
public void extractParameters_WHEN_varyingFormats_THEN_ShouldExtractRegardless() {
Map<String, String> metricsToPatterns = new HashMap<String, String>();
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<String, String> metricToPattern : metricsToPatterns.entrySet()) {
GraphiteNamePattern graphiteNamePattern = new GraphiteNamePattern(metricToPattern.getValue());
Entry<String, String> actual = graphiteNamePattern.extractParameters(metricToPattern.getKey()).entrySet().iterator().next();
Assertions.assertThat(actual.getKey()).isEqualTo("${0}");
Assertions.assertThat(actual.getValue()).isEqualToIgnoringCase("example");
}
}
}
6 changes: 0 additions & 6 deletions ...pwizard/src/test/java/io/prometheus/client/dropwizard/samplebuilder/MapperConfigTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit a2e759f

Please sign in to comment.