From b1dab58296355786c258bcd1b1c489a6f21f33aa Mon Sep 17 00:00:00 2001 From: Federico Mon Date: Wed, 17 Apr 2024 19:37:48 +0200 Subject: [PATCH] fix(internal): handle missing lib_config entry (#9022) This PR fixes an issue regarding the remote config module where a payload missing the ``lib_config`` entry would crash the application due to an unhandled ``KeyError`` exception. ``` KeyError: 'lib_config' lib_config = config["lib_config"] File "/usr/local/lib/python3.10/site-packages/ddtrace/settings/config.py", line 747, in _handle_remoteconfig self._callback(data, test_tracer=test_tracer) File "/usr/local/lib/python3.10/site-packages/ddtrace/internal/remoteconfig/_subscribers.py", line 36, in _exec_callback self._exec_callback(data, test_tracer=test_tracer) File "/usr/local/lib/python3.10/site-packages/ddtrace/internal/remoteconfig/_subscribers.py", line 41, in _get_data_from_connector_and_exec self._get_data_from_connector_and_exec() File "/usr/local/lib/python3.10/site-packages/ddtrace/internal/remoteconfig/_subscribers.py", line 46, in periodic Traceback (most recent call last): ``` ## Checklist - [x] Change(s) are motivated and described in the PR description - [x] Testing strategy is described if automated tests are not included in the PR - [x] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)) - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --------- Co-authored-by: kyle --- ddtrace/settings/config.py | 2 +- ...x-missing-lib_config-f2c0a6a40f525bff.yaml | 4 +++ tests/internal/test_settings.py | 30 +++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/fix-missing-lib_config-f2c0a6a40f525bff.yaml diff --git a/ddtrace/settings/config.py b/ddtrace/settings/config.py index 9047594b7b5..c49abc83bae 100644 --- a/ddtrace/settings/config.py +++ b/ddtrace/settings/config.py @@ -748,7 +748,7 @@ def _handle_remoteconfig(self, data, test_tracer=None): config = data["config"][0] base_rc_config = {n: None for n in self._config} - if config: + if config and "lib_config" in config: lib_config = config["lib_config"] if "tracing_sampling_rate" in lib_config: base_rc_config["_trace_sample_rate"] = lib_config["tracing_sampling_rate"] diff --git a/releasenotes/notes/fix-missing-lib_config-f2c0a6a40f525bff.yaml b/releasenotes/notes/fix-missing-lib_config-f2c0a6a40f525bff.yaml new file mode 100644 index 00000000000..2930273ef27 --- /dev/null +++ b/releasenotes/notes/fix-missing-lib_config-f2c0a6a40f525bff.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + internal: This fix resolves an error regarding the remote config module with payloads missing a ``lib_config`` entry diff --git a/tests/internal/test_settings.py b/tests/internal/test_settings.py index 747b33f9d46..faea554f489 100644 --- a/tests/internal/test_settings.py +++ b/tests/internal/test_settings.py @@ -164,6 +164,36 @@ def test_settings_parametrized(testcase, config, monkeypatch): assert config._get_source(expected_name) == expected_source +def test_settings_missing_lib_config(config, monkeypatch): + testcase = { + "env": {"DD_TRACE_ENABLED": "true"}, + "code": {"_tracing_enabled": True}, + "rc": {}, + "expected": {"_tracing_enabled": True}, + "expected_source": {"_tracing_enabled": "code"}, + } + for env_name, env_value in testcase.get("env", {}).items(): + monkeypatch.setenv(env_name, env_value) + config._reset() + + for code_name, code_value in testcase.get("code", {}).items(): + setattr(config, code_name, code_value) + + base_rc_config = _base_rc_config({}) + + # Delete "lib_config" from the remote config + del base_rc_config["config"][0]["lib_config"] + assert "lib_config" not in base_rc_config["config"][0] + + config._handle_remoteconfig(base_rc_config, None) + + for expected_name, expected_value in testcase["expected"].items(): + assert getattr(config, expected_name) == expected_value + + for expected_name, expected_source in testcase.get("expected_source", {}).items(): + assert config._get_source(expected_name) == expected_source + + def test_config_subscription(config): for s in ("_trace_sample_rate", "logs_injection", "trace_http_header_tags"): _handler = mock.MagicMock()