Ignore empty entries in tags config strings
Empty tags (or service names) do not make sense. Currently we ignore
whole config line which is too harsh. Instead just ignore a key with
empty value.
MODIFY
dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java
dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java
package datadog.opentracing;
+import static datadog.trace.common.DDTraceConfig.HEADER_TAGS;
+import static datadog.trace.common.DDTraceConfig.SERVICE_MAPPING;
+import static datadog.trace.common.DDTraceConfig.SERVICE_NAME;
+import static datadog.trace.common.DDTraceConfig.SPAN_TAGS;
import static datadog.trace.common.util.Config.parseMap;
import datadog.opentracing.decorators.AbstractDecorator;
@@ -89,12 +93,12 @@ public class DDTracer implements io.opentracing.Tracer {
public DDTracer(final Properties config) {
this(
- config.getProperty(DDTraceConfig.SERVICE_NAME),
+ config.getProperty(SERVICE_NAME),
Writer.Builder.forConfig(config),
Sampler.Builder.forConfig(config),
- parseMap(config.getProperty(DDTraceConfig.SPAN_TAGS)),
- parseMap(config.getProperty(DDTraceConfig.SERVICE_MAPPING)),
- parseMap(config.getProperty(DDTraceConfig.HEADER_TAGS)));
+ parseMap(config.getProperty(SPAN_TAGS), SPAN_TAGS),
+ parseMap(config.getProperty(SERVICE_MAPPING), SERVICE_MAPPING),
+ parseMap(config.getProperty(HEADER_TAGS), HEADER_TAGS));
log.debug("Using config: {}", config);
}
@@ -167,9 +171,9 @@ public class DDTracer implements io.opentracing.Tracer {
UNASSIGNED_DEFAULT_SERVICE_NAME,
writer,
new AllSampler(),
- parseMap(new DDTraceConfig().getProperty(DDTraceConfig.SPAN_TAGS)),
- parseMap(new DDTraceConfig().getProperty(DDTraceConfig.SERVICE_MAPPING)),
- parseMap(new DDTraceConfig().getProperty(DDTraceConfig.HEADER_TAGS)));
+ parseMap(new DDTraceConfig().getProperty(SPAN_TAGS), SPAN_TAGS),
+ parseMap(new DDTraceConfig().getProperty(SERVICE_MAPPING), SPAN_TAGS),
+ parseMap(new DDTraceConfig().getProperty(HEADER_TAGS), SPAN_TAGS));
}
/**

MODIFY
dd-trace-ot/src/main/java/datadog/trace/common/util/Config.java
dd-trace-ot/src/main/java/datadog/trace/common/util/Config.java
@@ -16,12 +16,13 @@ public abstract class Config {
return name.toUpperCase().replace(".", "_");
}
- public static Map<String, String> parseMap(final String str) {
+ public static Map<String, String> parseMap(final String str, final String settingName) {
if (str == null || str.trim().isEmpty()) {
return Collections.emptyMap();
}
- if (!str.matches("(([^,:]+:[^,:]+,)*([^,:]+:[^,:]+),?)?")) {
- log.warn("Invalid config '{}'. Must match 'key1:value1,key2:value2'.", str);
+ if (!str.matches("(([^,:]+:[^,:]*,)*([^,:]+:[^,:]*),?)?")) {
+ log.warn(
+ "Invalid config for {}: '{}'. Must match 'key1:value1,key2:value2'.", settingName, str);
return Collections.emptyMap();
}
@@ -31,7 +32,13 @@ public abstract class Config {
for (final String token : tokens) {
final String[] keyValue = token.split(":", -1);
if (keyValue.length == 2) {
- map.put(keyValue[0].trim(), keyValue[1].trim());
+ final String key = keyValue[0].trim();
+ final String value = keyValue[1].trim();
+ if (value.length() <= 0) {
+ log.warn("Ignoring empty value for key '{}' in config for {}", key, settingName);
+ continue;
+ }
+ map.put(key, value);
}
}
return Collections.unmodifiableMap(map);

MODIFY
dd-trace-ot/src/test/groovy/datadog/trace/DDTraceConfigTest.groovy
dd-trace-ot/src/test/groovy/datadog/trace/DDTraceConfigTest.groovy
@@ -129,7 +129,7 @@ class DDTraceConfigTest extends Specification {
where:
mapString | map
"a:1, a:2, a:3" | [a: "3"]
- "a:b,c:d" | [a: "b", c: "d"]
+ "a:b,c:d,e:" | [a: "b", c: "d"]
}
def "verify single override on #source for #key"() {
@@ -151,20 +151,21 @@ class DDTraceConfigTest extends Specification {
def "parsing valid string returns a map"() {
expect:
- parseMap(str) == map
+ parseMap(str, "test") == map
where:
str | map
+ "a:" | [:]
"a:a;" | [a: "a;"]
"a:1, a:2, a:3" | [a: "3"]
- "a:b,c:d" | [a: "b", c: "d"]
+ "a:b,c:d,e:" | [a: "b", c: "d"]
"key 1!:va|ue_1," | ["key 1!": "va|ue_1"]
" key1 :value1 ,\t key2: value2" | [key1: "value1", key2: "value2"]
}
def "parsing an invalid string returns an empty map"() {
expect:
- parseMap(str) == [:]
+ parseMap(str, "test") == [:]
where:
str | _
@@ -172,7 +173,6 @@ class DDTraceConfigTest extends Specification {
"" | _
"1" | _
"a" | _
- "a:" | _
"a,1" | _
"in:val:id" | _
"a:b:c:d" | _

Ignore empty entries in tags config strings

Empty tags (or service names) do not make sense. Currently we ignore
whole config line which is too harsh. Instead just ignore a key with
empty value.