Skip to content

LCORE-1794: Add OpenTelemetry spans to conversation v1 and v2 endpoints - #2475

Open
anik120 wants to merge 1 commit into
lightspeed-core:mainfrom
anik120:spans-for-convos
Open

LCORE-1794: Add OpenTelemetry spans to conversation v1 and v2 endpoints#2475
anik120 wants to merge 1 commit into
lightspeed-core:mainfrom
anik120:spans-for-convos

Conversation

@anik120

@anik120 anik120 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Description

  • Add coarse-grained OpenTelemetry spans to all 8 conversation endpoint handlers (4 in v1, 4 in v2)
  • Each handler emits a single span with operation-level metadata: conversation count (list), turn count and found flag (get), deleted flag (delete), updated flag (update)
  • All attributes are low-cardinality — no raw conversation IDs or user-derived free text in span attributes
  • Errors are auto-captured via start_as_current_span context manager setting StatusCode.ERROR

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library [pyproject.toml + uv.lock]
  • Bump-up dependent library [requirements.*.txt for Konflux]
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement
  • Benchmarks improvement

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: (e.g., Claude, CodeRabbit, Ollama, etc., N/A if not used)
  • Generated by: (e.g., tool name and version; N/A if not used)

Related Tickets & Documents

  • Related Issue #
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • Enhancements

    • Added OpenTelemetry tracing to conversation list, view, update, and delete operations.
    • Conversation telemetry now includes operation outcomes and relevant counts, statuses, and turn information.
    • Error conditions are recorded in tracing data to improve observability.
  • Tests

    • Added coverage verifying tracing for successful and failed conversation operations.

- Add coarse-grained OpenTelemetry spans to all 8 conversation endpoint handlers (4 in v1, 4 in v2)
- Each handler emits a single span with operation-level metadata: conversation count (list),
turn count and found flag (get), deleted flag (delete), updated flag (update)
- All attributes are low-cardinality — no raw conversation IDs or user-derived free text in span attributes
- Errors are auto-captured via start_as_current_span context manager setting StatusCode.ERROR

Signed-off-by: Anik Bhattacharjee <anbhatta@redhat.com>
@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8df94086-a7e9-45c0-9bb1-dd6bddbec415

📥 Commits

Reviewing files that changed from the base of the PR and between 6722d66 and 94c55e2.

📒 Files selected for processing (4)
  • src/app/endpoints/conversations_v1.py
  • src/app/endpoints/conversations_v2.py
  • tests/unit/app/endpoints/test_conversations.py
  • tests/unit/app/endpoints/test_conversations_v2.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (19)
  • GitHub Check: E2E Tests for Lightspeed Evaluation job
  • GitHub Check: E2E: library / ci / rbac
  • GitHub Check: E2E: server / ci / other
  • GitHub Check: E2E: server / ci / mcp
  • GitHub Check: E2E: server / ci / tls
  • GitHub Check: E2E: library / ci / authorized
  • GitHub Check: E2E: library / ci / skills
  • GitHub Check: E2E: library / ci / other
  • GitHub Check: E2E: server / ci / rbac
  • GitHub Check: E2E: library / ci / default
  • GitHub Check: E2E: library / ci / mcp
  • GitHub Check: E2E: server / ci / skills
  • GitHub Check: E2E: server / ci / default
  • GitHub Check: E2E: server / ci / authorized
  • GitHub Check: Red Hat Konflux / rag-content-0-8-e2e-tests / lightspeed-stack-0-8
  • GitHub Check: build-pr
  • GitHub Check: Red Hat Konflux / lightspeed-core-0-8-enterprise-contract / lightspeed-stack-0-8
  • GitHub Check: Red Hat Konflux / lightspeed-stack-0-8-e2e-tests / lightspeed-stack-0-8
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-8-on-pull-request
🧰 Additional context used
📓 Path-based instructions (3)
**/*

📄 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.

Files:

  • src/app/endpoints/conversations_v2.py
  • src/app/endpoints/conversations_v1.py
  • tests/unit/app/endpoints/test_conversations.py
  • tests/unit/app/endpoints/test_conversations_v2.py
src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Use absolute imports for internal modules and follow the prescribed FastAPI and Llama Stack import conventions.
All modules must begin with descriptive docstrings; use logger = get_logger(__name__) from log.py for module logging; package __init__.py files must contain brief package descriptions.
Define shared constants in the central constants.py module, add descriptive comments, and annotate constants with Final[type].
Use complete type annotations for function parameters, return types, class attributes, and type aliases; prefer specific types over Any, use modern union syntax, and use typing_extensions.Self for model validators.
All functions and classes require descriptive Google-style docstrings, including appropriate Parameters, Returns, Raises, and Attributes sections.
Use descriptive snake_case, action-oriented function names such as get_, validate_, and check_; use PascalCase class names with standard suffixes such as Configuration, Error/Exception, Resolver, and Interface.
Avoid modifying input parameters in place; return a newly constructed data structure instead.
Use async def for I/O operations and external API calls; API endpoints should raise FastAPI HTTPException with appropriate status codes and handle Llama Stack APIConnectionError.
Use from log import get_logger and standard logger levels: debug for diagnostics, info for general execution, warning for unexpected conditions or potential problems, and error for serious failures.
Configuration models must extend ConfigurationBase, set extra="forbid" to reject unknown fields, use Pydantic validators for custom validation, and use types such as Optional[FilePath], PositiveInt, and SecretStr where appropriate.
Abstract interfaces must use ABC and @abstractmethod decorators.
Never commit secrets or keys; use environment variables for sensitive data.

Files:

  • src/app/endpoints/conversations_v2.py
  • src/app/endpoints/conversations_v1.py
tests/unit/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use pytest for unit tests, shared fixtures in conftest.py, pytest-mock for mocks, pytest.mark.asyncio for async tests, and maintain at least 60% unit-test coverage.

Files:

  • tests/unit/app/endpoints/test_conversations.py
  • tests/unit/app/endpoints/test_conversations_v2.py
🧠 Learnings (4)
📚 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/conversations_v2.py
  • src/app/endpoints/conversations_v1.py
📚 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/app/endpoints/conversations_v2.py
  • src/app/endpoints/conversations_v1.py
  • tests/unit/app/endpoints/test_conversations.py
  • tests/unit/app/endpoints/test_conversations_v2.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/app/endpoints/conversations_v2.py
  • src/app/endpoints/conversations_v1.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/app/endpoints/conversations_v2.py
  • src/app/endpoints/conversations_v1.py
🔇 Additional comments (5)
src/app/endpoints/conversations_v1.py (1)

11-11: LGTM!

Also applies to: 60-60, 130-182, 214-297, 326-412, 441-558

src/app/endpoints/conversations_v2.py (1)

6-6: LGTM!

Also applies to: 38-38, 95-115, 129-157, 170-191, 202-238

tests/unit/app/endpoints/test_conversations.py (2)

12-15: LGTM!

Also applies to: 37-37, 2215-2530


2175-2213: 🩺 Stability & Availability

No fixture change is required. The default function-scoped otel fixture creates a new InMemorySpanExporter for each test and clears it during teardown, so both len(spans) == 1 assertions are isolated.

			> Likely an incorrect or invalid review comment.
tests/unit/app/endpoints/test_conversations_v2.py (1)

1-1: LGTM!

Also applies to: 10-13, 1022-1234


Walkthrough

Conversation v1 and v2 endpoints now emit OpenTelemetry spans for list, get, delete, and update operations. Spans record operation results and error status. Unit tests cover successful and failing paths.

Changes

Conversation endpoint tracing

Layer / File(s) Summary
V1 endpoint instrumentation
src/app/endpoints/conversations_v1.py
Adds named spans and operation attributes for listing, retrieval, deletion, and metadata updates.
V2 endpoint instrumentation
src/app/endpoints/conversations_v2.py
Adds named spans and operation attributes for listing, retrieval, deletion, and topic-summary updates.
Tracing test coverage
tests/unit/app/endpoints/test_conversations.py, tests/unit/app/endpoints/test_conversations_v2.py
Tests span names, success attributes, turn and conversation counts, and error statuses for both endpoint versions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 94c55

The change adds localized telemetry to conversation endpoints without any identified merge-blocking correctness or production risk. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: asimurka, tisnik

🚥 Pre-merge checks | ✅ 7
✅ Passed checks (7 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the addition of OpenTelemetry spans to both conversation endpoint versions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Performance And Algorithmic Complexity ✅ Passed PASS: The diff adds constant-time span setup and attribute writes; existing list/.all() calls and loops are unchanged, with no new N+1 work, unbounded growth, or pagination regression.
Security And Secret Handling ✅ Passed PASSED. The diff adds only tracing and formatting; all eight handlers retain auth dependencies and @authorize checks, and span attributes contain only counts or booleans. No credential literals or...
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant