LCORE-700: logging for info and health endpoints #559
Conversation
WalkthroughLogging namespaces were standardized and additional log statements were added across health and info endpoints. No endpoint logic, signatures, control flow, or error handling was changed. Observed behavior remains the same aside from added info/debug logs. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/app/endpoints/info.py (3)
61-61: Demote request log to DEBUG or enrich with structured fields.Info endpoint is low-traffic, but a bare INFO adds little value. Prefer DEBUG or include status/duration.
- logger.info("Response to /v1/info endpoint") + logger.debug("GET /v1/info requested")
69-71: Fix typo in log and (optional) avoid repeated config lookups.“LLama” → “Llama”. Optionally cache configuration once.
Apply:
- logger.debug("LLama Stack version: %s", llama_stack_version) + logger.debug("Llama Stack version: %s", llama_stack_version)Optional (outside changed lines):
cfg = configuration.configuration logger.debug("Service name: %s", cfg.name) # ... and use cfg.name in the response too
19-19: Use module-scoped loggers for endpoints.Multiple endpoint modules currently share logger "app.endpoints.handlers"; use module-scoped names for per-endpoint tuning.
- src/app/endpoints/info.py — logger = logging.getLogger("app.endpoints.info")
- src/app/endpoints/health.py — logger = logging.getLogger("app.endpoints.health")
- src/app/endpoints/root.py — logger = logging.getLogger("app.endpoints.root")
- src/app/endpoints/conversations.py — logger = logging.getLogger("app.endpoints.conversations")
- src/app/endpoints/query.py — logger = logging.getLogger("app.endpoints.query")
- src/app/endpoints/streaming_query.py — logger = logging.getLogger("app.endpoints.streaming_query")
Keep src/utils/mcp_headers.py as logger = logging.getLogger("app.endpoints.dependencies").
src/app/endpoints/health.py (1)
25-25: Replace shared logger "app.endpoints.handlers" with module-scoped loggersUse per-module names (e.g., "app.endpoints.health") so each endpoint can be filtered independently. Update the occurrences below:
- src/app/endpoints/health.py
- src/app/endpoints/info.py
- src/app/endpoints/streaming_query.py
- src/app/endpoints/query.py
- src/app/endpoints/conversations.py
- src/app/endpoints/root.py
Example diff (health.py):
-logger = logging.getLogger("app.endpoints.handlers") +logger = logging.getLogger("app.endpoints.health")src/utils/mcp_headers.py already uses "app.endpoints.dependencies" and can remain as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/endpoints/health.py(3 hunks)src/app/endpoints/info.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/app/endpoints/info.py (2)
src/client.py (2)
AsyncLlamaStackClientHolder(18-55)get_client(49-55)src/configuration.py (1)
configuration(60-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-pr
- GitHub Check: e2e_tests
| # Used only for authorization | ||
| _ = auth | ||
|
|
||
| logger.info("Response to /v1/readiness endpoint") |
There was a problem hiding this comment.
Health endpoints: avoid high-volume INFO logs; demote and log outcomes instead.
These endpoints are hit frequently (Kubernetes probes); INFO here will flood logs. Use DEBUG for requests and WARNING/INFO once per failure/transition with fields.
Apply:
- logger.info("Response to /v1/readiness endpoint")
+ logger.debug("GET /v1/readiness requested")- logger.info("Response to /v1/liveness endpoint")
+ logger.debug("GET /v1/liveness requested")Then, add outcome logs near the decision (outside the changed lines):
# after computing ready/reason/unhealthy_provider_names, before return
if unhealthy_providers:
logger.warning(
"Readiness failed",
extra={"ready": False, "reason": reason, "unhealthy": unhealthy_provider_names},
)
else:
logger.debug(
"Readiness ok",
extra={"ready": True, "reason": reason},
)Also applies to: 140-140
🤖 Prompt for AI Agents
In src/app/endpoints/health.py around lines 96 and 140, the health endpoint
currently emits high-volume INFO logs; change the logger.info calls on those
lines to logger.debug to avoid log flooding from frequent probes, and then add
outcome logs immediately after the readiness decision (after computing
ready/reason/unhealthy_provider_names) as described: if there are unhealthy
providers emit a logger.warning with extra={"ready": False, "reason": reason,
"unhealthy": unhealthy_provider_names}, otherwise emit a logger.debug with
extra={"ready": True, "reason": reason}. Ensure the new outcome logging is
placed outside the changed lines and uses the computed variables.
1 participant
Description
LCORE-700: logging for info and health endpoints
Type of change
Related Tickets & Documents
Summary by CodeRabbit