Logging clean up #178
Logging clean up #178
Conversation
- Moved logging configuration to cli - Configure only the main "vision_agents" logger
WalkthroughRemoved Agent-level logging setup and TurnEndedEvent subscription; renamed and centralized the SDK logging helper; refactored CLI runner to a launcher-based flow with a single Click command; adjusted logging formatter/propagation; updated tests and one Deepgram log call site. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI Runner
participant Launcher as AgentLauncher
participant Agent as Agent
participant Demo as Demo UI
participant Join as join_call
CLI->>Launcher: launch(call_type, call_id)
Launcher-->>CLI: Agent instance
alt demo enabled and edge supports demo
CLI->>Agent: edge.open_demo_for_agent()
Agent-->>Demo: open demo
end
alt join_call provided
CLI->>Launcher: join_call(...)
Join-->>CLI: result (sync or awaitable)
else no join_call
CLI-->>CLI: emit warning
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
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: 0
🧹 Nitpick comments (1)
agents-core/vision_agents/core/utils/logging.py (1)
21-22: Refresh the docstring to match current behaviorThe docstring still promises both the “vision_agents” and “getstream” loggers, but the implementation now configures only “vision_agents”. Please update the wording so it reflects the actual behavior.
Apply this diff to fix the docstring:
- """ - Sets the default configuration for "vision_agents" and "getstream" loggers to have a nice formatting - if it's not already configured. - """ + """ + Sets the default configuration for the "vision_agents" logger with a nice formatter + if it's not already configured. + """
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
agents-core/vision_agents/core/agents/agents.py(0 hunks)agents-core/vision_agents/core/cli/cli_runner.py(3 hunks)agents-core/vision_agents/core/utils/logging.py(2 hunks)plugins/deepgram/vision_agents/plugins/deepgram/deepgram_stt.py(1 hunks)tests/test_utils.py(2 hunks)
💤 Files with no reviewable changes (1)
- agents-core/vision_agents/core/agents/agents.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-13T22:00:34.300Z
Learnt from: dangusev
Repo: GetStream/Vision-Agents PR: 98
File: plugins/deepgram/vision_agents/plugins/deepgram/stt.py:135-150
Timestamp: 2025-10-13T22:00:34.300Z
Learning: In the Deepgram STT plugin (plugins/deepgram/vision_agents/plugins/deepgram/stt.py), the `started()` method is designed to wait for the connection attempt to complete, not to guarantee a successful connection. It's acceptable for the connection attempt to fail, and downstream code handles the case where `self.dg_connection` is `None`. The `_connected_once` event is set in the `finally` block intentionally to signal attempt completion.
Applied to files:
plugins/deepgram/vision_agents/plugins/deepgram/deepgram_stt.py
🧬 Code graph analysis (2)
tests/test_utils.py (1)
agents-core/vision_agents/core/utils/logging.py (1)
configure_sdk_logger(19-46)
agents-core/vision_agents/core/cli/cli_runner.py (3)
agents-core/vision_agents/core/utils/logging.py (1)
configure_sdk_logger(19-46)agents-core/vision_agents/core/agents/agent_launcher.py (2)
AgentLauncher(18-125)launch(111-125)plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (1)
open_demo_for_agent(354-360)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Ruff & mypy
🔇 Additional comments (1)
plugins/deepgram/vision_agents/plugins/deepgram/deepgram_stt.py (1)
256-256: Let me check one more detail about how the error parameter is used and whether there's conditional logging elsewhere in the codebase:Based on the verification, this change is acceptable. The logging level reduction aligns with the PR's objective to reduce verbosity while maintaining error visibility through separate error handling.
No action required. The logging change is reasonable.
The Deepgram SDK uses separate EventType.CLOSE and EventType.ERROR handlers, with
_on_erroralready logging at ERROR level and raising exceptions. The code in_on_closeonly clears the connection state flag; downstream code inprocess_audioalready logs a warning when the connection closes during active processing.Downgrading to DEBUG is appropriate because:
- Unexpected errors are captured by the separate
_on_errorhandler- Clean connection closures are normal operational events
- The connection state is properly managed and any disruption during message processing is already logged
cli_runnerinstead of theAgentvision_agentslogger, keep other loggers intact (must be atWARNINGlevel by default)Summary by CodeRabbit
Breaking Changes
Refactor
Updates
Tests