kubeflow / kubeflow/mcp-server
fix(core): log truncation drops the end of the log, where the failure is
- Dominant language
- Python
- Stars
- 44
- Forks
- 54
- Avg merge
- 2d 18h
- Merged PRs (30d)
- 29
Description
## Problem
`get_training_logs()` builds its response in two steps (`kubeflow_mcp/trainer/api/monitoring.py`):
```python
log_lines = list(deque(client.get_job_logs(...), maxlen=MAX_LOG_LINES))
...
logs = "\n".join(log_lines)
sanitized = truncate_log_output(logs)
```
The `deque` keeps the **last** `MAX_LOG_LINES` (1000) lines, which is what you want when a job has failed. `truncate_log_output()` then keeps the **first** 10000 characters of that window (`kubeflow_mcp/core/security.py`):
```python
output = output[:max_length] + f"\n... (truncated, {len(output)} total chars)"
```
1000 log lines are almost always more than 10000 characters, so what comes back is the oldest part of the tail window -- pip installs, shard downloads, and the traceback at the end is dropped.
The response can then contradict itself. `extract_failure_hint()` runs on the full text, so `failure_hint` reports the OOM while `logs` shows nothing failing.
## Reproduction
```python
from unittest.mock import patch, MagicMock
from kubeflow_mcp.trainer.api.monitoring import get_training_logs
lines = [f"[{i:05d}] downloading shard {i}/900 ..." for i in range(900)]
lines.append("torch.cuda.OutOfMemoryError: CUDA out of memory")
with (
patch("kubeflow_mcp.trainer.api.monitoring.check_namespace_allowed", return_value=None),
patch("kubeflow_mcp.trainer.api.monitoring.get_trainer_client_for_namespace") as gc,
):
client = MagicMock()
client.get_job_logs.return_value = iter(lines)
gc.return_value = client
data = get_training_logs("demo-job")["data"]
print(data["failure_hint"]["category"])
print("OutOfMemoryError" in data["logs"])
```
```
OOM
False
```
`test_log_truncation` in `monitoring_test.py` does not catch this. Its 1000-line fixture joins to 9499 characters, just under the 10000 limit, so the character branch of `truncate_log_output()` never runs in the suite.
## Proposed Fix
Keep the tail, and move the marker to the front so the reader still knows content was dropped:
```python
if len(output) > max_length:
output = f"... (truncated, {len(output)} total chars)\n" + output[-max_length:]
```
`get_training_logs()` is the only caller and already passes the most recent window, so no call site changes.
## Acceptance Criteria
- A failure signature at the end of a long log is present in the returned `logs`.
- `failure_hint` and `logs` no longer disagree about what the job printed.
- Output shorter than `max_length` is returned unchanged.
- Tests cover the direction of truncation, both on the helper and through `get_training_logs()`.
**P.S.**: I'll shortly open a PR addressing this. Thanks!
Contributor guide
Assessment
This issue has not been assessed yet.