kubeflow / kubeflow/mcp-server

fix(core): log truncation drops the end of the log, where the failure is

Open
#219 1 comment 0 reactions 1 assignee Claimed by @Neilblaze View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.