Skip to content

Commit f709476

Browse files
ygreeygree
committed
Collect null config defaults.
Properly serialize inner maps as string.
1 parent 1c33411 commit f709476

File tree

11 files changed

+159
-119
lines changed

11 files changed

+159
-119
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public static ConfigCollector get() {
2323
}
2424

2525
public void put(String key, Object value, ConfigOrigin origin) {
26-
ConfigSetting setting = new ConfigSetting(key, value, origin);
26+
ConfigSetting setting = ConfigSetting.of(key, value, origin);
2727
collected.put(key, setting);
2828
}
2929

@@ -32,7 +32,7 @@ public void putAll(Map<String, Object> keysAndValues, ConfigOrigin origin) {
3232
Map<String, ConfigSetting> merged =
3333
new ConcurrentHashMap<>(keysAndValues.size() + collected.size());
3434
for (Map.Entry<String, Object> entry : keysAndValues.entrySet()) {
35-
ConfigSetting setting = new ConfigSetting(entry.getKey(), entry.getValue(), origin);
35+
ConfigSetting setting = ConfigSetting.of(entry.getKey(), entry.getValue(), origin);
3636
merged.put(entry.getKey(), setting);
3737
}
3838
while (true) {

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package datadog.trace.api;
22

33
import java.util.Arrays;
4+
import java.util.BitSet;
45
import java.util.HashSet;
6+
import java.util.Map;
57
import java.util.Objects;
68
import java.util.Set;
79

@@ -10,6 +12,19 @@ public final class ConfigSetting {
1012
public final Object value;
1113
public final ConfigOrigin origin;
1214

15+
public static ConfigSetting of(String key, Object value, ConfigOrigin origin) {
16+
if (CONFIG_FILTER_LIST.contains(key)) {
17+
value = "<hidden>";
18+
} else if (value instanceof BitSet) {
19+
value = renderIntegerRange((BitSet) value);
20+
} else if (value instanceof Map) {
21+
value = renderMap((Map) value);
22+
} else if (value instanceof Iterable) {
23+
value = renderIterable((Iterable) value);
24+
}
25+
return new ConfigSetting(key, value, origin);
26+
}
27+
1328
private static final Set<String> CONFIG_FILTER_LIST =
1429
new HashSet<>(
1530
Arrays.asList("DD_API_KEY", "dd.api-key", "dd.profiling.api-key", "dd.profiling.apikey"));
@@ -18,6 +33,56 @@ private static Object filterConfigEntry(String key, Object value) {
1833
return CONFIG_FILTER_LIST.contains(key) ? "<hidden>" : value;
1934
}
2035

36+
private static String renderIntegerRange(BitSet bitset) {
37+
StringBuilder sb = new StringBuilder();
38+
int start = 0;
39+
while (true) {
40+
start = bitset.nextSetBit(start);
41+
if (start < 0) {
42+
break;
43+
}
44+
int end = bitset.nextClearBit(start);
45+
if (sb.length() > 0) {
46+
sb.append(',');
47+
}
48+
if (start < end - 1) {
49+
// interval
50+
sb.append(start);
51+
sb.append('-');
52+
sb.append(end);
53+
} else {
54+
// single value
55+
sb.append(start);
56+
}
57+
start = end;
58+
}
59+
return sb.toString();
60+
}
61+
62+
private static String renderMap(Map<Object, Object> merged) {
63+
StringBuilder result = new StringBuilder();
64+
for (Map.Entry entry : merged.entrySet()) {
65+
if (result.length() > 0) {
66+
result.append(',');
67+
}
68+
result.append(entry.getKey());
69+
result.append(':');
70+
result.append(entry.getValue());
71+
}
72+
return result.length() > 0 ? result.toString() : null;
73+
}
74+
75+
private static String renderIterable(Iterable iterable) {
76+
StringBuilder result = new StringBuilder();
77+
for (Object entry : iterable) {
78+
if (result.length() > 0) {
79+
result.append(',');
80+
}
81+
result.append(entry);
82+
}
83+
return result.length() > 0 ? result.toString() : null;
84+
}
85+
2186
public ConfigSetting(String key, Object value, ConfigOrigin origin) {
2287
this.key = key;
2388
this.value = filterConfigEntry(key, value);

‎internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigConverter.java‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -363,30 +363,4 @@ protected MethodHandle computeValue(Class<?> type) {
363363
}
364364
}
365365
}
366-
367-
public static String renderIntegerRange(BitSet bitset) {
368-
StringBuilder sb = new StringBuilder();
369-
int start = 0;
370-
while (true) {
371-
start = bitset.nextSetBit(start);
372-
if (start < 0) {
373-
break;
374-
}
375-
int end = bitset.nextClearBit(start);
376-
if (sb.length() > 0) {
377-
sb.append(',');
378-
}
379-
if (start < end - 1) {
380-
// interval
381-
sb.append(start);
382-
sb.append('-');
383-
sb.append(end);
384-
} else {
385-
// single value
386-
sb.append(start);
387-
}
388-
start = end;
389-
}
390-
return sb.toString();
391-
}
392366
}

‎internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java‎

Lines changed: 20 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ public <T extends Enum<T>> T getEnum(String key, Class<T> enumType, T defaultVal
6666
}
6767
}
6868
if (collectConfig) {
69-
ConfigCollector.get().put(key, String.valueOf(defaultValue), ConfigOrigin.DEFAULT);
69+
String valueStr = defaultValue == null ? null : defaultValue.name();
70+
ConfigCollector.get().put(key, valueStr, ConfigOrigin.DEFAULT);
7071
}
7172
return defaultValue;
7273
}
@@ -81,7 +82,7 @@ public String getString(String key, String defaultValue, String... aliases) {
8182
return value;
8283
}
8384
}
84-
if (collectConfig && defaultValue != null) {
85+
if (collectConfig) {
8586
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
8687
}
8788
return defaultValue;
@@ -101,7 +102,7 @@ public String getStringNotEmpty(String key, String defaultValue, String... alias
101102
return value;
102103
}
103104
}
104-
if (collectConfig && defaultValue != null) {
105+
if (collectConfig) {
105106
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
106107
}
107108
return defaultValue;
@@ -125,7 +126,7 @@ public String getStringExcludingSource(
125126
return value;
126127
}
127128
}
128-
if (collectConfig && defaultValue != null) {
129+
if (collectConfig) {
129130
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
130131
}
131132
return defaultValue;
@@ -203,7 +204,7 @@ private <T> T get(String key, T defaultValue, Class<T> type, String... aliases)
203204
// continue
204205
}
205206
}
206-
if (collectConfig && defaultValue != null) {
207+
if (collectConfig) {
207208
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
208209
}
209210
return defaultValue;
@@ -216,8 +217,8 @@ public List<String> getList(String key) {
216217
public List<String> getList(String key, List<String> defaultValue) {
217218
String list = getString(key);
218219
if (null == list) {
219-
if (collectConfig && defaultValue != null) {
220-
ConfigCollector.get().put(key, String.join(",", defaultValue), ConfigOrigin.DEFAULT);
220+
if (collectConfig) {
221+
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
221222
}
222223
return defaultValue;
223224
} else {
@@ -228,9 +229,8 @@ public List<String> getList(String key, List<String> defaultValue) {
228229
public Set<String> getSet(String key, Set<String> defaultValue) {
229230
String list = getString(key);
230231
if (null == list) {
231-
if (collectConfig && defaultValue != null) {
232-
String defaultValueStr = String.join(",", defaultValue);
233-
ConfigCollector.get().put(key, defaultValueStr, ConfigOrigin.DEFAULT);
232+
if (collectConfig) {
233+
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
234234
}
235235
return defaultValue;
236236
} else {
@@ -257,7 +257,9 @@ public Map<String, String> getMergedMap(String key) {
257257
}
258258
merged.putAll(parsedMap);
259259
}
260-
collectMapSetting(key, merged, origin);
260+
if (collectConfig) {
261+
ConfigCollector.get().put(key, merged, origin);
262+
}
261263
return merged;
262264
}
263265

@@ -276,7 +278,9 @@ public Map<String, String> getOrderedMap(String key) {
276278
}
277279
merged.putAll(parsedMap);
278280
}
279-
collectMapSetting(key, merged, origin);
281+
if (collectConfig) {
282+
ConfigCollector.get().put(key, merged, origin);
283+
}
280284
return merged;
281285
}
282286

@@ -298,7 +302,9 @@ public Map<String, String> getMergedMapWithOptionalMappings(
298302
}
299303
merged.putAll(parsedMap);
300304
}
301-
collectMapSetting(key, merged, origin);
305+
if (collectConfig) {
306+
ConfigCollector.get().put(key, merged, origin);
307+
}
302308
}
303309
return merged;
304310
}
@@ -313,8 +319,7 @@ public BitSet getIntegerRange(final String key, final BitSet defaultValue) {
313319
log.warn("Invalid configuration for {}", key, e);
314320
}
315321
if (collectConfig) {
316-
String defaultValueStr = ConfigConverter.renderIntegerRange(defaultValue);
317-
ConfigCollector.get().put(key, defaultValueStr, ConfigOrigin.DEFAULT);
322+
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
318323
}
319324
return defaultValue;
320325
}
@@ -407,22 +412,6 @@ public static ConfigProvider withPropertiesOverride(Properties properties) {
407412
}
408413
}
409414

410-
private void collectMapSetting(String key, Map<String, String> merged, ConfigOrigin origin) {
411-
if (!collectConfig || merged.isEmpty()) {
412-
return;
413-
}
414-
StringBuilder mergedValue = new StringBuilder();
415-
for (Map.Entry<String, String> entry : merged.entrySet()) {
416-
if (mergedValue.length() > 0) {
417-
mergedValue.append(',');
418-
}
419-
mergedValue.append(entry.getKey());
420-
mergedValue.append(':');
421-
mergedValue.append(entry.getValue());
422-
}
423-
ConfigCollector.get().put(key, mergedValue.toString(), origin);
424-
}
425-
426415
/**
427416
* Loads the optional configuration properties file into the global {@link Properties} object.
428417
*

‎internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy‎

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,10 @@ import datadog.trace.api.config.TraceInstrumentationConfig
99
import datadog.trace.api.config.TracerConfig
1010
import datadog.trace.api.iast.telemetry.Verbosity
1111
import datadog.trace.api.naming.SpanNaming
12-
import datadog.trace.bootstrap.config.provider.ConfigConverter
12+
import datadog.trace.bootstrap.config.provider.ConfigProvider
1313
import datadog.trace.test.util.DDSpecification
1414
import datadog.trace.util.Strings
1515

16-
import static datadog.trace.api.ConfigDefaults.DEFAULT_HTTP_CLIENT_ERROR_STATUSES
1716
import static datadog.trace.api.ConfigDefaults.DEFAULT_IAST_WEAK_HASH_ALGORITHMS
1817
import static datadog.trace.api.ConfigDefaults.DEFAULT_TELEMETRY_HEARTBEAT_INTERVAL
1918
import static datadog.trace.api.UserEventTrackingMode.SAFE
@@ -99,25 +98,31 @@ class ConfigCollectorTest extends DDSpecification {
9998
GeneralConfig.TELEMETRY_HEARTBEAT_INTERVAL | DEFAULT_TELEMETRY_HEARTBEAT_INTERVAL
10099
CiVisibilityConfig.CIVISIBILITY_JACOCO_GRADLE_SOURCE_SETS | "main"
101100
IastConfig.IAST_WEAK_HASH_ALGORITHMS | DEFAULT_IAST_WEAK_HASH_ALGORITHMS.join(",")
102-
TracerConfig.HTTP_CLIENT_ERROR_STATUSES | ConfigConverter.renderIntegerRange(DEFAULT_HTTP_CLIENT_ERROR_STATUSES)
101+
TracerConfig.HTTP_CLIENT_ERROR_STATUSES | "400-500"
103102
}
104103

105-
def "default NULL config settings are NOT collected"() {
106-
expect:
107-
ConfigCollector.get().collect().get(configKey) == null
104+
def "default null config settings are also collected"() {
105+
when:
106+
ConfigSetting cs = ConfigCollector.get().collect().get(configKey)
107+
108+
then:
109+
cs.key == configKey
110+
cs.value == null
111+
cs.origin == ConfigOrigin.DEFAULT
108112

109113
where:
110-
configKey | _
111-
GeneralConfig.APPLICATION_KEY | _
112-
TraceInstrumentationConfig.RESOLVER_USE_URL_CACHES | _
113-
JmxFetchConfig.JMX_FETCH_CHECK_PERIOD | _
114-
CiVisibilityConfig.CIVISIBILITY_MODULE_ID | _
115-
TracerConfig.TRACE_SAMPLE_RATE | _
116-
TraceInstrumentationConfig.JMS_PROPAGATION_DISABLED_TOPICS | _
117-
TracerConfig.PROXY_NO_PROXY | _
118-
TracerConfig.TRACE_PEER_SERVICE_MAPPING | _
119-
TracerConfig.TRACE_HTTP_SERVER_PATH_RESOURCE_NAME_MAPPING | _
120-
TracerConfig.HEADER_TAGS | _
114+
configKey << [
115+
GeneralConfig.APPLICATION_KEY,
116+
TraceInstrumentationConfig.RESOLVER_USE_URL_CACHES,
117+
JmxFetchConfig.JMX_FETCH_CHECK_PERIOD,
118+
CiVisibilityConfig.CIVISIBILITY_MODULE_ID,
119+
TracerConfig.TRACE_SAMPLE_RATE,
120+
TraceInstrumentationConfig.JMS_PROPAGATION_DISABLED_TOPICS,
121+
TracerConfig.PROXY_NO_PROXY,
122+
TracerConfig.TRACE_PEER_SERVICE_MAPPING,
123+
TracerConfig.TRACE_HTTP_SERVER_PATH_RESOURCE_NAME_MAPPING,
124+
TracerConfig.HEADER_TAGS,
125+
]
121126
}
122127

123128
def "put-get configurations"() {
@@ -132,9 +137,9 @@ class ConfigCollectorTest extends DDSpecification {
132137

133138
then:
134139
ConfigCollector.get().collect().values().toSet() == [
135-
new ConfigSetting('key1', 'replaced', ConfigOrigin.REMOTE),
136-
new ConfigSetting('key2', 'value2', ConfigOrigin.ENV),
137-
new ConfigSetting('key3', 'value3', ConfigOrigin.JVM_PROP)
140+
ConfigSetting.of('key1', 'replaced', ConfigOrigin.REMOTE),
141+
ConfigSetting.of('key2', 'value2', ConfigOrigin.ENV),
142+
ConfigSetting.of('key3', 'value3', ConfigOrigin.JVM_PROP)
138143
] as Set
139144
}
140145

0 commit comments

Comments
 (0)