diff --git a/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/latestDepTest/groovy/TomcatServer.groovy b/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/latestDepTest/groovy/TomcatServer.groovy index 1432a035609..1bfef641804 100644 --- a/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/latestDepTest/groovy/TomcatServer.groovy +++ b/dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/latestDepTest/groovy/TomcatServer.groovy @@ -66,7 +66,7 @@ class TomcatServer implements WebsocketServer { assert port > 0 if (Config.get().isExperimentalPropagateProcessTagsEnabled()) { server.getEngine().setName("tomcat") - def serverName = TraceUtils.normalizeTag(server.getEngine().getName()) + def serverName = TraceUtils.normalizeTagValue(server.getEngine().getName()) assert ProcessTags.getTagsAsStringList().containsAll(["server.type:tomcat", "server.name:" + serverName]) } else { assert ProcessTags.getTagsAsStringList() == null diff --git a/internal-api/src/main/java/datadog/trace/api/ProcessTags.java b/internal-api/src/main/java/datadog/trace/api/ProcessTags.java index 5ea482600a1..ef9fcb4d51c 100644 --- a/internal-api/src/main/java/datadog/trace/api/ProcessTags.java +++ b/internal-api/src/main/java/datadog/trace/api/ProcessTags.java @@ -146,7 +146,7 @@ static void calculate() { .map( entry -> UTF8BytesString.create( - entry.getKey() + ":" + TraceUtils.normalizeTag(entry.getValue()))); + entry.getKey() + ":" + TraceUtils.normalizeTagValue(entry.getValue()))); utf8ListForm = Collections.unmodifiableList(tagStream.collect(Collectors.toList())); stringListForm = Collections.unmodifiableList( diff --git a/internal-api/src/main/java/datadog/trace/util/TraceUtils.java b/internal-api/src/main/java/datadog/trace/util/TraceUtils.java index b5c1a1b76a4..2e4c52c606c 100644 --- a/internal-api/src/main/java/datadog/trace/util/TraceUtils.java +++ b/internal-api/src/main/java/datadog/trace/util/TraceUtils.java @@ -88,17 +88,34 @@ public static boolean isValidStatusCode(final int httpStatusCode) { // spotless:off /** - * Normalizes a tag value: + * Normalizes a full tag (key:value): * - Only letters, digits, ":", ".", "-", "_" and "/" are allowed. * - If a non-valid char is found, it's replaced with "_". If it's the last char, it's removed. * - It must start with a letter or ":". * - It applies lower case. * * @param tag value - * @return normalized tag value + * @return normalized full tag + * See https://docs.datadoghq.com/getting_started/tagging/ */ // spotless:on public static String normalizeTag(final String tag) { + return doNormalize(tag, true); + } + + /** + * Normalizes a tag value according to the datadog tag conventions - Only letters, digits, ":", + * ".", "-", "_" and "/" are allowed. - If a non-valid char is found, it's replaced with "_". If + * it's the last char, it's removed. - It applies lower case. + * + * @param tagValue the tag value + * @return normalized tag value See https://docs.datadoghq.com/getting_started/tagging/ + */ + public static String normalizeTagValue(final String tagValue) { + return doNormalize(tagValue, false); + } + + private static String doNormalize(String tag, boolean skipNumericalPrefixes) { if (tag == null || tag.isEmpty()) { return ""; } @@ -127,8 +144,10 @@ public static String normalizeTag(final String tag) { continue; } if (builder.length() == 0) { - // this character can't start the string, trim - continue; + if (skipNumericalPrefixes || !Character.isDigit(ch)) { + // this character can't start the string, trim + continue; + } } if (Character.isDigit(ch) || ch == '.' || ch == '/' || ch == '-') { isJumping = false; diff --git a/internal-api/src/test/groovy/datadog/trace/util/TraceUtilsTest.groovy b/internal-api/src/test/groovy/datadog/trace/util/TraceUtilsTest.groovy index ab617958750..49dd10cdea8 100644 --- a/internal-api/src/test/groovy/datadog/trace/util/TraceUtilsTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/util/TraceUtilsTest.groovy @@ -12,11 +12,11 @@ class TraceUtilsTest extends DDSpecification { normalized == expected where: - service | expected - null | TraceUtils.DEFAULT_SERVICE_NAME - "" | TraceUtils.DEFAULT_SERVICE_NAME - "good" | "good" - "bad\$service" | "bad_service" + service | expected + null | TraceUtils.DEFAULT_SERVICE_NAME + "" | TraceUtils.DEFAULT_SERVICE_NAME + "good" | "good" + "bad\$service" | "bad_service" "Too\$Long\$.Too\$Long\$.Too\$Long\$.Too\$Long\$.Too\$Long\$.Too\$Long\$.Too\$Long\$.Too\$Long\$.Too\$Long\$.Too\$Long\$.Too\$Long\$." | "too_long_.too_long_.too_long_.too_long_.too_long_.too_long_.too_long_.too_long_.too_long_.too_long_." } @@ -28,20 +28,20 @@ class TraceUtilsTest extends DDSpecification { normalized == expected where: - name | expected - null | TraceUtils.DEFAULT_OPERATION_NAME - "" | TraceUtils.DEFAULT_OPERATION_NAME - "good" | "good" - "bad-name" | "bad_name" + name | expected + null | TraceUtils.DEFAULT_OPERATION_NAME + "" | TraceUtils.DEFAULT_OPERATION_NAME + "good" | "good" + "bad-name" | "bad_name" "Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-." | "Too_Long.Too_Long.Too_Long.Too_Long.Too_Long.Too_Long.Too_Long.Too_Long.Too_Long.Too_Long." - "pylons.controller"|"pylons.controller" - "trace-api.request"|"trace_api.request" - "/"|"unnamed_operation" - "{çà]test"|"test" - "l___."|"l." - "a___b"|"a_b" - "a___"|"a" - "🐨🐶 繋"|"unnamed_operation" + "pylons.controller" | "pylons.controller" + "trace-api.request" | "trace_api.request" + "/" | "unnamed_operation" + "{çà]test" | "test" + "l___." | "l." + "a___b" | "a_b" + "a___" | "a" + "🐨🐶 繋" | "unnamed_operation" } @@ -53,40 +53,59 @@ class TraceUtilsTest extends DDSpecification { normalized == expected where: - tag | expected - null | "" - "" | "" - "ok" | "ok" - " " | "" - "#test_starting_hash"|"test_starting_hash" - "TestCAPSandSuch" | "testcapsandsuch" + tag | expected + null | "" + "" | "" + "ok" | "ok" + " " | "" + "#test_starting_hash" | "test_starting_hash" + "TestCAPSandSuch" | "testcapsandsuch" "Test Conversion Of Weird !@#\$%^&**() Characters" | "test_conversion_of_weird_characters" - "\$#weird_starting" | "weird_starting" - "allowed:c0l0ns" |"allowed:c0l0ns" - "1love" | "love" - "ünicöde" | "ünicöde" - "ünicöde:metäl"| "ünicöde:metäl" - "Data🐨dog🐶 繋がっ⛰てて"| "data_dog_繋がっ_てて" - " spaces "| "spaces" - " #hashtag!@#spaces #__<># "|"hashtag_spaces" - ":testing"|":testing" - "_foo"|"foo" - ":::test"| ":::test" - "contiguous_____underscores"| "contiguous_underscores" - "foo_"| "foo" - "\u017Fodd_\u017Fcase\u017F"| "\u017Fodd_\u017Fcase\u017F" - "™Ö™Ö™™Ö™"| "ö_ö_ö" - "AlsO:ök"| "also:ök" - ":still_ok"| ":still_ok" - "___trim"| "trim" - "12.:trim@"| ":trim" - "12.:trim@@"| ":trim" - "fun:ky__tag/1"| "fun:ky_tag/1" - "fun:ky@tag/2"| "fun:ky_tag/2" - "fun:ky@@@tag/3"| "fun:ky_tag/3" - "tag:1/2.3"| "tag:1/2.3" - "---fun:k####y_ta@#g/1_@@#"|"fun:k_y_ta_g/1" - "AlsO:œ#@ö))œk"|"also:œ_ö_œk" + "\$#weird_starting" | "weird_starting" + "allowed:c0l0ns" | "allowed:c0l0ns" + "1love" | "love" + "ünicöde" | "ünicöde" + "ünicöde:metäl" | "ünicöde:metäl" + "Data🐨dog🐶 繋がっ⛰てて" | "data_dog_繋がっ_てて" + " spaces " | "spaces" + " #hashtag!@#spaces #__<># " | "hashtag_spaces" + ":testing" | ":testing" + "_foo" | "foo" + ":::test" | ":::test" + "contiguous_____underscores" | "contiguous_underscores" + "foo_" | "foo" + "\u017Fodd_\u017Fcase\u017F" | "\u017Fodd_\u017Fcase\u017F" + "™Ö™Ö™™Ö™" | "ö_ö_ö" + "AlsO:ök" | "also:ök" + ":still_ok" | ":still_ok" + "___trim" | "trim" + "12.:trim@" | ":trim" + "12.:trim@@" | ":trim" + "fun:ky__tag/1" | "fun:ky_tag/1" + "fun:ky@tag/2" | "fun:ky_tag/2" + "fun:ky@@@tag/3" | "fun:ky_tag/3" + "tag:1/2.3" | "tag:1/2.3" + "---fun:k####y_ta@#g/1_@@#" | "fun:k_y_ta_g/1" + "AlsO:œ#@ö))œk" | "also:œ_ö_œk" + } + + def "test normalize tag value"() { + when: + String normalized = TraceUtils.normalizeTagValue(tag) + + then: + normalized == expected + + where: + tag | expected + null | "" + "" | "" + "ok" | "ok" + " " | "" + "TestCAPSandSuch" | "testcapsandsuch" + "Test Conversion Of Weird !@#\$%^&**() Characters" | "test_conversion_of_weird_characters" + "1.55.0-SNAPSHOT" | "1.55.0-snapshot" + "a,b" | "a_b" } @@ -98,11 +117,11 @@ class TraceUtilsTest extends DDSpecification { normalized == expected where: - spanType | expected - null | null - "" | "" - "ok" | "ok" - "VeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLong"|"VeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVery" + spanType | expected + null | null + "" | "" + "ok" | "ok" + "VeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLong" | "VeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVery" } def "test normalize env"() { @@ -113,11 +132,11 @@ class TraceUtilsTest extends DDSpecification { normalized == expected where: - env | expected + env | expected null | TraceUtils.DEFAULT_ENV "" | TraceUtils.DEFAULT_ENV - "ok" | "ok" - repeat("a",300)|repeat("a",200) + "ok" | "ok" + repeat("a", 300) | repeat("a", 200) } def "test is valid http status code"() { @@ -129,14 +148,14 @@ class TraceUtilsTest extends DDSpecification { where: httpStatusCode | expected - 100 | true - 404 | true - 600 | false + 100 | true + 404 | true + 600 | false } def repeat(String str, int length) { StringBuilder sb = new StringBuilder(length) - for(int i=0;i