feat(router): add --disable-access-log-for-endpoints to silence probe logs#979
feat(router): add --disable-access-log-for-endpoints to silence probe logs#979Anai-Guo wants to merge 3 commits into
Conversation
… logs Adds a router CLI flag to suppress uvicorn access-log lines for a comma-separated set of request paths (e.g. /health,/metrics), mirroring the probe-noise suppression already available on the model containers. Implemented as a logging.Filter attached to the uvicorn.access logger, which survives uvicorn's dictConfig in both text and json log formats. Closes vllm-project#967 Signed-off-by: Tai An <antai12232931@outlook.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a feature to suppress uvicorn access logs for specific endpoints (such as health probes) via a new --disable-access-log-for-endpoints command-line argument. The implementation includes a custom logging filter, _EndpointAccessLogFilter, which is registered on the uvicorn.access logger. The review feedback highlights two important issues: first, registering the filter before uvicorn.run() is ineffective because uvicorn's internal logging configuration clears existing filters, so it should instead be applied within the FastAPI lifespan context manager; second, a potential KeyError could occur if record.args is a dictionary rather than a sequence, which can be resolved by explicitly checking its type before indexing.
| # Optionally drop access logs for noisy probe endpoints. The filter is | ||
| # attached to the ``uvicorn.access`` logger before uvicorn configures its | ||
| # own logging; uvicorn's dictConfig replaces handlers but preserves | ||
| # existing logger filters, so the suppression survives both text and json | ||
| # log formats. | ||
| disabled_endpoints = parse_comma_separated_args( | ||
| args.disable_access_log_for_endpoints | ||
| ) | ||
| if disabled_endpoints: | ||
| logging.getLogger("uvicorn.access").addFilter( | ||
| _EndpointAccessLogFilter(disabled_endpoints) | ||
| ) |
There was a problem hiding this comment.
In Python's logging.config.dictConfig (which uvicorn calls during startup), any existing filters on loggers are cleared if they are not explicitly defined in the configuration dictionary (unless incremental=True is used, which is not the case here). As a result, adding the filter to uvicorn.access before calling uvicorn.run() will cause it to be silently removed during startup, making the suppression ineffective.
To resolve this, we should store the parsed endpoints in app.state and apply the filter inside the FastAPI lifespan context manager, which executes after uvicorn has completed its logging configuration.
For example, in lifespan:
@asynccontextmanager
async def lifespan(app: FastAPI):
# ... existing code ...
disabled_endpoints = getattr(app.state, "disable_access_log_for_endpoints", None)
if disabled_endpoints:
logging.getLogger("uvicorn.access").addFilter(
_EndpointAccessLogFilter(disabled_endpoints)
)
yield
# ...| # Optionally drop access logs for noisy probe endpoints. The filter is | |
| # attached to the ``uvicorn.access`` logger before uvicorn configures its | |
| # own logging; uvicorn's dictConfig replaces handlers but preserves | |
| # existing logger filters, so the suppression survives both text and json | |
| # log formats. | |
| disabled_endpoints = parse_comma_separated_args( | |
| args.disable_access_log_for_endpoints | |
| ) | |
| if disabled_endpoints: | |
| logging.getLogger("uvicorn.access").addFilter( | |
| _EndpointAccessLogFilter(disabled_endpoints) | |
| ) | |
| app.state.disable_access_log_for_endpoints = parse_comma_separated_args( | |
| args.disable_access_log_for_endpoints | |
| ) |
| def filter(self, record: logging.LogRecord) -> bool: | ||
| if self._endpoints and record.args and len(record.args) >= 3: | ||
| path = str(record.args[2]).split("?", 1)[0] | ||
| if path in self._endpoints: | ||
| return False | ||
| return True |
There was a problem hiding this comment.
If record.args is a dictionary (which can happen if a log message uses named placeholders), len(record.args) returns the number of keys. If the dictionary has 3 or more keys, record.args[2] will raise a KeyError because 2 is not a valid key. To prevent potential logging crashes, we should explicitly verify that record.args is a sequence (like a tuple or list) before indexing it.
| def filter(self, record: logging.LogRecord) -> bool: | |
| if self._endpoints and record.args and len(record.args) >= 3: | |
| path = str(record.args[2]).split("?", 1)[0] | |
| if path in self._endpoints: | |
| return False | |
| return True | |
| def filter(self, record: logging.LogRecord) -> bool: | |
| if ( | |
| self._endpoints | |
| and isinstance(record.args, (tuple, list)) | |
| and len(record.args) >= 3 | |
| ): | |
| path = str(record.args[2]).split("?", 1)[0] | |
| if path in self._endpoints: | |
| return False | |
| return True |
…re-commit end-of-file hook Signed-off-by: Tai An <antai12232931@outlook.com>
…nd-of-file hook Signed-off-by: Tai An <antai12232931@outlook.com>
cd3e850 to
4d0e939
Compare
Summary
Closes #967.
The router container logs every
liveness-/readinessProbehit, which floods the logs. The model containers can already suppress this with--disable-access-log-for-endpoints; this PR brings the same capability tovllm_router.What changed
--disable-access-log-for-endpoints(comma-separated paths, e.g./health,/metrics).logging.Filter(_EndpointAccessLogFilter) is attached to theuvicorn.accesslogger beforeuvicorn.run. It matches on the request path (ignoring any query string) and drops only the matching access records — error logs and--log-statsare untouched.Why this approach
uvicorn emits each access record with positional
argsof(client_addr, method, path, http_version, status_code), so matching onargs[2]is robust across formatters. uvicorn'sdictConfigreplaces handlers but preserves existing logger filters, so the suppression works in bothtextandjsonlog formats. When the flag is unset, behavior is unchanged.Usage
🤖 Generated with Claude Code