Skip to content

Commit deb782f

Browse files
LiamClarkeNZclaudejaydeluca
authored
fix: Relax metric name validation in Dropwizard5 (#1985)
Decided to revisit a very old PR of mine that addressed an issue that seems to reoccur periodically for others. ## Changes - Relaxes overly strict `METRIC_GLOB_REGEX` in `MapperConfig`/`GraphiteNamePattern` to accept a broader range of metric name patterns - The `CustomMappingSampleBuilder` can now remap metrics with underscores, colons, hyphens, and single-level names - Still rejects clearly invalid inputs (empty strings, double-star globs, trailing/double dots) ## Fixes - #461: Cannot match a Graphite metric with only one level - #518: Custom metric name remapping fails for non-Graphite-style names - #645: MapperConfig does not allow `:` in regex The validation was originally designed for Graphite bridge metrics but is too restrictive for `CustomMappingSampleBuilder` which needs to handle arbitrary Dropwizard metric names. @fstab @dhoard @zeitlinger @jaydeluca when you have a chance would appreciate your review :) --------- Signed-off-by: Liam Clarke-Hutchinson <liam@steelsky.co.nz> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Jay DeLuca <jaydeluca4@gmail.com>
1 parent 6beb7fd commit deb782f

File tree

4 files changed

+81
-12
lines changed

4 files changed

+81
-12
lines changed

prometheus-metrics-instrumentation-dropwizard5/src/main/java/io/prometheus/metrics/instrumentation/dropwizard5/labels/GraphiteNamePattern.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,16 @@
99

1010
/**
1111
* GraphiteNamePattern is initialised with a simplified glob pattern that only allows '*' as special
12-
* character. Examples of valid patterns:
12+
* character. Accepts a broad range of metric name patterns including single-level names, names with
13+
* underscores, hyphens, and colons. Examples of valid patterns:
1314
*
1415
* <ul>
1516
* <li>org.test.controller.gather.status.400
1617
* <li>org.test.controller.gather.status.*
1718
* <li>org.test.controller.*.status.*
1819
* <li>*.test.controller.*.status.*
20+
* <li>app_metric_some_count
21+
* <li>io.dropwizard.jetty.MutableServletContextHandler.*-requests
1922
* </ul>
2023
*
2124
* <p>It contains logic to match a metric name and to extract named parameters from it.
@@ -32,6 +35,10 @@ class GraphiteNamePattern {
3235
* @param pattern The glob style pattern to be used.
3336
*/
3437
GraphiteNamePattern(String pattern) throws IllegalArgumentException {
38+
if (pattern.contains("**")) {
39+
throw new IllegalArgumentException(
40+
String.format("Provided pattern [%s] must not contain '**' (double-star glob)", pattern));
41+
}
3542
if (!VALIDATION_PATTERN.matcher(pattern).matches()) {
3643
throw new IllegalArgumentException(
3744
String.format("Provided pattern [%s] does not matches [%s]", pattern, METRIC_GLOB_REGEX));

prometheus-metrics-instrumentation-dropwizard5/src/main/java/io/prometheus/metrics/instrumentation/dropwizard5/labels/MapperConfig.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,14 @@
1616
* and new labels based on this config.
1717
*/
1818
public final class MapperConfig {
19-
// each part of the metric name between dots
20-
private static final String METRIC_PART_REGEX = "[a-zA-Z_0-9](-?[a-zA-Z0-9_])+";
21-
// Simplified GLOB: we can have "*." at the beginning and "*" only at the end
19+
// Each part of the metric name between dots. Accepts letters, digits, underscores, hyphens,
20+
// colons, and glob wildcards (*) to support a broad range of metric naming conventions.
21+
private static final String METRIC_PART_REGEX = "[a-zA-Z_0-9*][a-zA-Z0-9_:*-]*";
22+
// Simplified GLOB: accepts single-level names, dot-separated names, and glob patterns with '*'.
23+
// The pattern requires at least one non-empty segment and does not allow empty segments (double
24+
// dots) or empty/whitespace-only strings. The '**' glob is rejected separately in validateMatch.
2225
static final String METRIC_GLOB_REGEX =
23-
"^(\\*\\.|" + METRIC_PART_REGEX + "\\.)+(\\*|" + METRIC_PART_REGEX + ")$";
26+
"^(" + METRIC_PART_REGEX + ")(\\." + METRIC_PART_REGEX + ")*$";
2427
// Labels validation.
2528
private static final String LABEL_REGEX = "^[a-zA-Z_][a-zA-Z0-9_]+$";
2629
private static final Pattern MATCH_EXPRESSION_PATTERN = Pattern.compile(METRIC_GLOB_REGEX);
@@ -109,6 +112,10 @@ public void setLabels(Map<String, String> labels) {
109112
}
110113

111114
private void validateMatch(String match) {
115+
if (match.contains("**")) {
116+
throw new IllegalArgumentException(
117+
String.format("Match expression [%s] must not contain '**' (double-star glob)", match));
118+
}
112119
if (!MATCH_EXPRESSION_PATTERN.matcher(match).matches()) {
113120
throw new IllegalArgumentException(
114121
String.format(

prometheus-metrics-instrumentation-dropwizard5/src/test/java/io/prometheus/metrics/instrumentation/dropwizard5/labels/GraphiteNamePatternTest.java

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,12 @@ void createNew_WHEN_InvalidPattern_THEN_ShouldThrowException() {
1717
List<String> invalidPatterns =
1818
Arrays.asList(
1919
"",
20-
"a",
21-
"1org",
2220
"1org.",
2321
"org.",
2422
"org.**",
25-
"org.**",
26-
"org.company-",
2723
"org.company-.",
28-
"org.company-*",
2924
"org.company.**",
3025
"org.company.**-",
31-
"org.com*pany.*",
3226
"org.test.contr.oller.gather.status..400",
3327
"org.test.controller.gather.status..400");
3428
for (String pattern : invalidPatterns) {
@@ -47,7 +41,22 @@ void createNew_WHEN_ValidPattern_THEN_ShouldCreateThePatternSuccessfully() {
4741
"org.test.controller.*.status.*",
4842
"*.test.controller.*.status.*",
4943
"*.test.controller-1.*.status.*",
50-
"*.amazing-test.controller-1.*.status.*");
44+
"*.amazing-test.controller-1.*.status.*",
45+
// Single-level names (previously rejected, fixes #461)
46+
"a",
47+
"1org",
48+
"app_metric_some_count",
49+
// Names with colons (fixes #645)
50+
"my:metric:name",
51+
"org.company:metric.*",
52+
// Names with hyphens at boundaries
53+
"org.company-",
54+
"org.company-*",
55+
// Embedded glob in segment (fixes #518)
56+
"org.com*pany.*",
57+
"io.dropwizard.jetty.MutableServletContextHandler.*-requests",
58+
// Standalone glob
59+
"*");
5160
for (String pattern : validPatterns) {
5261
new GraphiteNamePattern(pattern);
5362
}

prometheus-metrics-instrumentation-dropwizard5/src/test/java/io/prometheus/metrics/instrumentation/dropwizard5/labels/MapperConfigTest.java

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,52 @@ void setLabels_WHEN_ExpressionDoesnNotMatchPattern_ThrowException() {
4141
.isThrownBy(() -> mapperConfig.setLabels(labels));
4242
}
4343

44+
@Test
45+
void setMatch_WHEN_SingleLevelName_AllGood() {
46+
final MapperConfig mapperConfig = new MapperConfig();
47+
mapperConfig.setMatch("app_metric_some_count");
48+
assertThat(mapperConfig.getMatch()).isEqualTo("app_metric_some_count");
49+
}
50+
51+
@Test
52+
void setMatch_WHEN_NameWithColons_AllGood() {
53+
final MapperConfig mapperConfig = new MapperConfig();
54+
mapperConfig.setMatch("my:metric:name");
55+
assertThat(mapperConfig.getMatch()).isEqualTo("my:metric:name");
56+
}
57+
58+
@Test
59+
void setMatch_WHEN_EmbeddedGlobInSegment_AllGood() {
60+
final MapperConfig mapperConfig = new MapperConfig();
61+
mapperConfig.setMatch("io.dropwizard.jetty.MutableServletContextHandler.*-requests");
62+
assertThat(mapperConfig.getMatch())
63+
.isEqualTo("io.dropwizard.jetty.MutableServletContextHandler.*-requests");
64+
}
65+
66+
@Test
67+
void setMatch_WHEN_DoubleStarGlob_ThrowException() {
68+
assertThatExceptionOfType(IllegalArgumentException.class)
69+
.isThrownBy(() -> new MapperConfig().setMatch("org.**"));
70+
}
71+
72+
@Test
73+
void setMatch_WHEN_EmptyString_ThrowException() {
74+
assertThatExceptionOfType(IllegalArgumentException.class)
75+
.isThrownBy(() -> new MapperConfig().setMatch(""));
76+
}
77+
78+
@Test
79+
void setMatch_WHEN_TrailingDot_ThrowException() {
80+
assertThatExceptionOfType(IllegalArgumentException.class)
81+
.isThrownBy(() -> new MapperConfig().setMatch("org.company."));
82+
}
83+
84+
@Test
85+
void setMatch_WHEN_DoubleDot_ThrowException() {
86+
assertThatExceptionOfType(IllegalArgumentException.class)
87+
.isThrownBy(() -> new MapperConfig().setMatch("org..company"));
88+
}
89+
4490
@Test
4591
void toString_WHEN_EmptyConfig_AllGood() {
4692
final MapperConfig mapperConfig = new MapperConfig();

0 commit comments

Comments
 (0)