Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed metric name validation when using a CustomSampleMappingBuilder #519

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Removed metric name validation when using a CustomSampleMappingBuilder #519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions ...zard/src/main/java/io/prometheus/client/dropwizard/samplebuilder/GraphiteNamePattern.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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