Skip to content

Commit b7de091

Browse files
vitkyrkaclaude
andauthored
Fix false-positive discovery-candidate stability failures from log concatenation (DataDog#24536)
* Fix false-positive discovery-candidate stability failures from log concatenation assert_all_discovery_candidates_stable diffed docker logs by concatenating stdout and stderr before comparing against the previous snapshot. Any new stdout content (e.g. an access log line) shifts where the unchanged stderr tail sits in the concatenation, breaking the startswith-based diff and replaying the container's entire historical stderr output as "new" on every probe. This caused Kong's E2E discovery test to intermittently fail with "Container logs matched 'error'" whenever a benign startup-time nginx/ migrations message already sat in stderr. Diff stdout and stderr independently instead. Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add changelog entry for DataDog#24536 Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Trim internal helper docstring/comment to one line AGENTS.md requires internal helpers to use concise one-line docstrings/comments; the new _diff_logs docstring and the regression test comment were multi-line. Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent cb21492 commit b7de091

3 files changed

Lines changed: 47 additions & 6 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix false-positive discovery-candidate stability failures caused by incorrectly diffing concatenated stdout/stderr container logs.

datadog_checks_dev/datadog_checks/dev/docker.py

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ def assert_all_discovery_candidates_stable(
7272
compose_service = compose_service or _get_default_compose_service()
7373
container_id = _get_compose_container_id(compose_file, compose_service, project_name=project_name)
7474
initial_state = _inspect_container(container_id)
75-
previous_logs = _get_container_logs(container_id)
75+
previous_stdout, previous_stderr = _get_container_logs(container_id)
7676

7777
ports = tuple(SimpleNamespace(number=port, name='') for port in _get_container_ports(initial_state))
7878
service = SimpleNamespace(
@@ -100,12 +100,15 @@ def assert_all_discovery_candidates_stable(
100100
current_state = _inspect_container(current_container_id)
101101
_assert_container_stable(initial_state, current_state, index)
102102

103-
current_logs = _get_container_logs(current_container_id)
104-
new_logs = current_logs[len(previous_logs) :] if current_logs.startswith(previous_logs) else current_logs
103+
current_stdout, current_stderr = _get_container_logs(current_container_id)
104+
# Diffed separately: stdout/stderr interleaving varies across calls, breaking a combined diff.
105+
new_stdout = _diff_logs(previous_stdout, current_stdout)
106+
new_stderr = _diff_logs(previous_stderr, current_stderr)
107+
new_logs = new_stdout + new_stderr
105108
for line in new_logs.splitlines():
106109
logging.debug('New log line: %s', line)
107110
_assert_no_log_patterns(new_logs, log_patterns, index)
108-
previous_logs = current_logs
111+
previous_stdout, previous_stderr = current_stdout, current_stderr
109112

110113

111114
def _get_compose_container_id(
@@ -188,9 +191,14 @@ def _get_container_ports(inspect_data: Mapping[str, Any]) -> list[int]:
188191
return sorted(ports)
189192

190193

191-
def _get_container_logs(container_id: str) -> str:
194+
def _get_container_logs(container_id: str) -> tuple[str, str]:
192195
result = run_command(['docker', 'logs', container_id], capture=True)
193-
return result.stdout + result.stderr
196+
return result.stdout, result.stderr
197+
198+
199+
def _diff_logs(previous: str, current: str) -> str:
200+
"""Return the portion of `current` appended since `previous`."""
201+
return current[len(previous) :] if current.startswith(previous) else current
194202

195203

196204
def _assert_container_stable(

datadog_checks_dev/tests/test_docker.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,3 +394,35 @@ def fake_run_command(command, **kwargs):
394394
'/tmp/docker-compose.yml',
395395
'service',
396396
)
397+
398+
399+
def test_assert_all_discovery_candidates_stable_ignores_old_stderr_when_new_stdout_arrives():
400+
# Regression test: new stdout previously shifted unchanged stderr, replaying it as new.
401+
class DiscoveryCheck:
402+
@classmethod
403+
def generate_configs(cls, service):
404+
yield {'init_config': {}, 'instances': [{'url': f'http://{service.host}:8080/metrics'}]}
405+
406+
logs = iter(
407+
[
408+
_docker_result(stdout='', stderr='old startup error\n'),
409+
_docker_result(stdout='new access log line\n', stderr='old startup error\n'),
410+
]
411+
)
412+
413+
def fake_run_command(command, **kwargs):
414+
if command[:2] == ['docker', 'compose']:
415+
return _docker_result(stdout='container-id\n')
416+
if command[:2] == ['docker', 'inspect']:
417+
return _docker_result(stdout=json.dumps([_container_inspect()]))
418+
if command[:2] == ['docker', 'logs']:
419+
return next(logs)
420+
raise AssertionError(f'Unexpected command: {command}')
421+
422+
with mock.patch('datadog_checks.dev.docker.run_command', side_effect=fake_run_command):
423+
assert_all_discovery_candidates_stable(
424+
mock.Mock(),
425+
DiscoveryCheck,
426+
'/tmp/docker-compose.yml',
427+
'service',
428+
)

0 commit comments

Comments
 (0)