RSPEED-2997: extract Splunk telemetry from responses endpoint

[major]

Description

Extracts Splunk telemetry functions from responses.py into a dedicated responses_telemetry.py module. Centralizes the fire-and-forget dispatch pattern in observability/splunk.py as dispatch_splunk_event().

Reduces responses.py from 1201 to 1057 lines with no behavioral changes to request handling.

Changes:

• New src/app/endpoints/responses_telemetry.py with the four queue_* telemetry functions (previously _queue_* private functions in responses.py)
• New dispatch_splunk_event() in observability/splunk.py that handles both BackgroundTasks and asyncio.create_task fire-and-forget dispatch strategies
• Updated responses.py to import from the new telemetry module
• Updated test_responses_splunk.py to match new import paths
• New unit tests for dispatch_splunk_event() in test_splunk.py

Type of change

• Refactor
• Unit tests improvement

Tools used to create PR

• Assisted-by: Claude (Sisyphus via OpenCode)
• Generated by: N/A

Related Tickets & Documents

• Related Issue: RSPEED-2997

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

• All existing unit tests pass (64 test_responses + 21 test_responses_splunk + 13 test_splunk = 98 tests)
• New TestDispatchSplunkEvent class covers: no-op path, BackgroundTasks dispatch, fire-and-forget dispatch, fire-and-forget priority over BackgroundTasks
• All linters pass (make verify: black, pylint 10/10, pyright, ruff, pydocstyle, mypy)
• CodeRabbit review: 0 actionable findings remaining

Summary by CodeRabbit

• Refactor

  • Centralized and simplified telemetry handling for Responses to improve reliability and maintainability.

• Observability

  • More consistent emission of events for errors, blocked responses, and completed responses; improved background and fire-and-forget dispatching.

• Tests

  • Updated and expanded tests to cover telemetry dispatch behavior and integration across success/error and streaming/non-streaming flows.

[coderabbitai]

Warning

Rate limit exceeded

@major has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 52 minutes and 52 seconds before requesting another review.

To continue reviewing without waiting, purchase usage credits in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a98f8305-9e49-48e0-ba22-4705082d97ae

📥 Commits

Reviewing files that changed from the base of the PR and between 45337b0 and 890c8f7.

📒 Files selected for processing (6)

• src/app/endpoints/responses.py
• src/app/endpoints/responses_telemetry.py
• src/observability/__init__.py
• src/observability/splunk.py
• tests/unit/app/endpoints/test_responses_splunk.py
• tests/unit/observability/test_splunk.py

Walkthrough

Telemetry queuing for the Responses API was extracted into a new app.endpoints.responses_telemetry module and wired to a new dispatch helper observability.splunk.dispatch_splunk_event which routes delivery via FastAPI BackgroundTasks or fire-and-forget asyncio tasks. responses.py now calls the new telemetry helpers; tests are updated accordingly.

Changes

Telemetry extraction & dispatch

Layer / File(s)	Summary
Dispatch infra src/observability/splunk.py	Added dispatch_splunk_event(event, sourcetype, background_tasks=None, fire_and_forget=False) and a _fire_and_forget_tasks set to schedule via BackgroundTasks.add_task or asyncio.create_task with done-callback cleanup.
Public export src/observability/__init__.py	Re-exported dispatch_splunk_event in module __all__.
Telemetry API src/app/endpoints/responses_telemetry.py	New module providing queue_responses_splunk_event(...) plus high-level helpers: queue_responses_error_event(...), queue_blocked_response_event(...), queue_completed_response_event(...). These build ResponsesEventData, compute inference time, normalize conversation IDs, and call dispatch_splunk_event.
Endpoint wiring src/app/endpoints/responses.py	Removed local Splunk helper set and per-endpoint helper functions; replaced local calls with imports and invocations of queue_responses_error_event, queue_blocked_response_event, and queue_completed_response_event at error, shield-blocked, and completion sites (streaming and non-streaming).
Unit tests — observability tests/unit/observability/test_splunk.py	Added TestDispatchSplunkEvent to verify no-op, BackgroundTasks enqueueing, fire-and-forget task creation, done-callback cleanup, and precedence of fire_and_forget over background_tasks.
Unit tests — endpoints tests/unit/app/endpoints/test_responses_splunk.py	Refactored tests to target app.endpoints.responses_telemetry (patch queue_responses_splunk_event, build_responses_event, dispatch_splunk_event, and _fire_and_forget_tasks); updated integration assertions for success/error/blocked flows and Splunk-disabled scenario (calls with background_tasks=None).

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Client
    participant ResponsesEndpoint as Responses Endpoint
    participant Telemetry as ResponsesTelemetry
    participant Dispatcher as Observability.dispatch_splunk_event
    participant Scheduler as BackgroundTasks / asyncio
    participant Splunk as send_splunk_event (Splunk)

    Client->>ResponsesEndpoint: send request
    ResponsesEndpoint->>Telemetry: build event & call queue_*_response_event(...)
    Telemetry->>Dispatcher: dispatch_splunk_event(event, sourcetype, background_tasks, fire_and_forget)
    alt background_tasks provided and fire_and_forget False
        Dispatcher->>Scheduler: background_tasks.add_task(send_splunk_event, event, sourcetype)
    else fire_and_forget True
        Dispatcher->>Scheduler: asyncio.create_task(send_splunk_event(...)) (stored in _fire_and_forget_tasks)
        Scheduler-->>Dispatcher: task created, done-callback registered (removes from set)
    else neither provided
        Dispatcher-->>Telemetry: return (no-op)
    end
    Scheduler->>Splunk: send_splunk_event(event, sourcetype)
    Splunk-->>Scheduler: HTTP/async response

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately and clearly describes the main refactoring work—extracting Splunk telemetry logic from the responses endpoint into a dedicated module.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/unit/app/endpoints/test_responses_splunk.py (1)

697-759: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Docstring contradicts its own assertion.

The docstring on Line 702 says "When background_tasks is None, queue_responses_splunk_event is never called.", but the assertion on Line 758 is mock_queue.assert_called_once() — the function is called once with background_tasks=None. The no-op happens inside queue_responses_splunk_event (early-return guard), not at the call site.

📝 Proposed docstring fix

-        """When background_tasks is None, queue_responses_splunk_event is never called."""
+        """When background_tasks is None, queue_responses_splunk_event is called but is a no-op."""

🤖 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 `@tests/unit/app/endpoints/test_responses_splunk.py` around lines 697 - 759,
The test docstring in test_splunk_disabled_no_background_tasks contradicts the
assertion: update the docstring to state that queue_responses_splunk_event is
still called with background_tasks=None (and itself performs an early-return),
e.g. reference the test function name test_splunk_disabled_no_background_tasks
and the telemetry helper queue_responses_splunk_event so reviewers know you mean
the guard lives inside queue_responses_splunk_event called by
handle_non_streaming_response.

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

Outside diff comments:
In `@tests/unit/app/endpoints/test_responses_splunk.py`:
- Around line 697-759: The test docstring in
test_splunk_disabled_no_background_tasks contradicts the assertion: update the
docstring to state that queue_responses_splunk_event is still called with
background_tasks=None (and itself performs an early-return), e.g. reference the
test function name test_splunk_disabled_no_background_tasks and the telemetry
helper queue_responses_splunk_event so reviewers know you mean the guard lives
inside queue_responses_splunk_event called by handle_non_streaming_response.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0d5f404e-9c85-419a-b30c-3a29a1ea4a22

📥 Commits

Reviewing files that changed from the base of the PR and between fa19cbd and 4350a2e.

📒 Files selected for processing (6)

• src/app/endpoints/responses.py
• src/app/endpoints/responses_telemetry.py
• src/observability/__init__.py
• src/observability/splunk.py
• tests/unit/app/endpoints/test_responses_splunk.py
• tests/unit/observability/test_splunk.py

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

• GitHub Check: E2E: library mode / ci / group 3
• GitHub Check: E2E: server mode / ci / group 1
• GitHub Check: integration_tests (3.12)
• GitHub Check: build-pr
• GitHub Check: E2E Tests for Lightspeed Evaluation job

🧰 Additional context used

📓 Path-based instructions (6)

src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Use absolute imports for internal modules: from authentication import get_auth_dependency
All modules start with descriptive docstrings explaining purpose
Use logger = get_logger(__name__) from log.py for module logging
Type aliases defined at module level for clarity
Use Final[type] as type hint for all constants
All functions require docstrings with brief descriptions
Complete type annotations for function parameters and return types
Use Union types with modern syntax: str | int
Use Optional[Type] for optional type hints
Use snake_case with descriptive, action-oriented function names (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns: return new data structures instead of modifying parameters
Use async def for I/O operations and external API calls
Use logger.debug() for detailed diagnostic information
Use logger.info() for general information about program execution
Use logger.warning() for unexpected events or potential problems
Use logger.error() for serious problems that prevented function execution
All classes require descriptive docstrings explaining purpose
Use PascalCase for class names with descriptive names and standard suffixes: Configuration, Error/Exception, Resolver, Interface
Abstract classes use ABC with @abstractmethod decorators
Complete type annotations for all class attributes, use specific types, not Any
Follow Google Python docstring conventions for all modules, classes, and functions
Docstring Parameters section documents function parameters
Docstring Returns section documents function return values
Docstring Raises section documents exceptions that may be raised
Use black for code formatting
Use pylint for static analysis with source-roots configuration set to "src"
Use pyright for type checking
Use ruff for fast linting
Use pydocstyle for docstring style validation
Use mypy for additional type checking
Use bandit for security issue detection

Files:

• src/observability/__init__.py
• src/app/endpoints/responses_telemetry.py
• src/app/endpoints/responses.py
• src/observability/splunk.py

src/**/__init__.py

📄 CodeRabbit inference engine (AGENTS.md)

Package __init__.py files contain brief package descriptions

Files:

• src/observability/__init__.py

src/app/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use FastAPI dependencies: from fastapi import APIRouter, HTTPException, Request, status, Depends

Files:

• src/app/endpoints/responses_telemetry.py
• src/app/endpoints/responses.py

src/app/endpoints/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use FastAPI HTTPException with appropriate status codes for API endpoint error handling

Files:

• src/app/endpoints/responses_telemetry.py
• src/app/endpoints/responses.py

tests/unit/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

tests/unit/**/*.py: Use pytest for all unit and integration tests
Use pytest-mock for AsyncMock objects in unit tests
Use marker pytest.mark.asyncio for async tests

Files:

• tests/unit/app/endpoints/test_responses_splunk.py
• tests/unit/observability/test_splunk.py

tests/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Do not use unittest - pytest is the standard testing framework for this project

Files:

• tests/unit/app/endpoints/test_responses_splunk.py
• tests/unit/observability/test_splunk.py

🧠 Learnings (1)

📚 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/responses_telemetry.py
• src/app/endpoints/responses.py

🔇 Additional comments (5)

src/observability/splunk.py (1)

95-130: dispatch_splunk_event design and GC protection look correct.

The module-level _fire_and_forget_tasks set correctly prevents asyncio tasks from being garbage-collected before completion, and the add_done_callback(_fire_and_forget_tasks.discard) cleanup is safe even if a task completes or is cancelled before the callback is registered (since discard is a no-op for absent elements).

src/observability/__init__.py (1)

17-26: LGTM.

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

1-162: LGTM — clean extraction with correct dispatch semantics on all paths.

fire_and_forget=True on the error path (before HTTPException is raised) is the correct safeguard, fire_and_forget=False on blocked and completed paths is correct since those return 200 OK and FastAPI will execute BackgroundTasks normally.

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

33-37: LGTM — pure refactor with correct call sites.

tests/unit/observability/test_splunk.py (1)

201-265: LGTM — TestDispatchSplunkEvent correctly covers all dispatch branches including the done-callback cleanup lifecycle.

[major]

@coderabbitai Fixed the docstring complaint in 84ae4b8. Please verify.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
git show 84ae4b8

Unrecoverable error: Failed to clone repository

---

🏁 Script executed:

#!/bin/bash
gh api repos/lightspeed-core/lightspeed-stack/commits/84ae4b8 --jq '.files[] | {filename: .filename, patch: .patch}'

Length of output: 32128

---

@major: Verified ✅. The docstring fixes in commit 84ae4b8 look correct:

1. test_splunk_disabled_no_background_tasks — Updated from "_queue_responses_splunk_event is never called" to "queue_responses_splunk_event is called but is a no-op". This is semantically accurate: in the new design the function is always invoked, but returns early when background_tasks is None and fire_and_forget is False.
2. TestQueueResponsesSplunkEvent and TestSplunkTelemetryHooks class docstrings — Stripped the leading underscore from the function name to match the new public API.
3. dispatch_splunk_event() in splunk.py — Has a complete docstring covering both dispatch modes, the no-op condition, and an Args section.
4. All four functions in responses_telemetry.py — Each has a proper docstring with an Args section.

Everything looks good!

ᓚᘏᗢ

[coderabbitai]

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/observability/splunk.py`:
- Around line 125-129: The fire-and-forget path uses
asyncio.create_task(send_splunk_event(...)) and only discards the task via
task.add_done_callback(_fire_and_forget_tasks.discard), which allows exceptions
to go unobserved; implement a dedicated done-callback (e.g.,
_handle_fire_and_forget_done) that calls task.result() inside a try/except to
catch and log any exception (using the module logger) and then removes the task
from _fire_and_forget_tasks; replace the current add_done_callback call with
task.add_done_callback(_handle_fire_and_forget_done) so exceptions are consumed
and logged before discarding the task.

🪄 Autofix (Beta)

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

Run ID: 4b997b66-bb50-4605-b9d4-2137ca06f9b8

📥 Commits

Reviewing files that changed from the base of the PR and between 4350a2e and 45337b0.

📒 Files selected for processing (6)

• src/app/endpoints/responses.py
• src/app/endpoints/responses_telemetry.py
• src/observability/__init__.py
• src/observability/splunk.py
• tests/unit/app/endpoints/test_responses_splunk.py
• tests/unit/observability/test_splunk.py

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)

• GitHub Check: unit_tests (3.12)
• GitHub Check: unit_tests (3.13)
• GitHub Check: spectral
• GitHub Check: Pylinter
• GitHub Check: build-pr
• GitHub Check: E2E: library mode / ci / group 2
• GitHub Check: E2E: library mode / ci / group 1
• GitHub Check: E2E: server mode / ci / group 3
• GitHub Check: E2E: library mode / ci / group 3
• GitHub Check: E2E: server mode / ci / group 2
• GitHub Check: E2E: server mode / ci / group 1
• GitHub Check: E2E Tests for Lightspeed Evaluation job

🧰 Additional context used

📓 Path-based instructions (6)

src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Use absolute imports for internal modules: from authentication import get_auth_dependency
All modules start with descriptive docstrings explaining purpose
Use logger = get_logger(__name__) from log.py for module logging
Type aliases defined at module level for clarity
Use Final[type] as type hint for all constants
All functions require docstrings with brief descriptions
Complete type annotations for function parameters and return types
Use Union types with modern syntax: str | int
Use Optional[Type] for optional type hints
Use snake_case with descriptive, action-oriented function names (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns: return new data structures instead of modifying parameters
Use async def for I/O operations and external API calls
Use logger.debug() for detailed diagnostic information
Use logger.info() for general information about program execution
Use logger.warning() for unexpected events or potential problems
Use logger.error() for serious problems that prevented function execution
All classes require descriptive docstrings explaining purpose
Use PascalCase for class names with descriptive names and standard suffixes: Configuration, Error/Exception, Resolver, Interface
Abstract classes use ABC with @abstractmethod decorators
Complete type annotations for all class attributes, use specific types, not Any
Follow Google Python docstring conventions for all modules, classes, and functions
Docstring Parameters section documents function parameters
Docstring Returns section documents function return values
Docstring Raises section documents exceptions that may be raised
Use black for code formatting
Use pylint for static analysis with source-roots configuration set to "src"
Use pyright for type checking
Use ruff for fast linting
Use pydocstyle for docstring style validation
Use mypy for additional type checking
Use bandit for security issue detection

Files:

• src/observability/__init__.py
• src/observability/splunk.py
• src/app/endpoints/responses.py
• src/app/endpoints/responses_telemetry.py

src/**/__init__.py

📄 CodeRabbit inference engine (AGENTS.md)

Package __init__.py files contain brief package descriptions

Files:

• src/observability/__init__.py

tests/unit/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

tests/unit/**/*.py: Use pytest for all unit and integration tests
Use pytest-mock for AsyncMock objects in unit tests
Use marker pytest.mark.asyncio for async tests

Files:

• tests/unit/app/endpoints/test_responses_splunk.py
• tests/unit/observability/test_splunk.py

tests/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Do not use unittest - pytest is the standard testing framework for this project

Files:

• tests/unit/app/endpoints/test_responses_splunk.py
• tests/unit/observability/test_splunk.py

src/app/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use FastAPI dependencies: from fastapi import APIRouter, HTTPException, Request, status, Depends

Files:

• src/app/endpoints/responses.py
• src/app/endpoints/responses_telemetry.py

src/app/endpoints/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use FastAPI HTTPException with appropriate status codes for API endpoint error handling

Files:

• src/app/endpoints/responses.py
• src/app/endpoints/responses_telemetry.py

🧠 Learnings (1)

📚 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/responses.py
• src/app/endpoints/responses_telemetry.py

🔇 Additional comments (6)

src/observability/__init__.py (1)

17-25: Public re-export is clean and consistent.

Exposing dispatch_splunk_event through observability.__all__ matches the new telemetry module contract and keeps the package API coherent.

tests/unit/observability/test_splunk.py (1)

201-266: Good coverage for dispatcher behavior and priority rules.

The new TestDispatchSplunkEvent suite validates the core control-flow paths and task-retention lifecycle clearly.

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

23-163: Telemetry extraction is well-structured.

The split into focused queue helpers keeps endpoint flow cleaner while preserving dispatch semantics (background vs fire-and-forget) in one place.

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

33-37: Hook migration to telemetry module looks correct.

All key telemetry emission points (error, blocked, completed) are consistently routed through the new helper layer without disrupting control flow.

Also applies to: 211-212, 510-514, 927-933, 964-964, 1027-1033

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

22-212: Unit-level telemetry helper tests are aligned with the refactor.

Patch targets and assertions now correctly exercise responses_telemetry + dispatch integration.

---

219-760: Integration hook tests now match runtime telemetry flow.

The updated expectations for blocked/error/completed paths (including background_tasks=None no-op behavior) are consistent with the extracted telemetry architecture.

[tisnik]

LGTM