diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/URLAsResourceName.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/URLAsResourceName.java index c696c96cb8..c4c2e7e51e 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/URLAsResourceName.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/URLAsResourceName.java @@ -1,44 +1,24 @@ package datadog.opentracing.decorators; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import datadog.opentracing.DDSpanContext; import datadog.trace.api.DDTags; -import datadog.trace.common.util.ConfigUtils; import io.opentracing.tag.Tags; import java.net.MalformedURLException; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; +import java.util.regex.Pattern; /** Decorator for servlet contrib */ public class URLAsResourceName extends AbstractDecorator { - public static final String CONFIG_PATH = "dd-trace"; - public static final Config.Rule RULE_QPARAM = new Config.Rule("\\?.*$", ""); - public static final Config.Rule RULE_DIGIT = new Config.Rule("\\d+", "?"); - private List patterns = new ArrayList<>(); + // Matches everything after the ? character. + public static final Pattern QUERYSTRING = Pattern.compile("\\?.*$"); + // Matches any path segments with numbers in them. + public static final Pattern PATH_MIXED_ALPHANUMERICS = + Pattern.compile("/(?:[^\\/\\d\\?]*[\\d]+[^\\/\\?]*)"); public URLAsResourceName() { - this(CONFIG_PATH); - } - - public URLAsResourceName(final String configPath) { - super(); this.setMatchingTag(Tags.HTTP_URL.getKey()); this.setSetTag(DDTags.RESOURCE_NAME); - - try { - final Config config = ConfigUtils.loadConfigFromResource(configPath, Config.class); - for (final Config.Rule pattern : config.urlResourcePatterns) { - patterns.add(pattern); - } - } catch (final Throwable ex) { - // do nothing - } finally { - patterns.add(RULE_QPARAM); - patterns.add(RULE_DIGIT); - } } @Override @@ -76,95 +56,12 @@ public class URLAsResourceName extends AbstractDecorator { } // Method to normalise the url string - String norm(final String origin) { + private String norm(final String origin) { String norm = origin; - - // Apply rules - for (final Config.Rule p : patterns) { - norm = norm.replaceAll(p.regex, p.replacement); - // if the rule is final, so do not apply others functions - if (p.isFinal) { - break; - } - } + norm = QUERYSTRING.matcher(norm).replaceAll(""); + norm = PATH_MIXED_ALPHANUMERICS.matcher(norm).replaceAll("/?"); return norm; } - - // For tests - List getPatterns() { - return patterns; - } - - // For tests - void setPatterns(final List patterns) { - this.patterns = patterns; - } - - /** Properties concerning the UrlAsResourceDecorator in the YAML config */ - @JsonIgnoreProperties(ignoreUnknown = true) - public static class Config { - - List urlResourcePatterns; - - public List getUrlResourcePatterns() { - return urlResourcePatterns; - } - - public void setUrlResourcePatterns(final List urlResourcePatterns) { - this.urlResourcePatterns = urlResourcePatterns; - } - - public static class Rule { - - String regex; - String replacement; - boolean isFinal = false; - - public Rule() {} - - public Rule(final String regex, final String replacement) { - this.regex = regex; - this.replacement = replacement; - } - - public String getRegex() { - return regex; - } - - public void setRegex(final String regex) { - this.regex = regex; - } - - public String getReplacement() { - return replacement; - } - - public void setReplacement(final String replacement) { - this.replacement = replacement; - } - - public boolean isFinal() { - return isFinal; - } - - public void setFinal(final boolean isFinal) { - this.isFinal = isFinal; - } - - @Override - public boolean equals(final Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - final Rule rule = (Rule) o; - return Objects.equals(regex, rule.regex) && Objects.equals(replacement, rule.replacement); - } - - @Override - public int hashCode() { - return Objects.hash(regex, replacement, isFinal); - } - } - } } diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy index 1cac33db4c..c9d5be978d 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy @@ -1,32 +1,17 @@ package datadog.opentracing.decorators +import datadog.opentracing.DDSpanContext +import datadog.trace.common.sampling.PrioritySampling +import io.opentracing.tag.Tags import spock.lang.Specification +import spock.lang.Subject class URLAsResourceNameTest extends Specification { - def "load the config from the yaml files"() { - setup: - def patterns = new URLAsResourceName("dd-url-patterns").getPatterns() - - expect: - patterns.size() == 4 - // 0 and 1 are from the config file - patterns.get(0).regex == ".*" - patterns.get(0).replacement == "foo" - patterns.get(1).regex == "foo" - patterns.get(1).replacement == "bar" - - // the last and before-last are defaults - patterns.get(patterns.size() - 2) == URLAsResourceName.RULE_QPARAM - patterns.get(patterns.size() - 1) == URLAsResourceName.RULE_DIGIT - - } + @Subject + def decorator = new URLAsResourceName() def "remove query params"() { - - setup: - def decorator = new URLAsResourceName() - when: def norm = decorator.norm(input) @@ -34,16 +19,14 @@ class URLAsResourceNameTest extends Specification { norm == output where: - input << ["/search?id=100&status=true"] - output << ["/search"] - + input | output + "/search" | "/search" + "/search?" | "/search" + "/search?id=100&private=true" | "/search" + "/search?id=100&private=true?" | "/search" } def "should replace all digits"() { - - setup: - def decorator = new URLAsResourceName() - when: def norm = decorator.norm(input) @@ -51,18 +34,16 @@ class URLAsResourceNameTest extends Specification { norm == output where: - input << ["/user/100/repository/50"] - output << ["/user/?/repository/?"] - + input | output + "/1" | "/?" + "/9999" | "/?" + "/user/1" | "/user/?" + "/user/1/" | "/user/?/" + "/user/1/repo/50" | "/user/?/repo/?" + "/user/1/repo/50/" | "/user/?/repo/?/" } - def "norm should apply custom rules"() { - - setup: - def decorator = new URLAsResourceName() - def r1 = new URLAsResourceName.Config.Rule(/(\\/users\\/)([^\/]*)/, /$1:id/) - decorator.setPatterns(Arrays.asList(r1)) - + def "should replace segments with mixed-characters"() { when: def norm = decorator.norm(input) @@ -70,37 +51,58 @@ class URLAsResourceNameTest extends Specification { norm == output where: - input << ["/users/guillaume/list_repository/"] - output << ["/users/:id/list_repository/"] - + input | output + "/a1" | "/?" + "/1a" | "/?" + "/abc/-1?" | "/abc/?" + "/ABC/a-1/b_2/c.3/d4d/5f/6" | "/ABC/?/?/?/?/?/?" + "/user/asdf123/repository/01234567-9ABC-DEF0-1234" | "/user/?/repository/?" } - def "skip others rules if the current is set as final"() { - - // Same test as above except we replace :id by :id_01 - // And we want to stop the rule chain just after that. - - setup: - def decorator = new URLAsResourceName() - def r1 = new URLAsResourceName.Config.Rule(/(\\/users\\/)([^\/]*)/, /$1:id_01/) - decorator.setPatterns(Arrays.asList(r1, URLAsResourceName.RULE_DIGIT)) - + def "should leave other segments alone"() { when: - r1.setFinal(false) def norm = decorator.norm(input) then: - norm == "/users/:id_?/list_repository/" - - when: - r1.setFinal(true) - norm = decorator.norm(input) - - then: - norm == "/users/:id_01/list_repository/" + norm == input where: - input = "/users/guillaume/list_repository/" + input | _ + "/a-b" | _ + "/a_b" | _ + "/a.b" | _ + "/a-b/a-b" | _ + "/a_b/a_b" | _ + "/a.b/a.b" | _ + } + def "sets the resource name"() { + when: + final DDSpanContext context = + new DDSpanContext( + 1L, + 1L, + 0L, + "fakeService", + "fakeOperation", + "fakeResource", + PrioritySampling.UNSET, + Collections. emptyMap(), + false, + "fakeType", + tags, + null, + null) + + then: + decorator.afterSetTag(context, Tags.HTTP_URL.getKey(), value) + context.resourceName == resourceName + + where: + value | resourceName | tags + "/path" | "/path" | [:] + "/ABC/a-1/b_2/c.3/d4d/5f/6" | "/ABC/?/?/?/?/?/?" | [:] + "/not-found" | "fakeResource" | [(Tags.HTTP_STATUS.key): 404] + "/with-method" | "Post /with-method" | [(Tags.HTTP_METHOD.key): "Post"] } } diff --git a/dd-trace-ot/src/test/resources/dd-url-patterns.yaml b/dd-trace-ot/src/test/resources/dd-url-patterns.yaml deleted file mode 100644 index 61c392fb7c..0000000000 --- a/dd-trace-ot/src/test/resources/dd-url-patterns.yaml +++ /dev/null @@ -1,15 +0,0 @@ -decorators: - - type: HTTPComponent - matchingValue: hello - setValue: world - - type: HTTPComponent - matchingValue: foo - setValue: bar - - type: URLAsResourceName - -# Configuration of the URLAsResourceName -urlResourcePatterns: - - regex: ".*" - replacement: "foo" - - regex: "foo" - replacement: "bar"