Skip to content

Commit abe734a

Browse files
mtoffl01mtoffl01
committed
Introduce seqId to ConfigSetting, add tests to ConfigProvider to assert seq Id logic
1 parent c7fbbb3 commit abe734a

File tree

5 files changed

+145
-27
lines changed

5 files changed

+145
-27
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,13 @@ public void put(String key, Object value, ConfigOrigin origin) {
3030
configMap.put(key, setting); // replaces any previous value for this key at origin
3131
}
3232

33+
public void put(String key, Object value, ConfigOrigin origin, int seqId) {
34+
ConfigSetting setting = ConfigSetting.of(key, value, origin, seqId);
35+
Map<String, ConfigSetting> configMap =
36+
collected.computeIfAbsent(origin, k -> new ConcurrentHashMap<>());
37+
configMap.put(key, setting); // replaces any previous value for this key at origin
38+
}
39+
3340
public void putAll(Map<String, Object> keysAndValues, ConfigOrigin origin) {
3441
for (Map.Entry<String, Object> entry : keysAndValues.entrySet()) {
3542
put(entry.getKey(), entry.getValue(), origin);

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,28 @@ public final class ConfigSetting {
1111
public final String key;
1212
public final Object value;
1313
public final ConfigOrigin origin;
14+
public final int seqId;
15+
16+
public static final int DEFAULT_SEQ_ID = 1;
17+
private static final int ABSENT_SEQ_ID = 0;
1418

1519
private static final Set<String> CONFIG_FILTER_LIST =
1620
new HashSet<>(
1721
Arrays.asList("DD_API_KEY", "dd.api-key", "dd.profiling.api-key", "dd.profiling.apikey"));
1822

1923
public static ConfigSetting of(String key, Object value, ConfigOrigin origin) {
20-
return new ConfigSetting(key, value, origin);
24+
return new ConfigSetting(key, value, origin, ABSENT_SEQ_ID);
25+
}
26+
27+
public static ConfigSetting of(String key, Object value, ConfigOrigin origin, int seqId) {
28+
return new ConfigSetting(key, value, origin, seqId);
2129
}
2230

23-
private ConfigSetting(String key, Object value, ConfigOrigin origin) {
31+
private ConfigSetting(String key, Object value, ConfigOrigin origin, int seqId) {
2432
this.key = key;
2533
this.value = CONFIG_FILTER_LIST.contains(key) ? "<hidden>" : value;
2634
this.origin = origin;
35+
this.seqId = seqId;
2736
}
2837

2938
public String normalizedKey() {
@@ -99,12 +108,15 @@ public boolean equals(Object o) {
99108
if (this == o) return true;
100109
if (o == null || getClass() != o.getClass()) return false;
101110
ConfigSetting that = (ConfigSetting) o;
102-
return key.equals(that.key) && Objects.equals(value, that.value) && origin == that.origin;
111+
return key.equals(that.key)
112+
&& Objects.equals(value, that.value)
113+
&& origin == that.origin
114+
&& seqId == that.seqId;
103115
}
104116

105117
@Override
106118
public int hashCode() {
107-
return Objects.hash(key, value, origin);
119+
return Objects.hash(key, value, origin, seqId);
108120
}
109121

110122
@Override
@@ -117,6 +129,8 @@ public String toString() {
117129
+ stringValue()
118130
+ ", origin="
119131
+ origin
132+
+ ", seqId="
133+
+ seqId
120134
+ '}';
121135
}
122136
}

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

Lines changed: 48 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import datadog.environment.SystemProperties;
66
import datadog.trace.api.ConfigCollector;
77
import datadog.trace.api.ConfigOrigin;
8+
import datadog.trace.api.ConfigSetting;
89
import de.thetaphi.forbiddenapis.SuppressForbidden;
910
import java.io.File;
1011
import java.io.FileNotFoundException;
@@ -33,6 +34,7 @@ private static final class Singleton {
3334
private final boolean collectConfig;
3435

3536
private final ConfigProvider.Source[] sources;
37+
private final int numSources;
3638

3739
private ConfigProvider(ConfigProvider.Source... sources) {
3840
this(true, sources);
@@ -41,6 +43,7 @@ private ConfigProvider(ConfigProvider.Source... sources) {
4143
private ConfigProvider(boolean collectConfig, ConfigProvider.Source... sources) {
4244
this.collectConfig = collectConfig;
4345
this.sources = sources;
46+
this.numSources = sources.length;
4447
}
4548

4649
public String getConfigFileStatus() {
@@ -70,26 +73,29 @@ public <T extends Enum<T>> T getEnum(String key, Class<T> enumType, T defaultVal
7073
}
7174
if (collectConfig) {
7275
String valueStr = defaultValue == null ? null : defaultValue.name();
73-
ConfigCollector.get().put(key, valueStr, ConfigOrigin.DEFAULT);
76+
ConfigCollector.get().put(key, valueStr, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID);
7477
}
7578
return defaultValue;
7679
}
7780

7881
public String getString(String key, String defaultValue, String... aliases) {
7982
String foundValue = null;
80-
for (ConfigProvider.Source source : sources) {
83+
for (int i = 0; i < sources.length; i++) {
84+
ConfigProvider.Source source = sources[i];
8185
String value = source.get(key, aliases);
8286
if (value != null) {
8387
if (collectConfig) {
84-
ConfigCollector.get().put(key, value, source.origin());
88+
int seqId = sources.length - i + 1;
89+
ConfigCollector.get().put(key, value, source.origin(), seqId);
8590
}
8691
if (foundValue == null) {
8792
foundValue = value;
8893
}
8994
}
9095
}
9196
if (collectConfig) {
92-
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
97+
ConfigCollector.get()
98+
.put(key, defaultValue, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID);
9399
}
94100
return foundValue != null ? foundValue : defaultValue;
95101
}
@@ -100,19 +106,22 @@ public String getString(String key, String defaultValue, String... aliases) {
100106
*/
101107
public String getStringNotEmpty(String key, String defaultValue, String... aliases) {
102108
String foundValue = null;
103-
for (ConfigProvider.Source source : sources) {
109+
for (int i = 0; i < sources.length; i++) {
110+
ConfigProvider.Source source = sources[i];
104111
String value = source.get(key, aliases);
105112
if (value != null && !value.trim().isEmpty()) {
106113
if (collectConfig) {
107-
ConfigCollector.get().put(key, value, source.origin());
114+
int seqId = sources.length - i + 1;
115+
ConfigCollector.get().put(key, value, source.origin(), seqId);
108116
}
109117
if (foundValue == null) {
110118
foundValue = value;
111119
}
112120
}
113121
}
114122
if (collectConfig) {
115-
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
123+
ConfigCollector.get()
124+
.put(key, defaultValue, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID);
116125
}
117126
return foundValue != null ? foundValue : defaultValue;
118127
}
@@ -123,24 +132,26 @@ public String getStringExcludingSource(
123132
Class<? extends ConfigProvider.Source> excludedSource,
124133
String... aliases) {
125134
String foundValue = null;
126-
for (ConfigProvider.Source source : sources) {
135+
for (int i = 0; i < sources.length; i++) {
136+
ConfigProvider.Source source = sources[i];
127137
// Do we still want to report telemetry in this case?
128138
if (excludedSource.isAssignableFrom(source.getClass())) {
129139
continue;
130140
}
131-
132141
String value = source.get(key, aliases);
133142
if (value != null) {
134143
if (collectConfig) {
135-
ConfigCollector.get().put(key, value, source.origin());
144+
int seqId = sources.length - i + 1;
145+
ConfigCollector.get().put(key, value, source.origin(), seqId);
136146
}
137147
if (foundValue == null) {
138148
foundValue = value;
139149
}
140150
}
141151
}
142152
if (collectConfig) {
143-
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
153+
ConfigCollector.get()
154+
.put(key, defaultValue, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID);
144155
}
145156
return foundValue != null ? foundValue : defaultValue;
146157
}
@@ -204,13 +215,15 @@ public double getDouble(String key, double defaultValue) {
204215

205216
private <T> T get(String key, T defaultValue, Class<T> type, String... aliases) {
206217
T foundValue = null;
207-
for (ConfigProvider.Source source : sources) {
218+
for (int i = 0; i < sources.length; i++) {
219+
ConfigProvider.Source source = sources[i];
208220
try {
209221
String sourceValue = source.get(key, aliases);
210222
T value = ConfigConverter.valueOf(sourceValue, type);
211223
if (value != null) {
212224
if (collectConfig) {
213-
ConfigCollector.get().put(key, sourceValue, source.origin());
225+
int seqId = sources.length - i + 1;
226+
ConfigCollector.get().put(key, sourceValue, source.origin(), seqId);
214227
}
215228
if (foundValue == null) {
216229
foundValue = value;
@@ -221,7 +234,8 @@ private <T> T get(String key, T defaultValue, Class<T> type, String... aliases)
221234
}
222235
}
223236
if (collectConfig) {
224-
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
237+
ConfigCollector.get()
238+
.put(key, defaultValue, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID);
225239
}
226240
return foundValue != null ? foundValue : defaultValue;
227241
}
@@ -234,7 +248,8 @@ public List<String> getList(String key, List<String> defaultValue) {
234248
String list = getString(key);
235249
if (null == list) {
236250
if (collectConfig) {
237-
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT);
251+
ConfigCollector.get()
252+
.put(key, defaultValue, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID);
238253
}
239254
return defaultValue;
240255
} else {
@@ -265,19 +280,22 @@ public Map<String, String> getMergedMap(String key, String... aliases) {
265280
// prior art:
266281
// https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html
267282
// We reverse iterate to allow overrides
283+
int seqId = 2;
268284
for (int i = sources.length - 1; 0 <= i; i--) {
269285
String value = sources[i].get(key, aliases);
270286
Map<String, String> parsedMap = ConfigConverter.parseMap(value, key);
271287
if (!parsedMap.isEmpty()) {
272288
origin = sources[i].origin();
273289
if (collectConfig) {
274-
ConfigCollector.get().put(key, parsedMap, origin);
290+
seqId++;
291+
ConfigCollector.get().put(key, parsedMap, origin, seqId);
275292
}
276293
}
277294
merged.putAll(parsedMap);
278295
}
296+
// TODO: How to report telemetry about the final, mergedMap value? What is its correct origin?
279297
if (collectConfig && merged.isEmpty()) {
280-
ConfigCollector.get().put(key, merged, ConfigOrigin.DEFAULT);
298+
ConfigCollector.get().put(key, merged, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID);
281299
}
282300
return merged;
283301
}
@@ -289,20 +307,22 @@ public Map<String, String> getMergedTagsMap(String key, String... aliases) {
289307
// prior art:
290308
// https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html
291309
// We reverse iterate to allow overrides
310+
int seqId = 2;
292311
for (int i = sources.length - 1; 0 <= i; i--) {
293312
String value = sources[i].get(key, aliases);
294313
Map<String, String> parsedMap =
295314
ConfigConverter.parseTraceTagsMap(value, ':', Arrays.asList(',', ' '));
296315
if (!parsedMap.isEmpty()) {
297316
origin = sources[i].origin();
298317
if (collectConfig) {
299-
ConfigCollector.get().put(key, parsedMap, origin);
318+
seqId++;
319+
ConfigCollector.get().put(key, parsedMap, origin, seqId);
300320
}
301321
}
302322
merged.putAll(parsedMap);
303323
}
304324
if (collectConfig && merged.isEmpty()) {
305-
ConfigCollector.get().put(key, merged, ConfigOrigin.DEFAULT);
325+
ConfigCollector.get().put(key, merged, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID);
306326
}
307327
return merged;
308328
}
@@ -314,19 +334,21 @@ public Map<String, String> getOrderedMap(String key) {
314334
// prior art:
315335
// https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html
316336
// We reverse iterate to allow overrides
337+
int seqId = 2;
317338
for (int i = sources.length - 1; 0 <= i; i--) {
318339
String value = sources[i].get(key);
319340
Map<String, String> parsedMap = ConfigConverter.parseOrderedMap(value, key);
320341
if (!parsedMap.isEmpty()) {
321342
origin = sources[i].origin();
322343
if (collectConfig) {
323-
ConfigCollector.get().put(key, parsedMap, origin);
344+
seqId++;
345+
ConfigCollector.get().put(key, parsedMap, origin, seqId);
324346
}
325347
}
326348
merged.putAll(parsedMap);
327349
}
328350
if (collectConfig && merged.isEmpty()) {
329-
ConfigCollector.get().put(key, merged, ConfigOrigin.DEFAULT);
351+
ConfigCollector.get().put(key, merged, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID);
330352
}
331353
return merged;
332354
}
@@ -339,6 +361,7 @@ public Map<String, String> getMergedMapWithOptionalMappings(
339361
// prior art:
340362
// https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html
341363
// We reverse iterate to allow overrides
364+
int seqId = 2;
342365
for (String key : keys) {
343366
for (int i = sources.length - 1; 0 <= i; i--) {
344367
String value = sources[i].get(key);
@@ -347,13 +370,15 @@ public Map<String, String> getMergedMapWithOptionalMappings(
347370
if (!parsedMap.isEmpty()) {
348371
origin = sources[i].origin();
349372
if (collectConfig) {
350-
ConfigCollector.get().put(key, parsedMap, origin);
373+
seqId++;
374+
ConfigCollector.get().put(key, parsedMap, origin, seqId);
351375
}
352376
}
353377
merged.putAll(parsedMap);
354378
}
355379
if (collectConfig && merged.isEmpty()) {
356-
ConfigCollector.get().put(key, Collections.emptyMap(), ConfigOrigin.DEFAULT);
380+
ConfigCollector.get()
381+
.put(key, Collections.emptyMap(), ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID);
357382
}
358383
}
359384
return merged;

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,4 +239,33 @@ class ConfigCollectorTest extends DDSpecification {
239239
"logs.injection.enabled" | "false"
240240
"trace.sample.rate" | "0.3"
241241
}
242+
243+
def "config collector assigns creates ConfigSettings with correct seqId"() {
244+
setup:
245+
ConfigCollector.get().collect() // clear previous state
246+
247+
when:
248+
// Simulate three sources with increasing precedence and a default
249+
ConfigCollector.get().put("test.key", "default", ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID)
250+
ConfigCollector.get().put("test.key", "env", ConfigOrigin.ENV, 2)
251+
ConfigCollector.get().put("test.key", "jvm", ConfigOrigin.JVM_PROP, 3)
252+
ConfigCollector.get().put("test.key", "remote", ConfigOrigin.REMOTE, 4)
253+
254+
then:
255+
def collected = ConfigCollector.get().collect()
256+
def defaultSetting = collected.get(ConfigOrigin.DEFAULT).get("test.key")
257+
def envSetting = collected.get(ConfigOrigin.ENV).get("test.key")
258+
def jvmSetting = collected.get(ConfigOrigin.JVM_PROP).get("test.key")
259+
def remoteSetting = collected.get(ConfigOrigin.REMOTE).get("test.key")
260+
261+
defaultSetting.seqId == ConfigSetting.DEFAULT_SEQ_ID
262+
envSetting.seqId == 2
263+
jvmSetting.seqId == 3
264+
remoteSetting.seqId == 4
265+
266+
// Higher precedence = higher seqId
267+
assert remoteSetting.seqId > jvmSetting.seqId
268+
assert jvmSetting.seqId > envSetting.seqId
269+
assert envSetting.seqId > defaultSetting.seqId
270+
}
242271
}

0 commit comments

Comments
 (0)