Skip to content

Commit 4f059c8

Browse files
fix(remoteconfig): fix remoteconfig when flare data in payload [backport 2.9] (#9204)
Backport 69f91ee from #9196 to 2.9. Remote config was broken entirely due to the addition of tracer flare data in the config sent from the agent. Example incoming RC config: ``` data = {'metadata': [{'id': 'configuration_order', 'product_name': 'AGENT_CONFIG', 'sha256_hash': 'ddfc2c7b5ee1710aa915edfccd8a0d452784d946cebae0554485b5c0539a9e2c', 'length': 198, 'tuf_version': 2, 'apply_state': 2, 'apply_error': None}, {'id': 'f6c80fdcc00b702c54ff6ae5ff2ac7f16d9afef109bdf53ee990376455301ab2', 'product_name': 'APM_TRACING', 'sha256_hash': '098cb5a0d27fce648cdd4c6e686038282b64ffee7f42b7238a78552c91948d11', 'length': 616, 'tuf_version': 3, 'apply_state': 2, 'apply_error': None}], 'config': [{'internal_order': ['flare-log-level.trace', 'flare-log-level.debug', 'flare-log-level.info', 'flare-log-level.warn', 'flare-log-level.error', 'flare-log-level.critical', 'flare-log-level.off'], 'order': []}, {'id': 'f6c80fdcc00b702c54ff6ae5ff2ac7f16d9afef109bdf53ee990376455301ab2', 'revision': 1715109076236, 'schema_version': 'v1.0.0', 'action': 'enable', 'lib_config': {'library_language': 'all', 'library_version': 'latest', 'service_name': 'zachs-python-app', 'env': 'zachariah', 'tracing_enabled': True, 'dynamic_sampling_enabled': False, 'tracing_tags': ['rc:works'], 'tracing_sampling_rules': [{'service': 'zachs-python-app', 'provenance': 'customer', 'resource': 'GET /', 'sample_rate': 0.01}, {'service': 'zachs-python-app', 'provenance': 'customer', 'resource': '', 'sample_rate': 1}]}, 'service_target': {'service': 'zachs-python-app', 'env': 'zachariah'}}], 'shared_data_counter': 1} ``` The python tracer’s implementation of pulling RC rules was brittle and relied upon data["config"][0] always having the lib_config dict. However, with tracer flares it seems the agent sometimes sends the payload with the flare info in that 0 position in the list, so instead we need to sometimes grab data["config"][1] . ## 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: Zachary Groves <[email protected]>
1 parent 67cf19e commit 4f059c8

File tree

3 files changed

+32
-4
lines changed

3 files changed

+32
-4
lines changed

ddtrace/settings/config.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -748,8 +748,15 @@ def _handle_remoteconfig(self, data, test_tracer=None):
748748
log.warning("unexpected number of RC payloads %r", data)
749749
return
750750

751+
# Check if 'lib_config' is a key in the dictionary since other items can be sent in the payload
752+
config = None
753+
for config_item in data["config"]:
754+
if isinstance(config_item, Dict):
755+
if "lib_config" in config_item:
756+
config = config_item
757+
break
758+
751759
# If no data is submitted then the RC config has been deleted. Revert the settings.
752-
config = data["config"][0]
753760
base_rc_config = {n: None for n in self._config}
754761

755762
if config and "lib_config" in config:
@@ -782,7 +789,6 @@ def _handle_remoteconfig(self, data, test_tracer=None):
782789
if tags:
783790
tags = self._format_tags(lib_config["tracing_header_tags"])
784791
base_rc_config["trace_http_header_tags"] = tags
785-
786792
self._set_config_items([(k, v, "remote_config") for k, v in base_rc_config.items()])
787793
# called unconditionally to handle the case where header tags have been unset
788794
self._handle_remoteconfig_header_tags(base_rc_config)
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
fixes:
3+
- |
4+
RemoteConfig: This fix resolves an issue where remote config did not work for the tracer when using an agent
5+
that would add a flare item to the remote config payload. With this fix, the tracer will now correctly pull out
6+
the lib_config we need from the payload in order to implement remote config changes properly.
7+
8+

tests/internal/test_settings.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,25 @@ def _base_rc_config(cfg):
1616
return {
1717
"metadata": [],
1818
"config": [
19+
# this flare data can often come in and we want to make sure we're pulling the
20+
# actual lib_config data out correctly regardless
21+
{
22+
"internal_order": [
23+
"flare-log-level.trace",
24+
"flare-log-level.debug",
25+
"flare-log-level.info",
26+
"flare-log-level.warn",
27+
"flare-log-level.error",
28+
"flare-log-level.critical",
29+
"flare-log-level.off",
30+
],
31+
"order": [],
32+
},
1933
{
2034
"action": "enable",
2135
"service_target": {"service": None, "env": None},
2236
"lib_config": cfg,
23-
}
37+
},
2438
],
2539
}
2640

@@ -182,7 +196,7 @@ def test_settings_missing_lib_config(config, monkeypatch):
182196
base_rc_config = _base_rc_config({})
183197

184198
# Delete "lib_config" from the remote config
185-
del base_rc_config["config"][0]["lib_config"]
199+
del base_rc_config["config"][1]["lib_config"]
186200
assert "lib_config" not in base_rc_config["config"][0]
187201

188202
config._handle_remoteconfig(base_rc_config, None)

0 commit comments

Comments
 (0)