feat(stream): expose active session telemetry - #801
Conversation
Summary by CodeRabbit
Walkthrough扩展会话生命周期观测能力，记录停止原因、客户端标识、运行时长、通道活动时间和连接状态，并通过会话查询 JSON 返回；控制、视频、音频及主机终止路径均传递具体停止原因。 Changes会话生命周期可观测性
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ControlServer
participant CaptureThreads
participant stream_session_stop
participant SessionsAPI
ControlServer->>stream_session_stop: 传递控制断开、超时或协议错误原因
CaptureThreads->>stream_session_stop: 传递视频或音频结束原因
stream_session_stop->>SessionsAPI: 保存停止原因和活动时间
SessionsAPI-->>ControlServer: 返回运行、空闲和连接状态字段
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (1)
tests/unit/test_stream_session_observability.cpp (1)
10-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win补充会话遥测边界测试
当前断言正确覆盖了所有停止原因名称，但未验证首个停止原因、
uptime_ms、control_connected，以及首次活动前空闲字段应输出null。建议补充会话/API 层测试保护这些跨文件契约。As per path instructions：
tests/**需要验证测试覆盖率、边界情况和断言正确性。🤖 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/test_stream_session_observability.cpp` around lines 10 - 20, 在 StreamSessionObservability 测试中补充会话/API 层边界断言：验证首个停止原因、uptime_ms 和 control_connected，并确认首次活动前的空闲字段输出为 null。保留现有 stop_reason_name 全量断言，并确保新增断言覆盖这些跨文件遥测契约且断言值正确。Source: Path instructions
🤖 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/rtsp.cpp`:
- Around line 834-835: 在全局 shutdown 流程中，将 stream::session::stop 的
stop_reason_e::host_terminate 写入前移至广播关闭或终止发起处，确保其先于视频/音频线程写入 video_ended 或
audio_ended；保留 stream::session::join(slot) 的等待逻辑，并避免仅依赖 clear() 在 RUNNING
状态下覆盖已有原因。
In `@src/stream.cpp`:
- Around line 3052-3064: 在 stop(session_t&, stop_reason_e) 中，将 stop_reason 与
STOPPING 状态的发布改为原子一致的快照，并同步更新 get_all_sessions_info() 的读取逻辑，使其不会观察到 STOPPING 但仍为
none；确保并发停止调用仍只记录首个终止原因。仅调整内存序不足以解决问题，应复用锁或单一原子状态封装完成同步。
- Around line 3448-3457: Update the ENET_EVENT_TYPE_DISCONNECT handling so
stopping a session also clears its control connection state before API status is
read. Ensure the status-building code around session_p->control.peer no longer
reports control_connected=true after disconnect, using synchronized access or an
atomic connection-state field as appropriate; preserve the existing STOPPING
state behavior.
---
Nitpick comments:
In `@tests/unit/test_stream_session_observability.cpp`:
- Around line 10-20: 在 StreamSessionObservability 测试中补充会话/API
层边界断言：验证首个停止原因、uptime_ms 和 control_connected，并确认首次活动前的空闲字段输出为 null。保留现有
stop_reason_name 全量断言，并确保新增断言覆盖这些跨文件遥测契约且断言值正确。
🪄 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: CHILL
Plan: Pro
Run ID: 9df35dcc-0d94-442b-917e-1b6f40505eb0
📒 Files selected for processing (5)
src/nvhttp/sessions.cppsrc/rtsp.cppsrc/stream.cppsrc/stream.htests/unit/test_stream_session_observability.cpp
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
tests/**
⚙️ CodeRabbit configuration file
tests/**: 测试文件。验证测试覆盖率、边界情况和断言正确性。
Files:
tests/unit/test_stream_session_observability.cpp
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/rtsp.cppsrc/nvhttp/sessions.cppsrc/stream.hsrc/stream.cpp
🔇 Additional comments (3)
src/stream.h (1)
6-6: LGTM!Also applies to: 40-67, 92-92
src/stream.cpp (1)
111-124: LGTM!Also applies to: 506-512, 876-876, 891-894, 1668-1669, 1724-1725, 1733-1734, 1774-1774, 2313-2313, 2665-2665, 2972-2973, 2997-2998, 3021-3040, 3170-3172, 3278-3282
src/nvhttp/sessions.cpp (1)
69-78: LGTM!
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/stream.cpp (1)
3204-3219: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift给
session::start()补上异常边界和半启动清理这里没有
try/catch包住命令分发；std::thread构造一旦抛出std::system_error，异常会直接逃出handle_msg()，RTSP 线程随即终止。需要在src/rtsp.cpp的调用链外层兜住异常，并对已启动的线程执行stop/join后再返回错误。🤖 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/stream.cpp` around lines 3204 - 3219, 在 session::start() 的调用链外层增加异常边界，捕获 std::thread 创建等操作抛出的异常，避免其从 handle_msg() 继续传播并终止 RTSP 线程；若 audioThread 或 videoThread 仅部分启动，先执行 session 的 stop 流程并 join 已启动线程，完成半启动清理后再返回错误。Source: Path instructions
🧹 Nitpick comments (2)
src/stream.h (1)
116-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win为
session_info_t的标量字段补充默认初始值。新增的
session_id、uptime_ms、control_idle_ms、video_idle_ms、audio_idle_ms、control_connected等标量字段均未提供默认初始值。当前get_all_sessions_info()通过session_info_t info;（非info{}）构造后在 try 块内逐一赋值，异常路径会整体跳过push_back，目前不会真正暴露未初始化值，但这依赖于调用方始终完整赋值每个字段，一旦未来新增字段或调整赋值顺序遗漏某个分支，就会把未初始化的垂圾值序列化进遥测 JSON。建议直接给标量成员加默认值，消除这一隐患。🛡️ 建议的修复
struct session_info_t { std::string client_name; std::string client_uuid; std::string client_address; std::string state; std::string stop_reason; - uint32_t session_id; - std::int64_t uptime_ms; - std::int64_t control_idle_ms; - std::int64_t video_idle_ms; - std::int64_t audio_idle_ms; - bool control_connected; - int width; - int height; - int fps; - int bitrate; // Current bitrate in Kbps - bool host_audio; - bool enable_hdr; - bool enable_mic; + uint32_t session_id { 0 }; + std::int64_t uptime_ms { 0 }; + std::int64_t control_idle_ms { -1 }; + std::int64_t video_idle_ms { -1 }; + std::int64_t audio_idle_ms { -1 }; + bool control_connected { false }; + int width { 0 }; + int height { 0 }; + int fps { 0 }; + int bitrate { 0 }; // Current bitrate in Kbps + bool host_audio { false }; + bool enable_hdr { false }; + bool enable_mic { false }; std::string app_name; - int app_id; + int app_id { 0 }; };As per path instructions,
src/**/*.{cpp,c,h}审查要点包括内存安全，未初始化的 POD 成员属于此类隐患。🤖 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/stream.h` around lines 116 - 138, 为 session_info_t 中所有标量成员补充安全的默认初始值，至少覆盖 session_id、各项 *_ms、control_connected、width、height、fps、bitrate、host_audio、enable_hdr、enable_mic 和 app_id；保持现有字段类型及 get_all_sessions_info() 的赋值逻辑不变，确保默认构造 session_info_t 时不会产生未初始化值。Source: Path instructions
tests/unit/test_stream_session_observability.cpp (1)
67-84: 🎯 Functional Correctness | 🔵 Trivial | ⚖️ Poor tradeoff
SerializesSessionBoundaries未覆盖idle_ms()哨兵值本身的计算逻辑。该测试直接手写
control_idle_ms = -1等值，只验证了make_session_json对已知-1的序列化（映射为 JSONnull），但stream.cpp中idle_ms(now, last_activity)把last_activity == 0判定为"从未活动"并返回-1的这段核心逻辑本身并未被单测直接覆盖（该函数位于匿名命名空间，无法从测试文件直接调用）。可考虑通过集成测试（构造真实 session 并调用get_all_sessions_info()）或将该辅助函数移出匿名命名空间以便单测覆盖。As per path instructions，
tests/**需要验证测试覆盖率、边界情况和断言正确性。🤖 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/test_stream_session_observability.cpp` around lines 67 - 84, 扩展 SerializesSessionBoundaries 测试以覆盖 idle_ms() 对 last_activity == 0 的哨兵值计算逻辑，而不是仅手写 -1 验证 make_session_json 的序列化。优先通过真实 session 调用 get_all_sessions_info() 验证从未活动时返回 -1 并序列化为 JSON null；若现有结构不便集成测试，则将 idle_ms 移出匿名命名空间并直接补充边界单测，同时保留现有序列化断言。Source: Path instructions
🤖 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 `@src/stream.cpp`:
- Around line 3204-3219: 在 session::start() 的调用链外层增加异常边界，捕获 std::thread
创建等操作抛出的异常，避免其从 handle_msg() 继续传播并终止 RTSP 线程；若 audioThread 或 videoThread
仅部分启动，先执行 session 的 stop 流程并 join 已启动线程，完成半启动清理后再返回错误。
---
Nitpick comments:
In `@src/stream.h`:
- Around line 116-138: 为 session_info_t 中所有标量成员补充安全的默认初始值，至少覆盖 session_id、各项
*_ms、control_connected、width、height、fps、bitrate、host_audio、enable_hdr、enable_mic
和 app_id；保持现有字段类型及 get_all_sessions_info() 的赋值逻辑不变，确保默认构造 session_info_t
时不会产生未初始化值。
In `@tests/unit/test_stream_session_observability.cpp`:
- Around line 67-84: 扩展 SerializesSessionBoundaries 测试以覆盖 idle_ms() 对
last_activity == 0 的哨兵值计算逻辑，而不是仅手写 -1 验证 make_session_json 的序列化。优先通过真实 session
调用 get_all_sessions_info() 验证从未活动时返回 -1 并序列化为 JSON null；若现有结构不便集成测试，则将 idle_ms
移出匿名命名空间并直接补充边界单测，同时保留现有序列化断言。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 695de8fa-5c48-4ca8-9e4c-51f368fbdbf5
📒 Files selected for processing (5)
src/nvhttp/sessions.cppsrc/nvhttp/sessions.hsrc/stream.cppsrc/stream.htests/unit/test_stream_session_observability.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- src/nvhttp/sessions.cpp
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (2)
tests/**
⚙️ CodeRabbit configuration file
tests/**: 测试文件。验证测试覆盖率、边界情况和断言正确性。
Files:
tests/unit/test_stream_session_observability.cpp
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/nvhttp/sessions.hsrc/stream.hsrc/stream.cpp
🔇 Additional comments (5)
src/stream.h (2)
6-22: LGTM!Also applies to: 141-146
59-114:lifecycle_t的原子 + 互斥锁混合同步设计正确。
request_stop()/snapshot()均在同一把锁内完成_state与_stop_reason的写入/读取，避免了"状态已变为 STOPPING 但停止原因仍为 none"的不一致快照问题；state()仅无锁读取_state（用于自旋等待），不涉及_stop_reason，设计自洽。src/stream.cpp (1)
111-124: LGTM!
while_starting_do_nothing、各停止路径的具体原因传递、lifecycle/stop_reason的一致快照、以及control_connected依赖state == RUNNING的判定，均已正确修复此前评审中提到的一致性问题。Also applies to: 360-364, 506-512, 575-578, 874-874, 891-891, 1664-1664, 1720-1720, 1729-1731, 1770-1773, 2309-2309, 2661-2661, 2967-3000, 3021-3044, 3052-3064, 3167-3169, 3280-3284, 3370-3372, 3398-3398, 3449-3461, 3476-3476
src/nvhttp/sessions.h (1)
5-20: LGTM!使用
json_fwd.hpp与前置声明stream::session_info_t来声明make_session_json，减少了头文件的编译期耦合，是不错的实践。tests/unit/test_stream_session_observability.cpp (1)
1-15: LGTM!
PreservesFirstStopReason、PublishesOneConcurrentStopReason很好地覆盖了lifecycle_t的核心并发契约（首个停止原因保留、并发请求只有一个胜出者）。Also applies to: 29-42, 44-66
1 participant
改了啥呀
为啥要改
多客户端断开异常现在只能看到一个笼统的
-1，很难判断是控制通道断开、超时、协议错误，还是音视频线程结束。先把会话身份和生命周期信号补齐，后续才能安全地做按客户端取消和陈旧会话回收，哼，这次不靠猜啦。这层只增加观测信息，不新增超时、不主动回收会话，也不改变现有串流与全局终止逻辑。
*_idle_ms表示主机侧最近一次控制事件/媒体发送活动；活动尚未出现时返回null。客户端 UUID 仅通过现有本地管理接口暴露。验证
git diff --checksession::stop()调用均标注具体终止原因