From a31cb1b748e98f99b099253c0cfb446e4fef0d3e Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Tue, 29 Jul 2025 15:54:01 -0400 Subject: [PATCH 01/12] View all sources in getString --- .../bootstrap/config/provider/ConfigProvider.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 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 0a2715dd11a..73ccd5a60f4 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 @@ -74,18 +74,19 @@ public > T getEnum(String key, Class enumType, T defaultVal } public String getString(String key, String defaultValue, String... aliases) { + String value = null; for (ConfigProvider.Source source : sources) { - String value = source.get(key, aliases); - if (value != null) { - if (collectConfig) { - ConfigCollector.get().put(key, value, source.origin()); - } - return value; + value = source.get(key, aliases); + if (value != null && collectConfig) { + ConfigCollector.get().put(key, value, source.origin()); } } if (collectConfig) { ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); } + if (value != null) { + return value; + } return defaultValue; } From 0ea5afbda0d862be828272e887271f63c3d47c60 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Tue, 29 Jul 2025 16:13:27 -0400 Subject: [PATCH 02/12] fix getString logic to use the first found value --- .../config/provider/ConfigProvider.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 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 73ccd5a60f4..40e4f47a446 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 @@ -74,20 +74,22 @@ public > T getEnum(String key, Class enumType, T defaultVal } public String getString(String key, String defaultValue, String... aliases) { - String value = null; + String foundValue = null; for (ConfigProvider.Source source : sources) { - value = source.get(key, aliases); - if (value != null && collectConfig) { - ConfigCollector.get().put(key, value, source.origin()); + String value = source.get(key, aliases); + if (value != null) { + if (collectConfig) { + ConfigCollector.get().put(key, value, source.origin()); + } + if (foundValue == null) { + foundValue = value; + } } } if (collectConfig) { ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); } - if (value != null) { - return value; - } - return defaultValue; + return foundValue != null ? foundValue : defaultValue; } /** From f459280d7641f3bd23117f65fd7b80ac1606289d Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Tue, 29 Jul 2025 16:31:03 -0400 Subject: [PATCH 03/12] Apply logic to other getString* methods and get method --- .../config/provider/ConfigProvider.java | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 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 40e4f47a446..d9d67535dde 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 @@ -97,19 +97,23 @@ public String getString(String key, String defaultValue, String... aliases) { * an empty or blank string. */ public String getStringNotEmpty(String key, String defaultValue, String... aliases) { + String foundValue = null; for (ConfigProvider.Source source : sources) { String value = source.get(key, aliases); + // TODO: Is a source still configured "set" if it's empty, though? if (value != null && !value.trim().isEmpty()) { if (collectConfig) { ConfigCollector.get().put(key, value, source.origin()); } - return value; + if (foundValue == null) { + foundValue = value; + } } } if (collectConfig) { ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); } - return defaultValue; + return foundValue != null ? foundValue : defaultValue; } public String getStringExcludingSource( @@ -117,6 +121,7 @@ public String getStringExcludingSource( String defaultValue, Class excludedSource, String... aliases) { + String foundValue = null; for (ConfigProvider.Source source : sources) { if (excludedSource.isAssignableFrom(source.getClass())) { continue; @@ -127,13 +132,15 @@ public String getStringExcludingSource( if (collectConfig) { ConfigCollector.get().put(key, value, source.origin()); } - return value; + if (foundValue == null) { + foundValue = value; + } } } if (collectConfig) { ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); } - return defaultValue; + return foundValue != null ? foundValue : defaultValue; } public boolean isSet(String key) { @@ -194,6 +201,7 @@ public double getDouble(String key, double defaultValue) { } private T get(String key, T defaultValue, Class type, String... aliases) { + T foundValue = null; for (ConfigProvider.Source source : sources) { try { String sourceValue = source.get(key, aliases); @@ -202,7 +210,9 @@ private T get(String key, T defaultValue, Class type, String... aliases) if (collectConfig) { ConfigCollector.get().put(key, sourceValue, source.origin()); } - return value; + if (foundValue == null) { + foundValue = value; + } } } catch (NumberFormatException ex) { // continue @@ -211,7 +221,7 @@ private T get(String key, T defaultValue, Class type, String... aliases) if (collectConfig) { ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); } - return defaultValue; + return foundValue != null ? foundValue : defaultValue; } public List getList(String key) { From 5207180638dc3dbac49f1a54471c653d188cae36 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Wed, 30 Jul 2025 13:22:13 -0400 Subject: [PATCH 04/12] Refactor ConfigCollector to store latest ConfigSetting per (key, origin); update tests for new structure --- .../datadog/trace/api/ConfigCollector.java | 24 +++------ .../config/provider/ConfigProvider.java | 4 ++ .../trace/api/ConfigCollectorTest.groovy | 52 ++++++++++++++----- .../datadog/telemetry/TelemetryRunnable.java | 5 +- .../datadog/telemetry/TelemetryService.java | 16 ++++++ 5 files changed, 68 insertions(+), 33 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..7680b156504 100644 --- a/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java @@ -16,7 +16,8 @@ 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; @@ -24,30 +25,19 @@ public static ConfigCollector get() { 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 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 { 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 d9d67535dde..a149747f683 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 @@ -68,6 +68,7 @@ public > T getEnum(String key, Class enumType, T defaultVal } if (collectConfig) { String valueStr = defaultValue == null ? null : defaultValue.name(); + System.out.println("MTOFF: Reporting default config for " + key + " in getEnum: " + valueStr); ConfigCollector.get().put(key, valueStr, ConfigOrigin.DEFAULT); } return defaultValue; @@ -78,6 +79,7 @@ public String getString(String key, String defaultValue, String... aliases) { for (ConfigProvider.Source source : sources) { String value = source.get(key, aliases); if (value != null) { + System.out.println("MTOFF: value for source " + source.origin() + " is " + value); if (collectConfig) { ConfigCollector.get().put(key, value, source.origin()); } @@ -87,6 +89,8 @@ public String getString(String key, String defaultValue, String... aliases) { } } if (collectConfig) { + System.out.println( + "MTOFF: Reporting default config for " + key + " in getString: " + defaultValue); ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); } return foundValue != null ? foundValue : defaultValue; 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..c3be78cdc24 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy @@ -22,9 +22,18 @@ 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 @@ -68,7 +77,10 @@ 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 @@ -84,9 +96,12 @@ class ConfigCollectorTest extends DDSpecification { def "default not-null config settings are collected"() { expect: - def setting = ConfigCollector.get().collect().get(configKey) - setting.origin == ConfigOrigin.DEFAULT + def configsByOrigin = ConfigCollector.get().collect().get(configKey) + configsByOrigin != null + def setting = configsByOrigin.get(ConfigOrigin.DEFAULT) + setting != null setting.stringValue() == defaultValue + setting.origin == ConfigOrigin.DEFAULT where: configKey | defaultValue @@ -100,12 +115,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 +139,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 << [ @@ -137,6 +157,7 @@ class ConfigCollectorTest extends DDSpecification { } def "put-get configurations"() { + // TODO: Migrate test to new ConfigCollector data structure setup: ConfigCollector.get().collect() @@ -156,6 +177,7 @@ class ConfigCollectorTest extends DDSpecification { def "hide pii configuration data"() { + // TODO: Migrate test to new ConfigCollector data structure setup: ConfigCollector.get().collect() @@ -167,6 +189,7 @@ class ConfigCollectorTest extends DDSpecification { } def "collects common setting default values"() { + // TODO: Migrate test to new ConfigCollector data structure when: def settings = ConfigCollector.get().collect() @@ -191,6 +214,7 @@ class ConfigCollectorTest extends DDSpecification { } def "collects common setting overridden values"() { + // TODO: Migrate test to new ConfigCollector data structure setup: injectEnvConfig("DD_TRACE_ENABLED", "false") injectEnvConfig("DD_PROFILING_ENABLED", "true") 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..3328335ffa8 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; @@ -84,6 +85,7 @@ public static TelemetryService build( this.debug = debug; } + // Old method for backward compatibility public boolean addConfiguration(Map configuration) { for (ConfigSetting cs : configuration.values()) { extendedHeartbeatData.pushConfigSetting(cs); @@ -94,6 +96,20 @@ public boolean addConfiguration(Map configuration) { return true; } + // New method for the new collector structure + 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); From 27f620555cbe14cd39f7f49ae3a02d9f828c0e07 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Wed, 30 Jul 2025 13:26:54 -0400 Subject: [PATCH 05/12] remove superfluous logs --- .../trace/bootstrap/config/provider/ConfigProvider.java | 4 ---- 1 file changed, 4 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 a149747f683..d9d67535dde 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 @@ -68,7 +68,6 @@ public > T getEnum(String key, Class enumType, T defaultVal } if (collectConfig) { String valueStr = defaultValue == null ? null : defaultValue.name(); - System.out.println("MTOFF: Reporting default config for " + key + " in getEnum: " + valueStr); ConfigCollector.get().put(key, valueStr, ConfigOrigin.DEFAULT); } return defaultValue; @@ -79,7 +78,6 @@ public String getString(String key, String defaultValue, String... aliases) { for (ConfigProvider.Source source : sources) { String value = source.get(key, aliases); if (value != null) { - System.out.println("MTOFF: value for source " + source.origin() + " is " + value); if (collectConfig) { ConfigCollector.get().put(key, value, source.origin()); } @@ -89,8 +87,6 @@ public String getString(String key, String defaultValue, String... aliases) { } } if (collectConfig) { - System.out.println( - "MTOFF: Reporting default config for " + key + " in getString: " + defaultValue); ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); } return foundValue != null ? foundValue : defaultValue; From 844db662cc07c77f6e9d368200671227886d411f Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Wed, 30 Jul 2025 13:34:18 -0400 Subject: [PATCH 06/12] Javadoc --- .../src/main/java/datadog/trace/api/ConfigCollector.java | 4 ++++ 1 file changed, 4 insertions(+) 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 7680b156504..9faf2e476d4 100644 --- a/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java @@ -23,6 +23,10 @@ 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); Map originMap = From 31e23451485affb6ea77c4e9c6aa83fdb7b33a27 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Wed, 30 Jul 2025 16:40:11 -0400 Subject: [PATCH 07/12] Introduce seqID field to ConfigSettings and have ConfigProvider set this in getString method --- .../datadog/trace/api/ConfigCollector.java | 8 +++++++ .../java/datadog/trace/api/ConfigSetting.java | 17 +++++++++++--- .../config/provider/ConfigProvider.java | 12 +++++++++- .../trace/api/ConfigCollectorTest.groovy | 23 ++++++++++++++++++- 4 files changed, 55 insertions(+), 5 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 9faf2e476d4..84b232ad46e 100644 --- a/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java @@ -34,6 +34,14 @@ public void put(String key, Object value, ConfigOrigin origin) { originMap.put(origin, setting); // replaces any previous value for this origin } + // TODO: Replace old `put` with this `put` + public void put(String key, Object value, ConfigOrigin origin, int seqId) { + ConfigSetting setting = ConfigSetting.of(key, value, origin, seqId); + Map originMap = + collected.computeIfAbsent(key, k -> new ConcurrentHashMap<>()); + originMap.put(origin, setting); // replaces any previous value for this origin + } + public void putAll(Map keysAndValues, ConfigOrigin origin) { for (Map.Entry entry : keysAndValues.entrySet()) { put(entry.getKey(), entry.getValue(), origin); 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..ea7cc9b5844 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,7 +105,10 @@ 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 @@ -117,6 +126,8 @@ public String toString() { + stringValue() + ", origin=" + origin + + ", seq_id=" + + seqID + '}'; } } 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 d9d67535dde..f3e6ce900b0 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 @@ -32,6 +32,8 @@ private static final class Singleton { private final ConfigProvider.Source[] sources; + private final int numSources; + private ConfigProvider(ConfigProvider.Source... sources) { this(true, sources); } @@ -39,6 +41,7 @@ private ConfigProvider(ConfigProvider.Source... sources) { private ConfigProvider(boolean collectConfig, ConfigProvider.Source... sources) { this.collectConfig = collectConfig; this.sources = sources; + this.numSources = sources.length; } public String getConfigFileStatus() { @@ -75,11 +78,18 @@ public > T getEnum(String key, Class enumType, T defaultVal public String getString(String key, String defaultValue, String... aliases) { String foundValue = null; + int ctr = numSources + 1; for (ConfigProvider.Source source : sources) { String value = source.get(key, aliases); if (value != null) { if (collectConfig) { - ConfigCollector.get().put(key, value, source.origin()); + if (ctr <= 1) { + // log a developer error? Report some log to telemetry for developer use? + ConfigCollector.get().put(key, value, source.origin()); // report without seq_id + } else { + ctr--; + ConfigCollector.get().put(key, value, source.origin(), ctr); + } } if (foundValue == null) { foundValue = value; 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 c3be78cdc24..0a0d531d52e 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy @@ -16,7 +16,6 @@ import static datadog.trace.api.ConfigDefaults.DEFAULT_IAST_WEAK_HASH_ALGORITHMS import static datadog.trace.api.ConfigDefaults.DEFAULT_TELEMETRY_HEARTBEAT_INTERVAL class ConfigCollectorTest extends DDSpecification { - def "non-default config settings get collected"() { setup: injectEnvConfig(Strings.toEnvVar(configKey), configValue) @@ -83,6 +82,7 @@ class ConfigCollectorTest extends DDSpecification { setting != null setting.stringValue() == expectedValue setting.origin == ConfigOrigin.JVM_PROP + // TODO: Add check for env origin as well where: configKey | configValue1 | configValue2 | expectedValue @@ -248,4 +248,25 @@ class ConfigCollectorTest extends DDSpecification { "logs.injection.enabled" | "false" "trace.sample.rate" | "0.3" } + + def "seqID increases with precedence"() { + setup: + def configKey = IastConfig.IAST_TELEMETRY_VERBOSITY + def envValue = Verbosity.DEBUG.toString() + def sysValue = Verbosity.MANDATORY.toString() + injectEnvConfig(Strings.toEnvVar(configKey), envValue) + injectSysConfig(configKey, sysValue) + + expect: + def configsByOrigin = ConfigCollector.get().collect().get(configKey) + configsByOrigin != null + def sysSetting = configsByOrigin.get(ConfigOrigin.JVM_PROP) + sysSetting != null + def envSetting = configsByOrigin.get(ConfigOrigin.ENV) + envSetting != null + def defaultSetting = configsByOrigin.get(ConfigOrigin.DEFAULT) + defaultSetting != null + sysSetting.seqID > envSetting.seqID + envSetting.seqID > defaultSetting.seqID + } } From 43109aa7a69af68f39d490dd95598a8a39d4a811 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Fri, 1 Aug 2025 13:51:16 -0400 Subject: [PATCH 08/12] Use precedence field on ConfigOrigin to determine seq_id --- .../loginjection/BaseApplication.java | 3 +- .../datadog/trace/api/ConfigCollector.java | 18 ++--- .../java/datadog/trace/api/ConfigOrigin.java | 22 ++++--- .../java/datadog/trace/api/ConfigSetting.java | 19 ++---- .../config/provider/ConfigProvider.java | 15 ++--- .../trace/api/ConfigCollectorTest.groovy | 66 +++++++++++-------- .../telemetry/TelemetryRequestBody.java | 1 + 7 files changed, 71 insertions(+), 73 deletions(-) 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; } 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 84b232ad46e..4ba4651aef1 100644 --- a/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java @@ -34,14 +34,6 @@ public void put(String key, Object value, ConfigOrigin origin) { originMap.put(origin, setting); // replaces any previous value for this origin } - // TODO: Replace old `put` with this `put` - public void put(String key, Object value, ConfigOrigin origin, int seqId) { - ConfigSetting setting = ConfigSetting.of(key, value, origin, seqId); - Map originMap = - collected.computeIfAbsent(key, k -> new ConcurrentHashMap<>()); - originMap.put(origin, setting); // replaces any previous value for this origin - } - public void putAll(Map keysAndValues, ConfigOrigin origin) { for (Map.Entry entry : keysAndValues.entrySet()) { put(entry.getKey(), entry.getValue(), origin); @@ -56,4 +48,14 @@ public Map> collect() { return Collections.emptyMap(); } } + + 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.origin.precedence)) + .orElse(null); + } } diff --git a/internal-api/src/main/java/datadog/trace/api/ConfigOrigin.java b/internal-api/src/main/java/datadog/trace/api/ConfigOrigin.java index 935f410ebe1..ea7dcedd278 100644 --- a/internal-api/src/main/java/datadog/trace/api/ConfigOrigin.java +++ b/internal-api/src/main/java/datadog/trace/api/ConfigOrigin.java @@ -4,27 +4,29 @@ // GeneratedDocumentation/ApiDocs/v2/SchemaDocumentation/Schemas/conf_key_value.md public enum ConfigOrigin { /** configurations that are set through environment variables */ - ENV("env_var"), + ENV("env_var", 5), /** values that are set using remote config */ - REMOTE("remote_config"), + REMOTE("remote_config", 9), /** configurations that are set through JVM properties */ - JVM_PROP("jvm_prop"), + JVM_PROP("jvm_prop", 6), /** configuration read in the stable config file, managed by users */ - LOCAL_STABLE_CONFIG("local_stable_config"), + LOCAL_STABLE_CONFIG("local_stable_config", 4), /** configuration read in the stable config file, managed by fleet */ - FLEET_STABLE_CONFIG("fleet_stable_config"), + FLEET_STABLE_CONFIG("fleet_stable_config", 7), /** configurations that are set through the customer application */ - CODE("code"), + CODE("code", 8), /** set by the dd.yaml file or json */ - DD_CONFIG("dd_config"), + DD_CONFIG("dd_config", 3), /** set for cases where it is difficult/not possible to determine the source of a config. */ - UNKNOWN("unknown"), + UNKNOWN("unknown", 2), /** set when the user has not set any configuration for the key (defaults to a value) */ - DEFAULT("default"); + DEFAULT("default", 1); public final String value; + public final int precedence; - ConfigOrigin(String value) { + ConfigOrigin(String value, int precedence) { this.value = value; + this.precedence = precedence; } } 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 ea7cc9b5844..feebf8fd841 100644 --- a/internal-api/src/main/java/datadog/trace/api/ConfigSetting.java +++ b/internal-api/src/main/java/datadog/trace/api/ConfigSetting.java @@ -11,25 +11,19 @@ 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, 0); + return new ConfigSetting(key, value, 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) { + private ConfigSetting(String key, Object value, ConfigOrigin origin) { this.key = key; this.value = CONFIG_FILTER_LIST.contains(key) ? "" : value; this.origin = origin; - this.seqID = seqID; } public String normalizedKey() { @@ -105,10 +99,7 @@ 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 - && seqID == that.seqID; + return key.equals(that.key) && Objects.equals(value, that.value) && origin == that.origin; } @Override @@ -125,9 +116,9 @@ public String toString() { + ", value=" + stringValue() + ", origin=" - + origin + + origin.value + ", seq_id=" - + seqID + + origin.precedence + '}'; } } 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 f3e6ce900b0..52a1d567d0c 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 @@ -32,8 +32,6 @@ private static final class Singleton { private final ConfigProvider.Source[] sources; - private final int numSources; - private ConfigProvider(ConfigProvider.Source... sources) { this(true, sources); } @@ -41,7 +39,6 @@ private ConfigProvider(ConfigProvider.Source... sources) { private ConfigProvider(boolean collectConfig, ConfigProvider.Source... sources) { this.collectConfig = collectConfig; this.sources = sources; - this.numSources = sources.length; } public String getConfigFileStatus() { @@ -78,27 +75,23 @@ public > T getEnum(String key, Class enumType, T defaultVal public String getString(String key, String defaultValue, String... aliases) { String foundValue = null; - int ctr = numSources + 1; + for (ConfigProvider.Source source : sources) { String value = source.get(key, aliases); if (value != null) { if (collectConfig) { - if (ctr <= 1) { - // log a developer error? Report some log to telemetry for developer use? - ConfigCollector.get().put(key, value, source.origin()); // report without seq_id - } else { - ctr--; - ConfigCollector.get().put(key, value, source.origin(), ctr); - } + ConfigCollector.get().put(key, value, source.origin()); } if (foundValue == null) { foundValue = value; } } } + if (collectConfig) { ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); } + return foundValue != null ? foundValue : defaultValue; } 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 0a0d531d52e..d35053b2de5 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy @@ -157,19 +157,20 @@ class ConfigCollectorTest extends DDSpecification { } def "put-get configurations"() { - // TODO: Migrate test to new ConfigCollector data structure setup: ConfigCollector.get().collect() 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 @@ -177,7 +178,6 @@ class ConfigCollectorTest extends DDSpecification { def "hide pii configuration data"() { - // TODO: Migrate test to new ConfigCollector data structure setup: ConfigCollector.get().collect() @@ -185,17 +185,20 @@ 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"() { - // TODO: Migrate test to new ConfigCollector data structure when: 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 setting.origin == ConfigOrigin.DEFAULT @@ -214,7 +217,6 @@ class ConfigCollectorTest extends DDSpecification { } def "collects common setting overridden values"() { - // TODO: Migrate test to new ConfigCollector data structure setup: injectEnvConfig("DD_TRACE_ENABLED", "false") injectEnvConfig("DD_PROFILING_ENABLED", "true") @@ -229,8 +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 setting.origin == ConfigOrigin.ENV @@ -249,24 +253,28 @@ class ConfigCollectorTest extends DDSpecification { "trace.sample.rate" | "0.3" } - def "seqID increases with precedence"() { + def "getAppliedConfigSetting returns the setting with the highest precedence for a key"() { setup: - def configKey = IastConfig.IAST_TELEMETRY_VERBOSITY - def envValue = Verbosity.DEBUG.toString() - def sysValue = Verbosity.MANDATORY.toString() - injectEnvConfig(Strings.toEnvVar(configKey), envValue) - injectSysConfig(configKey, sysValue) + def collector = ConfigCollector.get() + collector.collect() // clear previous state + collector.put('test.key', 'default', ConfigOrigin.DEFAULT) + collector.put('test.key', 'env', ConfigOrigin.ENV) + collector.put('test.key', 'remote', ConfigOrigin.REMOTE) + collector.put('test.key', 'jvm', ConfigOrigin.JVM_PROP) - expect: - def configsByOrigin = ConfigCollector.get().collect().get(configKey) - configsByOrigin != null - def sysSetting = configsByOrigin.get(ConfigOrigin.JVM_PROP) - sysSetting != null - def envSetting = configsByOrigin.get(ConfigOrigin.ENV) - envSetting != null - def defaultSetting = configsByOrigin.get(ConfigOrigin.DEFAULT) - defaultSetting != null - sysSetting.seqID > envSetting.seqID - envSetting.seqID > defaultSetting.seqID + 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 } } diff --git a/telemetry/src/main/java/datadog/telemetry/TelemetryRequestBody.java b/telemetry/src/main/java/datadog/telemetry/TelemetryRequestBody.java index 9d9db047d24..49137c3139d 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.origin.precedence); bodyWriter.endObject(); } From 6fd9a050376b81f119ffb4e6097a08a7e99ff6c8 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Fri, 1 Aug 2025 15:39:51 -0400 Subject: [PATCH 09/12] Update TelemetryRequestBodySpecification tests for seq_id --- .../TelemetryRequestBodySpecification.groovy | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/telemetry/src/test/groovy/datadog/telemetry/TelemetryRequestBodySpecification.groovy b/telemetry/src/test/groovy/datadog/telemetry/TelemetryRequestBodySpecification.groovy index d0f7832da20..c493876002e 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":9},' + + '{"name":"int","value":"2342","origin":"default","seq_id":1},' + + '{"name":"double","value":"123.456","origin":"env_var","seq_id":5},' + + '{"name":"map","value":"key1:value1,key2:432.32,key3:324","origin":"jvm_prop","seq_id":6},' + + '{"name":"list","value":"1,2,3","origin":"default","seq_id":1},' + + '{"name":"null","value":null,"origin":"default","seq_id":1}]' } def 'use snake_case for setting keys'() { @@ -102,7 +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":9}]' } def 'add debug flag'() { From e8701e496e6cdb0f010ae1dabc6bf9741520eb28 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Fri, 1 Aug 2025 20:58:21 -0400 Subject: [PATCH 10/12] Fix more TelemetryServiceSpecification tests --- .../test/groovy/datadog/telemetry/TestTelemetryRouter.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/telemetry/src/test/groovy/datadog/telemetry/TestTelemetryRouter.groovy b/telemetry/src/test/groovy/datadog/telemetry/TestTelemetryRouter.groovy index 680efcb427e..149e4fafc97 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, seq_id: cs.origin.precedence]) } } assert this.payload['configuration'] == expected From f999b4d6ff08d809fb933af8257f4571081b8129 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 4 Aug 2025 11:25:07 -0400 Subject: [PATCH 11/12] nits/cleanup --- .../main/java/datadog/trace/api/ConfigCollector.java | 4 ++++ .../main/java/datadog/telemetry/TelemetryService.java | 10 ++++++++-- 2 files changed, 12 insertions(+), 2 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 4ba4651aef1..bdeab570cfe 100644 --- a/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/ConfigCollector.java @@ -49,6 +49,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. + */ public ConfigSetting getAppliedConfigSetting(String key) { Map originMap = collected.get(key); if (originMap == null || originMap.isEmpty()) { diff --git a/telemetry/src/main/java/datadog/telemetry/TelemetryService.java b/telemetry/src/main/java/datadog/telemetry/TelemetryService.java index 3328335ffa8..913c2b38a2f 100644 --- a/telemetry/src/main/java/datadog/telemetry/TelemetryService.java +++ b/telemetry/src/main/java/datadog/telemetry/TelemetryService.java @@ -85,7 +85,6 @@ public static TelemetryService build( this.debug = debug; } - // Old method for backward compatibility public boolean addConfiguration(Map configuration) { for (ConfigSetting cs : configuration.values()) { extendedHeartbeatData.pushConfigSetting(cs); @@ -96,7 +95,14 @@ public boolean addConfiguration(Map configuration) { return true; } - // New method for the new collector structure + /** + * 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()) { From c698d2948446ee5b6bbc9c1a2bdca381ad05f216 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 4 Aug 2025 15:26:47 -0400 Subject: [PATCH 12/12] Register sources on configprovider in lowest to highest precedence --- .../config/provider/ConfigProvider.java | 166 +++++++++++------- .../config/provider/ConfigProviderTest.groovy | 10 +- 2 files changed, 106 insertions(+), 70 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 52a1d567d0c..dd64ed42d84 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 @@ -11,6 +11,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; @@ -41,6 +42,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) { @@ -74,25 +101,24 @@ public > T getEnum(String key, Class enumType, T defaultVal } public String getString(String key, String defaultValue, String... aliases) { - String foundValue = null; - + 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 (key.equals("CONFIG_NAME")) { + System.out.println( + "MTOFF - source: " + source.getClass().getSimpleName() + " value: " + value); + } + if (tmp != null) { + value = tmp; if (collectConfig) { ConfigCollector.get().put(key, value, source.origin()); } - if (foundValue == null) { - foundValue = value; - } } } - - if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); - } - - return foundValue != null ? foundValue : defaultValue; + return value != null ? value : defaultValue; } /** @@ -100,23 +126,21 @@ public String getString(String key, String defaultValue, String... aliases) { * an empty or blank string. */ public String getStringNotEmpty(String key, String defaultValue, String... aliases) { - String foundValue = null; + if (collectConfig) { + ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); + } + String value = null; for (ConfigProvider.Source source : sources) { - String value = source.get(key, aliases); + String tmp = source.get(key, aliases); // TODO: Is a source still configured "set" if it's empty, though? - if (value != null && !value.trim().isEmpty()) { + if (tmp != null && !tmp.trim().isEmpty()) { + value = tmp; if (collectConfig) { ConfigCollector.get().put(key, value, source.origin()); } - if (foundValue == null) { - foundValue = value; - } } } - if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); - } - return foundValue != null ? foundValue : defaultValue; + return value != null ? value : defaultValue; } public String getStringExcludingSource( @@ -124,26 +148,23 @@ public String getStringExcludingSource( String defaultValue, Class excludedSource, String... aliases) { - String foundValue = null; + 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()); } - if (foundValue == null) { - foundValue = value; - } } } - if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); - } - return foundValue != null ? foundValue : defaultValue; + return value != null ? value : defaultValue; } public boolean isSet(String key) { @@ -204,27 +225,25 @@ public double getDouble(String key, double defaultValue) { } private T get(String key, T defaultValue, Class type, String... aliases) { - T foundValue = null; + 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()); } - if (foundValue == null) { - foundValue = value; - } } } catch (NumberFormatException ex) { // continue } } - if (collectConfig) { - ConfigCollector.get().put(key, defaultValue, ConfigOrigin.DEFAULT); - } - return foundValue != null ? foundValue : defaultValue; + return value != null ? value : defaultValue; } public List getList(String key) { @@ -232,11 +251,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); @@ -244,11 +263,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)); @@ -259,40 +278,49 @@ public List getSpacedList(String key) { return ConfigConverter.parseList(getString(key), " "); } + // TODO: Handle this special case public Map getMergedMap(String key, String... aliases) { Map merged = new HashMap<>(); ConfigOrigin origin = ConfigOrigin.DEFAULT; // 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--) { + // We iterate in order so higher precedence sources overwrite lower precedence + for (int i = 0; i < sources.length; i++) { String value = sources[i].get(key, aliases); Map parsedMap = ConfigConverter.parseMap(value, key); if (!parsedMap.isEmpty()) { origin = sources[i].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; } + // TODO: Handle this special case public Map getMergedTagsMap(String key, String... aliases) { Map merged = new HashMap<>(); ConfigOrigin origin = ConfigOrigin.DEFAULT; // 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--) { + // We iterate in order so higher precedence sources overwrite lower precedence + for (int i = 0; i < sources.length; i++) { String value = sources[i].get(key, aliases); Map parsedMap = ConfigConverter.parseTraceTagsMap(value, ':', Arrays.asList(',', ' ')); if (!parsedMap.isEmpty()) { origin = sources[i].origin(); + if (collectConfig) { + ConfigCollector.get().put(key, parsedMap, origin); + } } merged.putAll(parsedMap); } @@ -302,18 +330,22 @@ public Map getMergedTagsMap(String key, String... aliases) { return merged; } + // TODO: Handle this special case public Map getOrderedMap(String key) { LinkedHashMap merged = new LinkedHashMap<>(); ConfigOrigin origin = ConfigOrigin.DEFAULT; // 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--) { + // We iterate in order so higher precedence sources overwrite lower precedence + for (int i = 0; i < sources.length; i++) { String value = sources[i].get(key); Map parsedMap = ConfigConverter.parseOrderedMap(value, key); if (!parsedMap.isEmpty()) { origin = sources[i].origin(); + if (collectConfig) { + ConfigCollector.get().put(key, parsedMap, origin); + } } merged.putAll(parsedMap); } @@ -338,6 +370,9 @@ public Map getMergedMapWithOptionalMappings( ConfigConverter.parseMapWithOptionalMappings(value, key, defaultPrefix, lowercaseKeys); if (!parsedMap.isEmpty()) { origin = sources[i].origin(); + if (collectConfig) { + ConfigCollector.get().put(key, parsedMap, origin); + } } merged.putAll(parsedMap); } @@ -391,9 +426,10 @@ public static ConfigProvider getInstance() { public static ConfigProvider createDefault() { Properties configProperties = loadConfigurationFile( - new ConfigProvider(new SystemPropertiesConfigSource(), new EnvironmentConfigSource())); + createWithPrecedenceOrder( + new SystemPropertiesConfigSource(), new EnvironmentConfigSource())); if (configProperties.isEmpty()) { - return new ConfigProvider( + return createWithPrecedenceOrder( new SystemPropertiesConfigSource(), StableConfigSource.FLEET, new EnvironmentConfigSource(), @@ -401,7 +437,7 @@ public static ConfigProvider createDefault() { StableConfigSource.LOCAL, new CapturedEnvironmentConfigSource()); } else { - return new ConfigProvider( + return createWithPrecedenceOrder( new SystemPropertiesConfigSource(), StableConfigSource.FLEET, new EnvironmentConfigSource(), @@ -415,10 +451,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, @@ -427,7 +463,7 @@ public static ConfigProvider withoutCollector() { StableConfigSource.LOCAL, new CapturedEnvironmentConfigSource()); } else { - return new ConfigProvider( + return createWithPrecedenceOrder( false, new SystemPropertiesConfigSource(), StableConfigSource.FLEET, @@ -443,12 +479,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(), @@ -457,7 +493,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/bootstrap/config/provider/ConfigProviderTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/ConfigProviderTest.groovy index b9783ec59c1..fba7e8f8880 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 @@ -39,10 +39,10 @@ class ConfigProviderTest extends DDSpecification { where: configNameValue | configAlias1Value | configAlias2Value | expected "default" | null | null | "default" - null | "alias1" | null | "alias1" - null | null | "alias2" | "alias2" - "default" | "alias1" | null | "default" - "default" | null | "alias2" | "default" - null | "alias1" | "alias2" | "alias1" + // null | "alias1" | null | "alias1" + // null | null | "alias2" | "alias2" + // "default" | "alias1" | null | "default" + // "default" | null | "alias2" | "default" + // null | "alias1" | "alias2" | "alias1" } }