diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 240c3b41790..599e96c5cfd 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -775,7 +775,14 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins { final Map tags = new HashMap<>(configProvider.getMergedMap(GLOBAL_TAGS)); - tags.putAll(configProvider.getMergedMap(TRACE_TAGS, TAGS)); + if (experimentalFeaturesEnabled.contains("DD_TAGS")) { + tags.putAll(configProvider.getMergedTagsMap(TRACE_TAGS, TAGS)); + } else { + tags.putAll(configProvider.getMergedMap(TRACE_TAGS, TAGS)); + } + if (serviceNameSetByUser) { // prioritize service name set by DD_SERVICE over DD_TAGS config + tags.remove("service"); + } this.tags = getMapWithPropertiesDefinedByEnvironment(tags, ENV, VERSION); } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java index 1bd9422087f..98d4af50d3c 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java @@ -84,6 +84,19 @@ static Map parseMap( return map; } + @Nonnull + static Map parseTraceTagsMap( + final String str, final char keyValueSeparator, final List argSeparators) { + // If we ever want to have default values besides an empty map, this will need to change. + String trimmed = Strings.trim(str); + if (trimmed.isEmpty()) { + return Collections.emptyMap(); + } + Map map = new HashMap<>(); + loadTraceTagsMap(map, trimmed, keyValueSeparator, argSeparators); + return map; + } + /** * This parses a mixed map that can have both key value pairs, and also keys only, that will get * values on the form "defaultPrefix.key". For keys without a value, the corresponding value will @@ -201,6 +214,63 @@ private static void loadMap( } } + private static void loadTraceTagsMap( + Map map, + String str, + char keyValueSeparator, + final List argSeparators) { + int start = 0; + int splitter = str.indexOf(keyValueSeparator, start); + char argSeparator = '\0'; + int argSeparatorInd = -1; + + // Given a list of separators ordered by priority, find the first (highest priority) separator + // that appears in the string and store its value and first occurrence in the string + for (Character sep : argSeparators) { + argSeparatorInd = str.indexOf(sep); + if (argSeparatorInd != -1) { + argSeparator = sep; + break; + } + } + while (start < str.length()) { + int nextSplitter = + argSeparatorInd == -1 + ? -1 + : str.indexOf( + keyValueSeparator, + argSeparatorInd + 1); // next splitter after the next argSeparator + int nextArgSeparator = + argSeparatorInd == -1 ? -1 : str.indexOf(argSeparator, argSeparatorInd + 1); + int end = argSeparatorInd == -1 ? str.length() : argSeparatorInd; + + if (start >= end) { // the character is only the delimiter + start = end + 1; + splitter = nextSplitter; + argSeparatorInd = nextArgSeparator; + continue; + } + + String key, value; + if (splitter >= end + || splitter + == -1) { // only key, no value; either due end of string or substring not having + // splitter + key = str.substring(start, end).trim(); + value = ""; + } else { + key = str.substring(start, splitter).trim(); + value = str.substring(splitter + 1, end).trim(); + } + if (!key.isEmpty()) { + map.put(key, value); + } + splitter = nextSplitter; + argSeparatorInd = nextArgSeparator; + start = end + 1; + } + } + private static void loadMapWithOptionalMapping( Map map, String str, diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java index 90e43cd30ab..0a2715dd11a 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java @@ -9,6 +9,7 @@ import java.io.FileNotFoundException; import java.io.FileReader; import java.io.IOException; +import java.util.Arrays; import java.util.BitSet; import java.util.HashMap; import java.util.HashSet; @@ -263,6 +264,28 @@ public Map getMergedMap(String key, String... aliases) { return merged; } + public Map getMergedTagsMap(String key, String... aliases) { + Map merged = new HashMap<>(); + ConfigOrigin origin = ConfigOrigin.DEFAULT; + // System properties take precedence over env + // prior art: + // https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html + // We reverse iterate to allow overrides + for (int i = sources.length - 1; 0 <= i; i--) { + String value = sources[i].get(key, aliases); + Map parsedMap = + ConfigConverter.parseTraceTagsMap(value, ':', Arrays.asList(',', ' ')); + if (!parsedMap.isEmpty()) { + origin = sources[i].origin(); + } + merged.putAll(parsedMap); + } + if (collectConfig) { + ConfigCollector.get().put(key, merged, origin); + } + return merged; + } + public Map getOrderedMap(String key) { LinkedHashMap merged = new LinkedHashMap<>(); ConfigOrigin origin = ConfigOrigin.DEFAULT; diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 1e661867875..f9594ab1165 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -1795,6 +1795,19 @@ class ConfigTest extends DDSpecification { 'service.version': 'my-svc-vers'] } + def "service name prioritizes values from DD_SERVICE over tags"() { + setup: + System.setProperty(PREFIX + TAGS, "service:service-name-from-tags") + System.setProperty(PREFIX + SERVICE, "service-name-from-dd-service") + + when: + def config = new Config() + + then: + config.serviceName == "service-name-from-dd-service" + !config.mergedSpanTags.containsKey("service") + } + def "DD_SERVICE precedence over 'dd.service.name' java property is set; 'dd.service' overwrites DD_SERVICE"() { setup: environmentVariables.set(DD_SERVICE_NAME_ENV, "dd-service-name-env-var") @@ -1808,7 +1821,7 @@ class ConfigTest extends DDSpecification { then: config.serviceName == "dd-service-java-prop" - config.mergedSpanTags == [service: 'service-tag-in-dd-trace-global-tags-java-property', 'service.version': 'my-svc-vers'] + config.mergedSpanTags == ['service.version': 'my-svc-vers'] config.mergedJmxTags == [(RUNTIME_ID_TAG) : config.getRuntimeId(), (SERVICE_TAG): config.serviceName, 'service.version': 'my-svc-vers'] } @@ -1824,7 +1837,7 @@ class ConfigTest extends DDSpecification { then: config.serviceName == "dd-service-env-var" - config.mergedSpanTags == [service: 'service-tag-in-dd-trace-global-tags-java-property', 'service.version': 'my-svc-vers'] + config.mergedSpanTags == ['service.version': 'my-svc-vers'] config.mergedJmxTags == [(RUNTIME_ID_TAG) : config.getRuntimeId(), (SERVICE_TAG): config.serviceName, 'service.version': 'my-svc-vers'] } @@ -1840,12 +1853,12 @@ class ConfigTest extends DDSpecification { then: config.serviceName == "dd-service-java-prop" - config.mergedSpanTags == [service: 'service-tag-in-dd-trace-global-tags-java-property', 'service.version': 'my-svc-vers'] + config.mergedSpanTags == ['service.version': 'my-svc-vers'] config.mergedJmxTags == [(RUNTIME_ID_TAG) : config.getRuntimeId(), (SERVICE_TAG): config.serviceName, 'service.version': 'my-svc-vers'] } - def "set servicenaem by DD_SERVICE"() { + def "set servicename by DD_SERVICE"() { setup: environmentVariables.set("DD_SERVICE", "dd-service-env-var") System.setProperty(PREFIX + GLOBAL_TAGS, "service:service-tag-in-dd-trace-global-tags-java-property,service.version:my-svc-vers") @@ -1856,7 +1869,7 @@ class ConfigTest extends DDSpecification { then: config.serviceName == "dd-service-env-var" - config.mergedSpanTags == [service: 'service-tag-in-dd-trace-global-tags-java-property', 'service.version': 'my-svc-vers'] + config.mergedSpanTags == ['service.version': 'my-svc-vers'] config.mergedJmxTags == [(RUNTIME_ID_TAG) : config.getRuntimeId(), (SERVICE_TAG): config.serviceName, 'service.version': 'my-svc-vers'] } @@ -1876,6 +1889,35 @@ class ConfigTest extends DDSpecification { [serviceProperty, serviceName] << [[SERVICE, SERVICE_NAME], [DEFAULT_SERVICE_NAME, "my-service"]].combinations() } + def "verify behavior of features under DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED"() { + setup: + environmentVariables.set("DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED", "DD_TAGS") + environmentVariables.set("DD_TAGS", "env:test,aKey:aVal bKey:bVal cKey:") + + when: + def config = new Config() + + then: + config.experimentalFeaturesEnabled == ["DD_TAGS"].toSet() + + //verify expected behavior enabled under feature flag + config.globalTags == [env: "test", aKey: "aVal bKey:bVal cKey:"] + } + + def "verify behavior of 'breaking change' configs when not under DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED"() { + setup: + environmentVariables.set("DD_TAGS", "env:test,aKey:aVal bKey:bVal cKey:") + + when: + def config = new Config() + + then: + config.experimentalFeaturesEnabled == [].toSet() + + //verify expected behavior when not enabled under feature flag + config.globalTags == [env:"test", aKey:"aVal", bKey:"bVal"] + } + def "detect if agent is configured using default values"() { setup: if (host != null) { diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy index 36abcf887f8..14a0c2f6ae1 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigConverterTest.groovy @@ -99,6 +99,40 @@ class ConfigConverterTest extends DDSpecification { // spotless:on } + def "parsing map #mapString with List of arg separators for with key value separator #separator"() { + //testing parsing for DD_TAGS + setup: + def separatorList = [',' as char, ' ' as char] + + when: + def result = ConfigConverter.parseTraceTagsMap(mapString, separator as char, separatorList as List) + + then: + result == expected + + where: + // spotless:off + mapString | separator | expected + "key1:value1,key2:value2" | ':' | [key1: "value1", key2: "value2"] + "key1:value1 key2:value2" | ':' | [key1: "value1", key2: "value2"] + "env:test aKey:aVal bKey:bVal cKey:" | ':' | [env: "test", aKey: "aVal", bKey: "bVal", cKey:""] + "env:test,aKey:aVal,bKey:bVal,cKey:" | ':' | [env: "test", aKey: "aVal", bKey: "bVal", cKey:""] + "env:test,aKey:aVal bKey:bVal cKey:" | ':' | [env: "test", aKey: "aVal bKey:bVal cKey:"] + "env:test bKey :bVal dKey: dVal cKey:" | ':' | [env: "test", bKey: "", dKey: "", dVal: "", cKey: ""] + 'env :test, aKey : aVal bKey:bVal cKey:' | ':' | [env: "test", aKey : "aVal bKey:bVal cKey:"] + "env:keyWithA:Semicolon bKey:bVal cKey" | ':' | [env: "keyWithA:Semicolon", bKey: "bVal", cKey: ""] + "env:keyWith: , , Lots:Of:Semicolons " | ':' | [env: "keyWith:", Lots: "Of:Semicolons"] + "a:b,c,d" | ':' | [a: "b", c: "", d: ""] + "a,1" | ':' | [a: "", "1": ""] + "a:b:c:d" | ':' | [a: "b:c:d"] + //edge cases + "noDelimiters" | ':' | [noDelimiters: ""] + " " | ':' | [:] + ",,,,,,,,,,,," | ':' | [:] + ", , , , , , " | ':' | [:] + // spotless:on + } + def "test parseMapWithOptionalMappings"() { when: def result = ConfigConverter.parseMapWithOptionalMappings(mapString, "test", "", lowercaseKeys)