Use precedence field on ConfigOrigin to determine seq_id

Author: mtoffl01

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;
     }

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<ConfigOrigin, ConfigSetting> originMap =
-        collected.computeIfAbsent(key, k -> new ConcurrentHashMap<>());
-    originMap.put(origin, setting); // replaces any previous value for this origin
-  }
-
   public void putAll(Map<String, Object> keysAndValues, ConfigOrigin origin) {
     for (Map.Entry<String, Object> entry : keysAndValues.entrySet()) {
       put(entry.getKey(), entry.getValue(), origin);
@@ -56,4 +48,14 @@ public Map<String, Map<ConfigOrigin, ConfigSetting>> collect() {
       return Collections.emptyMap();
     }
   }
+
+  public ConfigSetting getAppliedConfigSetting(String key) {
+    Map<ConfigOrigin, ConfigSetting> 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);
+  }
 }

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;
   }
 }

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<String> 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) ? "<hidden>" : 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
         + '}';
   }
 }

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

@@ -32,16 +32,13 @@ private static final class Singleton {
 
   private final ConfigProvider.Source[] sources;
 
-  private final int numSources;
-
   private ConfigProvider(ConfigProvider.Source... sources) {
     this(true, 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 extends Enum<T>> T getEnum(String key, Class<T> 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;
   }
 

internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy

@@ -157,45 +157,48 @@ 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
   }
 
 
   def "hide pii configuration data"() {
-    // TODO: Migrate test to new ConfigCollector data structure
     setup:
     ConfigCollector.get().collect()
 
     when:
     ConfigCollector.get().put('DD_API_KEY', 'sensitive data', ConfigOrigin.ENV)
 
     then:
-    ConfigCollector.get().collect().get('DD_API_KEY').stringValue() == '<hidden>'
+    def configsByOrigin = ConfigCollector.get().collect().get('DD_API_KEY')
+    configsByOrigin != null
+    configsByOrigin.get(ConfigOrigin.ENV).stringValue() == '<hidden>'
   }
 
   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
   }
 }

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();
   }