LCORE-1792: Add OpenTelemetry instrumentation for POST /v1/query endpoint - #2342
LCORE-1792: Add OpenTelemetry instrumentation for POST /v1/query endpoint#2342anik120 wants to merge 1 commit into
Conversation
WalkthroughChangesOpenTelemetry tracing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant QueryEndpoint
participant AgentExecution
participant QuotaCheck
participant ShieldModeration
participant RAGRetrieval
QueryEndpoint->>AgentExecution: execute traced inference
AgentExecution->>QuotaCheck: check token availability
AgentExecution->>ShieldModeration: run moderation
AgentExecution->>RAGRetrieval: build RAG context
RAGRetrieval-->>AgentExecution: return retrieval result
AgentExecution-->>QueryEndpoint: return agent response
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (6 passed)
✨ 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. 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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/endpoints/query.py`:
- Around line 136-143: Complete the Google-style docstring for
_handle_query_with_tracing by documenting request, query_request, auth,
mcp_headers, and root_span under a Parameters: section, then add appropriate
Returns: and Raises: sections describing the QueryResponse result and exceptions
propagated by the helper.
- Around line 126-133: Prevent duplicate exception events by using a single
recording mechanism in the span blocks: in src/app/endpoints/query.py lines
126-133, src/utils/agents/query.py lines 264-348, src/utils/shields.py lines
160-169, and src/utils/vector_search.py lines 665-745, either remove the manual
record_exception calls and retain automatic span recording, or disable automatic
recording and keep explicit recording; apply the same consistent choice at every
affected site.
In `@src/utils/agents/query.py`:
- Around line 331-344: Move the add_span_event call for
SpanEvents.LLM_INFERENCE_COMPLETED below build_turn_summary_from_agent_run in
the current agent-run flow. Build and return the summary only after the builder
succeeds, emitting the completion event between successful summary construction
and return; preserve the existing vector_store_ids and rag_id_mapping inputs.
In `@src/utils/otel_tracing.py`:
- Around line 12-46: Move the SpanAttributes and SpanEvents classes from
otel_tracing.py into the central constants.py module, annotating every shared
constant with Final[str] and importing Final as needed. Update all consumers,
including otel_tracing.py, to import these classes from constants.py, and remove
the duplicate local definitions.
- Around line 49-62: Update anonymize_value so its result never contains any
substring of the input, including for short values and truncated long values;
replace the raw-prefix formatting with a non-retained keyed digest suitable for
correlation. Update related tests to assert that no input content appears in the
anonymized result while preserving the configured max_length contract.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 184678ed-e1de-4ad2-a995-8476074243cc
📒 Files selected for processing (7)
src/app/endpoints/query.pysrc/utils/agents/query.pysrc/utils/otel_tracing.pysrc/utils/quota_utils.pysrc/utils/shields.pysrc/utils/vector_search.pytests/unit/utils/test_otel_tracing.py
📜 Review details
⏰ Context from checks skipped due to timeout. (18)
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: build-pr
- GitHub Check: spectral
- GitHub Check: integration_tests (3.13)
- GitHub Check: Pylinter
- GitHub Check: integration_tests (3.12)
- GitHub Check: Pyright
- GitHub Check: bandit
- GitHub Check: unit_tests (3.12)
- GitHub Check: unit_tests (3.13)
- GitHub Check: mypy
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
⚠️ CI failures not shown inline (2)
GitHub Actions: Black / 0_black.txt: LCORE-1792: Add OpenTelemetry instrumentation for POST /v1/query endpoint
Conclusion: failure
##[group]Run uv tool run black --check src tests
�[36;1muv tool run black --check src tests�[0m
shell: /usr/bin/bash -e {0}
env:
UV_PYTHON: 3.12
VIRTUAL_ENV: /home/runner/work/lightspeed-stack/lightspeed-stack/.venv
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
##[endgroup]
Downloading black (1.8MiB)
Downloaded black
Installed 7 packages in 5ms
Warning: Python 3.12 cannot parse code formatted for Python 3.13. To fix this: run Black with Python 3.13, set --target-version to py312, or use --fast to skip the safety check. Black's safety check verifies equivalence by parsing the AST, which fails when the running Python is older than the target version.
would reformat /home/runner/work/lightspeed-stack/lightspeed-stack/src/utils/vector_search.py
Oh no! 💥 💔 💥
1 file would be reformatted, 486 files would be left unchanged.
##[error]Process completed with exit code 1.
GitHub Actions: Black / black: LCORE-1792: Add OpenTelemetry instrumentation for POST /v1/query endpoint
Conclusion: failure
##[group]Run uv tool run black --check src tests
�[36;1muv tool run black --check src tests�[0m
shell: /usr/bin/bash -e {0}
env:
UV_PYTHON: 3.12
VIRTUAL_ENV: /home/runner/work/lightspeed-stack/lightspeed-stack/.venv
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
##[endgroup]
Downloading black (1.8MiB)
Downloaded black
Installed 7 packages in 5ms
Warning: Python 3.12 cannot parse code formatted for Python 3.13. To fix this: run Black with Python 3.13, set --target-version to py312, or use --fast to skip the safety check. Black's safety check verifies equivalence by parsing the AST, which fails when the running Python is older than the target version.
would reformat /home/runner/work/lightspeed-stack/lightspeed-stack/src/utils/vector_search.py
Oh no! 💥 💔 💥
1 file would be reformatted, 486 files would be left unchanged.
##[error]Process completed with exit code 1.
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.
**/*: Useuv runfor project commands; useuv sync --group dev --group llslibdevto install development dependencies.
Before completion, runuv run make format,uv run make verify, create unit tests for new code, and ensure tests pass.
Never commit secrets or keys; use environment variables for sensitive data.
Files:
src/utils/shields.pytests/unit/utils/test_otel_tracing.pysrc/utils/otel_tracing.pysrc/utils/vector_search.pysrc/utils/agents/query.pysrc/utils/quota_utils.pysrc/app/endpoints/query.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use absolute imports for internal Python modules.
All modules must start with descriptive docstrings; uselogger = get_logger(__name__)fromlog.pyfor module logging.
Define shared constants in the centralconstants.pymodule, check it before adding constants, and annotate constants withFinal[type].
Define type aliases at module level for clarity.
Use complete type annotations for function parameters and return types; prefer modern union syntax (str | int) andOptional[Type]for optional values.
Usetyping_extensions.Selffor model validators.
Use descriptive, action-oriented snake_case function names, such asget_,validate_, andcheck_.
Functions must not modify mutable parameters in place; return a new data structure instead.
Useasync deffor I/O operations and external API calls.
HandleAPIConnectionErrorfrom Llama Stack.
Usefrom log import get_loggerand initialize module logging withlogger = get_logger(__name__).
Use standard log levels appropriately:debugfor diagnostics,infofor general execution,warningfor unexpected or potential problems, anderrorfor serious failures.
All classes must have descriptive docstrings, use PascalCase names, and use standard suffixes such asConfiguration,Error/Exception,Resolver, andInterfacewhere applicable.
Pydantic configuration models must extendConfigurationBase; data models must extendBaseModel.
Abstract interfaces must useABCwith@abstractmethoddecorators.
Use@model_validatorand@field_validatorfor Pydantic model validation.
Class attributes must have complete, specific type annotations; avoidAny.
Follow Google Python docstring conventions; provide required docstrings for modules, classes, and functions, including relevantParameters,Returns,Raises, andAttributessections.
Files:
src/utils/shields.pysrc/utils/otel_tracing.pysrc/utils/vector_search.pysrc/utils/agents/query.pysrc/utils/quota_utils.pysrc/app/endpoints/query.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (AGENTS.md)
Run and satisfy the configured Black, Pylint, Pyright, Ruff, pydocstyle, and type-checking standards.
Files:
src/utils/shields.pytests/unit/utils/test_otel_tracing.pysrc/utils/otel_tracing.pysrc/utils/vector_search.pysrc/utils/agents/query.pysrc/utils/quota_utils.pysrc/app/endpoints/query.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/unit/**/*.py: Use pytest for all unit tests; do not use unittest.
Useconftest.pyfor shared pytest fixtures andpytest-mockforAsyncMockobjects.
Mark asynchronous pytest tests withpytest.mark.asyncio.
Files:
tests/unit/utils/test_otel_tracing.py
src/app/endpoints/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use FastAPI
HTTPExceptionwith appropriate status codes for API endpoints.
Files:
src/app/endpoints/query.py
🧠 Learnings (5)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
src/utils/shields.pytests/unit/utils/test_otel_tracing.pysrc/utils/otel_tracing.pysrc/utils/vector_search.pysrc/utils/agents/query.pysrc/utils/quota_utils.pysrc/app/endpoints/query.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/utils/shields.pysrc/utils/otel_tracing.pysrc/utils/vector_search.pysrc/utils/agents/query.pysrc/utils/quota_utils.pysrc/app/endpoints/query.py
📚 Learning: 2026-07-17T19:25:05.325Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2166
File: src/utils/saved_prompts.py:129-157
Timestamp: 2026-07-17T19:25:05.325Z
Learning: For any endpoint that handles saved prompts and calls `src/utils/saved_prompts.py::create_saved_prompt`, treat the endpoint as the validation boundary. Before calling `create_saved_prompt`, validate the incoming saved-prompt name and content, specifically using `validate_saved_prompt_name` and then persist (store) the normalized value it returns. Do not call `create_saved_prompt` with unvalidated/raw name/content.
Applied to files:
src/utils/shields.pysrc/utils/otel_tracing.pysrc/utils/vector_search.pysrc/utils/agents/query.pysrc/utils/quota_utils.pysrc/app/endpoints/query.py
📚 Learning: 2026-01-14T09:37:51.612Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 988
File: src/app/endpoints/query.py:319-339
Timestamp: 2026-01-14T09:37:51.612Z
Learning: In the lightspeed-stack repository, when provider_id == "azure", the Azure provider with provider_type "remote::azure" is guaranteed to be present in the providers list. Therefore, avoid defensive StopIteration handling for next() when locating the Azure provider in providers within src/app/endpoints/query.py. This change applies specifically to this file (or nearby provider lookup code) and relies on the invariant that the Azure provider exists; if the invariant could be violated, keep the existing StopIteration handling.
Applied to files:
src/app/endpoints/query.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/query.py
🪛 ast-grep (0.45.0)
tests/unit/utils/test_otel_tracing.py
[warning] 119-119: Do not make http calls without encryption
Context: "http://example.com/doc1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 120-120: Do not make http calls without encryption
Context: "http://example.com/doc2"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 131-131: Do not make http calls without encryption
Context: "http://example.com/doc1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 132-132: Do not make http calls without encryption
Context: "http://example.com/doc2"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
🪛 GitHub Actions: Black / 0_black.txt
src/utils/vector_search.py
[error] 1-1: Black formatting check failed: the file would be reformatted. Run 'uv tool run black src/utils/vector_search.py' to fix it.
🪛 GitHub Actions: Black / black
src/utils/vector_search.py
[error] 1-1: Black formatting check failed: this file would be reformatted. Run 'uv tool run black src tests' to fix it.
🪛 GitHub Actions: Pydocstyle / 0_pydocstyle.txt
src/app/endpoints/query.py
[error] 143-143: pydocstyle D401: First line of the private function _handle_query_with_tracing docstring should be in imperative mood; rephrase the line beginning with 'Internal'. Command failed: uv tool run pydocstyle -v src tests.
🪛 GitHub Actions: Pydocstyle / pydocstyle
src/app/endpoints/query.py
[error] 143-143: pydocstyle D401: First line of the private function _handle_query_with_tracing docstring should be in imperative mood; rephrase the line beginning with 'Internal'. Command failed: uv tool run pydocstyle -v src tests.
dfb0ea3 to
838eda2
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/agents/query.py`:
- Around line 263-288: Move the blocked-moderation handling in the request flow
before the tracer.start_as_current_span("llm.inference") block. Preserve the
existing append_turn_items_to_conversation call and TurnSummary return, and only
create the inference span for requests that proceed to agent.run.
In `@src/utils/otel_tracing.py`:
- Around line 7-10: Initialize the required module logger in otel_tracing.py by
importing get_logger from log and defining logger = get_logger(__name__)
alongside the existing module imports.
- Around line 61-63: Update the helper producing the `hash:{digest}` value to
use HMAC-SHA-256 with a configuration-managed secret instead of an unkeyed
SHA-256 digest, and export at least 128 bits of output. Preserve the existing
length indicator and formatted return structure, then update its tests to assert
deterministic results without embedding or exposing the original input content.
- Around line 92-102: Update record_exception to set the provided span status to
StatusCode.ERROR in addition to recording the exception, ensuring handled
exceptions are reflected as span failures; import the required status symbol and
extend TestRecordException with an assertion verifying the span’s error status.
In `@src/utils/vector_search.py`:
- Around line 675-686: Update the `_fetch_byok_rag` and `_fetch_solr_rag`
exception handling paths in `vector_search.py` so they record a source-specific
failure marker before returning empty results. Keep the existing
`asyncio.gather` flow in `vector_search` unchanged, but ensure the span can
distinguish a true empty retrieval from a failed one by emitting a per-source
failure event or attribute from the catch blocks. Then have the
`RAG_RETRIEVAL_COMPLETED` reporting logic rely on that recorded failure state
rather than treating all empty results as successful retrieval.
In `@tests/unit/utils/test_otel_tracing.py`:
- Around line 60-66: Update test_exact_max_length_classified_as_long to cover
the actual boundary: assert a 50-character input with max_length=50 is
classified as short, and a 51-character input is classified as long. Keep
assertions verifying the corresponding output markers and lengths so both sides
of the boundary are tested.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1cd0dbb7-1a05-43ca-bb03-0a17a4af2624
📒 Files selected for processing (7)
src/app/endpoints/query.pysrc/utils/agents/query.pysrc/utils/otel_tracing.pysrc/utils/quota_utils.pysrc/utils/shields.pysrc/utils/vector_search.pytests/unit/utils/test_otel_tracing.py
📜 Review details
⏰ Context from checks skipped due to timeout. (13)
- GitHub Check: Pylinter
- GitHub Check: spectral
- GitHub Check: build-pr
- GitHub Check: integration_tests (3.13)
- GitHub Check: unit_tests (3.13)
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.
**/*: Useuv runfor project commands; useuv sync --group dev --group llslibdevto install development dependencies.
Before completion, runuv run make format,uv run make verify, create unit tests for new code, and ensure tests pass.
Never commit secrets or keys; use environment variables for sensitive data.
Files:
src/utils/shields.pysrc/utils/otel_tracing.pysrc/app/endpoints/query.pytests/unit/utils/test_otel_tracing.pysrc/utils/quota_utils.pysrc/utils/vector_search.pysrc/utils/agents/query.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use absolute imports for internal Python modules.
All modules must start with descriptive docstrings; uselogger = get_logger(__name__)fromlog.pyfor module logging.
Define shared constants in the centralconstants.pymodule, check it before adding constants, and annotate constants withFinal[type].
Define type aliases at module level for clarity.
Use complete type annotations for function parameters and return types; prefer modern union syntax (str | int) andOptional[Type]for optional values.
Usetyping_extensions.Selffor model validators.
Use descriptive, action-oriented snake_case function names, such asget_,validate_, andcheck_.
Functions must not modify mutable parameters in place; return a new data structure instead.
Useasync deffor I/O operations and external API calls.
HandleAPIConnectionErrorfrom Llama Stack.
Usefrom log import get_loggerand initialize module logging withlogger = get_logger(__name__).
Use standard log levels appropriately:debugfor diagnostics,infofor general execution,warningfor unexpected or potential problems, anderrorfor serious failures.
All classes must have descriptive docstrings, use PascalCase names, and use standard suffixes such asConfiguration,Error/Exception,Resolver, andInterfacewhere applicable.
Pydantic configuration models must extendConfigurationBase; data models must extendBaseModel.
Abstract interfaces must useABCwith@abstractmethoddecorators.
Use@model_validatorand@field_validatorfor Pydantic model validation.
Class attributes must have complete, specific type annotations; avoidAny.
Follow Google Python docstring conventions; provide required docstrings for modules, classes, and functions, including relevantParameters,Returns,Raises, andAttributessections.
Files:
src/utils/shields.pysrc/utils/otel_tracing.pysrc/app/endpoints/query.pysrc/utils/quota_utils.pysrc/utils/vector_search.pysrc/utils/agents/query.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (AGENTS.md)
Run and satisfy the configured Black, Pylint, Pyright, Ruff, pydocstyle, and type-checking standards.
Files:
src/utils/shields.pysrc/utils/otel_tracing.pysrc/app/endpoints/query.pytests/unit/utils/test_otel_tracing.pysrc/utils/quota_utils.pysrc/utils/vector_search.pysrc/utils/agents/query.py
src/app/endpoints/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use FastAPI
HTTPExceptionwith appropriate status codes for API endpoints.
Files:
src/app/endpoints/query.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/unit/**/*.py: Use pytest for all unit tests; do not use unittest.
Useconftest.pyfor shared pytest fixtures andpytest-mockforAsyncMockobjects.
Mark asynchronous pytest tests withpytest.mark.asyncio.
Files:
tests/unit/utils/test_otel_tracing.py
🧠 Learnings (5)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
src/utils/shields.pysrc/utils/otel_tracing.pysrc/app/endpoints/query.pytests/unit/utils/test_otel_tracing.pysrc/utils/quota_utils.pysrc/utils/vector_search.pysrc/utils/agents/query.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/utils/shields.pysrc/utils/otel_tracing.pysrc/app/endpoints/query.pysrc/utils/quota_utils.pysrc/utils/vector_search.pysrc/utils/agents/query.py
📚 Learning: 2026-07-17T19:25:05.325Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2166
File: src/utils/saved_prompts.py:129-157
Timestamp: 2026-07-17T19:25:05.325Z
Learning: For any endpoint that handles saved prompts and calls `src/utils/saved_prompts.py::create_saved_prompt`, treat the endpoint as the validation boundary. Before calling `create_saved_prompt`, validate the incoming saved-prompt name and content, specifically using `validate_saved_prompt_name` and then persist (store) the normalized value it returns. Do not call `create_saved_prompt` with unvalidated/raw name/content.
Applied to files:
src/utils/shields.pysrc/utils/otel_tracing.pysrc/app/endpoints/query.pysrc/utils/quota_utils.pysrc/utils/vector_search.pysrc/utils/agents/query.py
📚 Learning: 2026-01-14T09:37:51.612Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 988
File: src/app/endpoints/query.py:319-339
Timestamp: 2026-01-14T09:37:51.612Z
Learning: In the lightspeed-stack repository, when provider_id == "azure", the Azure provider with provider_type "remote::azure" is guaranteed to be present in the providers list. Therefore, avoid defensive StopIteration handling for next() when locating the Azure provider in providers within src/app/endpoints/query.py. This change applies specifically to this file (or nearby provider lookup code) and relies on the invariant that the Azure provider exists; if the invariant could be violated, keep the existing StopIteration handling.
Applied to files:
src/app/endpoints/query.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/query.py
🪛 ast-grep (0.45.0)
tests/unit/utils/test_otel_tracing.py
[warning] 148-148: Do not make http calls without encryption
Context: "http://example.com/doc1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 149-149: Do not make http calls without encryption
Context: "http://example.com/doc2"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 160-160: Do not make http calls without encryption
Context: "http://example.com/doc1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 161-161: Do not make http calls without encryption
Context: "http://example.com/doc2"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
🔇 Additional comments (5)
src/app/endpoints/query.py (1)
7-7: LGTM!Also applies to: 42-48, 67-67, 125-169, 188-190, 335-348
src/utils/quota_utils.py (1)
8-8: LGTM!Also applies to: 18-21, 85-103
src/utils/shields.py (1)
7-7: LGTM!Also applies to: 30-33, 157-161
src/utils/vector_search.py (1)
723-729: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSensitive Data Exposure (CWE-532): Insertion of Sensitive Information into Log File
Do not export raw document URLs in tracing attributes.
At Line 727,
doc.doc_urlis copied intoSpanAttributes.RAG_SOURCESwithout anonymization. Retrieved URLs can contain internal hosts, tenant identifiers, or signed query parameters. Telemetry storage can retain this data outside the request authorization boundary. Export a count or a redacted stable identifier instead.#!/bin/bash set -euo pipefail # Inspect the trace-attribute helper and the document URL contract. ast-grep outline src/utils/otel_tracing.py --items all rg -n -C 5 'def set_span_attributes|RAG_SOURCES|doc_url|class ReferencedDocument' \ src tests # Inspect configured trace exporters and their access-related configuration. rg -n -C 4 'OTLP|SpanExporter|TracerProvider|trace.*export|opentelemetry' \ src configuration* pyproject.toml uv.lock 2>/dev/null || truesrc/utils/otel_tracing.py (1)
13-47: Move the tracing contract toconstants.py.
SpanAttributesandSpanEventsremain shared contracts in this module. Their values also lackFinal[str]annotations. The prior review already contains the required remediation.As per coding guidelines, shared constants must be defined in
constants.pyand annotated withFinal[type].Source: Coding guidelines
| with tracer.start_as_current_span("llm.inference") as span: | ||
| # Extract provider and model from model_id | ||
| provider_id, model_id = extract_provider_and_model_from_model_id( | ||
| responses_params.model | ||
| ) | ||
| return TurnSummary( | ||
| id=moderation_result.moderation_id, | ||
| llm_response=moderation_result.message, | ||
| ) | ||
| try: | ||
| agent = build_agent( | ||
| client, | ||
| responses_params, | ||
| configuration, | ||
| shields=shield_ids, | ||
| no_tools=no_tools, | ||
|
|
||
| # Set LLM attributes | ||
| set_span_attributes( | ||
| span, | ||
| { | ||
| SpanAttributes.LLM_MODEL_ID: model_id, | ||
| SpanAttributes.LLM_PROVIDER_ID: provider_id, | ||
| }, | ||
| ) | ||
| logger.debug("Starting agent non-streaming response processing") | ||
| if image_attachments: | ||
| prompt = build_multimodal_input( | ||
| cast(str, responses_params.input), | ||
| image_attachments, | ||
|
|
||
| if moderation_result.decision == "blocked": | ||
| await append_turn_items_to_conversation( | ||
| client, | ||
| responses_params.conversation, | ||
| responses_params.input, | ||
| [moderation_result.refusal_response], | ||
| ) | ||
| else: | ||
| prompt = cast(str, responses_params.input) | ||
| run_result = await agent.run(prompt) | ||
| except (AgentRunError, APIStatusError, APIConnectionError, RuntimeError) as exc: | ||
| response = map_agent_inference_error(exc, responses_params.model) | ||
| raise HTTPException(**response.model_dump()) from exc | ||
|
|
||
| vector_store_ids = extract_vector_store_ids_from_tools(responses_params.tools) | ||
| rag_id_mapping = configuration.rag_id_mapping | ||
| return build_turn_summary_from_agent_run( | ||
| run_result, | ||
| model_id=responses_params.model, | ||
| endpoint_path=endpoint_path, | ||
| vector_store_ids=vector_store_ids, | ||
| rag_id_mapping=rag_id_mapping, | ||
| ) | ||
| return TurnSummary( | ||
| id=moderation_result.moderation_id, | ||
| llm_response=moderation_result.message, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not create an inference span for a moderated request.
At Line 263, llm.inference starts before the blocked-moderation branch. That branch returns without calling agent.run, so the trace reports an LLM inference that did not occur. Handle the blocked branch before starting llm.inference.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/agents/query.py` around lines 263 - 288, Move the
blocked-moderation handling in the request flow before the
tracer.start_as_current_span("llm.inference") block. Preserve the existing
append_turn_items_to_conversation call and TurnSummary return, and only create
the inference span for requests that proceed to agent.run.
| digest = hashlib.sha256(value.encode("utf-8")).hexdigest()[:8] | ||
| length_indicator = "long" if len(value) > max_length else "short" | ||
| return f"[hash:{digest}:{length_indicator}:len={len(value)}]" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Sensitive Data Exposure (CWE-200): Exposure of Sensitive Information to an Unauthorized Actor
Reachability: External
Use a keyed digest with sufficient output length.
src/app/endpoints/query.py passes external user IDs, queries, and LLM responses through this helper. Line 61 exports only a public 32-bit SHA-256 prefix. A telemetry reader can enumerate likely values and match this digest offline. Collisions also break correlation at normal tracing volume.
Use an HMAC with a configuration-managed secret. Export at least 128 bits. Update tests to verify deterministic output without exposing input content.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/otel_tracing.py` around lines 61 - 63, Update the helper producing
the `hash:{digest}` value to use HMAC-SHA-256 with a configuration-managed
secret instead of an unkeyed SHA-256 digest, and export at least 128 bits of
output. Preserve the existing length indicator and formatted return structure,
then update its tests to assert deterministic results without embedding or
exposing the original input content.
| def record_exception( | ||
| span: trace.Span, exception: Exception, attributes: Optional[dict[str, Any]] = None | ||
| ) -> None: | ||
| """Record an exception on a span. | ||
|
|
||
| Parameters: | ||
| span: The OpenTelemetry span to record the exception on. | ||
| exception: The exception to record. | ||
| attributes: Optional additional attributes for the exception event. | ||
| """ | ||
| span.record_exception(exception, attributes=attributes) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 '\brecord_exception\s*\(' src tests
rg -n -C 12 'except .+:' src -g '*.py'Repository: lightspeed-core/lightspeed-stack
Length of output: 50389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg '(^tests/.*otel|src/utils/otel_tracing\.py|telemetry|metrics|endpoint)' | head -200
echo "== direct imports/usages of utils.otel_tracing or record_exception =="
rg -n 'otel_tracing|record_exception|set_status\(' src tests --glob '*.py' | sed -n '1,260p'
echo "== test file candidates containing TestRecordException or otel =="
git ls-files 'tests/**/*.py' | while read -r f; do
if rg -q 'TestRecordException|record_exception|otel|open_tracer|tracer' "$f"; then
echo "--- $f"
rg -n -C 5 'TestRecordException|record_exception|otel|open_tracer|tracer' "$f" | sed -n '1,160p'
fi
done | sed -n '1,260p'
echo "== otel_tracing.py excerpt =="
wc -l src/utils/otel_tracing.py
cat -n src/utils/otel_tracing.py | sed -n '1,140p'Repository: lightspeed-core/lightspeed-stack
Length of output: 20883
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== utils/otel_tracing.py =="
cat -n src/utils/otel_tracing.py | sed -n '92,105p'
echo "== tests/unit/utils/test_otel_tracing.py around TestRecordException =="
wc -l tests/unit/utils/test_otel_tracing.py
cat -n tests/unit/utils/test_otel_tracing.py | sed -n '1,285p'
echo "== src/utils/quota_utils.py call sites =="
cat -n src/utils/quota_utils.py | sed -n '70,115p'
echo "== inspect imported SpanAttributes-only files =="
for f in src/utils/quota_utils.py src/utils/vector_search.py src/app/endpoints/query.py src/utils/agents/query.py; do
echo "--- $f imports/record_exception/status calls"
rg -n -C 4 'record_exception|set_status|try:|except .*Error|with (trace|Span)|start_as_current_span' "$f"
doneRepository: lightspeed-core/lightspeed-stack
Length of output: 21909
Mark handled exceptions as span errors.
src/utils/quota_utils.py calls record_exception() inside except blocks before raising, so the span exits as OK while the failure is only an event. Set StatusCode.ERROR before/after this helper records handled exceptions, and add a status assertion to TestRecordException.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/otel_tracing.py` around lines 92 - 102, Update record_exception to
set the provided span status to StatusCode.ERROR in addition to recording the
exception, ensuring handled exceptions are reflected as span failures; import
the required status symbol and extend TestRecordException with an assertion
verifying the span’s error status.
| byok_chunks_task = _fetch_byok_rag( | ||
| client, | ||
| query, | ||
| vector_store_ids, | ||
| max_chunks=constants.BYOK_RAG_MAX_CHUNKS, | ||
| ) | ||
| solr_chunks_task = _fetch_solr_rag(client, query, solr) | ||
|
|
||
| (byok_chunks, byok_documents), ( | ||
| solr_chunks, | ||
| solr_documents, | ||
| ) = await asyncio.gather(byok_chunks_task, solr_chunks_task) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Record source failures before reporting retrieval completion.
_fetch_byok_rag and _fetch_solr_rag catch exceptions and return empty lists. Therefore, asyncio.gather succeeds and Lines 731-736 emit RAG_RETRIEVAL_COMPLETED even when every source failed. Record a per-source failure attribute or event in the catch paths so the span distinguishes an empty result from failed retrieval.
Also applies to: 731-736
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/vector_search.py` around lines 675 - 686, Update the
`_fetch_byok_rag` and `_fetch_solr_rag` exception handling paths in
`vector_search.py` so they record a source-specific failure marker before
returning empty results. Keep the existing `asyncio.gather` flow in
`vector_search` unchanged, but ensure the span can distinguish a true empty
retrieval from a failed one by emitting a per-source failure event or attribute
from the catch blocks. Then have the `RAG_RETRIEVAL_COMPLETED` reporting logic
rely on that recorded failure state rather than treating all empty results as
successful retrieval.
|
Some screen shots from Jaeger collecting traces from lightspeed-stack The span hierarchy goes deeper here, this is all I could fit in in one screenshot: |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/otel_tracing.py`:
- Around line 73-79: Remove the hardcoded fallback from anonymize_value’s
OTEL_ANONYMIZATION_SECRET lookup and require a non-empty configured secret.
Validate this during startup/configuration loading, fail clearly when the
variable is absent, and add coverage for the missing-secret failure path.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c0298463-8ce8-4fa6-b0df-f5c2da0cf735
📒 Files selected for processing (8)
src/app/endpoints/query.pysrc/constants.pysrc/utils/agents/query.pysrc/utils/otel_tracing.pysrc/utils/quota_utils.pysrc/utils/shields.pysrc/utils/vector_search.pytests/unit/utils/test_otel_tracing.py
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: Pylinter
- GitHub Check: unit_tests (3.12)
- GitHub Check: list_outdated_dependencies
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.
**/*: Useuv runfor project commands; useuv sync --group dev --group llslibdevto install development dependencies.
Before completion, runuv run make format,uv run make verify, create unit tests for new code, and ensure tests pass.
Never commit secrets or keys; use environment variables for sensitive data.
Files:
src/constants.pysrc/utils/vector_search.pysrc/utils/agents/query.pytests/unit/utils/test_otel_tracing.pysrc/app/endpoints/query.pysrc/utils/otel_tracing.pysrc/utils/quota_utils.pysrc/utils/shields.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use absolute imports for internal Python modules.
All modules must start with descriptive docstrings; uselogger = get_logger(__name__)fromlog.pyfor module logging.
Define shared constants in the centralconstants.pymodule, check it before adding constants, and annotate constants withFinal[type].
Define type aliases at module level for clarity.
Use complete type annotations for function parameters and return types; prefer modern union syntax (str | int) andOptional[Type]for optional values.
Usetyping_extensions.Selffor model validators.
Use descriptive, action-oriented snake_case function names, such asget_,validate_, andcheck_.
Functions must not modify mutable parameters in place; return a new data structure instead.
Useasync deffor I/O operations and external API calls.
HandleAPIConnectionErrorfrom Llama Stack.
Usefrom log import get_loggerand initialize module logging withlogger = get_logger(__name__).
Use standard log levels appropriately:debugfor diagnostics,infofor general execution,warningfor unexpected or potential problems, anderrorfor serious failures.
All classes must have descriptive docstrings, use PascalCase names, and use standard suffixes such asConfiguration,Error/Exception,Resolver, andInterfacewhere applicable.
Pydantic configuration models must extendConfigurationBase; data models must extendBaseModel.
Abstract interfaces must useABCwith@abstractmethoddecorators.
Use@model_validatorand@field_validatorfor Pydantic model validation.
Class attributes must have complete, specific type annotations; avoidAny.
Follow Google Python docstring conventions; provide required docstrings for modules, classes, and functions, including relevantParameters,Returns,Raises, andAttributessections.
Files:
src/constants.pysrc/utils/vector_search.pysrc/utils/agents/query.pysrc/app/endpoints/query.pysrc/utils/otel_tracing.pysrc/utils/quota_utils.pysrc/utils/shields.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (AGENTS.md)
Run and satisfy the configured Black, Pylint, Pyright, Ruff, pydocstyle, and type-checking standards.
Files:
src/constants.pysrc/utils/vector_search.pysrc/utils/agents/query.pytests/unit/utils/test_otel_tracing.pysrc/app/endpoints/query.pysrc/utils/otel_tracing.pysrc/utils/quota_utils.pysrc/utils/shields.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/unit/**/*.py: Use pytest for all unit tests; do not use unittest.
Useconftest.pyfor shared pytest fixtures andpytest-mockforAsyncMockobjects.
Mark asynchronous pytest tests withpytest.mark.asyncio.
Files:
tests/unit/utils/test_otel_tracing.py
src/app/endpoints/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use FastAPI
HTTPExceptionwith appropriate status codes for API endpoints.
Files:
src/app/endpoints/query.py
🧠 Learnings (5)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
src/constants.pysrc/utils/vector_search.pysrc/utils/agents/query.pytests/unit/utils/test_otel_tracing.pysrc/app/endpoints/query.pysrc/utils/otel_tracing.pysrc/utils/quota_utils.pysrc/utils/shields.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/constants.pysrc/utils/vector_search.pysrc/utils/agents/query.pysrc/app/endpoints/query.pysrc/utils/otel_tracing.pysrc/utils/quota_utils.pysrc/utils/shields.py
📚 Learning: 2026-07-17T19:25:05.325Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2166
File: src/utils/saved_prompts.py:129-157
Timestamp: 2026-07-17T19:25:05.325Z
Learning: For any endpoint that handles saved prompts and calls `src/utils/saved_prompts.py::create_saved_prompt`, treat the endpoint as the validation boundary. Before calling `create_saved_prompt`, validate the incoming saved-prompt name and content, specifically using `validate_saved_prompt_name` and then persist (store) the normalized value it returns. Do not call `create_saved_prompt` with unvalidated/raw name/content.
Applied to files:
src/constants.pysrc/utils/vector_search.pysrc/utils/agents/query.pysrc/app/endpoints/query.pysrc/utils/otel_tracing.pysrc/utils/quota_utils.pysrc/utils/shields.py
📚 Learning: 2026-01-14T09:37:51.612Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 988
File: src/app/endpoints/query.py:319-339
Timestamp: 2026-01-14T09:37:51.612Z
Learning: In the lightspeed-stack repository, when provider_id == "azure", the Azure provider with provider_type "remote::azure" is guaranteed to be present in the providers list. Therefore, avoid defensive StopIteration handling for next() when locating the Azure provider in providers within src/app/endpoints/query.py. This change applies specifically to this file (or nearby provider lookup code) and relies on the invariant that the Azure provider exists; if the invariant could be violated, keep the existing StopIteration handling.
Applied to files:
src/app/endpoints/query.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/query.py
🪛 ast-grep (0.45.0)
tests/unit/utils/test_otel_tracing.py
[warning] 166-166: Do not make http calls without encryption
Context: "http://example.com/doc1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 167-167: Do not make http calls without encryption
Context: "http://example.com/doc2"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 178-178: Do not make http calls without encryption
Context: "http://example.com/doc1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 179-179: Do not make http calls without encryption
Context: "http://example.com/doc2"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
🔇 Additional comments (11)
src/utils/otel_tracing.py (1)
20-54: Move the tracing contract toconstants.py.
SpanAttributesandSpanEventsare shared constants. They remain outsideconstants.py, and their fields lackFinal[str]annotations.Source: Coding guidelines
tests/unit/utils/test_otel_tracing.py (2)
60-66: Test the actualmax_lengthboundary.This test uses a 56-character value. Assert that length 50 is
shortand length 51 islong.
20-58: LGTM!Also applies to: 68-305
src/utils/agents/query.py (2)
263-288: Do not createllm.inferencefor blocked moderation.The span starts before the blocked branch. That branch returns without calling
agent.run, but the trace reports LLM inference.
10-10: LGTM!Also applies to: 40-55, 191-225, 290-342
src/utils/vector_search.py (2)
731-736: Distinguish retrieval failure from an empty result.The source helpers catch failures and return empty collections. This event can report successful retrieval when every source failed.
16-36: LGTM!Also applies to: 664-729, 738-742
src/constants.py (1)
262-264: LGTM!src/app/endpoints/query.py (1)
7-7: LGTM!Also applies to: 42-48, 67-67, 125-189, 335-348
src/utils/quota_utils.py (1)
8-8: LGTM!Also applies to: 18-21, 85-103
src/utils/shields.py (1)
7-7: LGTM!Also applies to: 30-33, 157-161
| # Get HMAC secret from environment or use development default | ||
| secret = os.environ.get( | ||
| OTEL_ANONYMIZATION_SECRET_ENV_VAR, "dev-otel-secret-change-in-production" | ||
| ) | ||
| # Compute HMAC-SHA-256 and take first 16 hex chars (64 bits / 128 bits of security) | ||
| mac = hmac.new(secret.encode("utf-8"), value.encode("utf-8"), hashlib.sha256) | ||
| digest = mac.hexdigest()[:16] # 16 hex chars = 64 bits |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline =="
ast-grep outline src/utils/otel_tracing.py --view expanded || true
echo "== relevant otel_tracing.py =="
sed -n '1,140p' src/utils/otel_tracing.py | cat -n
echo "== tests around secret/env/anonymize =="
sed -n '1,180p' tests/unit/utils/test_otel_tracing.py | cat -n
echo "== imports/usages of anonymize_value and otel initialization =="
rg -n "anonymize_value|OTEL_ANONYMIZATION_SECRET|setup_.*otel|otel_tracing|add_span_attributes|set_span_attributes" src tests -SRepository: lightspeed-core/lightspeed-stack
Length of output: 19581
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import hmac, hashlib, os
secret = "dev-otel-secret-change-in-production"
value = "llm.response.content"
mac = hmac.new(secret.encode("utf-8"), value.encode("utf-8"), hashlib.sha256)
print("fallback_digest", mac.hexdigest()[:16])
value2 = "SensitiveUserInput"
mac2 = hmac.new(secret.encode("utf-8"), value2.encode("utf-8"), hashlib.sha256)
print("fallback_with_sensitivity", mac2.hexdigest()[:16])
print("deterministic_same_value_same_secret", mac.hexdigest()[:16] == hmac.new(secret.encode("utf-8"), value.encode("utf-8"), hashlib.sha256).hexdigest()[:16])
print(
"digest_excludes_sensitive_value" if "SensitiveUserInput" not in mac2.hexdigest()[:16] else "sensitive_value_appears_in_digest"
)
PYRepository: lightspeed-core/lightspeed-stack
Length of output: 320
Weak Cryptography (CWE-798): Use of Hard-coded Credentials
Reachability: External
Reachability path
● Entry
src/app/endpoints/query.py:268
retrieve_agent_response
│
▼
● Hop
src/utils/agents/query.py:157
build_turn_summary_from_agent_run: Build a turn summary from a completed agent run. Args: run_result: Completed agent run result. model_id: Model identifier in provider/mod…
│
▼
● Sink
src/utils/otel_tracing.py
Require a configured anonymization secret.
The hardcoded fallback key is reproducible for anyone with the source. When OTEL_ANONYMIZATION_SECRET is absent, anonymize_value() hashes user IDs, query input, and response output with that secret before attaching them to exported spans. Reject the missing secret during startup/configuration loading, and add a test for that failure path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/otel_tracing.py` around lines 73 - 79, Remove the hardcoded
fallback from anonymize_value’s OTEL_ANONYMIZATION_SECRET lookup and require a
non-empty configured secret. Validate this during startup/configuration loading,
fail clearly when the variable is absent, and add coverage for the
missing-secret failure path.
Source: Coding guidelines
There was a problem hiding this comment.
Addressed in the latest commit.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
asimurka
left a comment
asimurka
left a comment
There was a problem hiding this comment.
LGTM in overall, I have just some nits and questions.
|
|
||
| # Emit final events | ||
| add_span_event(root_span, SpanEvents.LLM_RESPONSE_COMPLETED) | ||
| add_span_event(root_span, SpanEvents.TURN_PERSISTED) |
There was a problem hiding this comment.
I would relocate right after store_query_results call, so it won't get separated by another expensive step in future.
| logger = get_logger(__name__) | ||
|
|
||
|
|
||
| class SpanAttributes: # pylint: disable=too-few-public-methods |
There was a problem hiding this comment.
Possibly use StrEnum for both classes
| # Get HMAC secret from environment or use development default | ||
| secret = os.environ.get( | ||
| OTEL_ANONYMIZATION_SECRET_ENV_VAR, "dev-otel-secret-change-in-production" | ||
| ) | ||
| # Compute HMAC-SHA-256 and take first 16 hex chars (64 bits / 128 bits of security) | ||
| mac = hmac.new(secret.encode("utf-8"), value.encode("utf-8"), hashlib.sha256) | ||
| digest = mac.hexdigest()[:16] # 16 hex chars = 64 bits |
| root_span, | ||
| { | ||
| SpanAttributes.USER_ID: anonymize_value(user_id), | ||
| SpanAttributes.INPUT: anonymize_value(query_request.query), |
There was a problem hiding this comment.
Despite the ticket says that we should anonymize input & output, I'm not sure if it makes sense. Wdyt?
There was a problem hiding this comment.
It's an interesting piece of design to think about. I like the anonymization route to start us off with, because that starts us out more restrictive to begin with, that we can then make less restrictive if we get requests to make them less restrictive.
So eg, with anonymization, a trace looks like:
WITH Anonymization:
{
"trace_id": "abc123",
"span_name": "query.handle_request",
"attributes": {
"user.id": "[hash:8a4f3d2e1c9b7a5f:short:len=24]",
"request.input": "[hash:f9e2d3c4b5a6f7e8:long:len=95]",
"request.attachments.count": 0,
"session.id": "conv-456",
"llm.model.id": "gpt-4o-mini",
"llm.provider.id": "openai",
"llm.usage.input_tokens": 45,
"llm.usage.output_tokens": 120,
"response.output": "[hash:a7f4e9d2c8b3f1a6:long:len=158]"
}
}
☝🏽 that's what implemented now.
Without anonymization, the trace will look like:
{
"trace_id": "abc123",
"span_name": "query.handle_request",
"attributes": {
"user.id": "jane.smith@company.com",
"request.input": "I need to create a secret for my database password 'MyS3cr3tP@ss!' in the prod namespace",
"request.attachments.count": 0,
"session.id": "conv-456",
"llm.model.id": "gpt-4o-mini",
"llm.provider.id": "openai",
"llm.usage.input_tokens": 45,
"llm.usage.output_tokens": 120,
"response.output": "To create a Kubernetes secret, run: kubectl create secret generic db-password
--from-literal=password='MyS3cr3tP@ss!' -n prod"
}
}
ie without anonymization there's potential for leaks. It's worth noting that even with anonymization, the following information is available:
- Session ID - Can track conversation flow
- Model and Provider - Know which LLM was used
- Token counts - Track usage and cost
- Attachments count - Know if files were uploaded
- Tool calls - See what tools were invoked
- RAG sources count - How many documents retrieved
- Timestamps - When things happened
- Error status - If something failed
That should be enough info to help with debugging, while preventing potential leaks, no?
There was a problem hiding this comment.
It's also worth mentioning that one way to go about this would be:
- Start off with anonymization
- If there are requests from some users to see the traces without anonymization, introduce a knob that can be turned on/off in the config file.
Something like
otel:
anonymize_sensitive_data: true # Default to safe
…oint
Implements comprehensive OpenTelemetry (OTEL) tracing instrumentation for the POST
/v1/query endpoint to enable distributed tracing and observability.
**Instrumented Components**
1. Query Endpoint Handler (`src/app/endpoints/query.py`)
2. Quota Check (`src/utils/quota_utils.py`)
3. Shield Moderation (`src/utils/shields.py`)
4. RAG Retrieval (`src/utils/vector_search.py`)
5. LLM Inference (`src/utils/agents/query.py`)
6. Tool Execution (`src/utils/agents/query.py`)
**Span Hierarchy**
```
query.handle_request (root span)
├── quota.check
├── shield.moderate
├── rag.retrieve
└── llm.inference
└── tool.execution (attributes only)
```
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/quota_utils.py`:
- Around line 85-103: Disable automatic exception recording when creating the
`quota.check` span via `tracer.start_as_current_span`, while preserving the
explicit `record_exception` calls in the database and `QuotaExceedError`
handlers so each failure records only one exception event.
In `@src/utils/vector_search.py`:
- Around line 722-729: Update the RAG attribute construction in the
vector-search flow to stop exporting raw doc_url values. Filter out missing URLs
and transform each remaining source into the project’s established anonymized
source identifier before assigning SpanAttributes.RAG_SOURCES; keep the count
aligned with the filtered sources.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2a9feb81-be82-4b4d-8cdd-ed57327ab6a6
📒 Files selected for processing (8)
src/app/endpoints/query.pysrc/constants.pysrc/utils/agents/query.pysrc/utils/otel_tracing.pysrc/utils/quota_utils.pysrc/utils/shields.pysrc/utils/vector_search.pytests/unit/utils/test_otel_tracing.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
🧰 Additional context used
📓 Path-based instructions (6)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.
**/*: Useuvfor dependency management and prefix project commands withuv run; synchronize development dependencies withuv sync --group dev --group llslibdevwhen required.
Before completing code changes, runuv run make format,uv run make verify, and the applicable tests; regenerate the OpenAPI schema when models change, and report completion only after checks pass.
Follow existing code patterns in the module being modified and write unit tests covering new functionality.
Files:
src/constants.pysrc/utils/vector_search.pysrc/utils/quota_utils.pysrc/utils/shields.pysrc/utils/otel_tracing.pysrc/app/endpoints/query.pytests/unit/utils/test_otel_tracing.pysrc/utils/agents/query.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use absolute imports for internal Python modules.
All modules must begin with descriptive docstrings; package__init__.pyfiles must contain brief package descriptions.
Uselogger = get_logger(__name__)fromlog.pyfor module logging.
UseFinal[type]type hints for constants and define shared constants in the centralconstants.pymodule.
Files:
src/constants.pysrc/utils/vector_search.pysrc/utils/quota_utils.pysrc/utils/shields.pysrc/utils/otel_tracing.pysrc/app/endpoints/query.pytests/unit/utils/test_otel_tracing.pysrc/utils/agents/query.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Functions must have descriptive docstrings, complete parameter and return type annotations, and descriptive snake_case action-oriented names.
Usetyping_extensions.Selffor model validators, modern union syntax such asstr | int, andOptional[Type]for optional types.
Avoid modifying mutable parameters in place; return a new data structure instead.
Useasync deffor I/O operations and external API calls.
Classes require descriptive docstrings, PascalCase descriptive names, complete attribute type annotations, and specific types instead ofAny. UseABCand@abstractmethodfor interfaces.
Follow Google Python docstring conventions and include relevantParameters,Returns,Raises, andAttributessections.
Files:
src/constants.pysrc/utils/vector_search.pysrc/utils/quota_utils.pysrc/utils/shields.pysrc/utils/otel_tracing.pysrc/app/endpoints/query.pysrc/utils/agents/query.py
**/*.{py,yaml,yml,json,toml}
📄 CodeRabbit inference engine (AGENTS.md)
Never commit secrets or keys; use environment variables for sensitive data.
Files:
src/constants.pysrc/utils/vector_search.pysrc/utils/quota_utils.pysrc/utils/shields.pysrc/utils/otel_tracing.pysrc/app/endpoints/query.pytests/unit/utils/test_otel_tracing.pysrc/utils/agents/query.py
src/app/endpoints/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use FastAPI
HTTPExceptionwith appropriate status codes for API endpoint errors, and handleAPIConnectionErrorfrom Llama Stack.
Files:
src/app/endpoints/query.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use pytest for unit tests; do not use unittest. Put shared fixtures in
conftest.py, usepytest-mockfor mocks, mark asynchronous tests withpytest.mark.asyncio, and maintain at least 60% unit-test coverage.
Files:
tests/unit/utils/test_otel_tracing.py
🧠 Learnings (5)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
src/constants.pysrc/utils/vector_search.pysrc/utils/quota_utils.pysrc/utils/shields.pysrc/utils/otel_tracing.pysrc/app/endpoints/query.pytests/unit/utils/test_otel_tracing.pysrc/utils/agents/query.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/constants.pysrc/utils/vector_search.pysrc/utils/quota_utils.pysrc/utils/shields.pysrc/utils/otel_tracing.pysrc/app/endpoints/query.pysrc/utils/agents/query.py
📚 Learning: 2026-07-17T19:25:05.325Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2166
File: src/utils/saved_prompts.py:129-157
Timestamp: 2026-07-17T19:25:05.325Z
Learning: For any endpoint that handles saved prompts and calls `src/utils/saved_prompts.py::create_saved_prompt`, treat the endpoint as the validation boundary. Before calling `create_saved_prompt`, validate the incoming saved-prompt name and content, specifically using `validate_saved_prompt_name` and then persist (store) the normalized value it returns. Do not call `create_saved_prompt` with unvalidated/raw name/content.
Applied to files:
src/constants.pysrc/utils/vector_search.pysrc/utils/quota_utils.pysrc/utils/shields.pysrc/utils/otel_tracing.pysrc/app/endpoints/query.pysrc/utils/agents/query.py
📚 Learning: 2026-01-14T09:37:51.612Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 988
File: src/app/endpoints/query.py:319-339
Timestamp: 2026-01-14T09:37:51.612Z
Learning: In the lightspeed-stack repository, when provider_id == "azure", the Azure provider with provider_type "remote::azure" is guaranteed to be present in the providers list. Therefore, avoid defensive StopIteration handling for next() when locating the Azure provider in providers within src/app/endpoints/query.py. This change applies specifically to this file (or nearby provider lookup code) and relies on the invariant that the Azure provider exists; if the invariant could be violated, keep the existing StopIteration handling.
Applied to files:
src/app/endpoints/query.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/query.py
🪛 ast-grep (0.45.0)
tests/unit/utils/test_otel_tracing.py
[warning] 190-190: Do not make http calls without encryption
Context: "http://example.com/doc1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 191-191: Do not make http calls without encryption
Context: "http://example.com/doc2"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 202-202: Do not make http calls without encryption
Context: "http://example.com/doc1"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 203-203: Do not make http calls without encryption
Context: "http://example.com/doc2"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
🔇 Additional comments (6)
src/constants.py (1)
262-264: LGTM!src/utils/otel_tracing.py (1)
92-115: LGTM!tests/unit/utils/test_otel_tracing.py (1)
68-82: LGTM!Also applies to: 137-145
src/app/endpoints/query.py (1)
188-190: LGTM!src/utils/agents/query.py (1)
191-225: LGTM!src/utils/shields.py (1)
157-161: LGTM!
| with tracer.start_as_current_span("quota.check") as span: | ||
| try: | ||
| # check available tokens using all configured quota limiters | ||
| for quota_limiter in quota_limiters: | ||
| quota_limiter.ensure_available_quota(subject_id=user_id) | ||
| span.set_attribute(SpanAttributes.QUOTA_CHECK_PASSED, True) | ||
| except (psycopg2.Error, sqlite3.Error) as pg_error: | ||
| message = "Error communicating with quota database backend" | ||
| logger.error(message) | ||
| span.set_attribute(SpanAttributes.QUOTA_CHECK_PASSED, False) | ||
| record_exception(span, pg_error) | ||
| response = InternalServerErrorResponse.database_error() | ||
| raise HTTPException(**response.model_dump()) from pg_error | ||
| except QuotaExceedError as e: | ||
| logger.error("The quota has been exceeded") | ||
| span.set_attribute(SpanAttributes.QUOTA_CHECK_PASSED, False) | ||
| record_exception(span, e) | ||
| response = QuotaExceededResponse.from_exception(e) | ||
| raise HTTPException(**response.model_dump()) from e |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '80,108p' src/utils/quota_utils.py
rg -n -C 5 'check_tokens_available|quota\.check|record_exception' tests src --glob '*.py'Repository: lightspeed-core/lightspeed-stack
Length of output: 36508
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the OpenTelemetry tracing helper without executing repository code.
sed -n '1,150p' src/utils/otel_tracing.py
# Inspect dependency declarations for OpenTelemetry Python package pins.
for f in pyproject.toml uv.lock; do
[ -f "$f" ] && { echo "=== $f ==="; sed -n '/opentelemetry/p' "$f"; }
doneRepository: lightspeed-core/lightspeed-stack
Length of output: 17026
🌐 Web query:
OpenTelemetry Python start_as_current_span record_exception default True
💡 Result:
In OpenTelemetry Python, the start_as_current_span method defaults to True for the record_exception parameter [1][2][3]. When record_exception is set to True (the default), the tracer automatically records any uncaught exceptions raised within the context manager block as an event on the span [1][3]. This functionality is coupled with the set_status_on_exception parameter, which also defaults to True and determines whether the span status is automatically set to ERROR when an exception occurs [1][2][3]. These behaviors are standard in the OpenTelemetry Python SDK [4].
Citations:
- 1: https://opentelemetry-python.readthedocs.io/en/stable/api/trace.html
- 2: https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-api/src/opentelemetry/trace/__init__.py
- 3: https://opentelemetry-python.readthedocs.io/en/latest/%5Fmodules/opentelemetry/trace.html
- 4: Document the default behavior on whether Trace SDK (and sugar around it) should record exceptions open-telemetry/opentelemetry-specification#4455
Disable automatic exception recording on quota.check.
record_exception() already adds one exception event for the database or quota failure. Since start_as_current_span() records uncaught exceptions by default, the re-raised HTTPException adds a second exception event on the same quota-span failure.
Proposed fix
-with tracer.start_as_current_span("quota.check") as span:
+with tracer.start_as_current_span(
+ "quota.check",
+ record_exception=False,
+) as span:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| with tracer.start_as_current_span("quota.check") as span: | |
| try: | |
| # check available tokens using all configured quota limiters | |
| for quota_limiter in quota_limiters: | |
| quota_limiter.ensure_available_quota(subject_id=user_id) | |
| span.set_attribute(SpanAttributes.QUOTA_CHECK_PASSED, True) | |
| except (psycopg2.Error, sqlite3.Error) as pg_error: | |
| message = "Error communicating with quota database backend" | |
| logger.error(message) | |
| span.set_attribute(SpanAttributes.QUOTA_CHECK_PASSED, False) | |
| record_exception(span, pg_error) | |
| response = InternalServerErrorResponse.database_error() | |
| raise HTTPException(**response.model_dump()) from pg_error | |
| except QuotaExceedError as e: | |
| logger.error("The quota has been exceeded") | |
| span.set_attribute(SpanAttributes.QUOTA_CHECK_PASSED, False) | |
| record_exception(span, e) | |
| response = QuotaExceededResponse.from_exception(e) | |
| raise HTTPException(**response.model_dump()) from e | |
| with tracer.start_as_current_span( | |
| "quota.check", | |
| record_exception=False, | |
| ) as span: | |
| try: | |
| # check available tokens using all configured quota limiters | |
| for quota_limiter in quota_limiters: | |
| quota_limiter.ensure_available_quota(subject_id=user_id) | |
| span.set_attribute(SpanAttributes.QUOTA_CHECK_PASSED, True) | |
| except (psycopg2.Error, sqlite3.Error) as pg_error: | |
| message = "Error communicating with quota database backend" | |
| logger.error(message) | |
| span.set_attribute(SpanAttributes.QUOTA_CHECK_PASSED, False) | |
| record_exception(span, pg_error) | |
| response = InternalServerErrorResponse.database_error() | |
| raise HTTPException(**response.model_dump()) from pg_error | |
| except QuotaExceedError as e: | |
| logger.error("The quota has been exceeded") | |
| span.set_attribute(SpanAttributes.QUOTA_CHECK_PASSED, False) | |
| record_exception(span, e) | |
| response = QuotaExceededResponse.from_exception(e) | |
| raise HTTPException(**response.model_dump()) from e |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/quota_utils.py` around lines 85 - 103, Disable automatic exception
recording when creating the `quota.check` span via
`tracer.start_as_current_span`, while preserving the explicit `record_exception`
calls in the database and `QuotaExceedError` handlers so each failure records
only one exception event.
Source: Coding guidelines
| # Set RAG attributes | ||
| set_span_attributes( | ||
| span, | ||
| { | ||
| SpanAttributes.RAG_SOURCES_COUNT: len(top_documents), | ||
| SpanAttributes.RAG_SOURCES: [doc.doc_url for doc in top_documents], | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'class ReferencedDocument|doc_url.*:' src --glob '*.py'
rg -n -C 5 'RAG_SOURCES|doc\.doc_url|ReferencedDocument\(' src tests --glob '*.py'Repository: lightspeed-core/lightspeed-stack
Length of output: 50388
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- VectorSearch relevant functions and constants ---'
wc -l src/utils/vector_search.py src/utils/otel_tracing.py
sed -n '330,860p' src/utils/vector_search.py
sed -n '1,90p' src/utils/otel_tracing.py
echo '--- Relevant tests around vector_search retrieval/filtering ---'
rg -n -C 5 '_filter_documents_for_chunks|_build.*referenced.*document|ReferencedDocument\(\)|RAG_SOURCES|sources_count\)|span\.attributes|span\.get_attribute' tests/unit tests/integration --glob '*.py' | sed -n '1,260p'Repository: lightspeed-core/lightspeed-stack
Length of output: 30896
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Static import and helper references ---'
python3 - <<'PY'
import ast
from pathlib import Path
p = Path('src/utils/vector_search.py')
tree = ast.parse(p.read_text())
print('vector_search.py functions/classes:')
for n in ast.walk(tree):
if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
if n.name in {'_filter_documents_for_chunks', '_process_byok_rag_chunks_for_documents', '_join_okp_doc_url', 'run_internal_vector_search'}:
print(f'{n.name} line {n.lineno}')
for filename in ['src/utils/otel_tracing.py', 'src/utils/vector_search.py']:
text = Path(filename).read_text()
print(f'\n--- {filename}: helper references in set_span_attributes caller area ---')
lines=text.splitlines()
for i,l in enumerate(lines,1):
if 'set_span_attributes' in l or 'SpanAttributes.RAG_SOURCES' in l or 'document_id' in l or 'source=' in l:
print(f'{i}: {l}')
PY
echo '--- Read behavioral probe for list comprehension values from model defaults ---'
python3 - <<'PY'
from pathlib import Path
import re
text=Path('src/models/common/turn_summary.py').read_text()
m=re.search(r'class ReferencedDocument\(BaseModel\):(.*?)(?=class \w+|$)', text, re.S)
print(m.group(1) if m else 'ReferencedDocument class not found')
vec=Path('src/utils/vector_search.py').read_text()
idx=vec.find('from src.models.common.turn_summary import ReferencedDocument')
print('ReferencedDocument import:', idx!=-1)
idx=vec.find('ReferencedDocument(doc_title=title,')
print('ReferencedDocument construction at', vec[:idx].rfind('\n'))
print(vec[idx:idx+450] if idx!=-1 else 'construction not found')
PYRepository: lightspeed-core/lightspeed-stack
Length of output: 2934
🌐 Web query:
OpenTelemetry Python attribute sequence value array unsupported null None
💡 Result:
In OpenTelemetry Python, the handling of 'None' (null) values in attributes and attribute sequences has evolved. Historically, 'None' values were considered invalid or undefined behavior [1][2][3]. However, the OpenTelemetry specification and the Python SDK have been updated to support 'None' as a valid attribute value [4][5][6]. Key details regarding current support: 1. Official Support: The OpenTelemetry specification now recognizes 'None' as a valid attribute value [5][7]. Recent updates to the OpenTelemetry Python SDK (e.g., Pull Requests #5266 and #5305) have expanded support to ensure that 'None' is accepted across the API, SDK, and encoders [4][6]. 2. Sequence Handling: While 'None' is a valid value, its use within homogeneous arrays/sequences is generally discouraged unless language constraints make it unavoidable [5][7]. If present, 'None' values within arrays must be preserved as-is by processors and exporters [5][7]. 3. Implementation: In the Python SDK, attribute values are processed by utility functions (such as those in opentelemetry.attributes) that ensure types are valid and homogeneous [8]. While historical issues existed where 'None' caused encoding failures or 'AttributeError' exceptions [1][9], these have been addressed by ensuring encoders correctly handle the 'None' type by encoding it as an empty 'AnyValue' [6]. If you are encountering issues with 'None' values, ensure your OpenTelemetry Python SDK is updated to the latest version, as older versions may lack this support and trigger validation errors or crashes [1]. If you are working with custom instrumentations, verify that your code conforms to the current attribute value types, which now include 'None' alongside standard primitives and sequences [4][8].
Citations:
- 1: 'None' values cause exporting to fail open-telemetry/opentelemetry-python#3677
- 2: Document None attribute values as undefined behavior open-telemetry/opentelemetry-python#1361
- 3: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.48.0/specification/common/README.md
- 4: Expand
attributevalue type to support complex values everywhere and cleanup surrounding code open-telemetry/opentelemetry-python#5266 - 5: https://opentelemetry.io/docs/specs/otel/common/
- 6: Update json and proto encoder to always accept None type, cleanup code / tests a little bit open-telemetry/opentelemetry-python#5305
- 7: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/common/README.md
- 8: https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-api/src/opentelemetry/attributes/__init__.py
- 9: AttributeError: 'NoneType' object has no attribute 'attributes' open-telemetry/opentelemetry-python#2743
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Inspect dependency constraints ---'
fd '.*' pyproject.toml uv.lock requirements*.txt 2>/dev/null | sort | sed -n '1,80p'
rg -n 'opentelemetry|otel|attribute' pyproject.toml uv.lock requirements*.txt 2>/dev/null || true
echo '--- Deterministic behavioral probe for current RAGSources construction ---'
python3 - <<'PY'
import ast
from pathlib import Path
vec = Path('src/utils/vector_search.py').read_text()
model = Path('src/models/common/turn_summary.py').read_text()
# Find set_span_attributes call and nearby assignments to top_documents
tree = ast.parse(vec)
for node in ast.walk(tree):
if isinstance(node, ast.Call):
func = node.func
if isinstance(func, ast.Name) and func.id == 'set_span_attributes':
print('set_span_attributes found at line', node.lineno)
for i, arg in enumerate(node.args):
print(f' arg {i} type', type(arg).__name__, ast.dump(arg))
if getattr(arg, 'arg', None) == 'attributes':
values = arg.value
for j, elt in enumerate(values):
if isinstance(elt, ast.Dict) and any(getattr(k, 'value', None) == 'rag.sources' for k in elt.keys):
print(' rag.sources dict index', j)
for idx, (k, v) in enumerate(zip(elt.items, elt.values)):
print(' key', ast.get_source_segment(vec, k))
if isinstance(k, ast.Attribute) and k.attr == 'RAG_SOURCES':
print(' value_type', type(v).__name__)
print(' value_source', ast.get_source_segment(vec, v))
# Find ReferencedDocument assignment defaults by static AST
for filename in ['src/models/common/turn_summary.py']:
tree = ast.parse(Path(filename).read_text())
for node in ast.walk(tree):
if isinstance(node, ast.Assign):
for target in node.targets:
if isinstance(target, ast.Name) and target.id == 'ReferencedDocument':
continue
for node in ast.walk(ast.parse(model)):
if isinstance(node, ast.AnnAssign):
attr = node.target
if isinstance(attr, ast.Attribute) and attr.attr == 'doc_url':
print('ReferencedDocument.doc_url ann_assign line', node.lineno, 'value', ast.get_source_segment(model, node.value))
PYRepository: lightspeed-core/lightspeed-stack
Length of output: 209
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- repository files ---'
git ls-files | rg 'pyproject\.toml|uv\.lock|requirements|lockfile|poetry' || true
echo '--- if pyproject exists ---'
if [ -f pyproject.toml ]; then
rg -n 'opentelemetry|otel' pyproject.toml
fi
echo '--- if uv.lock exists ---'
if [ -f uv.lock ]; then
rg -n 'name = "opentelemetry|opentelemetry' uv.lock
fiRepository: lightspeed-core/lightspeed-stack
Length of output: 13180
Do not export raw RAG URLs.
rag.sources currently stores full doc_url values, which can include internal paths, tenant identifiers, or signed query parameters. Filter any missing values and export anonymized source identifiers instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/vector_search.py` around lines 722 - 729, Update the RAG attribute
construction in the vector-search flow to stop exporting raw doc_url values.
Filter out missing URLs and transform each remaining source into the project’s
established anonymized source identifier before assigning
SpanAttributes.RAG_SOURCES; keep the count aligned with the filtered sources.
Source: Coding guidelines
2 participants
Description
Adds OpenTelemetry (OTEL) tracing instrumentation for the POST /v1/query endpoint to enable distributed tracing and observability.
Instrumented Components
src/app/endpoints/query.py)src/utils/quota_utils.py)src/utils/shields.py)src/utils/vector_search.py)src/utils/agents/query.py)src/utils/agents/query.py)Span Hierarchy
Type of change
pyproject.toml+uv.lock]requirements.*.txtfor Konflux]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
Summary by CodeRabbit
Release Notes
Observability Improvements
Quality Improvements