fix(library_config): Support order of precedence

Author: BaptisteFoy

ddtrace/internal/native/__init__.py

@@ -1,11 +1,17 @@
 import os
+import sys
 from typing import Dict
+from typing import List
+
+from ddtrace.internal.utils.formats import asbool
 
 from ._native import DDSketch  # noqa: F401
 from ._native import PyConfigurator
 
 
-def get_configuration_from_disk(debug_logs: bool = False, file_override="") -> Dict[str, str]:
+def get_configuration_from_disk(
+    debug_logs: bool = False, local_file_override="", managed_file_override=""
+) -> List[Dict[str, str]]:
     """
     Retrieves the tracer configuration from disk. Calls the PyConfigurator object
     to read the configuration from the disk using the libdatadog shared library
@@ -16,23 +22,38 @@ def get_configuration_from_disk(debug_logs: bool = False, file_override="") -> D
     configurator = PyConfigurator(debug_logs)
 
     # Set the file override if provided. Only used for testing purposes.
-    if file_override:
-        configurator.set_local_file_override(file_override)
+    if local_file_override:
+        configurator.set_local_file_override(local_file_override)
+    if managed_file_override:
+        configurator.set_managed_file_override(managed_file_override)
 
-    return configurator.get_configuration()
+    config = []
+    try:
+        config = configurator.get_configuration()
+    except Exception as e:
+        # No logger at this point, so we rely on good old print
+        if asbool(os.environ.get("DD_TRACE_DEBUG", "false")):
+            print("Error reading configuration from disk, skipping: %s" % e, file=sys.stderr)
+    return config
 
 
-def _apply_configuration_from_disk():
+def _apply_configuration_from_disk(debug_logs: bool = False, local_file_override="", managed_file_override=""):
     """
     Sets the configuration from disk as environment variables.
     This is not ideal and we should consider a better mechanism to
     apply this configuration to the tracer.
     Currently here is the order of precedence (higher takes precedence):
     1. Dynamic remote configuration
     2. Runtime configuration (ie fields set manually by customers / from the ddtrace code)
-    3. Configuration from disk
+    3. Managed configuration from disk ("fleet stable config")
     4. Environment variables
+    5. Local configuration from disk ("local stable config")
     5. Default values
     """
-    for key, value in get_configuration_from_disk().items():
-        os.environ[key] = str(value).lower()
+    for entry in get_configuration_from_disk(
+        debug_logs=debug_logs, local_file_override=local_file_override, managed_file_override=managed_file_override
+    ):
+        if entry["source"] == "fleet_stable_config" or (
+            entry["source"] == "local_stable_config" and entry["name"] not in list(os.environ.keys())
+        ):
+            os.environ[entry["name"]] = str(entry["value"])

ddtrace/internal/native/_native.pyi

@@ -1,4 +1,4 @@
-from typing import Dict
+from typing import Dict, List
 
 class DDSketch:
     def __init__(self): ...
@@ -22,13 +22,34 @@ class PyConfigurator:
         ...
     def set_local_file_override(self, file: str) -> None:
         """
-        Overrides the file path for the configuration. Should not be used outside of tests.
-        :param file: The path to the file to override.
+        Overrides the local file path for the configuration. Should not be used outside of tests.
+        :param file: The path to the local file to override.
         """
         ...
-    def get_configuration(self) -> Dict[str, str]:
+    def set_managed_file_override(self, file: str) -> None:
+        """
+        Overrides the managed file path for the configuration. Should not be used outside of tests.
+        :param file: The path to the managed file to override.
+        """
+        ...
+    def get_configuration(self) -> List[Dict[str, str]]:
         """
         Retrieve the on-disk configuration.
-        :return: A dictionary containing the current configuration of the form {key: value}.
+        :return: A list of dictionaries containing the configuration:
+            [{"source": ..., "key": ..., "value": ..., "config_id": ...}]
+        """
+        ...
+    @property
+    def local_stable_config_type(self) -> str:
+        """
+        Retrieve the local stable configuration type.
+        :return: A string representing the local stable configuration type.
+        """
+        ...
+    @property
+    def fleet_stable_config_type(self) -> str:
+        """
+        Retrieve the fleet stable configuration type.
+        :return: A string representing the fleet stable configuration type.
         """
         ...

src/native/library_config.rs

@@ -2,6 +2,7 @@ use datadog_library_config::{Configurator, ProcessInfo};
 use pyo3::exceptions::PyException;
 use pyo3::prelude::*;
 use pyo3::types::PyDict;
+use pyo3::types::PyList;
 
 #[pyclass(name = "PyConfigurator", module = "ddtrace.internal._native")]
 pub struct PyConfigurator {
@@ -26,6 +27,11 @@ impl PyConfigurator {
         Ok(())
     }
 
+    pub fn set_managed_file_override(&mut self, file: String) -> PyResult<()> {
+        self.fleet_file = file;
+        Ok(())
+    }
+
     pub fn get_configuration(&self, py: Python<'_>) -> PyResult<PyObject> {
         let res_config = self.configurator.get_config_from_file(
             self.local_file.as_ref(),
@@ -34,12 +40,16 @@ impl PyConfigurator {
         );
         match res_config {
             Ok(config) => {
-                let dict = PyDict::new_bound(py);
+                let list = PyList::empty_bound(py);
                 for c in config.iter() {
-                    let key = c.name.to_str().to_owned();
-                    let _ = dict.set_item(key, c.value.clone());
+                    let dict = PyDict::new_bound(py);
+                    dict.set_item("name", c.name.to_str().to_owned())?;
+                    dict.set_item("value", c.value.clone())?;
+                    dict.set_item("source", c.source.to_str().to_owned())?;
+                    dict.set_item("config_id", c.config_id.as_deref().unwrap_or("").to_owned())?;
+                    list.append(dict)?;
                 }
-                Ok(dict.into())
+                Ok(list.into())
             }
             Err(e) => {
                 let err_msg = format!("Failed to get configuration: {:?}", e);

tests/internal/test_native.py

@@ -1,6 +1,50 @@
+import os
+
+from ddtrace.internal.native import _apply_configuration_from_disk
 from ddtrace.internal.native import get_configuration_from_disk
 
 
+def test_get_configuration_from_disk__priority(tmp_path):
+    """
+    Verify the order:
+    local stable config < environment variables < managed stable config
+    """
+
+    local_config = tmp_path / "local_config.yaml"
+    local_config.write_text(
+        """
+apm_configuration_default:
+  DD_SERVICE: "a"
+""",
+        encoding="utf-8",
+    )
+
+    managed_config = tmp_path / "managed_config.yaml"
+    managed_config.write_text(
+        """
+apm_configuration_default:
+  DD_SERVICE: "c"
+""",
+        encoding="utf-8",
+    )
+
+    # First test, local config
+    _apply_configuration_from_disk(local_file_override=str(local_config))
+    assert os.environ["DD_SERVICE"] == "a"
+    del os.environ["DD_SERVICE"]
+
+    # Second test, local config + environment variable
+    os.environ["DD_SERVICE"] = "b"
+    _apply_configuration_from_disk(local_file_override=str(local_config))
+    assert os.environ["DD_SERVICE"] == "b"
+    del os.environ["DD_SERVICE"]
+
+    # Third test, local config + environment variable + managed config
+    _apply_configuration_from_disk(local_file_override=str(local_config), managed_file_override=str(managed_config))
+    assert os.environ["DD_SERVICE"] == "c"
+    del os.environ["DD_SERVICE"]
+
+
 def test_get_configuration_from_disk__host_selector(tmp_path):
     # First test -- config matches & should be returned
     config_1 = tmp_path / "config_1.yaml"
@@ -12,8 +56,10 @@ def test_get_configuration_from_disk__host_selector(tmp_path):
         encoding="utf-8",
     )
 
-    config = get_configuration_from_disk(file_override=str(config_1))
-    assert config == {"DD_RUNTIME_METRICS_ENABLED": "true"}
+    config = get_configuration_from_disk(local_file_override=str(config_1))
+    assert len(config) == 1
+    assert config[0]["name"] == "DD_RUNTIME_METRICS_ENABLED"
+    assert config[0]["value"] == "true"
 
 
 def test_get_configuration_from_disk__service_selector(tmp_path):
@@ -33,8 +79,10 @@ def test_get_configuration_from_disk__service_selector(tmp_path):
         encoding="utf-8",
     )
 
-    config = get_configuration_from_disk(file_override=str(config_1))
-    assert config == {"DD_SERVICE": "my-service"}
+    config = get_configuration_from_disk(local_file_override=str(config_1))
+    assert len(config) == 1
+    assert config[0]["name"] == "DD_SERVICE"
+    assert config[0]["value"] == "my-service"
 
     # Second test -- config does not match & should not be returned
     config_2 = tmp_path / "config_2.yaml"
@@ -52,8 +100,8 @@ def test_get_configuration_from_disk__service_selector(tmp_path):
         encoding="utf-8",
     )
 
-    config = get_configuration_from_disk(file_override=str(config_2))
-    assert config == {}
+    config = get_configuration_from_disk(local_file_override=str(config_2))
+    assert len(config) == 0
 
 
 def is_imported_before(module_a, module_b):