diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/RuleBasedTraceSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/RuleBasedTraceSampler.java index 78be5e7273b..8a8a040e94e 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/RuleBasedTraceSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/RuleBasedTraceSampler.java @@ -67,6 +67,7 @@ public static RuleBasedTraceSampler build( new TraceSamplingRule( rule.getService(), rule.getName(), + rule.getResource(), new DeterministicSampler.TraceSampler(rule.getSampleRate())); samplingRules.add(samplingRule); } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/SamplingRule.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/SamplingRule.java index e5fdbb9d8f7..59d1f47daed 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/SamplingRule.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/SamplingRule.java @@ -1,7 +1,8 @@ package datadog.trace.common.sampling; import datadog.trace.core.CoreSpan; -import datadog.trace.core.util.GlobPattern; +import datadog.trace.core.util.Matcher; +import datadog.trace.core.util.Matchers; import datadog.trace.core.util.SimpleRateLimiter; import java.util.regex.Pattern; @@ -74,28 +75,32 @@ protected > CharSequence getRelevantString(final T span) { } public static final class TraceSamplingRule extends SamplingRule { - private final String serviceName; - private final String operationName; + private final Matcher serviceMatcher; + private final Matcher operationMatcher; + private final Matcher resourceMatcher; public TraceSamplingRule( - final String exactServiceName, final String exactOperationName, final RateSampler sampler) { + final String serviceGlob, + final String operationGlob, + final String resourceGlob, + final RateSampler sampler) { super(sampler); - this.serviceName = exactServiceName; - this.operationName = exactOperationName; + serviceMatcher = Matchers.compileGlob(serviceGlob); + operationMatcher = Matchers.compileGlob(operationGlob); + resourceMatcher = Matchers.compileGlob(resourceGlob); } @Override public > boolean matches(T span) { - return (serviceName == null || serviceName.equals(span.getServiceName())) - && (operationName == null || operationName.contentEquals(span.getOperationName())); + return Matchers.matches(serviceMatcher, span.getServiceName()) + && Matchers.matches(operationMatcher, span.getOperationName()) + && Matchers.matches(resourceMatcher, span.getResourceName()); } } public static final class SpanSamplingRule extends SamplingRule { - private final String serviceExactName; - private final Pattern servicePattern; - private final String operationExactName; - private final Pattern operationPattern; + private final Matcher serviceMatcher; + private final Matcher operationMatcher; private final SimpleRateLimiter rateLimiter; @@ -106,43 +111,16 @@ public SpanSamplingRule( final SimpleRateLimiter rateLimiter) { super(sampler); - if (serviceName == null || "*".equals(serviceName)) { - this.serviceExactName = null; - this.servicePattern = null; - } else if (isExactMatcher(serviceName)) { - this.serviceExactName = serviceName; - this.servicePattern = null; - } else { - this.serviceExactName = null; - this.servicePattern = GlobPattern.globToRegexPattern(serviceName); - } - - if (operationName == null || "*".equals(operationName)) { - this.operationExactName = null; - this.operationPattern = null; - } else if (isExactMatcher(operationName)) { - this.operationExactName = operationName; - this.operationPattern = null; - } else { - this.operationExactName = null; - this.operationPattern = GlobPattern.globToRegexPattern(operationName); - } + serviceMatcher = Matchers.compileGlob(serviceName); + operationMatcher = Matchers.compileGlob(operationName); this.rateLimiter = rateLimiter; } - private boolean isExactMatcher(String serviceNameGlob) { - return !serviceNameGlob.contains("*") && !serviceNameGlob.contains("?"); - } - @Override public > boolean matches(T span) { - return (serviceExactName == null || serviceExactName.equals(span.getServiceName())) - && (servicePattern == null || servicePattern.matcher(span.getServiceName()).matches()) - && (operationExactName == null - || operationExactName.contentEquals(span.getOperationName())) - && (operationPattern == null - || operationPattern.matcher(span.getOperationName()).matches()); + return Matchers.matches(serviceMatcher, span.getServiceName()) + && Matchers.matches(operationMatcher, span.getOperationName()); } @Override diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/TraceSamplingRules.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/TraceSamplingRules.java index ebba4c1929c..9fb36366cb2 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/TraceSamplingRules.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/TraceSamplingRules.java @@ -17,34 +17,46 @@ public class TraceSamplingRules { private static final Logger log = LoggerFactory.getLogger(TraceSamplingRules.class); public static final class Rule { - public static Rule create(String service, String name, double sampleRate) { + public static Rule create(String service, String name, String resource, double sampleRate) { if (sampleRate < 0 || sampleRate > 1.0) { logError( - service, name, Double.toString(sampleRate), "sample_rate must be between 0.0 and 1.0"); + service, + name, + resource, + Double.toString(sampleRate), + "sample_rate must be between 0.0 and 1.0"); return null; } - return new Rule(service, name, sampleRate); + return new Rule(service, name, resource, sampleRate); } - public static Rule create(String service, String name, String sample_rate) { + public static Rule create(String service, String name, String resource, String sample_rate) { if (sample_rate == null) { - logError(service, name, null, "missing mandatory sample_rate"); + logError(service, name, resource, null, "missing mandatory sample_rate"); return null; } try { - return create(service, name, Double.parseDouble(sample_rate)); + return create(service, name, resource, Double.parseDouble(sample_rate)); } catch (NumberFormatException ex) { - logError(service, name, sample_rate, "sample_rate must be a number between 0.0 and 1.0"); + logError( + service, + name, + resource, + sample_rate, + "sample_rate must be a number between 0.0 and 1.0"); return null; } } - private static void logError(String service, String name, String sample_rate, String error) { + private static void logError( + String service, String name, String resource, String sample_rate, String error) { log.error( "Skipping invalid Trace Sampling Rule: { \"service\": \"" + service + "\", \"name\": \"" + name + + "\", \"resource\": \"" + + resource + "\", \"sample_rate\": " + sample_rate + " } - " @@ -52,14 +64,15 @@ private static void logError(String service, String name, String sample_rate, St } private final String service; - private final String name; + private final String resource; private final double sampleRate; - private Rule(String service, String name, double sampleRate) { + private Rule(String service, String name, String resource, double sampleRate) { this.service = service; this.name = name; + this.resource = resource; this.sampleRate = sampleRate; } @@ -71,6 +84,10 @@ public String getName() { return name; } + public String getResource() { + return resource; + } + public double getSampleRate() { return sampleRate; } @@ -88,10 +105,11 @@ public static TraceSamplingRules deserialize(String json) { private static final class JsonRule { String service; String name; + String resource; String sample_rate; private Rule toRule() { - return Rule.create(service, name, sample_rate); + return Rule.create(service, name, resource, sample_rate); } } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/util/GlobPattern.java b/dd-trace-core/src/main/java/datadog/trace/core/util/GlobPattern.java index 393b0e4d5ca..c0fd0499b95 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/util/GlobPattern.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/util/GlobPattern.java @@ -5,20 +5,11 @@ public final class GlobPattern { public static Pattern globToRegexPattern(String globPattern) { - if (globPattern == null) { - return null; - } String regex = globToRegex(globPattern); - if (regex == null) { - return null; - } return Pattern.compile(regex); } private static String globToRegex(String globPattern) { - if ("*".equals(globPattern)) { - return null; - } StringBuilder sb = new StringBuilder(64); sb.append('^'); for (int i = 0; i < globPattern.length(); i++) { diff --git a/dd-trace-core/src/main/java/datadog/trace/core/util/Matcher.java b/dd-trace-core/src/main/java/datadog/trace/core/util/Matcher.java new file mode 100644 index 00000000000..42cd2f71c97 --- /dev/null +++ b/dd-trace-core/src/main/java/datadog/trace/core/util/Matcher.java @@ -0,0 +1,7 @@ +package datadog.trace.core.util; + +public interface Matcher { + public boolean matches(String str); + + public boolean matches(CharSequence charSeq); +} diff --git a/dd-trace-core/src/main/java/datadog/trace/core/util/Matchers.java b/dd-trace-core/src/main/java/datadog/trace/core/util/Matchers.java new file mode 100644 index 00000000000..a3e590c6b93 --- /dev/null +++ b/dd-trace-core/src/main/java/datadog/trace/core/util/Matchers.java @@ -0,0 +1,69 @@ +package datadog.trace.core.util; + +import java.util.regex.Pattern; + +public final class Matchers { + private Matchers() {} + + public static final Matcher compileGlob(String glob) { + if (glob == null || glob.equals("*")) { + // DQH - Decided not to an anyMatcher because that's likely to + // cause our call site to go megamorphic + return null; + } else if (isExact(glob)) { + return new ExactMatcher(glob); + } else { + // DQH - not sure about the error handling here + Pattern pattern = GlobPattern.globToRegexPattern(glob); + return pattern == null ? null : new PatternMatcher(pattern); + } + } + + public static boolean matches(Matcher matcher, String str) { + return (matcher == null) || matcher.matches(str); + } + + public static boolean matches(Matcher matcher, CharSequence charSeq) { + return (matcher == null) || matcher.matches(charSeq); + } + + static boolean isExact(String glob) { + return (glob.indexOf('*') == -1) && (glob.indexOf('?') == -1); + } + + static final class ExactMatcher implements Matcher { + private final String exact; + + ExactMatcher(String exact) { + this.exact = exact; + } + + @Override + public boolean matches(String str) { + return exact.equals(str); + } + + @Override + public boolean matches(CharSequence charSeq) { + return exact.contentEquals(charSeq); + } + } + + static final class PatternMatcher implements Matcher { + private final Pattern pattern; + + PatternMatcher(Pattern pattern) { + this.pattern = pattern; + } + + @Override + public boolean matches(CharSequence charSeq) { + return pattern.matcher(charSeq).matches(); + } + + @Override + public boolean matches(String str) { + return pattern.matcher(str).matches(); + } + } +} diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/RuleBasedSamplingTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/RuleBasedSamplingTest.groovy index 7b470784310..3bde48912c6 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/RuleBasedSamplingTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/RuleBasedSamplingTest.groovy @@ -159,7 +159,9 @@ class RuleBasedSamplingTest extends DDCoreSpecification { DDSpan span = tracer.buildSpan("operation") .withServiceName("service") .withTag("env", "bar") - .ignoreActiveSpan().start() + .withResourceName("resource") + .ignoreActiveSpan() + .start() ((PrioritySampler) sampler).setSamplingPriority(span) then: @@ -214,6 +216,20 @@ class RuleBasedSamplingTest extends DDCoreSpecification { "[{\"name\": \"xxx\", \"sample_rate\": 0}, {\"name\": \"operation\", \"sample_rate\": 1}]" | null | 1.0 | 50 | null | USER_KEEP "[{\"name\": \"xxx\", \"sample_rate\": 1}, {\"name\": \"operation\", \"sample_rate\": 0}]" | null | 0 | null | null | USER_DROP + // Matching resource : keep + "[{\"resource\": \"resource\", \"sample_rate\": 1}]" | null | 1.0 | 50 | null | USER_KEEP + + // Matching operation: drop + "[{\"resource\": \"resource\", \"sample_rate\": 0}]" | null | 0 | null | null | USER_DROP + + // Matching operation overrides default rate + "[{\"resource\": \"resource\", \"sample_rate\": 1}]" | "0" | 1.0 | 50 | null | USER_KEEP + "[{\"resource\": \"resource\", \"sample_rate\": 0}]" | "1" | 0 | null | null | USER_DROP + + // multiple operation combinations + "[{\"resource\": \"xxx\", \"sample_rate\": 0}, {\"resource\": \"resource\", \"sample_rate\": 1}]" | null | 1.0 | 50 | null | USER_KEEP + "[{\"resource\": \"xxx\", \"sample_rate\": 1}, {\"resource\": \"resource\", \"sample_rate\": 0}]" | null | 0 | null | null | USER_DROP + // Service and operation name rules "[{\"service\": \"service\", \"sample_rate\": 1}, {\"name\": \"operation\", \"sample_rate\": 0}]" | null | 1.0 | 50 | null | USER_KEEP "[{\"service\": \"service\", \"sample_rate\": 1}, {\"name\": \"xxx\", \"sample_rate\": 0}]" | null | 1.0 | 50 | null | USER_KEEP @@ -227,6 +243,7 @@ class RuleBasedSamplingTest extends DDCoreSpecification { "[{\"service\": \"service\", \"name\": \"xxx\", \"sample_rate\": 0}, {\"service\": \"service\", \"name\": \"operation\", \"sample_rate\": 1}]" | null | 1.0 | 50 | null | USER_KEEP "[{\"service\": \"service\", \"name\": \"xxx\", \"sample_rate\": 0}, {\"service\": \"service\", \"sample_rate\": 1}]" | null | 1.0 | 50 | null | USER_KEEP "[{\"service\": \"service\", \"name\": \"xxx\", \"sample_rate\": 0}, {\"name\": \"operation\", \"sample_rate\": 1}]" | null | 1.0 | 50 | null | USER_KEEP + "[{\"service\": \"service\", \"resource\": \"xxx\", \"sample_rate\": 0}, {\"resource\": \"resource\", \"sample_rate\": 1}]" | null | 1.0 | 50 | null | USER_KEEP "[{\"service\": \"service\", \"name\": \"operation\", \"sample_rate\": 0}, {\"service\": \"service\", \"name\": \"operation\", \"sample_rate\": 1}]" | null | 0 | null | null | USER_DROP "[{\"service\": \"service\", \"name\": \"operation\", \"sample_rate\": 0}]" | null | 0 | null | null | USER_DROP } diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/TraceSamplingRulesTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/TraceSamplingRulesTest.groovy index 7962e2d42b8..32793c84ceb 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/TraceSamplingRulesTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/TraceSamplingRulesTest.groovy @@ -18,27 +18,37 @@ class TraceSamplingRulesTest extends DDCoreSpecification { {\"service\": \"usersvc\", \"name\": \"healthcheck\", \"sample_rate\": 0.0}, {\"service\": \"service-name\", \"sample_rate\": 0.5}, {\"name\": \"operation-name\", \"sample_rate\": 0.75}, + {\"resource\": \"resource-name\", \"sample_rate\": 1}, {\"sample_rate\": 0.25} ]""") then: - rules.rules.size() == 4 + rules.rules.size() == 5 rules.rules[0].service == "usersvc" rules.rules[0].name == "healthcheck" + rules.rules[0].resource == null rules.rules[0].sampleRate == 0.0d rules.rules[1].service == "service-name" rules.rules[1].name == null + rules.rules[1].resource == null rules.rules[1].sampleRate == 0.5d rules.rules[2].service == null rules.rules[2].name == "operation-name" + rules.rules[2].resource == null rules.rules[2].sampleRate == 0.75d rules.rules[3].service == null rules.rules[3].name == null - rules.rules[3].sampleRate == 0.25d + rules.rules[3].resource == "resource-name" + rules.rules[3].sampleRate == 1d + + rules.rules[4].service == null + rules.rules[4].name == null + rules.rules[4].resource == null + rules.rules[4].sampleRate == 0.25d } def "Skip Trace Sampling Rules with invalid rate values"() { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/util/GlobPatternTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/util/GlobPatternTest.groovy index 497ac4a68fd..9af94dc894c 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/util/GlobPatternTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/util/GlobPatternTest.groovy @@ -10,9 +10,13 @@ class GlobPatternTest extends DDSpecification { where: globPattern | expectedRegex - "*" | null + "*" | "^.*\$" + "?" | "^.\$" + "??" | "^..\$" "Foo*" | "^Foo.*\$" "abc" | "^abc\$" "?" | "^.\$" + "F?o" | "^F.o\$" + "Bar*" | "^Bar.*\$" } } diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/util/MatchersTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/util/MatchersTest.groovy new file mode 100644 index 00000000000..88fd38d7ad6 --- /dev/null +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/util/MatchersTest.groovy @@ -0,0 +1,54 @@ +package datadog.trace.core.util + +import datadog.trace.test.util.DDSpecification + +class MatchersTest extends DDSpecification { + + def "match-all scenarios must return a null matcher"() { + expect: + Matchers.compileGlob(glob) == null + + where: + glob << [null, "*"] + } + + def "pattern without * or ? must be an ExactMatcher"() { + expect: + Matchers.compileGlob(glob) instanceof Matchers.ExactMatcher + + where: + glob << ["a", "ogre", "bcoho34e2"] + } + + def "pattern with either * or ? must be a PatternMatcher"() { + expect: + Matchers.compileGlob(glob) instanceof Matchers.PatternMatcher + + where: + glob << ["?", "foo*", "*bar", "F?oB?r", "F?o*", "?*", "*?"] + } + + def "an exact matcher is self matching"() { + expect: + Matchers.compileGlob(pattern).matches(pattern) + + where: + pattern << ["", "a", "abc", "cde"] + } + + def "a pattern matcher test"() { + when: + def matcher = Matchers.compileGlob(pattern) + + then: + matcher.matches(matching) + !matcher.matches(nonMatching) + + where: + pattern | matching | nonMatching + "Fo?" | "Foo" | "Fooo" + "Fo*" | "Fo" | "Fa" + "F*B?r" | "FooBar" | "FooFar" + "" | "" | "non-empty" + } +}