Skip to content

Commit 760d436

Browse files
mtoffl01mtoffl01
committed
Allow config value parsing errors to bubble up from Sources to ConfigProvider, to report all configured sources
1 parent 2d5b53b commit 760d436

File tree

7 files changed

+196
-59
lines changed

7 files changed

+196
-59
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,10 @@ public Map<String, Map<ConfigOrigin, ConfigSetting>> collect() {
5858
}
5959

6060
/**
61-
* Returns the {@link ConfigSetting} with the highest precedence for the given key, or {@code
62-
* null} if no setting exists for that key.
61+
* Returns the {@link ConfigSetting} with the highest seq_id for the given key, or {@code null} if
62+
* no setting exists for that key.
6363
*/
64-
public ConfigSetting getAppliedConfigSetting(String key) {
64+
public ConfigSetting getHighestSeqIdConfig(String key) {
6565
Map<ConfigOrigin, ConfigSetting> originMap = collected.get(key);
6666
if (originMap == null || originMap.isEmpty()) {
6767
return null;

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,15 @@ public CapturedEnvironmentConfigSource(CapturedEnvironment env) {
1515
}
1616

1717
@Override
18-
protected String get(String key) {
19-
return env.getProperties().get(key);
18+
protected String get(String key) throws ConfigSourceException {
19+
Object value = env.getProperties().get(key);
20+
if (value == null) {
21+
return null;
22+
}
23+
if (!(value instanceof String)) {
24+
throw new ConfigSourceException(value);
25+
}
26+
return (String) value;
2027
}
2128

2229
@Override

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

Lines changed: 123 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.LinkedHashMap;
2020
import java.util.List;
2121
import java.util.Map;
22+
import java.util.Objects;
2223
import java.util.Properties;
2324
import java.util.Set;
2425
import org.slf4j.Logger;
@@ -101,20 +102,32 @@ public <T extends Enum<T>> T getEnum(String key, Class<T> enumType, T defaultVal
101102

102103
public String getString(String key, String defaultValue, String... aliases) {
103104
int seqId = ConfigSetting.DEFAULT_SEQ_ID;
105+
ConfigOrigin usedOrigin = null;
106+
String value = null;
104107
if (collectConfig) {
105108
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT, seqId);
106109
}
107-
String value = null;
108110
for (ConfigProvider.Source source : sources) {
109111
seqId++;
110-
String tmp = source.get(key, aliases);
111-
if (tmp != null) {
112-
value = tmp;
112+
try {
113+
String tmp = source.get(key, aliases);
114+
if (tmp != null) {
115+
value = tmp;
116+
usedOrigin = source.origin();
117+
if (collectConfig) {
118+
ConfigCollector.get().put(key, tmp, source.origin(), seqId);
119+
}
120+
}
121+
} catch (ConfigSourceException e) {
113122
if (collectConfig) {
114-
ConfigCollector.get().put(key, value, source.origin(), seqId);
123+
ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId);
115124
}
116125
}
117126
}
127+
// Re-report the used value to ensure it has the highest seqId
128+
if (collectConfig && value != null && usedOrigin != null) {
129+
ConfigCollector.get().put(key, value, usedOrigin, seqId + 1);
130+
}
118131
return value != null ? value : defaultValue;
119132
}
120133

@@ -124,20 +137,32 @@ public String getString(String key, String defaultValue, String... aliases) {
124137
*/
125138
public String getStringNotEmpty(String key, String defaultValue, String... aliases) {
126139
int seqId = ConfigSetting.DEFAULT_SEQ_ID;
140+
ConfigOrigin usedOrigin = null;
141+
String value = null;
127142
if (collectConfig) {
128143
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT, seqId);
129144
}
130-
String value = null;
131145
for (ConfigProvider.Source source : sources) {
132146
seqId++;
133-
String tmp = source.get(key, aliases);
134-
if (tmp != null && !tmp.trim().isEmpty()) {
135-
value = tmp;
147+
try {
148+
String tmp = source.get(key, aliases);
149+
if (tmp != null && !tmp.trim().isEmpty()) {
150+
value = tmp;
151+
usedOrigin = source.origin();
152+
if (collectConfig) {
153+
ConfigCollector.get().put(key, tmp, source.origin(), seqId);
154+
}
155+
}
156+
} catch (ConfigSourceException e) {
136157
if (collectConfig) {
137-
ConfigCollector.get().put(key, value, source.origin(), seqId);
158+
ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId);
138159
}
139160
}
140161
}
162+
// Re-report the used value to ensure it has the highest seqId
163+
if (collectConfig && value != null && usedOrigin != null) {
164+
ConfigCollector.get().put(key, value, usedOrigin, seqId + 1);
165+
}
141166
return value != null ? value : defaultValue;
142167
}
143168

@@ -147,23 +172,35 @@ public String getStringExcludingSource(
147172
Class<? extends ConfigProvider.Source> excludedSource,
148173
String... aliases) {
149174
int seqId = ConfigSetting.DEFAULT_SEQ_ID;
175+
ConfigOrigin usedOrigin = null;
176+
String value = null;
150177
if (collectConfig) {
151178
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT, seqId);
152179
}
153-
String value = null;
154180
for (ConfigProvider.Source source : sources) {
155181
if (excludedSource.isAssignableFrom(source.getClass())) {
156182
continue;
157183
}
158184
seqId++;
159-
String tmp = source.get(key, aliases);
160-
if (tmp != null) {
161-
value = tmp;
185+
try {
186+
String tmp = source.get(key, aliases);
187+
if (tmp != null) {
188+
value = tmp;
189+
usedOrigin = source.origin();
190+
if (collectConfig) {
191+
ConfigCollector.get().put(key, tmp, source.origin(), seqId);
192+
}
193+
}
194+
} catch (ConfigSourceException e) {
162195
if (collectConfig) {
163-
ConfigCollector.get().put(key, value, source.origin(), seqId);
196+
ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId);
164197
}
165198
}
166199
}
200+
// Re-report the used value to ensure it has the highest seqId
201+
if (collectConfig && value != null && usedOrigin != null) {
202+
ConfigCollector.get().put(key, value, usedOrigin, seqId + 1);
203+
}
167204
return value != null ? value : defaultValue;
168205
}
169206

@@ -226,25 +263,40 @@ public double getDouble(String key, double defaultValue) {
226263

227264
private <T> T get(String key, T defaultValue, Class<T> type, String... aliases) {
228265
int seqId = ConfigSetting.DEFAULT_SEQ_ID;
266+
ConfigOrigin usedOrigin = null;
267+
String usedSourceValue = null;
268+
T value = null;
229269
if (collectConfig) {
230270
ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT, seqId);
231271
}
232-
T value = null;
233272
for (ConfigProvider.Source source : sources) {
273+
seqId++;
234274
try {
235-
seqId++;
236275
String sourceValue = source.get(key, aliases);
237276
T tmp = ConfigConverter.valueOf(sourceValue, type);
238277
if (tmp != null) {
239278
value = tmp;
279+
usedOrigin = source.origin();
280+
usedSourceValue = sourceValue;
240281
if (collectConfig) {
241282
ConfigCollector.get().put(key, sourceValue, source.origin(), seqId);
242283
}
243284
}
285+
} catch (ConfigSourceException e) {
286+
if (Objects.equals(key, "CONFIG_NAME")) {
287+
System.out.println("MTOFF: exception thrown");
288+
}
289+
if (collectConfig) {
290+
ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId);
291+
}
244292
} catch (NumberFormatException ex) {
245293
// continue
246294
}
247295
}
296+
// Re-report the used value to ensure it has the highest seqId
297+
if (collectConfig && value != null && usedOrigin != null && usedSourceValue != null) {
298+
ConfigCollector.get().put(key, usedSourceValue, usedOrigin, seqId + 1);
299+
}
248300
return value != null ? value : defaultValue;
249301
}
250302

@@ -293,15 +345,21 @@ public Map<String, String> getMergedMap(String key, String... aliases) {
293345
// https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html
294346
// We iterate in order so higher precedence sources overwrite lower precedence
295347
for (Source source : sources) {
296-
String value = source.get(key, aliases);
297-
Map<String, String> parsedMap = ConfigConverter.parseMap(value, key);
298-
if (!parsedMap.isEmpty()) {
299-
origin = source.origin();
348+
try {
349+
String value = source.get(key, aliases);
350+
Map<String, String> parsedMap = ConfigConverter.parseMap(value, key);
351+
if (!parsedMap.isEmpty()) {
352+
origin = source.origin();
353+
if (collectConfig) {
354+
ConfigCollector.get().put(key, parsedMap, origin, seqId);
355+
}
356+
}
357+
merged.putAll(parsedMap);
358+
} catch (ConfigSourceException e) {
300359
if (collectConfig) {
301-
ConfigCollector.get().put(key, parsedMap, origin, seqId);
360+
ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId);
302361
}
303362
}
304-
merged.putAll(parsedMap);
305363
seqId++;
306364
}
307365
if (collectConfig) {
@@ -325,16 +383,22 @@ public Map<String, String> getMergedTagsMap(String key, String... aliases) {
325383
// https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html
326384
// We iterate in order so higher precedence sources overwrite lower precedence
327385
for (Source source : sources) {
328-
String value = source.get(key, aliases);
329-
Map<String, String> parsedMap =
330-
ConfigConverter.parseTraceTagsMap(value, ':', Arrays.asList(',', ' '));
331-
if (!parsedMap.isEmpty()) {
332-
origin = source.origin();
386+
try {
387+
String value = source.get(key, aliases);
388+
Map<String, String> parsedMap =
389+
ConfigConverter.parseTraceTagsMap(value, ':', Arrays.asList(',', ' '));
390+
if (!parsedMap.isEmpty()) {
391+
origin = source.origin();
392+
if (collectConfig) {
393+
ConfigCollector.get().put(key, parsedMap, origin, seqId);
394+
}
395+
}
396+
merged.putAll(parsedMap);
397+
} catch (ConfigSourceException e) {
333398
if (collectConfig) {
334-
ConfigCollector.get().put(key, parsedMap, origin, seqId);
399+
ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId);
335400
}
336401
}
337-
merged.putAll(parsedMap);
338402
seqId++;
339403
}
340404
if (collectConfig) {
@@ -356,19 +420,26 @@ public Map<String, String> getOrderedMap(String key) {
356420
// https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html
357421
// We iterate in order so higher precedence sources overwrite lower precedence
358422
for (Source source : sources) {
359-
String value = source.get(key);
360-
Map<String, String> parsedMap = ConfigConverter.parseOrderedMap(value, key);
361-
if (!parsedMap.isEmpty()) {
362-
origin = source.origin();
423+
try {
424+
String value = source.get(key);
425+
Map<String, String> parsedMap = ConfigConverter.parseOrderedMap(value, key);
426+
if (!parsedMap.isEmpty()) {
427+
origin = source.origin();
428+
if (collectConfig) {
429+
ConfigCollector.get().put(key, parsedMap, origin, seqId);
430+
}
431+
}
432+
merged.putAll(parsedMap);
433+
} catch (ConfigSourceException e) {
363434
if (collectConfig) {
364-
ConfigCollector.get().put(key, parsedMap, origin, seqId);
435+
ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId);
365436
}
366437
}
367-
merged.putAll(parsedMap);
368438
seqId++;
369439
}
370440
if (collectConfig) {
371441
if (origin != null) {
442+
// TO DISCUSS: But if multiple sources have been set, origin isn't exactly accurate here...?
372443
ConfigCollector.get().put(key, merged, origin, seqId);
373444
} else {
374445
ConfigCollector.get().put(key, merged, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID);
@@ -388,16 +459,23 @@ public Map<String, String> getMergedMapWithOptionalMappings(
388459
// We iterate in order so higher precedence sources overwrite lower precedence
389460
for (String key : keys) {
390461
for (Source source : sources) {
391-
String value = source.get(key);
392-
Map<String, String> parsedMap =
393-
ConfigConverter.parseMapWithOptionalMappings(value, key, defaultPrefix, lowercaseKeys);
394-
if (!parsedMap.isEmpty()) {
395-
origin = source.origin();
462+
try {
463+
String value = source.get(key);
464+
Map<String, String> parsedMap =
465+
ConfigConverter.parseMapWithOptionalMappings(
466+
value, key, defaultPrefix, lowercaseKeys);
467+
if (!parsedMap.isEmpty()) {
468+
origin = source.origin();
469+
if (collectConfig) {
470+
ConfigCollector.get().put(key, parsedMap, origin, seqId);
471+
}
472+
}
473+
merged.putAll(parsedMap);
474+
} catch (ConfigSourceException e) {
396475
if (collectConfig) {
397-
ConfigCollector.get().put(key, parsedMap, origin, seqId);
476+
ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId);
398477
}
399478
}
400-
merged.putAll(parsedMap);
401479
seqId++;
402480
}
403481
if (collectConfig) {
@@ -580,7 +658,7 @@ private static Properties loadConfigurationFile(ConfigProvider configProvider) {
580658
}
581659

582660
public abstract static class Source {
583-
public final String get(String key, String... aliases) {
661+
public final String get(String key, String... aliases) throws ConfigSourceException {
584662
String value = get(key);
585663
if (value != null) {
586664
return value;
@@ -594,7 +672,7 @@ public final String get(String key, String... aliases) {
594672
return null;
595673
}
596674

597-
protected abstract String get(String key);
675+
protected abstract String get(String key) throws ConfigSourceException;
598676

599677
public abstract ConfigOrigin origin();
600678
}

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package datadog.trace.bootstrap.config.provider;
2+
3+
/**
4+
* Exception thrown when a ConfigProvider.Source encounters an error (e.g., parsing, IO, or format
5+
* error) while retrieving a configuration value.
6+
*/
7+
public class ConfigSourceException extends Exception {
8+
private final Object rawValue;
9+
10+
public ConfigSourceException(String message, Object rawValue, Throwable cause) {
11+
super(message, cause);
12+
this.rawValue = rawValue;
13+
}
14+
15+
public ConfigSourceException(Object rawValue) {
16+
this.rawValue = rawValue;
17+
}
18+
19+
/** Returns the raw value that caused the exception, if available. */
20+
public Object getRawValue() {
21+
return rawValue;
22+
}
23+
}

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,18 @@ public final class StableConfigSource extends ConfigProvider.Source {
4747
this.config = cfg;
4848
}
4949

50-
@Override
51-
public String get(String key) {
50+
public String get(String key) throws ConfigSourceException {
5251
if (this.config == StableConfig.EMPTY) {
5352
return null;
5453
}
55-
return this.config.get(propertyNameToEnvironmentVariableName(key));
54+
Object value = this.config.get(propertyNameToEnvironmentVariableName(key));
55+
if (value == null) {
56+
return null;
57+
}
58+
if (!(value instanceof String)) {
59+
throw new ConfigSourceException(value);
60+
}
61+
return (String) value;
5662
}
5763

5864
@Override
@@ -78,9 +84,8 @@ public StableConfig(String configId, Map<String, Object> configMap) {
7884
this.apmConfiguration = configMap;
7985
}
8086

81-
public String get(String key) {
82-
Object value = this.apmConfiguration.get(key);
83-
return (value == null) ? null : String.valueOf(value);
87+
public Object get(String key) {
88+
return this.apmConfiguration.get(key);
8489
}
8590

8691
public Set<String> getKeys() {

0 commit comments

Comments
 (0)