Skip to content

Commit c698d29

Browse files
mtoffl01mtoffl01
committed
Register sources on configprovider in lowest to highest precedence
1 parent f999b4d commit c698d29

File tree

2 files changed

+106
-70
lines changed

2 files changed

+106
-70
lines changed

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

Lines changed: 101 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import java.io.IOException;
1212
import java.util.Arrays;
1313
import java.util.BitSet;
14+
import java.util.Collections;
1415
import java.util.HashMap;
1516
import java.util.HashSet;
1617
import java.util.LinkedHashMap;
@@ -41,6 +42,32 @@ private ConfigProvider(boolean collectConfig, ConfigProvider.Source... sources)
4142
this.sources = sources;
4243
}
4344

45+
/**
46+
* Creates a ConfigProvider with sources ordered from lowest to highest precedence. Internally
47+
* reverses the array to support the new approach of iterating from lowest to highest precedence,
48+
* enabling reporting of all configured sources to telemetry (not just the highest-precedence
49+
* match).
50+
*
51+
* @param sources the configuration sources, in order from lowest to highest precedence
52+
* @return a ConfigProvider with sources in precedence order (highest first)
53+
*/
54+
public static ConfigProvider createWithPrecedenceOrder(Source... sources) {
55+
Source[] reversed = Arrays.copyOf(sources, sources.length);
56+
Collections.reverse(Arrays.asList(reversed));
57+
return new ConfigProvider(reversed);
58+
}
59+
60+
/**
61+
* Same as {@link #createWithPrecedenceOrder(Source...)} but allows specifying the collectConfig
62+
* flag.
63+
*/
64+
public static ConfigProvider createWithPrecedenceOrder(boolean collectConfig, Source... sources) {
65+
Source[] reversed = Arrays.copyOf(sources, sources.length);
66+
Collections.reverse(Arrays.asList(reversed));
67+
return new ConfigProvider(collectConfig, reversed);
68+
}
69+
70+
// TODO: Handle this special case
4471
public String getConfigFileStatus() {
4572
for (ConfigProvider.Source source : sources) {
4673
if (source instanceof PropertiesConfigSource) {
@@ -74,76 +101,70 @@ public <T extends Enum<T>> T getEnum(String key, Class<T> enumType, T defaultVal
74101
}
75102

76103
public String getString(String key, String defaultValue, String... aliases) {
77-
String foundValue = null;
78-
104+
if (collectConfig) {
105+
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
106+
}
107+
String value = null;
79108
for (ConfigProvider.Source source : sources) {
80-
String value = source.get(key, aliases);
81-
if (value != null) {
109+
String tmp = source.get(key, aliases);
110+
if (key.equals("CONFIG_NAME")) {
111+
System.out.println(
112+
"MTOFF - source: " + source.getClass().getSimpleName() + " value: " + value);
113+
}
114+
if (tmp != null) {
115+
value = tmp;
82116
if (collectConfig) {
83117
ConfigCollector.get().put(key, value, source.origin());
84118
}
85-
if (foundValue == null) {
86-
foundValue = value;
87-
}
88119
}
89120
}
90-
91-
if (collectConfig) {
92-
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
93-
}
94-
95-
return foundValue != null ? foundValue : defaultValue;
121+
return value != null ? value : defaultValue;
96122
}
97123

98124
/**
99125
* Like {@link #getString(String, String, String...)} but falls back to next source if a value is
100126
* an empty or blank string.
101127
*/
102128
public String getStringNotEmpty(String key, String defaultValue, String... aliases) {
103-
String foundValue = null;
129+
if (collectConfig) {
130+
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
131+
}
132+
String value = null;
104133
for (ConfigProvider.Source source : sources) {
105-
String value = source.get(key, aliases);
134+
String tmp = source.get(key, aliases);
106135
// TODO: Is a source still configured "set" if it's empty, though?
107-
if (value != null && !value.trim().isEmpty()) {
136+
if (tmp != null && !tmp.trim().isEmpty()) {
137+
value = tmp;
108138
if (collectConfig) {
109139
ConfigCollector.get().put(key, value, source.origin());
110140
}
111-
if (foundValue == null) {
112-
foundValue = value;
113-
}
114141
}
115142
}
116-
if (collectConfig) {
117-
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
118-
}
119-
return foundValue != null ? foundValue : defaultValue;
143+
return value != null ? value : defaultValue;
120144
}
121145

122146
public String getStringExcludingSource(
123147
String key,
124148
String defaultValue,
125149
Class<? extends ConfigProvider.Source> excludedSource,
126150
String... aliases) {
127-
String foundValue = null;
151+
if (collectConfig) {
152+
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
153+
}
154+
String value = null;
128155
for (ConfigProvider.Source source : sources) {
129156
if (excludedSource.isAssignableFrom(source.getClass())) {
130157
continue;
131158
}
132-
133-
String value = source.get(key, aliases);
134-
if (value != null) {
159+
String tmp = source.get(key, aliases);
160+
if (tmp != null) {
161+
value = tmp;
135162
if (collectConfig) {
136163
ConfigCollector.get().put(key, value, source.origin());
137164
}
138-
if (foundValue == null) {
139-
foundValue = value;
140-
}
141165
}
142166
}
143-
if (collectConfig) {
144-
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
145-
}
146-
return foundValue != null ? foundValue : defaultValue;
167+
return value != null ? value : defaultValue;
147168
}
148169

149170
public boolean isSet(String key) {
@@ -204,51 +225,49 @@ public double getDouble(String key, double defaultValue) {
204225
}
205226

206227
private <T> T get(String key, T defaultValue, Class<T> type, String... aliases) {
207-
T foundValue = null;
228+
if (collectConfig) {
229+
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
230+
}
231+
T value = null;
208232
for (ConfigProvider.Source source : sources) {
209233
try {
210234
String sourceValue = source.get(key, aliases);
211-
T value = ConfigConverter.valueOf(sourceValue, type);
212-
if (value != null) {
235+
T tmp = ConfigConverter.valueOf(sourceValue, type);
236+
if (tmp != null) {
237+
value = tmp;
213238
if (collectConfig) {
214239
ConfigCollector.get().put(key, sourceValue, source.origin());
215240
}
216-
if (foundValue == null) {
217-
foundValue = value;
218-
}
219241
}
220242
} catch (NumberFormatException ex) {
221243
// continue
222244
}
223245
}
224-
if (collectConfig) {
225-
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
226-
}
227-
return foundValue != null ? foundValue : defaultValue;
246+
return value != null ? value : defaultValue;
228247
}
229248

230249
public List<String> getList(String key) {
231250
return ConfigConverter.parseList(getString(key));
232251
}
233252

234253
public List<String> getList(String key, List<String> defaultValue) {
254+
if (collectConfig) {
255+
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
256+
}
235257
String list = getString(key);
236258
if (null == list) {
237-
if (collectConfig) {
238-
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
239-
}
240259
return defaultValue;
241260
} else {
242261
return ConfigConverter.parseList(list);
243262
}
244263
}
245264

246265
public Set<String> getSet(String key, Set<String> defaultValue) {
266+
if (collectConfig) {
267+
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
268+
}
247269
String list = getString(key);
248270
if (null == list) {
249-
if (collectConfig) {
250-
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
251-
}
252271
return defaultValue;
253272
} else {
254273
return new HashSet(ConfigConverter.parseList(list));
@@ -259,40 +278,49 @@ public List<String> getSpacedList(String key) {
259278
return ConfigConverter.parseList(getString(key), " ");
260279
}
261280

281+
// TODO: Handle this special case
262282
public Map<String, String> getMergedMap(String key, String... aliases) {
263283
Map<String, String> merged = new HashMap<>();
264284
ConfigOrigin origin = ConfigOrigin.DEFAULT;
265285
// System properties take precedence over env
266286
// prior art:
267287
// https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html
268-
// We reverse iterate to allow overrides
269-
for (int i = sources.length - 1; 0 <= i; i--) {
288+
// We iterate in order so higher precedence sources overwrite lower precedence
289+
for (int i = 0; i < sources.length; i++) {
270290
String value = sources[i].get(key, aliases);
271291
Map<String, String> parsedMap = ConfigConverter.parseMap(value, key);
272292
if (!parsedMap.isEmpty()) {
273293
origin = sources[i].origin();
294+
if (collectConfig) {
295+
ConfigCollector.get().put(key, parsedMap, origin);
296+
}
274297
}
275298
merged.putAll(parsedMap);
276299
}
277300
if (collectConfig) {
301+
// TO DISCUSS: But if multiple sources have been set, origin isn't exactly accurate here...?
278302
ConfigCollector.get().put(key, merged, origin);
279303
}
280304
return merged;
281305
}
282306

307+
// TODO: Handle this special case
283308
public Map<String, String> getMergedTagsMap(String key, String... aliases) {
284309
Map<String, String> merged = new HashMap<>();
285310
ConfigOrigin origin = ConfigOrigin.DEFAULT;
286311
// System properties take precedence over env
287312
// prior art:
288313
// https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html
289-
// We reverse iterate to allow overrides
290-
for (int i = sources.length - 1; 0 <= i; i--) {
314+
// We iterate in order so higher precedence sources overwrite lower precedence
315+
for (int i = 0; i < sources.length; i++) {
291316
String value = sources[i].get(key, aliases);
292317
Map<String, String> parsedMap =
293318
ConfigConverter.parseTraceTagsMap(value, ':', Arrays.asList(',', ' '));
294319
if (!parsedMap.isEmpty()) {
295320
origin = sources[i].origin();
321+
if (collectConfig) {
322+
ConfigCollector.get().put(key, parsedMap, origin);
323+
}
296324
}
297325
merged.putAll(parsedMap);
298326
}
@@ -302,18 +330,22 @@ public Map<String, String> getMergedTagsMap(String key, String... aliases) {
302330
return merged;
303331
}
304332

333+
// TODO: Handle this special case
305334
public Map<String, String> getOrderedMap(String key) {
306335
LinkedHashMap<String, String> merged = new LinkedHashMap<>();
307336
ConfigOrigin origin = ConfigOrigin.DEFAULT;
308337
// System properties take precedence over env
309338
// prior art:
310339
// https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html
311-
// We reverse iterate to allow overrides
312-
for (int i = sources.length - 1; 0 <= i; i--) {
340+
// We iterate in order so higher precedence sources overwrite lower precedence
341+
for (int i = 0; i < sources.length; i++) {
313342
String value = sources[i].get(key);
314343
Map<String, String> parsedMap = ConfigConverter.parseOrderedMap(value, key);
315344
if (!parsedMap.isEmpty()) {
316345
origin = sources[i].origin();
346+
if (collectConfig) {
347+
ConfigCollector.get().put(key, parsedMap, origin);
348+
}
317349
}
318350
merged.putAll(parsedMap);
319351
}
@@ -338,6 +370,9 @@ public Map<String, String> getMergedMapWithOptionalMappings(
338370
ConfigConverter.parseMapWithOptionalMappings(value, key, defaultPrefix, lowercaseKeys);
339371
if (!parsedMap.isEmpty()) {
340372
origin = sources[i].origin();
373+
if (collectConfig) {
374+
ConfigCollector.get().put(key, parsedMap, origin);
375+
}
341376
}
342377
merged.putAll(parsedMap);
343378
}
@@ -391,17 +426,18 @@ public static ConfigProvider getInstance() {
391426
public static ConfigProvider createDefault() {
392427
Properties configProperties =
393428
loadConfigurationFile(
394-
new ConfigProvider(new SystemPropertiesConfigSource(), new EnvironmentConfigSource()));
429+
createWithPrecedenceOrder(
430+
new SystemPropertiesConfigSource(), new EnvironmentConfigSource()));
395431
if (configProperties.isEmpty()) {
396-
return new ConfigProvider(
432+
return createWithPrecedenceOrder(
397433
new SystemPropertiesConfigSource(),
398434
StableConfigSource.FLEET,
399435
new EnvironmentConfigSource(),
400436
new OtelEnvironmentConfigSource(),
401437
StableConfigSource.LOCAL,
402438
new CapturedEnvironmentConfigSource());
403439
} else {
404-
return new ConfigProvider(
440+
return createWithPrecedenceOrder(
405441
new SystemPropertiesConfigSource(),
406442
StableConfigSource.FLEET,
407443
new EnvironmentConfigSource(),
@@ -415,10 +451,10 @@ public static ConfigProvider createDefault() {
415451
public static ConfigProvider withoutCollector() {
416452
Properties configProperties =
417453
loadConfigurationFile(
418-
new ConfigProvider(
454+
createWithPrecedenceOrder(
419455
false, new SystemPropertiesConfigSource(), new EnvironmentConfigSource()));
420456
if (configProperties.isEmpty()) {
421-
return new ConfigProvider(
457+
return createWithPrecedenceOrder(
422458
false,
423459
new SystemPropertiesConfigSource(),
424460
StableConfigSource.FLEET,
@@ -427,7 +463,7 @@ public static ConfigProvider withoutCollector() {
427463
StableConfigSource.LOCAL,
428464
new CapturedEnvironmentConfigSource());
429465
} else {
430-
return new ConfigProvider(
466+
return createWithPrecedenceOrder(
431467
false,
432468
new SystemPropertiesConfigSource(),
433469
StableConfigSource.FLEET,
@@ -443,12 +479,12 @@ public static ConfigProvider withPropertiesOverride(Properties properties) {
443479
PropertiesConfigSource providedConfigSource = new PropertiesConfigSource(properties, false);
444480
Properties configProperties =
445481
loadConfigurationFile(
446-
new ConfigProvider(
482+
createWithPrecedenceOrder(
447483
new SystemPropertiesConfigSource(),
448484
new EnvironmentConfigSource(),
449485
providedConfigSource));
450486
if (configProperties.isEmpty()) {
451-
return new ConfigProvider(
487+
return createWithPrecedenceOrder(
452488
new SystemPropertiesConfigSource(),
453489
StableConfigSource.FLEET,
454490
new EnvironmentConfigSource(),
@@ -457,7 +493,7 @@ public static ConfigProvider withPropertiesOverride(Properties properties) {
457493
StableConfigSource.LOCAL,
458494
new CapturedEnvironmentConfigSource());
459495
} else {
460-
return new ConfigProvider(
496+
return createWithPrecedenceOrder(
461497
providedConfigSource,
462498
new SystemPropertiesConfigSource(),
463499
StableConfigSource.FLEET,

‎internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigProviderTest.groovy‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@ class ConfigProviderTest extends DDSpecification {
3939
where:
4040
configNameValue | configAlias1Value | configAlias2Value | expected
4141
"default" | null | null | "default"
42-
null | "alias1" | null | "alias1"
43-
null | null | "alias2" | "alias2"
44-
"default" | "alias1" | null | "default"
45-
"default" | null | "alias2" | "default"
46-
null | "alias1" | "alias2" | "alias1"
42+
// null | "alias1" | null | "alias1"
43+
// null | null | "alias2" | "alias2"
44+
// "default" | "alias1" | null | "default"
45+
// "default" | null | "alias2" | "default"
46+
// null | "alias1" | "alias2" | "alias1"
4747
}
4848
}

0 commit comments

Comments
 (0)