Skip to content

fix(#50): durable vLLM logs that survive restart + model logs - #51

Merged
OriNachum merged 3 commits into
mainfrom
fix/durable-crash-logs-50
Jun 20, 2026
Merged

fix(#50): durable vLLM logs that survive restart + model logs#51
OriNachum merged 3 commits into
mainfrom
fix/durable-crash-logs-50

Conversation

@OriNachum

Copy link
Copy Markdown
Contributor

What & why

A vLLM EngineCore crash (#50)
took the engine down on a tool-calling request, and by the time it was looked
at, a restart had wiped docker logs — so the crash trace was gone and the root
cause could not be investigated for lack of data.
This PR closes that
observability gap so the next crash leaves a durable, readable trace.

Per the issue owner's steer, auto-restart/autoheal is intentionally out of
scope
here — the blocker was losing the data, not the recovery. Pinning the
EngineCore root cause (MTP speculative decoding + tools vs FP4) needs a controlled
repro with the durable trace in hand; this PR is that prerequisite.

How

model init now scaffolds mg-logwrap.sh, bind-mounted as each vLLM
service's entrypoint (the command: arg list is unchanged). It:

  • tees stdout+stderr to a per-boot file <service>-<boot>.log under a
    host-mounted log dir (${MODEL_GEAR_LOG_DIR:-<deploy>/logs}/logs/model-gear)
    and passes them through to the console (so docker logs still works);
  • execs the real command, so vLLM stays the signal target (graceful
    docker stop) and the exit code / restart: policy are unchanged` (verified:
    exit code propagates through the wrapper);
  • captures both Python tracebacks and native CUDA/C++/OOM aborts (it tees
    below Python logging);
  • falls back to a plain exec "$@" if logging can't be set up — it never blocks
    serving
    .

The crash boot is preserved as its own file across the restart that follows
it. Wired into the single-model and fleet (primary/embed/rerank) compose
templates; init/serve/fleet up pre-create the log dir user-owned so it
is never created root-owned by the bind mount.

New verb: model logs (read-only)

Reads the host files directly, so it works even after the crashed container is
gone (docker logs would not):

model logs                  # list per-boot files (newest first)
model logs vllm             # tail the latest boot for a service
model logs vllm --previous  # tail the boot that CRASHED (after a restart made a fresh one)

OTEL — evaluated, rejected for crash logs

vLLM's OpenTelemetry support is traces-only (--otlp-traces-endpoint); it has
no native OTLP log export, and a crash traceback is not a span (the engine
dies). OTEL log capture would need a Collector + filelog sidecar reading the same
stderr — new infra for no gain over a host file. So crash durability is done at
the file level; OTEL traces remain a future opt-in for request observability.
Details in docs/durable-logs.md.

Tests / validation

  • 320 tests pass; new tests/test_cli_logs.py covers collect_logs/tail_lines,
    --previous crash-boot recovery, JSON, empty/unknown-service paths; test_init
    extended for the scaffolded wrapper + per-service MG_LOG_NAME.
  • mg-logwrap.sh verified against a throwaway local bash run: captures
    stdout+stderr, propagates the exit code, per-boot file + latest symlink, and
    the unwritable-dir fallback still execs.
  • Both composes pass docker compose config with the new entrypoint/volumes.
  • black / isort / flake8 / bandit / markdownlint / rubric-gate all clean.
  • Did not touch the live mesh backend (it was actively serving).

Closes #50.

  • model-gear (Claude)

🤖 Generated with Claude Code

A vLLM EngineCore crash (issue #50) took the engine down, and by the time it was
looked at a restart had wiped `docker logs` — so the crash trace was gone and the
root cause could not be investigated for lack of data. This closes that
observability gap (recovery/auto-restart is intentionally out of scope here).

`model init` now scaffolds `mg-logwrap.sh`, bind-mounted as each vLLM service's
entrypoint. It tees stdout+stderr to a per-boot file `<service>-<boot>.log` under
a host-mounted log dir (`${MODEL_GEAR_LOG_DIR:-<deploy>/logs}` -> `/logs/model-gear`),
then `exec`s the real command so vLLM stays the signal target (graceful SIGTERM)
and the exit code / `restart:` policy are unchanged. Teeing at the process-I/O
level captures BOTH python tracebacks AND native CUDA/C++ aborts; it falls back to
a plain exec if logging can't be set up, so it never blocks serving. The crash
boot is preserved as its own file across the restart that follows it.

New read-only verb `model logs` reads the host files directly (works even after
the container is gone): list boots, tail the latest, or `--previous` to tail the
boot that crashed. Wired into the single-model and fleet (primary/embed/rerank)
templates; init/serve/fleet-up pre-create the log dir user-owned.

OTEL was evaluated and rejected for crash logs: vLLM's OTLP support is
traces-only and a crash traceback is not a span; file capture is the right tool.
See docs/durable-logs.md.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnDfFkZkXz8C68hr3AA9Qa
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

fix(#50): Durable vLLM logs that survive restart + model logs verb
🐞 Bug fix ✨ Enhancement 🕐 20-40 Minutes

Grey Divider

Description

• Introduces mg-logwrap.sh, a bash entrypoint wrapper scaffolded by model init that tees each
 vLLM service's stdout+stderr to a per-boot host file, preserving crash traces across container
 restarts/recreates.
• Adds a new read-only model logs CLI verb to list and tail durable per-boot log files directly
 from the host, including --previous to access the crashed boot after a restart.
• Wires the log wrapper into all vLLM compose services (single-model vllm, fleet
 primary/embed/rerank) with MG_LOG_NAME env vars and bind-mounts for the log dir and wrapper
 script.
• Pre-creates the host log dir (user-owned) in model init, model serve, and model fleet up to
 prevent Docker from creating it root-owned.
• Adds docs/durable-logs.md documenting the mechanism, paths, model logs usage, pruning, and why
 OTEL was rejected for crash logs.
Diagram

graph TD
    A(["model init / serve / fleet up"]) --> B["ensure_log_dir()"]
    B --> C[("Host log dir\n<deploy>/logs")]
    A --> D["write_scaffold()\n→ mg-logwrap.sh"]

    D --> E["mg-logwrap.sh\n(container entrypoint)"]
    C --> F["bind-mount\n→ /logs/model-gear"]
    E --> F
    F --> G["vLLM service\n(primary/embed/rerank/vllm)"]
    G -->|"tee stdout+stderr"| C
    G -->|"exec → signal target"| G

    C --> H(["model logs"])
    H --> I["collect_logs()\ntail_lines()"]  
    I --> J["CLI output\n(list / tail / --previous)"]

    subgraph Legend
        direction LR
        _cli(["CLI command"]) ~~~ _file["Module / File"] ~~~ _db[("Host storage")]
    end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Docker logging driver with rotation
  • ➕ No extra file in the deploy dir
  • ➕ Daemon-managed rotation
  • ➖ Logs still lost on docker compose down + recreate (not just restart)
  • ➖ Requires daemon-level config (/etc/docker/daemon.json), not portable
  • ➖ Does not capture native CUDA/OOM aborts any better than docker logs
2. OTEL Collector + filelog receiver sidecar
  • ➕ Structured, queryable log storage
  • ➕ Integrates with existing observability stacks
  • ➖ Significant new infrastructure (Collector, backend)
  • ➖ vLLM has no native OTLP log export; crash traceback is not a span
  • ➖ Overkill for the stated goal of preserving a crash trace

Recommendation: The file-level tee approach is the right choice here. OTEL was correctly evaluated and rejected: vLLM's OTLP support is traces-only and a crash traceback is not a span. The main alternative worth noting is a Docker logging driver (e.g. json-file with max-size/max-file rotation, or local driver), but that still loses logs on docker compose down + recreate and requires daemon-level config. The bash exec &gt; &gt;(tee -a) pattern is the minimal, dependency-free solution that captures both Python and native CUDA/C++ aborts below the Python logging layer.

Files changed (20) +639 / -7

Enhancement (6) +56 / -2
_compose.pyAdd durable-log dir helpers and register mg-logwrap.sh in scaffold templates +42/-0

Add durable-log dir helpers and register mg-logwrap.sh in scaffold templates

• Adds 'LOG_WRAPPER', 'LOG_DIRNAME', and 'LOG_DIR_ENV' constants. Implements 'durable_log_dir()' (mirrors compose's '${MODEL_GEAR_LOG_DIR:-./logs}' resolution) and 'ensure_log_dir()' (best-effort mkdir before compose bind-mounts). Registers 'mg-logwrap.sh' in both 'SINGLE_TEMPLATES' and 'FLEET_TEMPLATES' so 'model init' scaffolds it.

model_gear/runtime/_compose.py

init.pyPre-create the durable log dir (user-owned) during 'model init --apply' +4/-0

Pre-create the durable log dir (user-owned) during 'model init --apply'

• Calls '_compose.ensure_log_dir(target)' after writing the scaffold so the bind-mount source directory exists before 'model serve' or 'fleet up' runs, preventing Docker from creating it root-owned.

model_gear/cli/_commands/init.py

serve.pyPre-create the durable log dir before 'docker compose up' in 'model serve' +2/-0

Pre-create the durable log dir before 'docker compose up' in 'model serve'

• Calls '_compose.ensure_log_dir()' with the configured 'MODEL_GEAR_LOG_DIR' value before starting compose, ensuring the log dir is user-owned on first serve.

model_gear/cli/_commands/serve.py

fleet.pyPre-create the durable log dir before 'docker compose up' in 'model fleet up' +3/-1

Pre-create the durable log dir before 'docker compose up' in 'model fleet up'

• Imports '_env' and calls '_compose.ensure_log_dir()' before the fleet compose-up, mirroring the same user-owned dir pre-creation done in 'model serve'.

model_gear/cli/_commands/fleet.py

__init__.pyRegister the new 'logs' subcommand in the CLI parser +2/-0

Register the new 'logs' subcommand in the CLI parser

• Imports 'logs' from '_commands' and calls '_logs_cmd.register(sub)' to wire the new verb into the argument parser.

model_gear/cli/init.py

fleet.pyImport _env module needed for log dir resolution in fleet up +3/-1

Import _env module needed for log dir resolution in fleet up

• Adds '_env' to the runtime imports so '_env.read_env()' can be called to resolve 'MODEL_GEAR_LOG_DIR' before compose starts.

model_gear/cli/_commands/fleet.py

Tests (2) +186 / -2
test_cli_logs.pyNew test suite for 'model logs' and durable-log helpers +158/-0

New test suite for 'model logs' and durable-log helpers

• Covers 'durable_log_dir' resolution (default, relative, absolute), 'ensure_log_dir' creation, 'collect_logs' ordering/filtering/symlink-skipping, 'tail_lines' windowing, and the full CLI verb (list text/JSON, tail latest, '--previous' crash-boot recovery, empty dir, unknown service). Also asserts the shipped 'mg-logwrap.sh' template contains the required 'exec', 'tee -a', and 'MG_LOG_NAME' patterns.

tests/test_cli_logs.py

test_init.pyExtend init tests to assert mg-logwrap scaffolding and log dir creation +28/-2

Extend init tests to assert mg-logwrap scaffolding and log dir creation

• Adds assertions for 'mg-logwrap.sh' file presence, 'logs/' dir creation, entrypoint line, 'MG_LOG_NAME' env vars, and '/logs/model-gear' volume mount in both single-model and fleet init scenarios. Updates JSON payload assertions to include 'mg-logwrap.sh' in the scaffolded file set.

tests/test_init.py

Documentation (7) +146 / -2
env.exampleDocument MODEL_GEAR_LOG_DIR in the single-model env template +6/-0

Document MODEL_GEAR_LOG_DIR in the single-model env template

• Adds the 'MODEL_GEAR_LOG_DIR=' variable with a comment explaining its purpose, default, and how to override it.

model_gear/templates/env.example

env.exampleDocument MODEL_GEAR_LOG_DIR in the fleet env template +6/-0

Document MODEL_GEAR_LOG_DIR in the fleet env template

• Same 'MODEL_GEAR_LOG_DIR=' addition as the single-model template, adapted for fleet per-boot file naming.

model_gear/templates/fleet/env.example

durable-logs.mdNew documentation for the durable-log mechanism, paths, and model logs usage +97/-0

New documentation for the durable-log mechanism, paths, and model logs usage

• Explains the observability gap (issue #50), how 'mg-logwrap.sh' works, path resolution table, 'model logs' usage examples, pruning guidance, and why OTEL was evaluated and rejected for crash log capture.

docs/durable-logs.md

overview.pyAdd 'logs' to the overview verb listing +1/-0

Add 'logs' to the overview verb listing

• Inserts a one-line description of 'model logs' into the overview command's verb list so it appears in the agent-facing snapshot.

model_gear/cli/_commands/overview.py

CLAUDE.mdRegister logs.py in the command module listing and read-only verb list +2/-2

Register logs.py in the command module listing and read-only verb list

• Adds 'logs.py' to the '_commands/' directory listing and 'logs' to the read-only verbs description.

CLAUDE.md

gateway-fleet.mdNote mg-logwrap coverage for fleet vLLM gears in gateway docs +5/-0

Note mg-logwrap coverage for fleet vLLM gears in gateway docs

• Adds a paragraph pointing readers to 'model logs {primary,embed,rerank}' and 'docs/durable-logs.md' for crash log access.

docs/gateway-fleet.md

CHANGELOG.mdAdd v0.23.0 changelog entry for durable logs and model logs verb +29/-0

Add v0.23.0 changelog entry for durable logs and model logs verb

• Documents the new durable-log feature, 'model logs' verb, and the pre-creation of the user-owned log dir under Added/Changed/Fixed sections.

CHANGELOG.md

Other (5) +251 / -1
mg-logwrap.shNew bash entrypoint wrapper that tees vLLM output to a durable per-boot log file +42/-0

New bash entrypoint wrapper that tees vLLM output to a durable per-boot log file

• Scaffolded by 'model init' and bind-mounted as each vLLM service's container entrypoint. Opens a per-boot '<service>-<ISO8601>.log' file under '/logs/model-gear', tees stdout+stderr to it while passing through to the console, then 'exec's the real command so vLLM remains the signal target. Falls back to plain 'exec "$@"' if logging setup fails, ensuring it never blocks serving.

model_gear/templates/mg-logwrap.sh

logs.pyNew 'model logs' CLI verb for listing and tailing durable per-boot log files +180/-0

New 'model logs' CLI verb for listing and tailing durable per-boot log files

• Implements 'collect_logs()' (pure file I/O, newest-first, skips '-latest.log' symlinks) and 'tail_lines()' (reads only the final 'max_bytes' window for large files). The 'cmd_logs' handler supports listing all boots, tailing the latest, '--previous' to tail the crashed boot, and '--json' output. Reads host files directly so it works even after the container is gone.

model_gear/cli/_commands/logs.py

docker-compose.ymlWire mg-logwrap entrypoint and log-dir bind-mount into the single-model vLLM service +11/-0

Wire mg-logwrap entrypoint and log-dir bind-mount into the single-model vLLM service

• Adds 'MG_LOG_NAME=vllm' env var, the '${MODEL_GEAR_LOG_DIR:-./logs}:/logs/model-gear' and './mg-logwrap.sh:/usr/local/bin/mg-logwrap:ro' volume mounts, and 'entrypoint: ["bash", "/usr/local/bin/mg-logwrap"]' to the vllm service.

model_gear/templates/docker-compose.yml

docker-compose.ymlWire mg-logwrap entrypoint and log-dir bind-mounts into all three fleet vLLM services +17/-0

Wire mg-logwrap entrypoint and log-dir bind-mounts into all three fleet vLLM services

• Adds 'MG_LOG_NAME' env vars ('primary', 'embed', 'rerank'), log-dir and wrapper bind-mounts, and 'entrypoint: ["bash", "/usr/local/bin/mg-logwrap"]' to the 'vllm-primary', 'vllm-embed', and 'vllm-rerank' services.

model_gear/templates/fleet/docker-compose.yml

pyproject.tomlBump version to 0.23.0 +1/-1

Bump version to 0.23.0

• Version increment from 0.22.1 to 0.23.0 for the durable-logs release.

pyproject.toml

@qodo-code-review

qodo-code-review Bot commented Jun 20, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 35 rules
✅ Skills: 4 invoked
  sonarclaude
  version-bump
  cicd
  doc-test-alignment

Grey Divider


Action required

1. --previous tails latest silently ✓ Resolved 🐞 Bug ≡ Correctness
Description
cmd_logs() treats --previous as “boot before latest”, but when only one boot file exists it
silently falls back to the latest boot (idx=0). This can mislead crash investigations into thinking
they’re looking at the crashed boot when they’re not.
Code

model_gear/cli/_commands/logs.py[R112-116]

+        # --previous tails the boot *before* the latest — i.e. the boot that crashed,
+        # the one to investigate after a restart created a fresh (healthy) boot file.
+        idx = 1 if getattr(args, "previous", False) and len(entries) > 1 else 0
+        latest = entries[idx]
+        n = int(getattr(args, "lines", 40))
Relevance

⭐⭐⭐ High

Team has accepted fixes for misleading CLI behavior/text; likely to warn/error instead of silently
tailing latest.

PR-#23
PR-#34

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The index selection explicitly forces idx=0 (latest) whenever there is only one entry, even if
--previous was requested.

model_gear/cli/_commands/logs.py[112-116]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`model logs <service> --previous` is intended to show the boot *before* the latest, but the current implementation silently falls back to the latest boot when there is only one entry. This produces incorrect output for the flag’s semantics and can send operators to the wrong log during incident/debug workflows.

### Issue Context
This is in the new `model logs` command added for durable crash traces.

### Fix Focus Areas
- model_gear/cli/_commands/logs.py[112-116]

### Suggested fix
- If `args.previous` is set and `len(entries) < 2`, emit a clear message like `no previous boot for '<service>' (only 1 boot file found)`.
- Prefer a non-zero exit code (or at least a distinct JSON shape) so automation can detect the condition.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Symlink log targets readable ✓ Resolved 🐞 Bug ⛨ Security
Description
collect_logs() accepts any *.log file that isn’t named *-latest.log, and
Path.is_file()/stat() follow symlinks; tail_lines() then opens the selected path. If symlinks
exist in the log dir (e.g., created accidentally or by another process/user), model logs may read
an unintended file target.
Code

model_gear/cli/_commands/logs.py[R36-48]

+    if not log_dir.is_dir():
+        return []
+    out: list[dict] = []
+    for p in log_dir.glob("*.log"):
+        if p.name.endswith(_LATEST_SUFFIX) or not p.is_file():
+            continue
+        svc = p.name.split("-", 1)[0]
+        if service and svc != service:
+            continue
+        try:
+            st = p.stat()
+        except OSError:
+            continue
Relevance

⭐⭐ Medium

No prior symlink-traversal findings; team does accept security hardening elsewhere (e.g.,
trust-remote-code gating).

PR-#5
PR-#36

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The collector uses is_file() and stat() (both follow symlinks) and the tailer then opens the
resulting path, so a symlink named like a boot log will be treated as a readable log file.

model_gear/cli/_commands/logs.py[36-48]
model_gear/cli/_commands/logs.py[62-78]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`model logs` should operate on the durable per-boot log files produced by `mg-logwrap`, but the current file selection logic will also accept symlinks (except for the `*-latest.log` name). That means `model logs` may tail the symlink’s target instead of an actual boot log.

### Issue Context
This is a local file I/O path. Impact depends on who/what can write into the configured log directory, but it’s straightforward to harden with minimal behavior change.

### Fix Focus Areas
- model_gear/cli/_commands/logs.py[36-58]
- model_gear/cli/_commands/logs.py[62-78]

### Suggested fix
- In `collect_logs()`, skip symlinks explicitly (e.g., `if p.is_symlink(): continue`) and/or use `p.lstat()` + `stat.S_ISREG(...)` to ensure the entry is a regular file.
- Optionally, in `tail_lines()`, guard against symlinks as a second line of defense (especially if entries can come from elsewhere in the future).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread model_gear/cli/_commands/logs.py Outdated
SonarCloud:
- logs.py `cmd_logs`: extract `_emit_tail` / `_emit_listing` so it has a single
  return (S3516) and its cognitive complexity drops from 18 to under 15 (S3776).
- _compose.py: use the `LOG_WRAPPER` constant for the template dict keys instead
  of repeating the "mg-logwrap.sh" literal (S1192).

Colleague review:
- Set `MG_LOG_DIR=/logs/model-gear` explicitly in each vLLM service's
  `environment:` so the in-container log path can't drift from the volume mount
  and can't be silently mis-overridden.
- Comment the two `exec` uses in mg-logwrap.sh (redirect-only vs replace-shell).
- `model logs <svc> --previous` with only one boot now says "(only 1 boot —
  showing latest)" instead of silently showing the latest as if it were the
  crashed boot; add `only_boot` to the JSON.
- Add a drift-guard test linking the compose `${MODEL_GEAR_LOG_DIR:-./logs}`
  default + `/logs/model-gear` mount to the Python `LOG_DIRNAME`.

322 tests pass; black/isort/flake8 clean; both composes pass `docker compose config`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnDfFkZkXz8C68hr3AA9Qa
…ogs (Qodo)

Qodo flagged that collect_logs() used is_file()/stat() (which follow symlinks)
and tail_lines() then opened the path, so a symlink planted in the log dir and
named like a boot log (e.g. vllm-x.log -> /etc/shadow) would be listed and
tailed. mg-logwrap only ever writes regular per-boot files, so:

- collect_logs(): skip any symlink (also covers the <service>-latest.log pointer).
- tail_lines(): refuse to read through a symlink as defense in depth.
- Tests for both: a planted symlink is neither listed nor read.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnDfFkZkXz8C68hr3AA9Qa
@OriNachum

Copy link
Copy Markdown
Contributor Author

Addressed the remaining review + SonarCloud findings:

Qodo bug #2 — symlink traversal (security) → fixed in 2ca66c7. collect_logs() now skips any symlink (not just the *-latest.log pointer) so a symlink planted in the log dir and named like a boot file can't be listed/followed out of the dir; tail_lines() refuses to read through a symlink as defense-in-depth. mg-logwrap only ever writes regular per-boot files. Tests added for both.

SonarCloud (3) → fixed in 530caa0:

  • cmd_logs split into _emit_tail / _emit_listing → single return (S3516) + cognitive complexity 18 → under 15 (S3776).
  • template dict keys use the LOG_WRAPPER constant instead of repeating the "mg-logwrap.sh" literal (S1192).

Colleague (different-model) review folded into 530caa0: explicit MG_LOG_DIR=/logs/model-gear per service (no drift / no silent mis-override), the two-exec semantics commented, and a drift-guard test linking the compose default to LOG_DIRNAME.

324 tests pass; black/isort/flake8/bandit clean; both composes pass docker compose config. The live mesh backend was not touched.

  • model-gear (Claude)

@sonarqubecloud

Copy link
Copy Markdown

@OriNachum
OriNachum merged commit 46dc46f into main Jun 20, 2026
8 checks passed
@OriNachum
OriNachum deleted the fix/durable-crash-logs-50 branch June 20, 2026 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vLLM server crashes (EngineCore 500, then unrecoverable) on tool-calling requests for Qwen3.6-27B FP4

1 participant