Merge pull request #418 from DataDog/mar-kolya/ignore-empty-tags-in-config

Ignore empty entries in tags config strings

Author: mar-kolya

dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java

@@ -1,5 +1,9 @@
 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 DDTracer(final String serviceName) {
 
   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 DDTracer(final Writer writer) {
         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));
   }
 
   /**

dd-trace-ot/src/main/java/datadog/trace/common/util/Config.java

@@ -16,12 +16,13 @@ public static String propToEnvName(final String name) {
     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 static Map<String, String> parseMap(final String str) {
     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);

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,28 +151,28 @@ 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         | _
     null        | _
     ""          | _
     "1"         | _
     "a"         | _
-    "a:"        | _
     "a,1"       | _
     "in:val:id" | _
     "a:b:c:d"   | _