Skip to content

Commit 4985d55

Browse files
amarzialiamarziali
authored
Disambiguate normalizeTag from normalizeTagValue (#9816)
1 parent aca2731 commit 4985d55

File tree

4 files changed

+107
-69
lines changed

4 files changed

+107
-69
lines changed

‎dd-java-agent/instrumentation/tomcat/tomcat-5.5/src/latestDepTest/groovy/TomcatServer.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ class TomcatServer implements WebsocketServer {
6666
assert port > 0
6767
if (Config.get().isExperimentalPropagateProcessTagsEnabled()) {
6868
server.getEngine().setName("tomcat")
69-
def serverName = TraceUtils.normalizeTag(server.getEngine().getName())
69+
def serverName = TraceUtils.normalizeTagValue(server.getEngine().getName())
7070
assert ProcessTags.getTagsAsStringList().containsAll(["server.type:tomcat", "server.name:" + serverName])
7171
} else {
7272
assert ProcessTags.getTagsAsStringList() == null

‎internal-api/src/main/java/datadog/trace/api/ProcessTags.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ static void calculate() {
146146
.map(
147147
entry ->
148148
UTF8BytesString.create(
149-
entry.getKey() + ":" + TraceUtils.normalizeTag(entry.getValue())));
149+
entry.getKey() + ":" + TraceUtils.normalizeTagValue(entry.getValue())));
150150
utf8ListForm = Collections.unmodifiableList(tagStream.collect(Collectors.toList()));
151151
stringListForm =
152152
Collections.unmodifiableList(

‎internal-api/src/main/java/datadog/trace/util/TraceUtils.java‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,17 +88,34 @@ public static boolean isValidStatusCode(final int httpStatusCode) {
8888
// spotless:off
8989

9090
/**
91-
* Normalizes a tag value:
91+
* Normalizes a full tag (key:value):
9292
* - Only letters, digits, ":", ".", "-", "_" and "/" are allowed.
9393
* - If a non-valid char is found, it's replaced with "_". If it's the last char, it's removed.
9494
* - It must start with a letter or ":".
9595
* - It applies lower case.
9696
*
9797
* @param tag value
98-
* @return normalized tag value
98+
* @return normalized full tag
99+
* See https://docs.datadoghq.com/getting_started/tagging/
99100
*/
100101
// spotless:on
101102
public static String normalizeTag(final String tag) {
103+
return doNormalize(tag, true);
104+
}
105+
106+
/**
107+
* Normalizes a tag value according to the datadog tag conventions - Only letters, digits, ":",
108+
* ".", "-", "_" and "/" are allowed. - If a non-valid char is found, it's replaced with "_". If
109+
* it's the last char, it's removed. - It applies lower case.
110+
*
111+
* @param tagValue the tag value
112+
* @return normalized tag value See https://docs.datadoghq.com/getting_started/tagging/
113+
*/
114+
public static String normalizeTagValue(final String tagValue) {
115+
return doNormalize(tagValue, false);
116+
}
117+
118+
private static String doNormalize(String tag, boolean skipNumericalPrefixes) {
102119
if (tag == null || tag.isEmpty()) {
103120
return "";
104121
}
@@ -127,8 +144,10 @@ public static String normalizeTag(final String tag) {
127144
continue;
128145
}
129146
if (builder.length() == 0) {
130-
// this character can't start the string, trim
131-
continue;
147+
if (skipNumericalPrefixes || !Character.isDigit(ch)) {
148+
// this character can't start the string, trim
149+
continue;
150+
}
132151
}
133152
if (Character.isDigit(ch) || ch == '.' || ch == '/' || ch == '-') {
134153
isJumping = false;

‎internal-api/src/test/groovy/datadog/trace/util/TraceUtilsTest.groovy‎

Lines changed: 82 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ class TraceUtilsTest extends DDSpecification {
1212
normalized == expected
1313

1414
where:
15-
service | expected
16-
null | TraceUtils.DEFAULT_SERVICE_NAME
17-
"" | TraceUtils.DEFAULT_SERVICE_NAME
18-
"good" | "good"
19-
"bad\$service" | "bad_service"
15+
service | expected
16+
null | TraceUtils.DEFAULT_SERVICE_NAME
17+
"" | TraceUtils.DEFAULT_SERVICE_NAME
18+
"good" | "good"
19+
"bad\$service" | "bad_service"
2020
"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_."
2121
}
2222

@@ -28,20 +28,20 @@ class TraceUtilsTest extends DDSpecification {
2828
normalized == expected
2929

3030
where:
31-
name | expected
32-
null | TraceUtils.DEFAULT_OPERATION_NAME
33-
"" | TraceUtils.DEFAULT_OPERATION_NAME
34-
"good" | "good"
35-
"bad-name" | "bad_name"
31+
name | expected
32+
null | TraceUtils.DEFAULT_OPERATION_NAME
33+
"" | TraceUtils.DEFAULT_OPERATION_NAME
34+
"good" | "good"
35+
"bad-name" | "bad_name"
3636
"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."
37-
"pylons.controller"|"pylons.controller"
38-
"trace-api.request"|"trace_api.request"
39-
"/"|"unnamed_operation"
40-
"{çà]test"|"test"
41-
"l___."|"l."
42-
"a___b"|"a_b"
43-
"a___"|"a"
44-
"🐨🐶 繋"|"unnamed_operation"
37+
"pylons.controller" | "pylons.controller"
38+
"trace-api.request" | "trace_api.request"
39+
"/" | "unnamed_operation"
40+
"{çà]test" | "test"
41+
"l___." | "l."
42+
"a___b" | "a_b"
43+
"a___" | "a"
44+
"🐨🐶 繋" | "unnamed_operation"
4545
}
4646

4747

@@ -53,40 +53,59 @@ class TraceUtilsTest extends DDSpecification {
5353
normalized == expected
5454

5555
where:
56-
tag | expected
57-
null | ""
58-
"" | ""
59-
"ok" | "ok"
60-
" " | ""
61-
"#test_starting_hash"|"test_starting_hash"
62-
"TestCAPSandSuch" | "testcapsandsuch"
56+
tag | expected
57+
null | ""
58+
"" | ""
59+
"ok" | "ok"
60+
" " | ""
61+
"#test_starting_hash" | "test_starting_hash"
62+
"TestCAPSandSuch" | "testcapsandsuch"
6363
"Test Conversion Of Weird !@#\$%^&**() Characters" | "test_conversion_of_weird_characters"
64-
"\$#weird_starting" | "weird_starting"
65-
"allowed:c0l0ns" |"allowed:c0l0ns"
66-
"1love" | "love"
67-
"ünicöde" | "ünicöde"
68-
"ünicöde:metäl"| "ünicöde:metäl"
69-
"Data🐨dog🐶 繋がっ⛰てて"| "data_dog_繋がっ_てて"
70-
" spaces "| "spaces"
71-
" #hashtag!@#spaces #__<># "|"hashtag_spaces"
72-
":testing"|":testing"
73-
"_foo"|"foo"
74-
":::test"| ":::test"
75-
"contiguous_____underscores"| "contiguous_underscores"
76-
"foo_"| "foo"
77-
"\u017Fodd_\u017Fcase\u017F"| "\u017Fodd_\u017Fcase\u017F"
78-
"™Ö™Ö™™Ö™"| "ö_ö_ö"
79-
"AlsO:ök"| "also:ök"
80-
":still_ok"| ":still_ok"
81-
"___trim"| "trim"
82-
"12.:trim@"| ":trim"
83-
"12.:trim@@"| ":trim"
84-
"fun:ky__tag/1"| "fun:ky_tag/1"
85-
"fun:ky@tag/2"| "fun:ky_tag/2"
86-
"fun:ky@@@tag/3"| "fun:ky_tag/3"
87-
"tag:1/2.3"| "tag:1/2.3"
88-
"---fun:k####y_ta@#g/1_@@#"|"fun:k_y_ta_g/1"
89-
"AlsO:œ#@ö))œk"|"also:œ_ö_œk"
64+
"\$#weird_starting" | "weird_starting"
65+
"allowed:c0l0ns" | "allowed:c0l0ns"
66+
"1love" | "love"
67+
"ünicöde" | "ünicöde"
68+
"ünicöde:metäl" | "ünicöde:metäl"
69+
"Data🐨dog🐶 繋がっ⛰てて" | "data_dog_繋がっ_てて"
70+
" spaces " | "spaces"
71+
" #hashtag!@#spaces #__<># " | "hashtag_spaces"
72+
":testing" | ":testing"
73+
"_foo" | "foo"
74+
":::test" | ":::test"
75+
"contiguous_____underscores" | "contiguous_underscores"
76+
"foo_" | "foo"
77+
"\u017Fodd_\u017Fcase\u017F" | "\u017Fodd_\u017Fcase\u017F"
78+
"™Ö™Ö™™Ö™" | "ö_ö_ö"
79+
"AlsO:ök" | "also:ök"
80+
":still_ok" | ":still_ok"
81+
"___trim" | "trim"
82+
"12.:trim@" | ":trim"
83+
"12.:trim@@" | ":trim"
84+
"fun:ky__tag/1" | "fun:ky_tag/1"
85+
"fun:ky@tag/2" | "fun:ky_tag/2"
86+
"fun:ky@@@tag/3" | "fun:ky_tag/3"
87+
"tag:1/2.3" | "tag:1/2.3"
88+
"---fun:k####y_ta@#g/1_@@#" | "fun:k_y_ta_g/1"
89+
"AlsO:œ#@ö))œk" | "also:œ_ö_œk"
90+
}
91+
92+
def "test normalize tag value"() {
93+
when:
94+
String normalized = TraceUtils.normalizeTagValue(tag)
95+
96+
then:
97+
normalized == expected
98+
99+
where:
100+
tag | expected
101+
null | ""
102+
"" | ""
103+
"ok" | "ok"
104+
" " | ""
105+
"TestCAPSandSuch" | "testcapsandsuch"
106+
"Test Conversion Of Weird !@#\$%^&**() Characters" | "test_conversion_of_weird_characters"
107+
"1.55.0-SNAPSHOT" | "1.55.0-snapshot"
108+
"a,b" | "a_b"
90109
}
91110

92111

@@ -98,11 +117,11 @@ class TraceUtilsTest extends DDSpecification {
98117
normalized == expected
99118

100119
where:
101-
spanType | expected
102-
null | null
103-
"" | ""
104-
"ok" | "ok"
105-
"VeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLong"|"VeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVery"
120+
spanType | expected
121+
null | null
122+
"" | ""
123+
"ok" | "ok"
124+
"VeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLong" | "VeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVeryLongVery"
106125
}
107126

108127
def "test normalize env"() {
@@ -113,11 +132,11 @@ class TraceUtilsTest extends DDSpecification {
113132
normalized == expected
114133

115134
where:
116-
env | expected
135+
env | expected
117136
null | TraceUtils.DEFAULT_ENV
118137
"" | TraceUtils.DEFAULT_ENV
119-
"ok" | "ok"
120-
repeat("a",300)|repeat("a",200)
138+
"ok" | "ok"
139+
repeat("a", 300) | repeat("a", 200)
121140
}
122141

123142
def "test is valid http status code"() {
@@ -129,14 +148,14 @@ class TraceUtilsTest extends DDSpecification {
129148

130149
where:
131150
httpStatusCode | expected
132-
100 | true
133-
404 | true
134-
600 | false
151+
100 | true
152+
404 | true
153+
600 | false
135154
}
136155

137156
def repeat(String str, int length) {
138157
StringBuilder sb = new StringBuilder(length)
139-
for(int i=0;i<length;i++) {
158+
for (int i = 0; i < length; i++) {
140159
sb.append(str)
141160
}
142161
return sb.toString()

0 commit comments

Comments
 (0)