feat: json logging for inference (vLLM)#2466
Conversation
Adds [log] config to InferenceConfig and routes vLLM's stdlib loggers (plus uvicorn / fastapi / root) through a JSON formatter matching the trainer/orchestrator shape. Driven via VLLM_LOGGING_CONFIG_PATH so the config crosses the multiprocessing.spawn boundary into vLLM workers.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a78f02a93e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
vLLM raises at import if VLLM_CONFIGURE_LOGGING=0 and VLLM_LOGGING_CONFIG_PATH is set. Setting json_logging is an explicit opt-in to configure vLLM's logger, so override the env var rather than crash on jobs that have it preset to 0.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Match loguru's tz-aware ISO output so the platform's JsonLogsViewer parses inference timestamps correctly. Naive timestamps are interpreted as the viewer's local time, off by the viewer-vs-pod offset.
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cb0a1629b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
RLConfig.auto_setup_logs already mirrors [log] into trainer and orchestrator subconfigs. Now that InferenceConfig has its own [log] field, propagate there too — without this, an RL run with [log] json_logging=true at the root still emits text vLLM logs because inference.log defaults to non-JSON.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 523749c. Configure here.
RLConfig.inference is Optional and defaults to None for setups like elastic-inference-pool runs. The previous commit propagated [log] unconditionally, crashing the validator on any RLConfig without an [inference] section (caught by the test_load_configs unit tests on configs/elastic/rl.toml).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b880db530
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Mirrors the auto_setup_logs propagation to inference, so teacher inference servers also pick up shared level / json_logging when they are configured separately or auto-created without a rollout inference to deep-copy from.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31a789b21b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Stdlib logging does not know TRACE/SUCCESS, so setting [log] level = "trace" or PRIME_LOG_LEVEL=trace would crash setup_vllm_env when JSON logging is enabled. Map TRACE→DEBUG and SUCCESS→INFO before building the dictConfig payload.
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9d2fd6304
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The docstring claimed transformers was covered but the named-loggers tuple omitted it, so HF model-loading warnings/info were emitted as plain text in otherwise-JSON inference logs (transformers sets propagate=False on its own logger).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2c5db01c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2: SharedLogConfig.json_logging defaulted to False, so auto_setup_logs would clobber any per-component (inference, teacher_inference) override back to text whenever the shared [log] section wasn't set. Mirrors the existing 'level' pattern: nullable on SharedLogConfig, gate propagation on the shared value being explicitly set.
1 participant
Adds
[log]toInferenceConfigso the inference pod can emit JSON logs in the same shape as trainer / orchestrator.JsonFormattermatchingbuild_log_entry's flat shape — renders identically in the platform'sJsonLogsViewerand prime-cli's pretty modeVLLM_LOGGING_CONFIG_PATHso config crosses themultiprocessing.spawnboundary into vLLM workersconfig.log.json_logging/config.log.levelCaveat: uvicorn's access logger may revert to text after server start — vLLM logs (the bulk) and root logger are unaffected.
Note
Medium Risk
Moderate risk because it changes inference startup/logging behavior and injects vLLM logging-related environment variables and stdlib
loggingconfiguration that affects spawned worker processes.Overview
Adds an explicit
log: LogConfigsection toInferenceConfigand wires the inference entrypoint to useconfig.log.levelandconfig.log.json_logginginstead of a hardcoded log level.Introduces
prime_rl.inference.json_loggingto emit newline-delimited JSON via stdliblogging(matching trainer/orchestrator log shape) and updatessetup_vllm_envto enable and propagate this config to vLLM (and its spawned workers) viaVLLM_LOGGING_CONFIG_PATH/VLLM_CONFIGURE_LOGGINGplus an in-processdictConfig.Updates RL shared logging propagation so
SharedLogConfig.json_loggingis tri-state (only overrides when set) and extends shared log propagation to also coverinferenceandteacher_inferencewhen present.Reviewed by Cursor Bugbot for commit b726169. Bugbot is set up for automated code reviews on this repo. Configure here.