Skip to content

ConfigProvider.Source bubbles up parsing errors to ConfigProvider with ConfigSourceException #9325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

ConfigProvider.Source bubbles up parsing errors to ConfigProvider with ConfigSourceException #9325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 83 additions & 37 deletions internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,18 @@ public <T extends Enum<T>> T getEnum(String key, Class<T> enumType, T defaultVal

public String getString(String key, String defaultValue, String... aliases) {
for (ConfigProvider.Source source : sources) {
String value = source.get(key, aliases);
if (value != null) {
try {
String value = source.get(key, aliases);
if (value != null) {
if (collectConfig) {
ConfigCollector.get().put(key, value, source.origin());
}
return value;
}
} catch (ConfigSourceException e) {
if (collectConfig) {
ConfigCollector.get().put(key, value, source.origin());
ConfigCollector.get().put(key, e.getRawValue(), source.origin());
}
return value;
}
}
if (collectConfig) {
Expand All @@ -96,12 +102,18 @@ public String getString(String key, String defaultValue, String... aliases) {
*/
public String getStringNotEmpty(String key, String defaultValue, String... aliases) {
for (ConfigProvider.Source source : sources) {
String value = source.get(key, aliases);
if (value != null && !value.trim().isEmpty()) {
try {
String value = source.get(key, aliases);
if (value != null && !value.trim().isEmpty()) {
if (collectConfig) {
ConfigCollector.get().put(key, value, source.origin());
}
return value;
}
} catch (ConfigSourceException e) {
if (collectConfig) {
ConfigCollector.get().put(key, value, source.origin());
ConfigCollector.get().put(key, e.getRawValue(), source.origin());
}
return value;
}
}
if (collectConfig) {
Expand All @@ -119,13 +131,18 @@ public String getStringExcludingSource(
if (excludedSource.isAssignableFrom(source.getClass())) {
continue;
}

String value = source.get(key, aliases);
if (value != null) {
try {
String value = source.get(key, aliases);
if (value != null) {
if (collectConfig) {
ConfigCollector.get().put(key, value, source.origin());
}
return value;
}
} catch (ConfigSourceException e) {
if (collectConfig) {
ConfigCollector.get().put(key, value, source.origin());
ConfigCollector.get().put(key, e.getRawValue(), source.origin());
}
return value;
}
}
if (collectConfig) {
Expand Down Expand Up @@ -202,6 +219,10 @@ private <T> T get(String key, T defaultValue, Class<T> type, String... aliases)
}
return value;
}
} catch (ConfigSourceException e) {
if (collectConfig) {
ConfigCollector.get().put(key, e.getRawValue(), source.origin());
}
} catch (NumberFormatException ex) {
// continue
}
Expand Down Expand Up @@ -252,12 +273,18 @@ public Map<String, String> getMergedMap(String key, String... aliases) {
// 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<String, String> parsedMap = ConfigConverter.parseMap(value, key);
if (!parsedMap.isEmpty()) {
origin = sources[i].origin();
try {
String value = sources[i].get(key, aliases);
Map<String, String> parsedMap = ConfigConverter.parseMap(value, key);
if (!parsedMap.isEmpty()) {
origin = sources[i].origin();
}
merged.putAll(parsedMap);
} catch (ConfigSourceException e) {
if (collectConfig) {
ConfigCollector.get().put(key, e.getRawValue(), sources[i].origin());
}
}
merged.putAll(parsedMap);
}
if (collectConfig) {
ConfigCollector.get().put(key, merged, origin);
Expand All @@ -273,13 +300,19 @@ public Map<String, String> getMergedTagsMap(String key, String... aliases) {
// 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<String, String> parsedMap =
ConfigConverter.parseTraceTagsMap(value, ':', Arrays.asList(',', ' '));
if (!parsedMap.isEmpty()) {
origin = sources[i].origin();
try {
String value = sources[i].get(key, aliases);
Map<String, String> parsedMap =
ConfigConverter.parseTraceTagsMap(value, ':', Arrays.asList(',', ' '));
if (!parsedMap.isEmpty()) {
origin = sources[i].origin();
}
merged.putAll(parsedMap);
} catch (ConfigSourceException e) {
if (collectConfig) {
ConfigCollector.get().put(key, e.getRawValue(), sources[i].origin());
}
}
merged.putAll(parsedMap);
}
if (collectConfig) {
ConfigCollector.get().put(key, merged, origin);
Expand All @@ -295,12 +328,18 @@ public Map<String, String> getOrderedMap(String key) {
// 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);
Map<String, String> parsedMap = ConfigConverter.parseOrderedMap(value, key);
if (!parsedMap.isEmpty()) {
origin = sources[i].origin();
try {
String value = sources[i].get(key);
Map<String, String> parsedMap = ConfigConverter.parseOrderedMap(value, key);
if (!parsedMap.isEmpty()) {
origin = sources[i].origin();
}
merged.putAll(parsedMap);
} catch (ConfigSourceException e) {
if (collectConfig) {
ConfigCollector.get().put(key, e.getRawValue(), sources[i].origin());
}
}
merged.putAll(parsedMap);
}
if (collectConfig) {
ConfigCollector.get().put(key, merged, origin);
Expand All @@ -318,13 +357,20 @@ public Map<String, String> getMergedMapWithOptionalMappings(
// We reverse iterate to allow overrides
for (String key : keys) {
for (int i = sources.length - 1; 0 <= i; i--) {
String value = sources[i].get(key);
Map<String, String> parsedMap =
ConfigConverter.parseMapWithOptionalMappings(value, key, defaultPrefix, lowercaseKeys);
if (!parsedMap.isEmpty()) {
origin = sources[i].origin();
try {
String value = sources[i].get(key);
Map<String, String> parsedMap =
ConfigConverter.parseMapWithOptionalMappings(
value, key, defaultPrefix, lowercaseKeys);
if (!parsedMap.isEmpty()) {
origin = sources[i].origin();
}
merged.putAll(parsedMap);
} catch (ConfigSourceException e) {
if (collectConfig) {
ConfigCollector.get().put(key, e.getRawValue(), sources[i].origin());
}
}
merged.putAll(parsedMap);
}
if (collectConfig) {
ConfigCollector.get().put(key, merged, origin);
Expand Down Expand Up @@ -500,7 +546,7 @@ private static Properties loadConfigurationFile(ConfigProvider configProvider) {
}

public abstract static class Source {
public final String get(String key, String... aliases) {
public final String get(String key, String... aliases) throws ConfigSourceException {
String value = get(key);
if (value != null) {
return value;
Expand All @@ -514,7 +560,7 @@ public final String get(String key, String... aliases) {
return null;
}

protected abstract String get(String key);
protected abstract String get(String key) throws ConfigSourceException;

public abstract ConfigOrigin origin();
}
Expand Down
23 changes: 23 additions & 0 deletions ...rnal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigSourceException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package datadog.trace.bootstrap.config.provider;

/**
* Exception thrown when a ConfigProvider.Source encounters an error (e.g., parsing, IO, or format
* error) while retrieving a configuration value.
*/
public class ConfigSourceException extends Exception {
private final Object rawValue;

public ConfigSourceException(String message, Object rawValue, Throwable cause) {
super(message, cause);
this.rawValue = rawValue;
}

public ConfigSourceException(Object rawValue) {
this.rawValue = rawValue;
}

/** Returns the raw value that caused the exception, if available. */
public Object getRawValue() {
return rawValue;
}
}
60 changes: 60 additions & 0 deletions ...nal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigProviderTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package datadog.trace.bootstrap.config.provider

import datadog.trace.api.ConfigOrigin
import datadog.trace.test.util.DDSpecification
import spock.lang.Shared

Expand Down Expand Up @@ -45,4 +46,63 @@ class ConfigProviderTest extends DDSpecification {
"default" | null | "alias2" | "default"
null | "alias1" | "alias2" | "alias1"
}

def "ConfigProvider handles ConfigSourceException gracefully"() {
given:
def throwingSource = new ConfigProvider.Source() {
@Override
protected String get(String key) throws ConfigSourceException {
throw new ConfigSourceException("raw")
}
@Override
ConfigOrigin origin() {
ConfigOrigin.ENV
}
}
// Create a provider with a collector
def provider = new ConfigProvider(true, throwingSource)

expect:
//Any "get" method should return the default value, if provided
provider.getString("any.key", "default") == "default"
provider.getBoolean("any.key", true) == true
provider.getInteger("any.key", 42) == 42
provider.getLong("any.key", 123L) == 123L
provider.getFloat("any.key", 1.23f) == 1.23f
provider.getDouble("any.key", 2.34d) == 2.34d
provider.getList("any.key", ["a", "b"]) == ["a", "b"]
provider.getSet("any.key", ["x", "y"] as Set) == ["x", "y"] as Set
}

def "ConfigProvider skips sources that throw ConfigSourceException and uses next available value"() {
given:
def throwingSource = new ConfigProvider.Source() {
@Override
protected String get(String key) throws ConfigSourceException {
throw new ConfigSourceException("raw")
}
@Override
ConfigOrigin origin() {
ConfigOrigin.ENV
}
}

def workingSource = new ConfigProvider.Source() {
@Override
protected String get(String key) throws ConfigSourceException {
if (key == "any.key") {
return "fromSecondSource"
}
return null
}
@Override
ConfigOrigin origin() {
ConfigOrigin.JVM_PROP
}
}
def provider = new ConfigProvider(true, throwingSource, workingSource)

expect:
provider.getString("any.key", "default") == "fromSecondSource"
}
}