From 48d0e5105d691db0abfe9b36e5964ad6323c2e7c Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 4 Aug 2025 15:58:21 -0400 Subject: [PATCH 01/10] Change ConfigProvider to iterate from lowest to highest precedence source, adn report all to ConfigCollector --- .../config/provider/ConfigProvider.java | 159 ++++++++++++------ .../trace/api/ConfigCollectorTest.groovy | 58 +++---- 2 files changed, 139 insertions(+), 78 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java index 1edeb072c80..6ffbe0fb6b9 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java @@ -5,6 +5,7 @@ import datadog.environment.SystemProperties; import datadog.trace.api.ConfigCollector; import datadog.trace.api.ConfigOrigin; +import datadog.trace.api.config.AppSecConfig; import de.thetaphi.forbiddenapis.SuppressForbidden; import java.io.File; import java.io.FileNotFoundException; @@ -12,6 +13,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.BitSet; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; @@ -42,6 +44,32 @@ private ConfigProvider(boolean collectConfig, ConfigProvider.Source... sources) this.sources = sources; } + /** + * Creates a ConfigProvider with sources ordered from lowest to highest precedence. Internally + * reverses the array to support the new approach of iterating from lowest to highest precedence, + * enabling reporting of all configured sources to telemetry (not just the highest-precedence + * match). + * + * @param sources the configuration sources, in order from lowest to highest precedence + * @return a ConfigProvider with sources in precedence order (highest first) + */ + public static ConfigProvider createWithPrecedenceOrder(Source... sources) { + Source[] reversed = Arrays.copyOf(sources, sources.length); + Collections.reverse(Arrays.asList(reversed)); + return new ConfigProvider(reversed); + } + + /** + * Same as {@link #createWithPrecedenceOrder(Source...)} but allows specifying the collectConfig + * flag. + */ + public static ConfigProvider createWithPrecedenceOrder(boolean collectConfig, Source... sources) { + Source[] reversed = Arrays.copyOf(sources, sources.length); + Collections.reverse(Arrays.asList(reversed)); + return new ConfigProvider(collectConfig, reversed); + } + + // TODO: Handle this special case public String getConfigFileStatus() { for (ConfigProvider.Source source : sources) { if (source instanceof PropertiesConfigSource) { @@ -75,19 +103,20 @@ public > T getEnum(String key, Class enumType, T defaultVal } public String getString(String key, String defaultValue, String... aliases) { + if (collectConfig) { + ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); + } + String value = null; for (ConfigProvider.Source source : sources) { - String value = source.get(key, aliases); - if (value != null) { + String tmp = source.get(key, aliases); + if (tmp != null) { + value = tmp; if (collectConfig) { ConfigCollector.get().put(key, value, source.origin()); } - return value; } } - if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); - } - return defaultValue; + return value != null ? value : defaultValue; } /** @@ -95,19 +124,30 @@ public String getString(String key, String defaultValue, String... aliases) { * an empty or blank string. */ public String getStringNotEmpty(String key, String defaultValue, String... aliases) { + if (collectConfig) { + ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); + } + String value = null; for (ConfigProvider.Source source : sources) { - String value = source.get(key, aliases); - if (value != null && !value.trim().isEmpty()) { + String tmp = source.get(key, aliases); + if (key.equals(AppSecConfig.APPSEC_AUTOMATED_USER_EVENTS_TRACKING)) { + System.out.println("MTOFF - source: " + source.getClass().getSimpleName() + " tmp: " + tmp); + } + if (tmp != null && !tmp.trim().isEmpty()) { + value = tmp; + if (key.equals(AppSecConfig.APPSEC_AUTOMATED_USER_EVENTS_TRACKING)) { + System.out.println( + "MTOFF - source: " + source.getClass().getSimpleName() + " value: " + value); + } if (collectConfig) { ConfigCollector.get().put(key, value, source.origin()); } - return value; } } if (collectConfig) { ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); } - return defaultValue; + return value != null ? value : defaultValue; } public String getStringExcludingSource( @@ -115,23 +155,27 @@ public String getStringExcludingSource( String defaultValue, Class excludedSource, String... aliases) { + if (collectConfig) { + ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); + } + String value = null; for (ConfigProvider.Source source : sources) { if (excludedSource.isAssignableFrom(source.getClass())) { continue; } - String value = source.get(key, aliases); - if (value != null) { + String tmp = source.get(key, aliases); + if (tmp != null) { + value = tmp; if (collectConfig) { ConfigCollector.get().put(key, value, source.origin()); } - return value; } } if (collectConfig) { ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); } - return defaultValue; + return value != null ? value : defaultValue; } public boolean isSet(String key) { @@ -192,15 +236,19 @@ public double getDouble(String key, double defaultValue) { } private T get(String key, T defaultValue, Class type, String... aliases) { + if (collectConfig) { + ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); + } + T value = null; for (ConfigProvider.Source source : sources) { try { String sourceValue = source.get(key, aliases); - T value = ConfigConverter.valueOf(sourceValue, type); - if (value != null) { + T tmp = ConfigConverter.valueOf(sourceValue, type); + if (tmp != null) { + value = tmp; if (collectConfig) { ConfigCollector.get().put(key, sourceValue, source.origin()); } - return value; } } catch (NumberFormatException ex) { // continue @@ -209,7 +257,7 @@ private T get(String key, T defaultValue, Class type, String... aliases) if (collectConfig) { ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); } - return defaultValue; + return value != null ? value : defaultValue; } public List getList(String key) { @@ -217,11 +265,11 @@ public List getList(String key) { } public List getList(String key, List defaultValue) { + if (collectConfig) { + ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); + } String list = getString(key); if (null == list) { - if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); - } return defaultValue; } else { return ConfigConverter.parseList(list); @@ -229,11 +277,11 @@ public List getList(String key, List defaultValue) { } public Set getSet(String key, Set defaultValue) { + if (collectConfig) { + ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); + } String list = getString(key); if (null == list) { - if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); - } return defaultValue; } else { return new HashSet(ConfigConverter.parseList(list)); @@ -250,16 +298,20 @@ public Map getMergedMap(String key, String... aliases) { // System properties take precedence over env // prior art: // 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); + // We iterate in order so higher precedence sources overwrite lower precedence + for (Source source : sources) { + String value = source.get(key, aliases); Map parsedMap = ConfigConverter.parseMap(value, key); if (!parsedMap.isEmpty()) { - origin = sources[i].origin(); + origin = source.origin(); + if (collectConfig) { + ConfigCollector.get().put(key, parsedMap, origin); + } } merged.putAll(parsedMap); } if (collectConfig) { + // TO DISCUSS: But if multiple sources have been set, origin isn't exactly accurate here...? ConfigCollector.get().put(key, merged, origin); } return merged; @@ -271,13 +323,16 @@ public Map getMergedTagsMap(String key, String... aliases) { // System properties take precedence over env // prior art: // 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); + // We iterate in order so higher precedence sources overwrite lower precedence + for (Source source : sources) { + String value = source.get(key, aliases); Map parsedMap = ConfigConverter.parseTraceTagsMap(value, ':', Arrays.asList(',', ' ')); if (!parsedMap.isEmpty()) { - origin = sources[i].origin(); + origin = source.origin(); + if (collectConfig) { + ConfigCollector.get().put(key, parsedMap, origin); + } } merged.putAll(parsedMap); } @@ -293,12 +348,15 @@ public Map getOrderedMap(String key) { // System properties take precedence over env // prior art: // 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); + // We iterate in order so higher precedence sources overwrite lower precedence + for (Source source : sources) { + String value = source.get(key); Map parsedMap = ConfigConverter.parseOrderedMap(value, key); if (!parsedMap.isEmpty()) { - origin = sources[i].origin(); + origin = source.origin(); + if (collectConfig) { + ConfigCollector.get().put(key, parsedMap, origin); + } } merged.putAll(parsedMap); } @@ -315,14 +373,17 @@ public Map getMergedMapWithOptionalMappings( // System properties take precedence over env // prior art: // https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html - // We reverse iterate to allow overrides + // We iterate in order so higher precedence sources overwrite lower precedence for (String key : keys) { - for (int i = sources.length - 1; 0 <= i; i--) { - String value = sources[i].get(key); + for (Source source : sources) { + String value = source.get(key); Map parsedMap = ConfigConverter.parseMapWithOptionalMappings(value, key, defaultPrefix, lowercaseKeys); if (!parsedMap.isEmpty()) { - origin = sources[i].origin(); + origin = source.origin(); + if (collectConfig) { + ConfigCollector.get().put(key, parsedMap, origin); + } } merged.putAll(parsedMap); } @@ -378,7 +439,7 @@ public static ConfigProvider createDefault() { loadConfigurationFile( new ConfigProvider(new SystemPropertiesConfigSource(), new EnvironmentConfigSource())); if (configProperties.isEmpty()) { - return new ConfigProvider( + return createWithPrecedenceOrder( new SystemPropertiesConfigSource(), StableConfigSource.FLEET, new EnvironmentConfigSource(), @@ -386,7 +447,7 @@ public static ConfigProvider createDefault() { StableConfigSource.LOCAL, new CapturedEnvironmentConfigSource()); } else { - return new ConfigProvider( + return createWithPrecedenceOrder( new SystemPropertiesConfigSource(), StableConfigSource.FLEET, new EnvironmentConfigSource(), @@ -400,10 +461,10 @@ public static ConfigProvider createDefault() { public static ConfigProvider withoutCollector() { Properties configProperties = loadConfigurationFile( - new ConfigProvider( + createWithPrecedenceOrder( false, new SystemPropertiesConfigSource(), new EnvironmentConfigSource())); if (configProperties.isEmpty()) { - return new ConfigProvider( + return createWithPrecedenceOrder( false, new SystemPropertiesConfigSource(), StableConfigSource.FLEET, @@ -412,7 +473,7 @@ public static ConfigProvider withoutCollector() { StableConfigSource.LOCAL, new CapturedEnvironmentConfigSource()); } else { - return new ConfigProvider( + return createWithPrecedenceOrder( false, new SystemPropertiesConfigSource(), StableConfigSource.FLEET, @@ -428,12 +489,12 @@ public static ConfigProvider withPropertiesOverride(Properties properties) { PropertiesConfigSource providedConfigSource = new PropertiesConfigSource(properties, false); Properties configProperties = loadConfigurationFile( - new ConfigProvider( + createWithPrecedenceOrder( new SystemPropertiesConfigSource(), new EnvironmentConfigSource(), providedConfigSource)); if (configProperties.isEmpty()) { - return new ConfigProvider( + return createWithPrecedenceOrder( new SystemPropertiesConfigSource(), StableConfigSource.FLEET, new EnvironmentConfigSource(), @@ -442,7 +503,7 @@ public static ConfigProvider withPropertiesOverride(Properties properties) { StableConfigSource.LOCAL, new CapturedEnvironmentConfigSource()); } else { - return new ConfigProvider( + return createWithPrecedenceOrder( providedConfigSource, new SystemPropertiesConfigSource(), StableConfigSource.FLEET, diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy index 12a52136eb0..5864cf321d4 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy @@ -28,38 +28,38 @@ class ConfigCollectorTest extends DDSpecification { where: configKey | configValue - // ConfigProvider.getEnum - IastConfig.IAST_TELEMETRY_VERBOSITY | Verbosity.DEBUG.toString() - // ConfigProvider.getString - TracerConfig.TRACE_SPAN_ATTRIBUTE_SCHEMA | "v1" + // // ConfigProvider.getEnum + // IastConfig.IAST_TELEMETRY_VERBOSITY | Verbosity.DEBUG.toString() + // // ConfigProvider.getString + // TracerConfig.TRACE_SPAN_ATTRIBUTE_SCHEMA | "v1" // ConfigProvider.getStringNotEmpty AppSecConfig.APPSEC_AUTOMATED_USER_EVENTS_TRACKING | UserEventTrackingMode.EXTENDED.toString() // ConfigProvider.getStringExcludingSource - GeneralConfig.APPLICATION_KEY | "app-key" - // ConfigProvider.getBoolean - TraceInstrumentationConfig.RESOLVER_USE_URL_CACHES | "true" - // ConfigProvider.getInteger - JmxFetchConfig.JMX_FETCH_CHECK_PERIOD | "60" - // ConfigProvider.getLong - CiVisibilityConfig.CIVISIBILITY_GIT_COMMAND_TIMEOUT_MILLIS | "450273" - // ConfigProvider.getFloat - GeneralConfig.TELEMETRY_HEARTBEAT_INTERVAL | "1.5" - // ConfigProvider.getDouble - TracerConfig.TRACE_SAMPLE_RATE | "2.2" - // ConfigProvider.getList - TraceInstrumentationConfig.JMS_PROPAGATION_DISABLED_TOPICS | "someTopic,otherTopic" - // ConfigProvider.getSet - IastConfig.IAST_WEAK_HASH_ALGORITHMS | "SHA1,SHA-1" - // ConfigProvider.getSpacedList - TracerConfig.PROXY_NO_PROXY | "a b c" - // ConfigProvider.getMergedMap - TracerConfig.TRACE_PEER_SERVICE_MAPPING | "service1:best_service,userService:my_service" - // ConfigProvider.getOrderedMap - TracerConfig.TRACE_HTTP_SERVER_PATH_RESOURCE_NAME_MAPPING | "/asdf/*:/test" - // ConfigProvider.getMergedMapWithOptionalMappings - TracerConfig.HEADER_TAGS | "e:five" - // ConfigProvider.getIntegerRange - TracerConfig.TRACE_HTTP_CLIENT_ERROR_STATUSES | "400-402" + // GeneralConfig.APPLICATION_KEY | "app-key" + // // ConfigProvider.getBoolean + // TraceInstrumentationConfig.RESOLVER_USE_URL_CACHES | "true" + // // ConfigProvider.getInteger + // JmxFetchConfig.JMX_FETCH_CHECK_PERIOD | "60" + // // ConfigProvider.getLong + // CiVisibilityConfig.CIVISIBILITY_GIT_COMMAND_TIMEOUT_MILLIS | "450273" + // // ConfigProvider.getFloat + // GeneralConfig.TELEMETRY_HEARTBEAT_INTERVAL | "1.5" + // // ConfigProvider.getDouble + // TracerConfig.TRACE_SAMPLE_RATE | "2.2" + // // ConfigProvider.getList + // TraceInstrumentationConfig.JMS_PROPAGATION_DISABLED_TOPICS | "someTopic,otherTopic" + // // ConfigProvider.getSet + // IastConfig.IAST_WEAK_HASH_ALGORITHMS | "SHA1,SHA-1" + // // ConfigProvider.getSpacedList + // TracerConfig.PROXY_NO_PROXY | "a b c" + // // ConfigProvider.getMergedMap + // TracerConfig.TRACE_PEER_SERVICE_MAPPING | "service1:best_service,userService:my_service" + // // ConfigProvider.getOrderedMap + // TracerConfig.TRACE_HTTP_SERVER_PATH_RESOURCE_NAME_MAPPING | "/asdf/*:/test" + // // ConfigProvider.getMergedMapWithOptionalMappings + // TracerConfig.HEADER_TAGS | "e:five" + // // ConfigProvider.getIntegerRange + // TracerConfig.TRACE_HTTP_CLIENT_ERROR_STATUSES | "400-402" } def "should collect merged data from multiple sources"() { From 90d56445ff489bb6180e33fbff4b699d20afef51 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 4 Aug 2025 16:00:10 -0400 Subject: [PATCH 02/10] Support seq_id in ConfigSetting --- .../java/datadog/trace/api/ConfigSetting.java | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/ConfigSetting.java b/internal-api/src/main/java/datadog/trace/api/ConfigSetting.java index b688d5b477d..4c58bb8e172 100644 --- a/internal-api/src/main/java/datadog/trace/api/ConfigSetting.java +++ b/internal-api/src/main/java/datadog/trace/api/ConfigSetting.java @@ -11,19 +11,25 @@ public final class ConfigSetting { public final String key; public final Object value; public final ConfigOrigin origin; + public final int seqId; private static final Set CONFIG_FILTER_LIST = new HashSet<>( Arrays.asList("DD_API_KEY", "dd.api-key", "dd.profiling.api-key", "dd.profiling.apikey")); public static ConfigSetting of(String key, Object value, ConfigOrigin origin) { - return new ConfigSetting(key, value, origin); + return new ConfigSetting(key, value, origin, 0); } - private ConfigSetting(String key, Object value, ConfigOrigin origin) { + public static ConfigSetting of(String key, Object value, ConfigOrigin origin, int seqId) { + return new ConfigSetting(key, value, origin, seqId); + } + + private ConfigSetting(String key, Object value, ConfigOrigin origin, int seqId) { this.key = key; this.value = CONFIG_FILTER_LIST.contains(key) ? "" : value; this.origin = origin; + this.seqId = seqId; } public String normalizedKey() { @@ -99,12 +105,15 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; ConfigSetting that = (ConfigSetting) o; - return key.equals(that.key) && Objects.equals(value, that.value) && origin == that.origin; + return key.equals(that.key) + && Objects.equals(value, that.value) + && origin == that.origin + && seqId == that.seqId; } @Override public int hashCode() { - return Objects.hash(key, value, origin); + return Objects.hash(key, value, origin, seqId); } @Override @@ -117,6 +126,8 @@ public String toString() { + stringValue() + ", origin=" + origin + + ", seq_id=" + + seqId + '}'; } } From 48e3af76e33b1d10d85dc2611b210f0441087c53 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 4 Aug 2025 16:01:07 -0400 Subject: [PATCH 03/10] nits: remove superfluous debug logs --- .../trace/bootstrap/config/provider/ConfigProvider.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java index 6ffbe0fb6b9..c851a9b0687 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java @@ -5,7 +5,6 @@ import datadog.environment.SystemProperties; import datadog.trace.api.ConfigCollector; import datadog.trace.api.ConfigOrigin; -import datadog.trace.api.config.AppSecConfig; import de.thetaphi.forbiddenapis.SuppressForbidden; import java.io.File; import java.io.FileNotFoundException; @@ -130,15 +129,8 @@ public String getStringNotEmpty(String key, String defaultValue, String... alias String value = null; for (ConfigProvider.Source source : sources) { String tmp = source.get(key, aliases); - if (key.equals(AppSecConfig.APPSEC_AUTOMATED_USER_EVENTS_TRACKING)) { - System.out.println("MTOFF - source: " + source.getClass().getSimpleName() + " tmp: " + tmp); - } if (tmp != null && !tmp.trim().isEmpty()) { value = tmp; - if (key.equals(AppSecConfig.APPSEC_AUTOMATED_USER_EVENTS_TRACKING)) { - System.out.println( - "MTOFF - source: " + source.getClass().getSimpleName() + " value: " + value); - } if (collectConfig) { ConfigCollector.get().put(key, value, source.origin()); } From 3ac04052573a4d83744bb18d417f13a841fbb1d5 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 4 Aug 2025 16:20:47 -0400 Subject: [PATCH 04/10] Report ConfigSettings with seq_id in ConfigProvider and ConfigCollector --- .../datadog/trace/api/ConfigCollector.java | 5 + .../java/datadog/trace/api/ConfigSetting.java | 2 + .../config/provider/ConfigProvider.java | 103 +++++++++++------- 3 files changed, 72 insertions(+), 38 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java b/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java index f4cfda6877b..d531ad9716a 100644 --- a/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java @@ -27,6 +27,11 @@ public void put(String key, Object value, ConfigOrigin origin) { collected.put(key, setting); } + public void put(String key, Object value, ConfigOrigin origin, int seqId) { + ConfigSetting setting = ConfigSetting.of(key, value, origin, seqId); + collected.put(key, setting); + } + public void putAll(Map keysAndValues, ConfigOrigin origin) { // attempt merge+replace to avoid collector seeing partial update Map merged = diff --git a/internal-api/src/main/java/datadog/trace/api/ConfigSetting.java b/internal-api/src/main/java/datadog/trace/api/ConfigSetting.java index 4c58bb8e172..aa18c6c9287 100644 --- a/internal-api/src/main/java/datadog/trace/api/ConfigSetting.java +++ b/internal-api/src/main/java/datadog/trace/api/ConfigSetting.java @@ -13,6 +13,8 @@ public final class ConfigSetting { public final ConfigOrigin origin; public final int seqId; + public static final int DEFAULT_SEQ_ID = 1; + private static final Set CONFIG_FILTER_LIST = new HashSet<>( Arrays.asList("DD_API_KEY", "dd.api-key", "dd.profiling.api-key", "dd.profiling.apikey")); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java index c851a9b0687..43bce888ae4 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java @@ -5,6 +5,7 @@ import datadog.environment.SystemProperties; import datadog.trace.api.ConfigCollector; import datadog.trace.api.ConfigOrigin; +import datadog.trace.api.ConfigSetting; import de.thetaphi.forbiddenapis.SuppressForbidden; import java.io.File; import java.io.FileNotFoundException; @@ -85,6 +86,7 @@ public String getString(String key) { return getString(key, null); } + // TODO: Change this logic too public > T getEnum(String key, Class enumType, T defaultValue) { String value = getString(key); if (null != value) { @@ -94,24 +96,22 @@ public > T getEnum(String key, Class enumType, T defaultVal log.debug("failed to parse {} for {}, defaulting to {}", value, key, defaultValue); } } - if (collectConfig) { - String valueStr = defaultValue == null ? null : defaultValue.name(); - ConfigCollector.get().put(key, valueStr, ConfigOrigin.DEFAULT); - } return defaultValue; } public String getString(String key, String defaultValue, String... aliases) { + int seqId = ConfigSetting.DEFAULT_SEQ_ID; if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); + ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT, seqId); } String value = null; for (ConfigProvider.Source source : sources) { + seqId++; String tmp = source.get(key, aliases); if (tmp != null) { value = tmp; if (collectConfig) { - ConfigCollector.get().put(key, value, source.origin()); + ConfigCollector.get().put(key, value, source.origin(), seqId); } } } @@ -123,22 +123,21 @@ public String getString(String key, String defaultValue, String... aliases) { * an empty or blank string. */ public String getStringNotEmpty(String key, String defaultValue, String... aliases) { + int seqId = ConfigSetting.DEFAULT_SEQ_ID; if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); + ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT, seqId); } String value = null; for (ConfigProvider.Source source : sources) { + seqId++; String tmp = source.get(key, aliases); if (tmp != null && !tmp.trim().isEmpty()) { value = tmp; if (collectConfig) { - ConfigCollector.get().put(key, value, source.origin()); + ConfigCollector.get().put(key, value, source.origin(), seqId); } } } - if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); - } return value != null ? value : defaultValue; } @@ -147,26 +146,24 @@ public String getStringExcludingSource( String defaultValue, Class excludedSource, String... aliases) { + int seqId = ConfigSetting.DEFAULT_SEQ_ID; if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); + ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT, seqId); } String value = null; for (ConfigProvider.Source source : sources) { if (excludedSource.isAssignableFrom(source.getClass())) { continue; } - + seqId++; String tmp = source.get(key, aliases); if (tmp != null) { value = tmp; if (collectConfig) { - ConfigCollector.get().put(key, value, source.origin()); + ConfigCollector.get().put(key, value, source.origin(), seqId); } } } - if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); - } return value != null ? value : defaultValue; } @@ -228,27 +225,26 @@ public double getDouble(String key, double defaultValue) { } private T get(String key, T defaultValue, Class type, String... aliases) { + int seqId = ConfigSetting.DEFAULT_SEQ_ID; if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); + ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT, seqId); } T value = null; for (ConfigProvider.Source source : sources) { try { + seqId++; String sourceValue = source.get(key, aliases); T tmp = ConfigConverter.valueOf(sourceValue, type); if (tmp != null) { value = tmp; if (collectConfig) { - ConfigCollector.get().put(key, sourceValue, source.origin()); + ConfigCollector.get().put(key, sourceValue, source.origin(), seqId); } } } catch (NumberFormatException ex) { // continue } } - if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); - } return value != null ? value : defaultValue; } @@ -257,8 +253,10 @@ public List getList(String key) { } public List getList(String key, List defaultValue) { + // Do we need this, if getString will already report the default? if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); + ConfigCollector.get() + .put(key, defaultValue, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID); } String list = getString(key); if (null == list) { @@ -269,8 +267,10 @@ public List getList(String key, List defaultValue) { } public Set getSet(String key, Set defaultValue) { + // Do we need this, if getString will already report the default? if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); + ConfigCollector.get() + .put(key, defaultValue, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID); } String list = getString(key); if (null == list) { @@ -286,7 +286,8 @@ public List getSpacedList(String key) { public Map getMergedMap(String key, String... aliases) { Map merged = new HashMap<>(); - ConfigOrigin origin = ConfigOrigin.DEFAULT; + ConfigOrigin origin = null; + int seqId = ConfigSetting.DEFAULT_SEQ_ID + 1; // System properties take precedence over env // prior art: // https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html @@ -297,21 +298,28 @@ public Map getMergedMap(String key, String... aliases) { if (!parsedMap.isEmpty()) { origin = source.origin(); if (collectConfig) { - ConfigCollector.get().put(key, parsedMap, origin); + ConfigCollector.get().put(key, parsedMap, origin, seqId); } } merged.putAll(parsedMap); + seqId++; } if (collectConfig) { - // TO DISCUSS: But if multiple sources have been set, origin isn't exactly accurate here...? - ConfigCollector.get().put(key, merged, origin); + if (origin != null) { + // TO DISCUSS: But if multiple sources have been set, origin isn't exactly accurate here...? + ConfigCollector.get().put(key, merged, origin, seqId); + } else { + ConfigCollector.get() + .put(key, new HashMap<>(), ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID); + } } return merged; } public Map getMergedTagsMap(String key, String... aliases) { Map merged = new HashMap<>(); - ConfigOrigin origin = ConfigOrigin.DEFAULT; + ConfigOrigin origin = null; + int seqId = ConfigSetting.DEFAULT_SEQ_ID + 1; // System properties take precedence over env // prior art: // https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html @@ -323,20 +331,26 @@ public Map getMergedTagsMap(String key, String... aliases) { if (!parsedMap.isEmpty()) { origin = source.origin(); if (collectConfig) { - ConfigCollector.get().put(key, parsedMap, origin); + ConfigCollector.get().put(key, parsedMap, origin, seqId); } } merged.putAll(parsedMap); + seqId++; } if (collectConfig) { - ConfigCollector.get().put(key, merged, origin); + if (origin != null) { + ConfigCollector.get().put(key, merged, origin, seqId); + } else { + ConfigCollector.get().put(key, merged, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID); + } } return merged; } public Map getOrderedMap(String key) { LinkedHashMap merged = new LinkedHashMap<>(); - ConfigOrigin origin = ConfigOrigin.DEFAULT; + ConfigOrigin origin = null; + int seqId = ConfigSetting.DEFAULT_SEQ_ID + 1; // System properties take precedence over env // prior art: // https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html @@ -347,13 +361,18 @@ public Map getOrderedMap(String key) { if (!parsedMap.isEmpty()) { origin = source.origin(); if (collectConfig) { - ConfigCollector.get().put(key, parsedMap, origin); + ConfigCollector.get().put(key, parsedMap, origin, seqId); } } merged.putAll(parsedMap); + seqId++; } if (collectConfig) { - ConfigCollector.get().put(key, merged, origin); + if (origin != null) { + ConfigCollector.get().put(key, merged, origin, seqId); + } else { + ConfigCollector.get().put(key, merged, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID); + } } return merged; } @@ -361,7 +380,8 @@ public Map getOrderedMap(String key) { public Map getMergedMapWithOptionalMappings( String defaultPrefix, boolean lowercaseKeys, String... keys) { Map merged = new HashMap<>(); - ConfigOrigin origin = ConfigOrigin.DEFAULT; + ConfigOrigin origin = null; + int seqId = ConfigSetting.DEFAULT_SEQ_ID + 1; // System properties take precedence over env // prior art: // https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html @@ -374,13 +394,19 @@ public Map getMergedMapWithOptionalMappings( if (!parsedMap.isEmpty()) { origin = source.origin(); if (collectConfig) { - ConfigCollector.get().put(key, parsedMap, origin); + ConfigCollector.get().put(key, parsedMap, origin, seqId); } } merged.putAll(parsedMap); + seqId++; } if (collectConfig) { - ConfigCollector.get().put(key, merged, origin); + if (origin != null) { + ConfigCollector.get().put(key, merged, origin, seqId); + } else { + ConfigCollector.get() + .put(key, merged, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID); + } } } return merged; @@ -396,7 +422,8 @@ public BitSet getIntegerRange(final String key, final BitSet defaultValue, Strin log.warn("Invalid configuration for {}", key, e); } if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); + ConfigCollector.get() + .put(key, defaultValue, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID); } return defaultValue; } From 7933b4e32671c42315e819d2598121dd0825b1b0 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 4 Aug 2025 16:25:43 -0400 Subject: [PATCH 05/10] Support seq_id in TelemetryRequestBody --- .../datadog/telemetry/TelemetryRequestBody.java | 1 + .../TelemetryRequestBodySpecification.groovy | 15 +++++++-------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/telemetry/src/main/java/datadog/telemetry/TelemetryRequestBody.java b/telemetry/src/main/java/datadog/telemetry/TelemetryRequestBody.java index ac8466a1913..5f61b3d5586 100644 --- a/telemetry/src/main/java/datadog/telemetry/TelemetryRequestBody.java +++ b/telemetry/src/main/java/datadog/telemetry/TelemetryRequestBody.java @@ -230,6 +230,7 @@ public void writeConfiguration(ConfigSetting configSetting) throws IOException { bodyWriter.name("value").value(configSetting.stringValue()); bodyWriter.setSerializeNulls(false); bodyWriter.name("origin").value(configSetting.origin.value); + bodyWriter.name("seq_id").value(configSetting.seqId); bodyWriter.endObject(); } diff --git a/telemetry/src/test/groovy/datadog/telemetry/TelemetryRequestBodySpecification.groovy b/telemetry/src/test/groovy/datadog/telemetry/TelemetryRequestBodySpecification.groovy index d0f7832da20..33a390b5af9 100644 --- a/telemetry/src/test/groovy/datadog/telemetry/TelemetryRequestBodySpecification.groovy +++ b/telemetry/src/test/groovy/datadog/telemetry/TelemetryRequestBodySpecification.groovy @@ -79,12 +79,12 @@ class TelemetryRequestBodySpecification extends DDSpecification { then: drainToString(req) == ',"configuration":[' + - '{"name":"string","value":"bar","origin":"remote_config"},' + - '{"name":"int","value":"2342","origin":"default"},' + - '{"name":"double","value":"123.456","origin":"env_var"},' + - '{"name":"map","value":"key1:value1,key2:432.32,key3:324","origin":"jvm_prop"},' + - '{"name":"list","value":"1,2,3","origin":"default"},' + - '{"name":"null","value":null,"origin":"default"}]' + '{"name":"string","value":"bar","origin":"remote_config","seq_id":0},' + + '{"name":"int","value":"2342","origin":"default","seq_id":0},' + + '{"name":"double","value":"123.456","origin":"env_var","seq_id":0},' + + '{"name":"map","value":"key1:value1,key2:432.32,key3:324","origin":"jvm_prop","seq_id":0},' + + '{"name":"list","value":"1,2,3","origin":"default","seq_id":0},' + + '{"name":"null","value":null,"origin":"default","seq_id":0}]' } def 'use snake_case for setting keys'() { @@ -102,8 +102,7 @@ class TelemetryRequestBodySpecification extends DDSpecification { req.endConfiguration() then: - drainToString(req) == ',"configuration":[{"name":"this_is_a_key","value":"value","origin":"remote_config"}]' - } + drainToString(req) == ',"configuration":[{"name":"this_is_a_key","value":"value","origin":"remote_config","seq_id":0}]' } def 'add debug flag'() { setup: From 076e9e1564cf05dfc8819361e514db5c261e396b Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 4 Aug 2025 16:50:31 -0400 Subject: [PATCH 06/10] Modify ConfigCollector to support multiple configs for same key, grouped by origin --- .../datadog/trace/api/ConfigCollector.java | 47 ++++-- .../trace/api/ConfigCollectorTest.groovy | 154 ++++++++++++------ 2 files changed, 133 insertions(+), 68 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java b/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java index d531ad9716a..6fe676c95c7 100644 --- a/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java @@ -16,47 +16,58 @@ public class ConfigCollector { private static final AtomicReferenceFieldUpdater COLLECTED_UPDATER = AtomicReferenceFieldUpdater.newUpdater(ConfigCollector.class, Map.class, "collected"); - private volatile Map collected = new ConcurrentHashMap<>(); + private volatile Map> collected = + new ConcurrentHashMap<>(); public static ConfigCollector get() { return INSTANCE; } + /** + * Records the latest ConfigSetting for the given key and origin, replacing any previous value for + * that (key, origin) pair. + */ public void put(String key, Object value, ConfigOrigin origin) { ConfigSetting setting = ConfigSetting.of(key, value, origin); - collected.put(key, setting); + Map originMap = + collected.computeIfAbsent(key, k -> new ConcurrentHashMap<>()); + originMap.put(origin, setting); // replaces any previous value for this origin } public void put(String key, Object value, ConfigOrigin origin, int seqId) { ConfigSetting setting = ConfigSetting.of(key, value, origin, seqId); - collected.put(key, setting); + Map originMap = + collected.computeIfAbsent(key, k -> new ConcurrentHashMap<>()); + originMap.put(origin, setting); // replaces any previous value for this origin } + // TODO: Does this need a counterpart with seqId? public void putAll(Map keysAndValues, ConfigOrigin origin) { - // attempt merge+replace to avoid collector seeing partial update - Map merged = - new ConcurrentHashMap<>(keysAndValues.size() + collected.size()); for (Map.Entry entry : keysAndValues.entrySet()) { - ConfigSetting setting = ConfigSetting.of(entry.getKey(), entry.getValue(), origin); - merged.put(entry.getKey(), setting); - } - while (true) { - Map current = collected; - current.forEach(merged::putIfAbsent); - if (COLLECTED_UPDATER.compareAndSet(this, current, merged)) { - break; // success - } - // roll back to original update before next attempt - merged.keySet().retainAll(keysAndValues.keySet()); + put(entry.getKey(), entry.getValue(), origin); } } @SuppressWarnings("unchecked") - public Map collect() { + public Map> collect() { if (!collected.isEmpty()) { return COLLECTED_UPDATER.getAndSet(this, new ConcurrentHashMap<>()); } else { return Collections.emptyMap(); } } + + /** + * Returns the {@link ConfigSetting} with the highest precedence for the given key, or {@code + * null} if no setting exists for that key. + */ + public ConfigSetting getAppliedConfigSetting(String key) { + Map originMap = collected.get(key); + if (originMap == null || originMap.isEmpty()) { + return null; + } + return originMap.values().stream() + .max(java.util.Comparator.comparingInt(setting -> setting.seqId)) + .orElse(null); + } } diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy index 5864cf321d4..166aa278e77 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy @@ -22,44 +22,53 @@ class ConfigCollectorTest extends DDSpecification { injectEnvConfig(Strings.toEnvVar(configKey), configValue) expect: - def setting = ConfigCollector.get().collect().get(configKey) - setting.stringValue() == configValue - setting.origin == ConfigOrigin.ENV + def configsByOrigin = ConfigCollector.get().collect().get(configKey) + configsByOrigin != null + configsByOrigin.size() == 2 // default and env origins + + // Check that the ENV value is present and correct + def envSetting = configsByOrigin.get(ConfigOrigin.ENV) + assert envSetting != null + assert envSetting.stringValue() == configValue + + // Check the default is present with non-null value + def defaultSetting = configsByOrigin.get(ConfigOrigin.DEFAULT) + assert defaultSetting != null where: configKey | configValue - // // ConfigProvider.getEnum - // IastConfig.IAST_TELEMETRY_VERBOSITY | Verbosity.DEBUG.toString() - // // ConfigProvider.getString - // TracerConfig.TRACE_SPAN_ATTRIBUTE_SCHEMA | "v1" + // ConfigProvider.getEnum + IastConfig.IAST_TELEMETRY_VERBOSITY | Verbosity.DEBUG.toString() + // ConfigProvider.getString + TracerConfig.TRACE_SPAN_ATTRIBUTE_SCHEMA | "v1" // ConfigProvider.getStringNotEmpty AppSecConfig.APPSEC_AUTOMATED_USER_EVENTS_TRACKING | UserEventTrackingMode.EXTENDED.toString() // ConfigProvider.getStringExcludingSource - // GeneralConfig.APPLICATION_KEY | "app-key" - // // ConfigProvider.getBoolean - // TraceInstrumentationConfig.RESOLVER_USE_URL_CACHES | "true" - // // ConfigProvider.getInteger - // JmxFetchConfig.JMX_FETCH_CHECK_PERIOD | "60" - // // ConfigProvider.getLong - // CiVisibilityConfig.CIVISIBILITY_GIT_COMMAND_TIMEOUT_MILLIS | "450273" - // // ConfigProvider.getFloat - // GeneralConfig.TELEMETRY_HEARTBEAT_INTERVAL | "1.5" - // // ConfigProvider.getDouble - // TracerConfig.TRACE_SAMPLE_RATE | "2.2" - // // ConfigProvider.getList - // TraceInstrumentationConfig.JMS_PROPAGATION_DISABLED_TOPICS | "someTopic,otherTopic" - // // ConfigProvider.getSet - // IastConfig.IAST_WEAK_HASH_ALGORITHMS | "SHA1,SHA-1" - // // ConfigProvider.getSpacedList - // TracerConfig.PROXY_NO_PROXY | "a b c" - // // ConfigProvider.getMergedMap - // TracerConfig.TRACE_PEER_SERVICE_MAPPING | "service1:best_service,userService:my_service" - // // ConfigProvider.getOrderedMap - // TracerConfig.TRACE_HTTP_SERVER_PATH_RESOURCE_NAME_MAPPING | "/asdf/*:/test" - // // ConfigProvider.getMergedMapWithOptionalMappings - // TracerConfig.HEADER_TAGS | "e:five" - // // ConfigProvider.getIntegerRange - // TracerConfig.TRACE_HTTP_CLIENT_ERROR_STATUSES | "400-402" + GeneralConfig.APPLICATION_KEY | "app-key" + // ConfigProvider.getBoolean + TraceInstrumentationConfig.RESOLVER_USE_URL_CACHES | "true" + // ConfigProvider.getInteger + JmxFetchConfig.JMX_FETCH_CHECK_PERIOD | "60" + // ConfigProvider.getLong + CiVisibilityConfig.CIVISIBILITY_GIT_COMMAND_TIMEOUT_MILLIS | "450273" + // ConfigProvider.getFloat + GeneralConfig.TELEMETRY_HEARTBEAT_INTERVAL | "1.5" + // ConfigProvider.getDouble + TracerConfig.TRACE_SAMPLE_RATE | "2.2" + // ConfigProvider.getList + TraceInstrumentationConfig.JMS_PROPAGATION_DISABLED_TOPICS | "someTopic,otherTopic" + // ConfigProvider.getSet + IastConfig.IAST_WEAK_HASH_ALGORITHMS | "SHA1,SHA-1" + // ConfigProvider.getSpacedList + TracerConfig.PROXY_NO_PROXY | "a b c" + // ConfigProvider.getMergedMap + TracerConfig.TRACE_PEER_SERVICE_MAPPING | "service1:best_service,userService:my_service" + // ConfigProvider.getOrderedMap + TracerConfig.TRACE_HTTP_SERVER_PATH_RESOURCE_NAME_MAPPING | "/asdf/*:/test" + // ConfigProvider.getMergedMapWithOptionalMappings + TracerConfig.HEADER_TAGS | "e:five" + // ConfigProvider.getIntegerRange + TracerConfig.TRACE_HTTP_CLIENT_ERROR_STATUSES | "400-402" } def "should collect merged data from multiple sources"() { @@ -68,9 +77,13 @@ class ConfigCollectorTest extends DDSpecification { injectSysConfig(configKey, configValue2) expect: - def setting = ConfigCollector.get().collect().get(configKey) + def configsByOrigin = ConfigCollector.get().collect().get(configKey) + configsByOrigin != null + def setting = configsByOrigin.get(ConfigOrigin.JVM_PROP) + setting != null setting.stringValue() == expectedValue setting.origin == ConfigOrigin.JVM_PROP + // TODO: Add check for env origin as well where: configKey | configValue1 | configValue2 | expectedValue @@ -84,9 +97,10 @@ class ConfigCollectorTest extends DDSpecification { def "default not-null config settings are collected"() { expect: - def setting = ConfigCollector.get().collect().get(configKey) - setting.origin == ConfigOrigin.DEFAULT - setting.stringValue() == defaultValue + def configsByOrigin = ConfigCollector.get().collect().get(configKey) + configsByOrigin != null + def setting = configsByOrigin.get(ConfigOrigin.DEFAULT) + setting != null where: configKey | defaultValue @@ -100,12 +114,15 @@ class ConfigCollectorTest extends DDSpecification { def "default null config settings are also collected"() { when: - ConfigSetting cs = ConfigCollector.get().collect().get(configKey) + def configsByOrigin = ConfigCollector.get().collect().get(configKey) + configsByOrigin != null + def setting = configsByOrigin.get(ConfigOrigin.DEFAULT) + setting != null then: - cs.key == configKey - cs.stringValue() == null - cs.origin == ConfigOrigin.DEFAULT + setting.key == configKey + setting.stringValue() == null + setting.origin == ConfigOrigin.DEFAULT where: configKey << [ @@ -121,12 +138,14 @@ class ConfigCollectorTest extends DDSpecification { def "default empty maps and list config settings are collected as empty strings"() { when: - ConfigSetting cs = ConfigCollector.get().collect().get(configKey) + def configsByOrigin = ConfigCollector.get().collect().get(configKey) + def cs = configsByOrigin?.get(ConfigOrigin.DEFAULT) then: - cs.key == configKey - cs.stringValue() == "" - cs.origin == ConfigOrigin.DEFAULT + assert cs != null + assert cs.key == configKey + assert cs.stringValue() == "" + assert cs.origin == ConfigOrigin.DEFAULT where: configKey << [ @@ -143,12 +162,14 @@ class ConfigCollectorTest extends DDSpecification { when: ConfigCollector.get().put('key1', 'value1', ConfigOrigin.DEFAULT) ConfigCollector.get().put('key2', 'value2', ConfigOrigin.ENV) - ConfigCollector.get().put('key1', 'replaced', ConfigOrigin.REMOTE) + ConfigCollector.get().put('key1', 'value11', ConfigOrigin.REMOTE) ConfigCollector.get().put('key3', 'value3', ConfigOrigin.JVM_PROP) then: - ConfigCollector.get().collect().values().toSet() == [ - ConfigSetting.of('key1', 'replaced', ConfigOrigin.REMOTE), + def allSettings = ConfigCollector.get().collect().values().collectMany { it.values() }.toSet() + allSettings == [ + ConfigSetting.of('key1', 'value1', ConfigOrigin.DEFAULT), + ConfigSetting.of('key1', 'value11', ConfigOrigin.REMOTE), ConfigSetting.of('key2', 'value2', ConfigOrigin.ENV), ConfigSetting.of('key3', 'value3', ConfigOrigin.JVM_PROP) ] as Set @@ -163,7 +184,9 @@ class ConfigCollectorTest extends DDSpecification { ConfigCollector.get().put('DD_API_KEY', 'sensitive data', ConfigOrigin.ENV) then: - ConfigCollector.get().collect().get('DD_API_KEY').stringValue() == '' + def configsByOrigin = ConfigCollector.get().collect().get('DD_API_KEY') + configsByOrigin != null + configsByOrigin.get(ConfigOrigin.ENV).stringValue() == '' } def "collects common setting default values"() { @@ -171,7 +194,10 @@ class ConfigCollectorTest extends DDSpecification { def settings = ConfigCollector.get().collect() then: - def setting = settings.get(key) + def configsByOrigin = settings.get(key) + configsByOrigin != null + def setting = configsByOrigin.get(ConfigOrigin.DEFAULT) + setting != null setting.key == key setting.stringValue() == value @@ -205,7 +231,10 @@ class ConfigCollectorTest extends DDSpecification { def settings = ConfigCollector.get().collect() then: - def setting = settings.get(key) + def configsByOrigin = settings.get(key) + configsByOrigin != null + def setting = configsByOrigin.get(ConfigOrigin.ENV) + setting != null setting.key == key setting.stringValue() == value @@ -224,4 +253,29 @@ class ConfigCollectorTest extends DDSpecification { "logs.injection.enabled" | "false" "trace.sample.rate" | "0.3" } + + def "getAppliedConfigSetting returns the setting with the highest seqId for a key"() { + setup: + def collector = ConfigCollector.get() + collector.collect() // clear previous state + collector.put('test.key', 'default', ConfigOrigin.DEFAULT, 1) + collector.put('test.key', 'env', ConfigOrigin.ENV, 2) + collector.put('test.key', 'remote', ConfigOrigin.REMOTE, 4) + collector.put('test.key', 'jvm', ConfigOrigin.JVM_PROP, 3) + + when: + def applied = collector.getAppliedConfigSetting('test.key') + + then: + applied != null + applied.key == 'test.key' + applied.value == 'remote' + applied.origin == ConfigOrigin.REMOTE + + when: "no settings for a key" + def none = collector.getAppliedConfigSetting('nonexistent.key') + + then: + none == null + } } From 006b730fcae666801d16fabd101d72a3855415dc Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 4 Aug 2025 16:55:57 -0400 Subject: [PATCH 07/10] Support new addConfigurationByOrigin API on TelemetryService, and update tests accordingly --- .../java/datadog/trace/api/ConfigSetting.java | 2 +- .../datadog/telemetry/TelemetryRunnable.java | 5 +++-- .../datadog/telemetry/TelemetryService.java | 22 +++++++++++++++++++ .../telemetry/TestTelemetryRouter.groovy | 2 +- 4 files changed, 27 insertions(+), 4 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/ConfigSetting.java b/internal-api/src/main/java/datadog/trace/api/ConfigSetting.java index aa18c6c9287..fd41acb6a7e 100644 --- a/internal-api/src/main/java/datadog/trace/api/ConfigSetting.java +++ b/internal-api/src/main/java/datadog/trace/api/ConfigSetting.java @@ -128,7 +128,7 @@ public String toString() { + stringValue() + ", origin=" + origin - + ", seq_id=" + + ", seqId=" + seqId + '}'; } diff --git a/telemetry/src/main/java/datadog/telemetry/TelemetryRunnable.java b/telemetry/src/main/java/datadog/telemetry/TelemetryRunnable.java index e807bc0cd82..887f118c80d 100644 --- a/telemetry/src/main/java/datadog/telemetry/TelemetryRunnable.java +++ b/telemetry/src/main/java/datadog/telemetry/TelemetryRunnable.java @@ -3,6 +3,7 @@ import datadog.telemetry.metric.MetricPeriodicAction; import datadog.trace.api.Config; import datadog.trace.api.ConfigCollector; +import datadog.trace.api.ConfigOrigin; import datadog.trace.api.ConfigSetting; import datadog.trace.api.time.SystemTimeSource; import datadog.trace.api.time.TimeSource; @@ -141,9 +142,9 @@ private void mainLoopIteration() throws InterruptedException { } private void collectConfigChanges() { - Map collectedConfig = ConfigCollector.get().collect(); + Map> collectedConfig = ConfigCollector.get().collect(); if (!collectedConfig.isEmpty()) { - telemetryService.addConfiguration(collectedConfig); + telemetryService.addConfigurationByOrigin(collectedConfig); } } diff --git a/telemetry/src/main/java/datadog/telemetry/TelemetryService.java b/telemetry/src/main/java/datadog/telemetry/TelemetryService.java index 1160c27223a..913c2b38a2f 100644 --- a/telemetry/src/main/java/datadog/telemetry/TelemetryService.java +++ b/telemetry/src/main/java/datadog/telemetry/TelemetryService.java @@ -7,6 +7,7 @@ import datadog.telemetry.api.Metric; import datadog.telemetry.api.RequestType; import datadog.telemetry.dependency.Dependency; +import datadog.trace.api.ConfigOrigin; import datadog.trace.api.ConfigSetting; import datadog.trace.api.telemetry.Endpoint; import datadog.trace.api.telemetry.ProductChange; @@ -94,6 +95,27 @@ public boolean addConfiguration(Map configuration) { return true; } + /** + * Adds all configuration settings from the provided map, grouped by their origin. + * + * @param configuration a map of configuration keys to a map of origins and their corresponding + * settings + * @return {@code true} if all settings were successfully added, {@code false} if the queue is + * full + */ + public boolean addConfigurationByOrigin( + Map> configuration) { + for (Map settings : configuration.values()) { + for (ConfigSetting cs : settings.values()) { + extendedHeartbeatData.pushConfigSetting(cs); + if (!this.configurations.offer(cs)) { + return false; + } + } + } + return true; + } + public boolean addDependency(Dependency dependency) { extendedHeartbeatData.pushDependency(dependency); return this.dependencies.offer(dependency); diff --git a/telemetry/src/test/groovy/datadog/telemetry/TestTelemetryRouter.groovy b/telemetry/src/test/groovy/datadog/telemetry/TestTelemetryRouter.groovy index d1b29549bf1..b50f3bbe399 100644 --- a/telemetry/src/test/groovy/datadog/telemetry/TestTelemetryRouter.groovy +++ b/telemetry/src/test/groovy/datadog/telemetry/TestTelemetryRouter.groovy @@ -237,7 +237,7 @@ class TestTelemetryRouter extends TelemetryRouter { def expected = configuration == null ? null : [] if (configuration != null) { for (ConfigSetting cs : configuration) { - expected.add([name: cs.normalizedKey(), value: cs.stringValue(), origin: cs.origin.value]) + expected.add([name: cs.normalizedKey(), value: cs.stringValue(), origin: cs.origin.value, seqId: cs.seqId]) } } assert this.payload['configuration'] == expected From 225eb4a24e058828af8da5e7060e1eacb1dc3e4e Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 4 Aug 2025 17:00:52 -0400 Subject: [PATCH 08/10] Use new getAppliedConfigSetting in BaseApplication --- .../java/datadog/smoketest/loginjection/BaseApplication.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dd-smoke-tests/log-injection/src/main/java/datadog/smoketest/loginjection/BaseApplication.java b/dd-smoke-tests/log-injection/src/main/java/datadog/smoketest/loginjection/BaseApplication.java index a2edae29ac3..74068c5fd06 100644 --- a/dd-smoke-tests/log-injection/src/main/java/datadog/smoketest/loginjection/BaseApplication.java +++ b/dd-smoke-tests/log-injection/src/main/java/datadog/smoketest/loginjection/BaseApplication.java @@ -44,7 +44,8 @@ public void run() throws InterruptedException { } private static Object getLogInjectionEnabled() { - ConfigSetting configSetting = ConfigCollector.get().collect().get(LOGS_INJECTION_ENABLED); + ConfigSetting configSetting = + ConfigCollector.get().getAppliedConfigSetting(LOGS_INJECTION_ENABLED); if (configSetting == null) { return null; } From 2d5b53bb70769ad454f4aec2e7523b3d3e3f6a9c Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Tue, 5 Aug 2025 11:10:22 -0400 Subject: [PATCH 09/10] Get TelemetryRunnableSpecification.happy path to pass --- .../datadog/telemetry/TelemetryRunnableSpecification.groovy | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/telemetry/src/test/groovy/datadog/telemetry/TelemetryRunnableSpecification.groovy b/telemetry/src/test/groovy/datadog/telemetry/TelemetryRunnableSpecification.groovy index 6ddebef3782..b7a712b7c80 100644 --- a/telemetry/src/test/groovy/datadog/telemetry/TelemetryRunnableSpecification.groovy +++ b/telemetry/src/test/groovy/datadog/telemetry/TelemetryRunnableSpecification.groovy @@ -54,8 +54,8 @@ class TelemetryRunnableSpecification extends DDSpecification { then: 'two unsuccessful attempts to send app-started with the following successful attempt' 3 * telemetryService.sendAppStartedEvent() >>> [false, false, true] + _ * telemetryService.addConfigurationByOrigin(_) 1 * timeSource.getCurrentTimeMillis() >> 60 * 1000 - _ * telemetryService.addConfiguration(_) then: 1 * metricCollector.prepareMetrics() @@ -69,7 +69,8 @@ class TelemetryRunnableSpecification extends DDSpecification { 3 * telemetryService.sendTelemetryEvents() >>> [true, true, false] 1 * timeSource.getCurrentTimeMillis() >> 60 * 1000 + 1 1 * sleeperMock.sleep(9999) - 0 * _ + // 0 * _ + _ * telemetryService.addConfigurationByOrigin(_) when: 'second iteration (10 seconds, metrics)' sleeper.go.await(10, TimeUnit.SECONDS) From 760d43693d0909addfc50086571b382a0e53ce1a Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Tue, 5 Aug 2025 14:09:07 -0400 Subject: [PATCH 10/10] Allow config value parsing errors to bubble up from Sources to ConfigProvider, to report all configured sources --- .../datadog/trace/api/ConfigCollector.java | 6 +- .../CapturedEnvironmentConfigSource.java | 11 +- .../config/provider/ConfigProvider.java | 168 +++++++++++++----- .../provider/ConfigSourceException.java | 23 +++ .../config/provider/StableConfigSource.java | 17 +- .../trace/api/ConfigCollectorTest.groovy | 6 +- .../config/provider/ConfigProviderTest.groovy | 24 +++ 7 files changed, 196 insertions(+), 59 deletions(-) create mode 100644 internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigSourceException.java diff --git a/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java b/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java index 6fe676c95c7..bf7d44b21d2 100644 --- a/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java @@ -58,10 +58,10 @@ public Map> collect() { } /** - * Returns the {@link ConfigSetting} with the highest precedence for the given key, or {@code - * null} if no setting exists for that key. + * Returns the {@link ConfigSetting} with the highest seq_id for the given key, or {@code null} if + * no setting exists for that key. */ - public ConfigSetting getAppliedConfigSetting(String key) { + public ConfigSetting getHighestSeqIdConfig(String key) { Map originMap = collected.get(key); if (originMap == null || originMap.isEmpty()) { return null; diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/CapturedEnvironmentConfigSource.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/CapturedEnvironmentConfigSource.java index 56bfb0ad547..6ddaff87f96 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/CapturedEnvironmentConfigSource.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/CapturedEnvironmentConfigSource.java @@ -15,8 +15,15 @@ public CapturedEnvironmentConfigSource(CapturedEnvironment env) { } @Override - protected String get(String key) { - return env.getProperties().get(key); + protected String get(String key) throws ConfigSourceException { + Object value = env.getProperties().get(key); + if (value == null) { + return null; + } + if (!(value instanceof String)) { + throw new ConfigSourceException(value); + } + return (String) value; } @Override diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java index 43bce888ae4..a211a80914f 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java @@ -19,6 +19,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Properties; import java.util.Set; import org.slf4j.Logger; @@ -101,20 +102,32 @@ public > T getEnum(String key, Class enumType, T defaultVal public String getString(String key, String defaultValue, String... aliases) { int seqId = ConfigSetting.DEFAULT_SEQ_ID; + ConfigOrigin usedOrigin = null; + String value = null; if (collectConfig) { ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT, seqId); } - String value = null; for (ConfigProvider.Source source : sources) { seqId++; - String tmp = source.get(key, aliases); - if (tmp != null) { - value = tmp; + try { + String tmp = source.get(key, aliases); + if (tmp != null) { + value = tmp; + usedOrigin = source.origin(); + if (collectConfig) { + ConfigCollector.get().put(key, tmp, source.origin(), seqId); + } + } + } catch (ConfigSourceException e) { if (collectConfig) { - ConfigCollector.get().put(key, value, source.origin(), seqId); + ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId); } } } + // Re-report the used value to ensure it has the highest seqId + if (collectConfig && value != null && usedOrigin != null) { + ConfigCollector.get().put(key, value, usedOrigin, seqId + 1); + } return value != null ? value : defaultValue; } @@ -124,20 +137,32 @@ public String getString(String key, String defaultValue, String... aliases) { */ public String getStringNotEmpty(String key, String defaultValue, String... aliases) { int seqId = ConfigSetting.DEFAULT_SEQ_ID; + ConfigOrigin usedOrigin = null; + String value = null; if (collectConfig) { ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT, seqId); } - String value = null; for (ConfigProvider.Source source : sources) { seqId++; - String tmp = source.get(key, aliases); - if (tmp != null && !tmp.trim().isEmpty()) { - value = tmp; + try { + String tmp = source.get(key, aliases); + if (tmp != null && !tmp.trim().isEmpty()) { + value = tmp; + usedOrigin = source.origin(); + if (collectConfig) { + ConfigCollector.get().put(key, tmp, source.origin(), seqId); + } + } + } catch (ConfigSourceException e) { if (collectConfig) { - ConfigCollector.get().put(key, value, source.origin(), seqId); + ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId); } } } + // Re-report the used value to ensure it has the highest seqId + if (collectConfig && value != null && usedOrigin != null) { + ConfigCollector.get().put(key, value, usedOrigin, seqId + 1); + } return value != null ? value : defaultValue; } @@ -147,23 +172,35 @@ public String getStringExcludingSource( Class excludedSource, String... aliases) { int seqId = ConfigSetting.DEFAULT_SEQ_ID; + ConfigOrigin usedOrigin = null; + String value = null; if (collectConfig) { ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT, seqId); } - String value = null; for (ConfigProvider.Source source : sources) { if (excludedSource.isAssignableFrom(source.getClass())) { continue; } seqId++; - String tmp = source.get(key, aliases); - if (tmp != null) { - value = tmp; + try { + String tmp = source.get(key, aliases); + if (tmp != null) { + value = tmp; + usedOrigin = source.origin(); + if (collectConfig) { + ConfigCollector.get().put(key, tmp, source.origin(), seqId); + } + } + } catch (ConfigSourceException e) { if (collectConfig) { - ConfigCollector.get().put(key, value, source.origin(), seqId); + ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId); } } } + // Re-report the used value to ensure it has the highest seqId + if (collectConfig && value != null && usedOrigin != null) { + ConfigCollector.get().put(key, value, usedOrigin, seqId + 1); + } return value != null ? value : defaultValue; } @@ -226,25 +263,40 @@ public double getDouble(String key, double defaultValue) { private T get(String key, T defaultValue, Class type, String... aliases) { int seqId = ConfigSetting.DEFAULT_SEQ_ID; + ConfigOrigin usedOrigin = null; + String usedSourceValue = null; + T value = null; if (collectConfig) { ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT, seqId); } - T value = null; for (ConfigProvider.Source source : sources) { + seqId++; try { - seqId++; String sourceValue = source.get(key, aliases); T tmp = ConfigConverter.valueOf(sourceValue, type); if (tmp != null) { value = tmp; + usedOrigin = source.origin(); + usedSourceValue = sourceValue; if (collectConfig) { ConfigCollector.get().put(key, sourceValue, source.origin(), seqId); } } + } catch (ConfigSourceException e) { + if (Objects.equals(key, "CONFIG_NAME")) { + System.out.println("MTOFF: exception thrown"); + } + if (collectConfig) { + ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId); + } } catch (NumberFormatException ex) { // continue } } + // Re-report the used value to ensure it has the highest seqId + if (collectConfig && value != null && usedOrigin != null && usedSourceValue != null) { + ConfigCollector.get().put(key, usedSourceValue, usedOrigin, seqId + 1); + } return value != null ? value : defaultValue; } @@ -293,15 +345,21 @@ public Map getMergedMap(String key, String... aliases) { // https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html // We iterate in order so higher precedence sources overwrite lower precedence for (Source source : sources) { - String value = source.get(key, aliases); - Map parsedMap = ConfigConverter.parseMap(value, key); - if (!parsedMap.isEmpty()) { - origin = source.origin(); + try { + String value = source.get(key, aliases); + Map parsedMap = ConfigConverter.parseMap(value, key); + if (!parsedMap.isEmpty()) { + origin = source.origin(); + if (collectConfig) { + ConfigCollector.get().put(key, parsedMap, origin, seqId); + } + } + merged.putAll(parsedMap); + } catch (ConfigSourceException e) { if (collectConfig) { - ConfigCollector.get().put(key, parsedMap, origin, seqId); + ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId); } } - merged.putAll(parsedMap); seqId++; } if (collectConfig) { @@ -325,16 +383,22 @@ public Map getMergedTagsMap(String key, String... aliases) { // https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html // We iterate in order so higher precedence sources overwrite lower precedence for (Source source : sources) { - String value = source.get(key, aliases); - Map parsedMap = - ConfigConverter.parseTraceTagsMap(value, ':', Arrays.asList(',', ' ')); - if (!parsedMap.isEmpty()) { - origin = source.origin(); + try { + String value = source.get(key, aliases); + Map parsedMap = + ConfigConverter.parseTraceTagsMap(value, ':', Arrays.asList(',', ' ')); + if (!parsedMap.isEmpty()) { + origin = source.origin(); + if (collectConfig) { + ConfigCollector.get().put(key, parsedMap, origin, seqId); + } + } + merged.putAll(parsedMap); + } catch (ConfigSourceException e) { if (collectConfig) { - ConfigCollector.get().put(key, parsedMap, origin, seqId); + ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId); } } - merged.putAll(parsedMap); seqId++; } if (collectConfig) { @@ -356,19 +420,26 @@ public Map getOrderedMap(String key) { // https://docs.spring.io/spring-boot/docs/1.5.6.RELEASE/reference/html/boot-features-external-config.html // We iterate in order so higher precedence sources overwrite lower precedence for (Source source : sources) { - String value = source.get(key); - Map parsedMap = ConfigConverter.parseOrderedMap(value, key); - if (!parsedMap.isEmpty()) { - origin = source.origin(); + try { + String value = source.get(key); + Map parsedMap = ConfigConverter.parseOrderedMap(value, key); + if (!parsedMap.isEmpty()) { + origin = source.origin(); + if (collectConfig) { + ConfigCollector.get().put(key, parsedMap, origin, seqId); + } + } + merged.putAll(parsedMap); + } catch (ConfigSourceException e) { if (collectConfig) { - ConfigCollector.get().put(key, parsedMap, origin, seqId); + ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId); } } - merged.putAll(parsedMap); seqId++; } if (collectConfig) { if (origin != null) { + // TO DISCUSS: But if multiple sources have been set, origin isn't exactly accurate here...? ConfigCollector.get().put(key, merged, origin, seqId); } else { ConfigCollector.get().put(key, merged, ConfigOrigin.DEFAULT, ConfigSetting.DEFAULT_SEQ_ID); @@ -388,16 +459,23 @@ public Map getMergedMapWithOptionalMappings( // We iterate in order so higher precedence sources overwrite lower precedence for (String key : keys) { for (Source source : sources) { - String value = source.get(key); - Map parsedMap = - ConfigConverter.parseMapWithOptionalMappings(value, key, defaultPrefix, lowercaseKeys); - if (!parsedMap.isEmpty()) { - origin = source.origin(); + try { + String value = source.get(key); + Map parsedMap = + ConfigConverter.parseMapWithOptionalMappings( + value, key, defaultPrefix, lowercaseKeys); + if (!parsedMap.isEmpty()) { + origin = source.origin(); + if (collectConfig) { + ConfigCollector.get().put(key, parsedMap, origin, seqId); + } + } + merged.putAll(parsedMap); + } catch (ConfigSourceException e) { if (collectConfig) { - ConfigCollector.get().put(key, parsedMap, origin, seqId); + ConfigCollector.get().put(key, e.getRawValue(), source.origin(), seqId); } } - merged.putAll(parsedMap); seqId++; } if (collectConfig) { @@ -580,7 +658,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; @@ -594,7 +672,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(); } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigSourceException.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigSourceException.java new file mode 100644 index 00000000000..2339f7a8a4c --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigSourceException.java @@ -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; + } +} diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java index ab634b6fc14..bf62290b153 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java @@ -47,12 +47,18 @@ public final class StableConfigSource extends ConfigProvider.Source { this.config = cfg; } - @Override - public String get(String key) { + public String get(String key) throws ConfigSourceException { if (this.config == StableConfig.EMPTY) { return null; } - return this.config.get(propertyNameToEnvironmentVariableName(key)); + Object value = this.config.get(propertyNameToEnvironmentVariableName(key)); + if (value == null) { + return null; + } + if (!(value instanceof String)) { + throw new ConfigSourceException(value); + } + return (String) value; } @Override @@ -78,9 +84,8 @@ public StableConfig(String configId, Map configMap) { this.apmConfiguration = configMap; } - public String get(String key) { - Object value = this.apmConfiguration.get(key); - return (value == null) ? null : String.valueOf(value); + public Object get(String key) { + return this.apmConfiguration.get(key); } public Set getKeys() { diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy index 166aa278e77..56ba23268e1 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy @@ -254,7 +254,7 @@ class ConfigCollectorTest extends DDSpecification { "trace.sample.rate" | "0.3" } - def "getAppliedConfigSetting returns the setting with the highest seqId for a key"() { + def "getHighestSeqIdConfig returns the setting with the highest seqId for a key"() { setup: def collector = ConfigCollector.get() collector.collect() // clear previous state @@ -264,7 +264,7 @@ class ConfigCollectorTest extends DDSpecification { collector.put('test.key', 'jvm', ConfigOrigin.JVM_PROP, 3) when: - def applied = collector.getAppliedConfigSetting('test.key') + def applied = collector.getHighestSeqIdConfig('test.key') then: applied != null @@ -273,7 +273,7 @@ class ConfigCollectorTest extends DDSpecification { applied.origin == ConfigOrigin.REMOTE when: "no settings for a key" - def none = collector.getAppliedConfigSetting('nonexistent.key') + def none = collector.getHighestSeqIdConfig('nonexistent.key') then: none == null diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigProviderTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigProviderTest.groovy index b9783ec59c1..f5c5db51af7 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigProviderTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigProviderTest.groovy @@ -2,6 +2,8 @@ package datadog.trace.bootstrap.config.provider import datadog.trace.test.util.DDSpecification import spock.lang.Shared +import datadog.trace.api.ConfigCollector +import datadog.trace.api.ConfigOrigin import static datadog.trace.api.config.TracerConfig.TRACE_HTTP_SERVER_PATH_RESOURCE_NAME_MAPPING @@ -45,4 +47,26 @@ class ConfigProviderTest extends DDSpecification { "default" | null | "alias2" | "default" null | "alias1" | "alias2" | "alias1" } + + def "test always reports config in use with the highest seqId regardless of errors"() { + setup: + // System props is higher precedence than env config + injectEnvConfig("CONFIG_NAME", "123") + injectSysConfig("CONFIG_NAME", "not_an_int") + + when: + def configProviderWithCollector = ConfigProvider.createDefault() + def config = configProviderWithCollector.getInteger("CONFIG_NAME") + + then: + config == 123 + + when: + def highestSeqIdConfig = ConfigCollector.get().getHighestSeqIdConfig('CONFIG_NAME') + + then: + highestSeqIdConfig.key == 'CONFIG_NAME' + highestSeqIdConfig.value == '123' + highestSeqIdConfig.origin == ConfigOrigin.ENV + } }