RSPEED-2974: add privacy-safe RLSAPI observability logs#1646
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe changes add extensive info-level logging throughout the rlsapi v1 endpoint to track request handling stages, model selection, moderation decisions, inference completion, and quota enforcement. Error-to-HTTP mapping logging was adjusted to log only exception type names instead of full exception objects. Two new tests verify logging behavior and ensure sensitive data is not leaked in error logs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Review rate limit: 0/1 reviews remaining, refill in 56 minutes and 20 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/endpoints/rlsapi_v1.py (1)
475-488:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRedact the exception payload before queueing the failure event.
str(error)can still contain backend-echoed prompt text. The newmock_api_status_error_with_private_texttest fixture proves that anAPIStatusErrormay embedPRIVATE prompt sk-backend-secret, and this path forwards that raw string into_queue_splunk_event(...). That leaves the logger sanitized, but not the observability event.🔒 Suggested fix
_queue_splunk_event( background_tasks, infer_request, request, request_id, - str(error), + type(error).__name__, inference_time, "infer_error", )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/endpoints/rlsapi_v1.py` around lines 475 - 488, The exception string passed into _queue_splunk_event may contain private prompt text (e.g., APIStatusError with "PRIVATE prompt sk-backend-secret"), so before calling _queue_splunk_event replace or sanitize sensitive substrings in str(error) (for example via a helper like redact_sensitive_info or a short regex that removes "PRIVATE" blocks and sk- tokens) and pass the redacted string instead; update the call site using _queue_splunk_event(..., redacted_error, ...) referencing the variables infer_request and request_id to preserve context.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/app/endpoints/rlsapi_v1.py`:
- Around line 475-488: The exception string passed into _queue_splunk_event may
contain private prompt text (e.g., APIStatusError with "PRIVATE prompt
sk-backend-secret"), so before calling _queue_splunk_event replace or sanitize
sensitive substrings in str(error) (for example via a helper like
redact_sensitive_info or a short regex that removes "PRIVATE" blocks and sk-
tokens) and pass the redacted string instead; update the call site using
_queue_splunk_event(..., redacted_error, ...) referencing the variables
infer_request and request_id to preserve context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ae9af949-e0da-4b1f-8112-4d8399b0ab21
📒 Files selected for processing (2)
src/app/endpoints/rlsapi_v1.pytests/unit/app/endpoints/test_rlsapi_v1.py
📜 Review details
⏰ 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). (14)
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: Pylinter
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: build-pr
- GitHub Check: integration_tests (3.12)
- GitHub Check: unit_tests (3.12)
- GitHub Check: list_outdated_dependencies
- GitHub Check: unit_tests (3.13)
- GitHub Check: spectral
🧰 Additional context used
📓 Path-based instructions (5)
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/unit/**/*.py: Use pytest for all unit and integration tests
Usepytest-mockfor AsyncMock objects in unit tests
Use markerpytest.mark.asynciofor async tests
Files:
tests/unit/app/endpoints/test_rlsapi_v1.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Do not use unittest - pytest is the standard testing framework for this project
Files:
tests/unit/app/endpoints/test_rlsapi_v1.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use absolute imports for internal modules:from authentication import get_auth_dependency
All modules start with descriptive docstrings explaining purpose
Uselogger = get_logger(__name__)fromlog.pyfor module logging
Type aliases defined at module level for clarity
Use Final[type] as type hint for all constants
All functions require docstrings with brief descriptions
Complete type annotations for function parameters and return types
Use Union types with modern syntax:str | int
UseOptional[Type]for optional type hints
Use snake_case with descriptive, action-oriented function names (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns: return new data structures instead of modifying parameters
Useasync deffor I/O operations and external API calls
Uselogger.debug()for detailed diagnostic information
Uselogger.info()for general information about program execution
Uselogger.warning()for unexpected events or potential problems
Uselogger.error()for serious problems that prevented function execution
All classes require descriptive docstrings explaining purpose
Use PascalCase for class names with descriptive names and standard suffixes:Configuration,Error/Exception,Resolver,Interface
Abstract classes use ABC with@abstractmethoddecorators
Complete type annotations for all class attributes, use specific types, notAny
Follow Google Python docstring conventions for all modules, classes, and functions
Docstring Parameters section documents function parameters
Docstring Returns section documents function return values
Docstring Raises section documents exceptions that may be raised
Use black for code formatting
Use pylint for static analysis with source-roots configuration set to "src"
Use pyright for type checking
Use ruff for fast linting
Use pydocstyle for docstring style validation
Use mypy for additional type checking
Use bandit for security issue detection
Files:
src/app/endpoints/rlsapi_v1.py
src/app/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use FastAPI dependencies:
from fastapi import APIRouter, HTTPException, Request, status, Depends
Files:
src/app/endpoints/rlsapi_v1.py
src/app/endpoints/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use FastAPI
HTTPExceptionwith appropriate status codes for API endpoint error handling
Files:
src/app/endpoints/rlsapi_v1.py
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-29T15:45:22.854Z
Learning: PR titles MUST start with JIRA issue key prefix: `LCORE-`, `RSPEED-`, `MGTM-`, `OLS-`, `RHDHPAI-`, or `LEADS-`
📚 Learning: 2026-04-29T15:45:22.854Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-29T15:45:22.854Z
Learning: Applies to src/runners/**/*.py : Use Llama Stack imports: `from llama_stack_client import AsyncLlamaStackClient`
Applied to files:
tests/unit/app/endpoints/test_rlsapi_v1.py
📚 Learning: 2026-04-29T15:45:22.854Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-29T15:45:22.854Z
Learning: Applies to src/runners/**/*.py : Handle `APIConnectionError` from Llama Stack in modules integrating with Llama Stack
Applied to files:
tests/unit/app/endpoints/test_rlsapi_v1.py
📚 Learning: 2026-04-07T14:44:42.022Z
Learnt from: major
Repo: lightspeed-core/lightspeed-stack PR: 1469
File: src/models/config.py:1928-1933
Timestamp: 2026-04-07T14:44:42.022Z
Learning: In lightspeed-core/lightspeed-stack, `allow_verbose_infer` (previously `customization.allow_verbose_infer`, now `rlsapi_v1.allow_verbose_infer`) is only used internally by the `rlsapi_v1` `/infer` endpoint and has a single known consumer (the PR author). Backward compatibility for this config field relocation is intentionally not required and should not be flagged in future reviews.
Applied to files:
src/app/endpoints/rlsapi_v1.py
📚 Learning: 2026-04-29T15:45:22.854Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-29T15:45:22.854Z
Learning: Applies to src/app/endpoints/**/*.py : Use FastAPI `HTTPException` with appropriate status codes for API endpoint error handling
Applied to files:
src/app/endpoints/rlsapi_v1.py
📚 Learning: 2026-04-06T20:18:07.852Z
Learnt from: major
Repo: lightspeed-core/lightspeed-stack PR: 1463
File: src/app/endpoints/rlsapi_v1.py:266-271
Timestamp: 2026-04-06T20:18:07.852Z
Learning: In the lightspeed-stack codebase, within `src/app/endpoints/` inference/MCP endpoints, treat `tools: Optional[list[Any]]` in MCP tool definitions as an intentional, consistent typing pattern (used across `query`, `responses`, `streaming_query`, `rlsapi_v1`). Do not raise or suggest this as a typing issue during code review; changing it in isolation could break endpoint typing consistency across the codebase.
Applied to files:
src/app/endpoints/rlsapi_v1.py
|
@coderabbitai review I cleared up the redaction issue you highlighted outside the diff. |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
major
force-pushed
the
rspeed-2974-rlsapi-safe-logging
branch
from
8e7ab18 to
dbd85e5
Compare
RSPEED-2974 Signed-off-by: Major Hayden <major@redhat.com>
major
force-pushed
the
rspeed-2974-rlsapi-safe-logging
branch
from
dbd85e5 to
db59c76
Compare
2 participants
Description
Adds INFO-level breadcrumbs around the RLSAPI v1 infer path so production can trace request progress without exposing user-provided data. The logs cover safe operational metadata only, including request IDs, quota state, shield outcome, model/provider selection, MCP tool counts, token counts, durations, and Splunk sourcetype.
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
uv run black "src/app/endpoints/rlsapi_v1.py" "tests/unit/app/endpoints/test_rlsapi_v1.py"uv run pytest tests/unit/app/endpoints/test_rlsapi_v1.pyuv run make verifyNote:
uv run make formatstill fails on pre-existing unparsable demo docs filesdocs/demos/lcore/weak_points_for_ai/ex1.pyanddocs/demos/lcore/weak_points_for_ai/ex5.py; unrelated formatting changes from that attempt were reverted.Summary by CodeRabbit
Release Notes
Bug Fixes
Improvements