ConfigProvider iterates over all sources and reports all non-null values to telemetry

dd-smoke-tests/log-injection/src/main/java/datadog/smoketest/loginjection/BaseApplication.java

@@ -44,7 +44,9 @@ public void run() throws InterruptedException {
   }
 
   private static Object getLogInjectionEnabled() {
-    ConfigSetting configSetting = ConfigCollector.get().collect().get(LOGS_INJECTION_ENABLED);
+    ConfigSetting configSetting =
+        ConfigCollector.getAppliedConfigSetting(
+            LOGS_INJECTION_ENABLED, ConfigCollector.get().collect());
     if (configSetting == null) {
       return null;
     }

internal-api/src/main/java/datadog/trace/api/Config.java

@@ -170,6 +170,7 @@
 import static datadog.trace.api.ConfigDefaults.DEFAULT_WEBSOCKET_MESSAGES_SEPARATE_TRACES;
 import static datadog.trace.api.ConfigDefaults.DEFAULT_WEBSOCKET_TAG_SESSION_ID;
 import static datadog.trace.api.ConfigDefaults.DEFAULT_WRITER_BAGGAGE_INJECT;
+import static datadog.trace.api.ConfigSetting.DEFAULT_SEQ_ID;
 import static datadog.trace.api.DDTags.APM_ENABLED;
 import static datadog.trace.api.DDTags.HOST_TAG;
 import static datadog.trace.api.DDTags.INTERNAL_HOST_NAME;
@@ -5219,7 +5220,8 @@ private static boolean isWindowsOS() {
   private static String getEnv(String name) {
     String value = EnvironmentVariables.get(name);
     if (value != null) {
-      ConfigCollector.get().put(name, value, ConfigOrigin.ENV);
+      // Reporting default sequence id to be consistent with ConfigProvider
+      ConfigCollector.get().put(name, value, ConfigOrigin.ENV, DEFAULT_SEQ_ID);
     }
     return value;
   }
@@ -5242,7 +5244,8 @@ private static String getProp(String name) {
   private static String getProp(String name, String def) {
     String value = SystemProperties.getOrDefault(name, def);
     if (value != null) {
-      ConfigCollector.get().put(name, value, ConfigOrigin.JVM_PROP);
+      // Reporting default sequence id to be consistent with ConfigProvider
+      ConfigCollector.get().put(name, value, ConfigOrigin.JVM_PROP, DEFAULT_SEQ_ID);
     }
     return value;
   }

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

@@ -1,5 +1,8 @@
 package datadog.trace.api;
 
+import static datadog.trace.api.ConfigOrigin.DEFAULT;
+import static datadog.trace.api.ConfigSetting.DEFAULT_SEQ_ID;
+
 import java.util.Collections;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -16,47 +19,79 @@ public class ConfigCollector {
   private static final AtomicReferenceFieldUpdater<ConfigCollector, Map> COLLECTED_UPDATER =
       AtomicReferenceFieldUpdater.newUpdater(ConfigCollector.class, Map.class, "collected");
 
-  private volatile Map<String, ConfigSetting> collected = new ConcurrentHashMap<>();
+  private volatile Map<ConfigOrigin, Map<String, ConfigSetting>> collected =
+      new ConcurrentHashMap<>();
 
   public static ConfigCollector get() {
     return INSTANCE;
   }
 
   public void put(String key, Object value, ConfigOrigin origin) {
     ConfigSetting setting = ConfigSetting.of(key, value, origin);
-    collected.put(key, setting);
+    Map<String, ConfigSetting> configMap =
+        collected.computeIfAbsent(origin, k -> new ConcurrentHashMap<>());
+    configMap.put(key, setting); // replaces any previous value for this key at origin
+  }
+
+  public void put(String key, Object value, ConfigOrigin origin, int seqId) {
+    ConfigSetting setting = ConfigSetting.of(key, value, origin, seqId);
+    Map<String, ConfigSetting> configMap =
+        collected.computeIfAbsent(origin, k -> new ConcurrentHashMap<>());
+    configMap.put(key, setting); // replaces any previous value for this key at origin
   }
 
   public void put(String key, Object value, ConfigOrigin origin, String configId) {
     ConfigSetting setting = ConfigSetting.of(key, value, origin, configId);
-    collected.put(key, setting);
+    Map<String, ConfigSetting> configMap =
+        collected.computeIfAbsent(origin, k -> new ConcurrentHashMap<>());
+    configMap.put(key, setting); // replaces any previous value for this key at origin
+  }
+
+  public void put(String key, Object value, ConfigOrigin origin, int seqId, String configId) {
+    ConfigSetting setting = ConfigSetting.of(key, value, origin, seqId, configId);
+    Map<String, ConfigSetting> configMap =
+        collected.computeIfAbsent(origin, k -> new ConcurrentHashMap<>());
+    configMap.put(key, setting); // replaces any previous value for this key at origin
+  }
+
+  // put method specifically for DEFAULT origins. We don't allow overrides for configs from DEFAULT
+  // origins
+  public void putDefault(String key, Object value) {
+    ConfigSetting setting = ConfigSetting.of(key, value, DEFAULT, DEFAULT_SEQ_ID);
+    Map<String, ConfigSetting> configMap =
+        collected.computeIfAbsent(DEFAULT, k -> new ConcurrentHashMap<>());
+    if (!configMap.containsKey(key) || configMap.get(key).value == null) {
+      configMap.put(key, setting);
+    }
   }
 
   public void putAll(Map<String, Object> keysAndValues, ConfigOrigin origin) {
-    // attempt merge+replace to avoid collector seeing partial update
-    Map<String, ConfigSetting> merged =
-        new ConcurrentHashMap<>(keysAndValues.size() + collected.size());
     for (Map.Entry<String, Object> entry : keysAndValues.entrySet()) {
-      ConfigSetting setting = ConfigSetting.of(entry.getKey(), entry.getValue(), origin);
-      merged.put(entry.getKey(), setting);
-    }
-    while (true) {
-      Map<String, ConfigSetting> 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<String, ConfigSetting> collect() {
+  public Map<ConfigOrigin, Map<String, ConfigSetting>> collect() {
     if (!collected.isEmpty()) {
       return COLLECTED_UPDATER.getAndSet(this, new ConcurrentHashMap<>());
     } else {
       return Collections.emptyMap();
     }
   }
+
+  // NOTE: Only used to preserve legacy behavior for with smoke tests
+  public static ConfigSetting getAppliedConfigSetting(
+      String key, Map<ConfigOrigin, Map<String, ConfigSetting>> configMap) {
+    ConfigSetting best = null;
+    for (Map<String, ConfigSetting> originConfigMap : configMap.values()) {
+      ConfigSetting setting = originConfigMap.get(key);
+      if (setting != null) {
+        if (best == null || setting.seqId > best.seqId) {
+          best = setting;
+        }
+      }
+    }
+    return best;
+  }
 }

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

@@ -11,6 +11,11 @@ public final class ConfigSetting {
   public final String key;
   public final Object value;
   public final ConfigOrigin origin;
+
+  public final int seqId;
+  public static final int DEFAULT_SEQ_ID = 1;
+  public static final int ABSENT_SEQ_ID = 0;
+
   /** The config ID associated with this setting, or {@code null} if not applicable. */
   public final String configId;
 
@@ -19,17 +24,27 @@ public final class ConfigSetting {
           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, null);
+    return new ConfigSetting(key, value, origin, ABSENT_SEQ_ID, null);
+  }
+
+  public static ConfigSetting of(String key, Object value, ConfigOrigin origin, int seqId) {
+    return new ConfigSetting(key, value, origin, seqId, null);
   }
 
   public static ConfigSetting of(String key, Object value, ConfigOrigin origin, String configId) {
-    return new ConfigSetting(key, value, origin, configId);
+    return new ConfigSetting(key, value, origin, ABSENT_SEQ_ID, configId);
+  }
+
+  public static ConfigSetting of(
+      String key, Object value, ConfigOrigin origin, int seqId, String configId) {
+    return new ConfigSetting(key, value, origin, seqId, configId);
   }
 
-  private ConfigSetting(String key, Object value, ConfigOrigin origin, String configId) {
+  private ConfigSetting(String key, Object value, ConfigOrigin origin, int seqId, String configId) {
     this.key = key;
     this.value = CONFIG_FILTER_LIST.contains(key) ? "<hidden>" : value;
     this.origin = origin;
+    this.seqId = seqId;
     this.configId = configId;
   }
 
@@ -109,12 +124,13 @@ public boolean equals(Object o) {
     return key.equals(that.key)
         && Objects.equals(value, that.value)
         && origin == that.origin
+        && seqId == that.seqId
         && Objects.equals(configId, that.configId);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(key, value, origin, configId);
+    return Objects.hash(key, value, origin, seqId, configId);
   }
 
   @Override
@@ -127,6 +143,8 @@ public String toString() {
         + stringValue()
         + ", origin="
         + origin
+        + ", seqId="
+        + seqId
         + ", configId="
         + configId
         + '}';