[Feat] Add --log-format json option for structured logging#849
[Feat] Add --log-format json option for structured logging#849ruizhang0101 merged 5 commits intovllm-project:mainfrom
Conversation
Summary of ChangesHello @keyuchen21, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement to the router's logging system by providing an option for structured JSON output. This change allows users to easily switch between human-readable text logs and machine-parseable JSON logs via a new command-line argument. The primary impact is improved log analysis and integration with external logging systems, offering greater flexibility and observability for the application. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The PR introduces structured JSON logging which is a great addition for observability. The implementation correctly integrates with both the internal router loggers and Uvicorn. I've suggested some improvements to the JsonFormatter to ensure it captures all relevant context (like extra fields and stack traces) and handles non-serializable data gracefully. I also noted a minor inconsistency in how Uvicorn logs are routed compared to the router's own logs.
src/vllm_router/log.py
Outdated
| formatter = JsonFormatter() if _LOG_FORMAT == "json" else None | ||
| for logger in _loggers: | ||
| for handler in logger.handlers: | ||
| if _LOG_FORMAT == "json": | ||
| handler.setFormatter(formatter) | ||
| elif isinstance(handler.formatter, JsonFormatter): | ||
| handler.setFormatter(CustomFormatter()) |
There was a problem hiding this comment.
When switching back to 'text' format, a new CustomFormatter instance is created for every handler in every logger. It would be more efficient to instantiate the formatter once outside the loops.
| formatter = JsonFormatter() if _LOG_FORMAT == "json" else None | |
| for logger in _loggers: | |
| for handler in logger.handlers: | |
| if _LOG_FORMAT == "json": | |
| handler.setFormatter(formatter) | |
| elif isinstance(handler.formatter, JsonFormatter): | |
| handler.setFormatter(CustomFormatter()) | |
| if _LOG_FORMAT == "json": | |
| formatter = JsonFormatter() | |
| else: | |
| formatter = CustomFormatter() | |
| for logger in _loggers: | |
| for handler in logger.handlers: | |
| if _LOG_FORMAT == "json": | |
| handler.setFormatter(formatter) | |
| elif isinstance(handler.formatter, JsonFormatter): | |
| handler.setFormatter(formatter) |
There was a problem hiding this comment.
Good catch — applied. Formatter is now instantiated once before the loop to avoid redundant allocations. Fixed in 8f7003a.
src/vllm_router/log.py
Outdated
| def format(self, record): | ||
| log_record = { | ||
| "timestamp": self.formatTime(record, self.datefmt), | ||
| "level": record.levelname, | ||
| "logger": record.name, | ||
| "message": record.getMessage(), | ||
| "filename": record.filename, | ||
| "lineno": record.lineno, | ||
| } | ||
| if record.exc_info and record.exc_info[0] is not None: | ||
| log_record["exception"] = self.formatException(record.exc_info) | ||
| return json.dumps(log_record) |
There was a problem hiding this comment.
The JsonFormatter currently omits several important pieces of information for structured logging, such as extra fields passed to the logger and stack_info. Additionally, json.dumps may raise a TypeError if any field (especially in extra) is not JSON-serializable.
Consider updating the formatter to include these fields and using default=str to handle non-serializable objects. You might also want to ensure timestamps are in UTC for consistency in cloud environments.
def format(self, record):
log_record = {
"timestamp": self.formatTime(record, self.datefmt),
"level": record.levelname,
"logger": record.name,
"message": record.getMessage(),
"filename": record.filename,
"lineno": record.lineno,
}
if record.exc_info and record.exc_info[0] is not None:
log_record["exception"] = self.formatException(record.exc_info)
if record.stack_info:
log_record["stack_info"] = self.formatStack(record.stack_info)
# Include extra fields
standard_attrs = {
"args", "asctime", "created", "exc_info", "exc_text", "filename",
"funcName", "levelname", "levelno", "lineno", "module", "msecs",
"message", "msg", "name", "pathname", "process", "processName",
"relativeCreated", "stack_info", "thread", "threadName"
}
for key, value in record.__dict__.items():
if key not in standard_attrs and not key.startswith("_"):
log_record[key] = value
return json.dumps(log_record, default=str)There was a problem hiding this comment.
Partially addressed in 8f7003a:
- Added
default=strtojson.dumpsas a safety net for non-serializable objects. - Added
stack_infosupport. - Skipping the
extrafields iteration for now — the codebase doesn't useextrain any logging calls, and blindly dumpingrecord.__dict__could leak unexpected data into structured logs. Happy to revisit if there's an actual use case.
| "handlers": { | ||
| "default": { | ||
| "formatter": "json", | ||
| "class": "logging.StreamHandler", | ||
| "stream": "ext://sys.stdout", | ||
| }, | ||
| "access": { | ||
| "formatter": "json", | ||
| "class": "logging.StreamHandler", | ||
| "stream": "ext://sys.stdout", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
In the JSON log configuration for Uvicorn, both the default and access handlers are routed to sys.stdout. This is inconsistent with init_logger (used for the router's own logs), which routes logs with level WARNING and above to stderr.
While some users prefer all structured logs on stdout, maintaining consistency across the application is usually better for log collection. Consider routing the default handler (which handles errors) to stderr to match the router's behavior.
There was a problem hiding this comment.
Intentionally keeping both uvicorn handlers on stdout — JSON logging is typically consumed by log aggregators (CloudWatch, Datadog, etc.) where a single stream is standard practice. Splitting stdout/stderr adds complexity for JSON consumers with no real benefit. The router's own split is useful for colored text output but less relevant for machine-parsed JSON.
Add a JsonFormatter that outputs log records as JSON with timestamp, level, logger, message, filename, and lineno fields. The new --log-format flag (choices: text, json) controls the output format for both the router loggers and uvicorn. Signed-off-by: Keyu Chen <54015474+keyuchen21@users.noreply.github.com>
Add TestJsonFormatter class covering JSON output validation, exception inclusion/exclusion, format switching via set_log_format, and init_logger format respect. Add parser tests verifying --log-format defaults to text and accepts json. Update README logging options documentation. Signed-off-by: Keyu Chen <54015474+keyuchen21@users.noreply.github.com>
Instantiate formatter once outside the loop in set_log_format to avoid redundant allocations. Add stack_info support and default=str fallback to JsonFormatter for robustness. Add tests for stack_info inclusion and non-serializable object handling. Signed-off-by: Keyu Chen <54015474+keyuchen21@users.noreply.github.com>
keyuchen21
force-pushed
the
feat/log-format-json
branch
from
8f7003a to
63141d1
Compare
Signed-off-by: Keyu Chen <54015474+keyuchen21@users.noreply.github.com>
Signed-off-by: Rui Zhang <51696593+ruizhang0101@users.noreply.github.com>
2 participants
Summary
--log-format jsonCLI option that outputs structured JSON log lines withtimestamp,level,logger,message,filename, andlinenofields--log-format jsonis setJsonFormatter,set_log_format, and the--log-formatparser argument--log-leveland--log-formatoptionsCloses #416
Test plan
pytest src/tests/test_log.py -v— all 21 tests pass (6 newTestJsonFormattertests)pytest src/tests/test_parser.py -v— all 12 tests pass (2 new--log-formattests)