Skip to content

Commit

Permalink
Incorporating feedback from code review
Browse files Browse the repository at this point in the history
1.) splitting out the default/regex tests
2.) updating the configuration reference and
3.) supporting caching of compiled regex expressions
  • Loading branch information
bradley.schmidt committed Feb 23, 2015
1 parent c60c892 commit 8f8fa15
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 2 deletions.
1 change: 1 addition & 0 deletions docs/source/manual/configuration.rst
Expand Up @@ -630,6 +630,7 @@ durationUnit milliseconds The unit to report durations as. Overrides
rateUnit seconds The unit to report rates as. Overrides per-metric rate units. rateUnit seconds The unit to report rates as. Overrides per-metric rate units.
excludes (none) Metrics to exclude from reports, by name. When defined, matching metrics will not be reported. excludes (none) Metrics to exclude from reports, by name. When defined, matching metrics will not be reported.
includes (all) Metrics to include in reports, by name. When defined, only these metrics will be reported. includes (all) Metrics to include in reports, by name. When defined, only these metrics will be reported.
useRegexFilters false Indicates whether the values of the 'includes' and 'excludes' fields should be treated as regular expressions or not.
frequency (none) The frequency to report metrics. Overrides the default. frequency (none) The frequency to report metrics. Overrides the default.
====================== ============= =========== ====================== ============= ===========


Expand Down
Expand Up @@ -5,6 +5,9 @@
import com.codahale.metrics.ScheduledReporter; import com.codahale.metrics.ScheduledReporter;
import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.base.Optional; import com.google.common.base.Optional;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import io.dropwizard.util.Duration; import io.dropwizard.util.Duration;


Expand Down Expand Up @@ -203,10 +206,23 @@ public boolean containsMatch(ImmutableSet<String> matchExpressions, String metri
} }


private static class RegexStringMatchingStrategy implements StringMatchingStrategy { private static class RegexStringMatchingStrategy implements StringMatchingStrategy {
private final LoadingCache<String, Pattern> patternCache;

private RegexStringMatchingStrategy() {
patternCache = CacheBuilder.newBuilder()
.expireAfterWrite(1, TimeUnit.HOURS)
.build(new CacheLoader<String, Pattern>() {
@Override
public Pattern load(String regex) throws Exception {
return Pattern.compile(regex);
}
});
}

@Override @Override
public boolean containsMatch(ImmutableSet<String> matchExpressions, String metricName) { public boolean containsMatch(ImmutableSet<String> matchExpressions, String metricName) {
for (String regexExpression : matchExpressions) { for (String regexExpression : matchExpressions) {
if (Pattern.matches(regexExpression, metricName)) { if (patternCache.getUnchecked(regexExpression).matcher(metricName).matches()) {
// just need to match on a single value - return as soon as we do // just need to match on a single value - return as soon as we do
return true; return true;
} }
Expand Down
Expand Up @@ -103,14 +103,20 @@ public ScheduledReporter build(MetricRegistry registry) {
private final Metric metric = mock(Metric.class); private final Metric metric = mock(Metric.class);


@Test @Test
public void matches() { public void testDefaultMatching() {
factory.setIncludes(includes); factory.setIncludes(includes);
factory.setExcludes(excludes); factory.setExcludes(excludes);


factory.setUseRegexFilters(false); factory.setUseRegexFilters(false);
assertThat(factory.getFilter().matches(name, metric)) assertThat(factory.getFilter().matches(name, metric))
.overridingErrorMessage(msg + ": expected 'matches(%s)=%s' for default matcher", name, expectedDefaultResult) .overridingErrorMessage(msg + ": expected 'matches(%s)=%s' for default matcher", name, expectedDefaultResult)
.isEqualTo(expectedDefaultResult); .isEqualTo(expectedDefaultResult);
}

@Test
public void testRegexMatching() {
factory.setIncludes(includes);
factory.setExcludes(excludes);


factory.setUseRegexFilters(true); factory.setUseRegexFilters(true);
assertThat(factory.getFilter().matches(name, metric)) assertThat(factory.getFilter().matches(name, metric))
Expand Down

0 comments on commit 8f8fa15

Please sign in to comment.