feat(tools): add openmonitor — TUI and web dashboard for openads_serverd#5
feat(tools): add openmonitor — TUI and web dashboard for openads_serverd#5Admnwk wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces openmonitor, a Rust-based terminal (TUI) and web dashboard monitor for the openads_serverd service, reading live telemetry from wire and HTTP APIs. The feedback highlights several critical issues: a compilation error in http_api.rs due to an invalid unwrap_or_default() call, a terminal state restoration bug in main.rs on early exit, blocking synchronous calls inside async Axum handlers in web.rs, a potential out-of-memory vulnerability in wire.rs from unconstrained network frame allocations, silently ignored JSON deserialization errors, and a potential out-of-bounds panic when the session list shrinks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| client: reqwest::blocking::Client::builder() | ||
| .timeout(std::time::Duration::from_secs(5)) | ||
| .build() | ||
| .unwrap_or_default(), |
There was a problem hiding this comment.
unwrap_or_default() is called on the result of reqwest::blocking::Client::builder().build(). However, reqwest::blocking::Client does not implement Default. This will cause a compilation error. Using .expect("failed to build reqwest client") or handling the error is a safer and correct approach.
| client: reqwest::blocking::Client::builder() | |
| .timeout(std::time::Duration::from_secs(5)) | |
| .build() | |
| .unwrap_or_default(), | |
| client: reqwest::blocking::Client::builder() | |
| .timeout(std::time::Duration::from_secs(5)) | |
| .build() | |
| .expect("failed to build reqwest client"), |
| fn run_tui(app: &mut TuiState, cfg: &MonitorConfig, args: &Args) -> Result<()> { | ||
| enable_raw_mode()?; | ||
| stdout().execute(EnterAlternateScreen)?; | ||
| let mut term = ratatui::init(); | ||
|
|
||
| let tick = Duration::from_secs(cfg.interval_secs); | ||
| let mut needs_redraw = true; | ||
|
|
||
| loop { | ||
| if needs_redraw || app.last_refresh.elapsed() >= tick { | ||
| app.refresh(cfg); | ||
| needs_redraw = true; | ||
| } | ||
|
|
||
| if needs_redraw { | ||
| term.draw(|f| ui(f, app, cfg, args))?; | ||
| needs_redraw = false; | ||
| } | ||
|
|
||
| if event::poll(Duration::from_millis(200))? { | ||
| if let Event::Key(key) = event::read()? { | ||
| if key.kind != KeyEventKind::Press { | ||
| continue; | ||
| } | ||
| match key.code { | ||
| KeyCode::Char('q') | KeyCode::Esc => break, | ||
| KeyCode::Char('r') => { | ||
| app.refresh(cfg); | ||
| app.status_msg = "refreshed".into(); | ||
| needs_redraw = true; | ||
| } | ||
| KeyCode::Up => { | ||
| app.selected_session = app.selected_session.saturating_sub(1); | ||
| needs_redraw = true; | ||
| } | ||
| KeyCode::Down => { | ||
| if !app.inner.http.sessions.is_empty() { | ||
| app.selected_session = | ||
| (app.selected_session + 1).min(app.inner.http.sessions.len() - 1); | ||
| } | ||
| needs_redraw = true; | ||
| } | ||
| KeyCode::Char('g') => { | ||
| app.show_graphs = !app.show_graphs; | ||
| app.status_msg = if app.show_graphs { | ||
| "graphs on".into() | ||
| } else { | ||
| "graphs off".into() | ||
| }; | ||
| needs_redraw = true; | ||
| } | ||
| KeyCode::Char('k') => { | ||
| if let (Some(base), Some(sess)) = | ||
| (&cfg.http, app.inner.http.sessions.get(app.selected_session)) | ||
| { | ||
| let client = HttpClient::new(base); | ||
| match client.kill_session(sess.id) { | ||
| Ok(()) => { | ||
| app.status_msg = format!("killed session {}", sess.id); | ||
| app.refresh(cfg); | ||
| } | ||
| Err(e) => app.status_msg = format!("kill failed: {e}"), | ||
| } | ||
| needs_redraw = true; | ||
| } else { | ||
| app.status_msg = "kill needs --http and a selected session".into(); | ||
| needs_redraw = true; | ||
| } | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| disable_raw_mode()?; | ||
| stdout().execute(LeaveAlternateScreen)?; | ||
| ratatui::restore(); | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
If any of the ? operators inside run_tui (such as event::poll or event::read) return an error, the function will exit early before restoring the terminal state. This leaves the user's terminal in raw mode and alternate screen, which is a poor user experience. Wrapping the main loop in a closure or block and ensuring the cleanup code always runs before returning the result prevents this issue.
fn run_tui(app: &mut TuiState, cfg: &MonitorConfig, args: &Args) -> Result<()> {
enable_raw_mode()?;
stdout().execute(EnterAlternateScreen)?;
let mut term = ratatui::init();
let tick = Duration::from_secs(cfg.interval_secs);
let mut needs_redraw = true;
let res = (|| -> Result<()> {
loop {
if needs_redraw || app.last_refresh.elapsed() >= tick {
app.refresh(cfg);
needs_redraw = true;
}
if needs_redraw {
term.draw(|f| ui(f, app, cfg, args))?;
needs_redraw = false;
}
if event::poll(Duration::from_millis(200))? {
if let Event::Key(key) = event::read()? {
if key.kind != KeyEventKind::Press {
continue;
}
match key.code {
KeyCode::Char('q') | KeyCode::Esc => break,
KeyCode::Char('r') => {
app.refresh(cfg);
app.status_msg = "refreshed".into();
needs_redraw = true;
}
KeyCode::Up => {
app.selected_session = app.selected_session.saturating_sub(1);
needs_redraw = true;
}
KeyCode::Down => {
if !app.inner.http.sessions.is_empty() {
app.selected_session =
(app.selected_session + 1).min(app.inner.http.sessions.len() - 1);
}
needs_redraw = true;
}
KeyCode::Char('g') => {
app.show_graphs = !app.show_graphs;
app.status_msg = if app.show_graphs {
"graphs on".into()
} else {
"graphs off".into()
};
needs_redraw = true;
}
KeyCode::Char('k') => {
if let (Some(base), Some(sess)) =
(&cfg.http, app.inner.http.sessions.get(app.selected_session))
{
let client = HttpClient::new(base);
match client.kill_session(sess.id) {
Ok(()) => {
app.status_msg = format!("killed session {}", sess.id);
app.refresh(cfg);
}
Err(e) => app.status_msg = format!("kill failed: {e}"),
}
needs_redraw = true;
} else {
app.status_msg = "kill needs --http and a selected session".into();
needs_redraw = true;
}
}
_ => {}
}
}
}
}
Ok(())
})();
disable_raw_mode()?;
stdout().execute(LeaveAlternateScreen)?;
ratatui::restore();
res
}| async fn api_kill_session( | ||
| State(st): State<WebState>, | ||
| Path(id): Path<u64>, | ||
| ) -> impl IntoResponse { | ||
| let Some(base) = st.cfg.http.clone() else { | ||
| return (StatusCode::BAD_REQUEST, "monitor started without --http").into_response(); | ||
| }; | ||
| match HttpClient::new(&base).kill_session(id) { | ||
| Ok(()) => { | ||
| if let Ok(mut state) = st.poll.write() { | ||
| state.refresh(&st.cfg); | ||
| if let Ok(mut out) = st.snap.write() { | ||
| *out = state.to_snapshot(&st.cfg); | ||
| } | ||
| } | ||
| StatusCode::OK.into_response() | ||
| } | ||
| Err(e) => (StatusCode::BAD_GATEWAY, e.to_string()).into_response(), | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Calling synchronous blocking operations like HttpClient::kill_session and state.refresh inside an async Axum handler blocks the Tokio executor thread, which can starve other tasks and degrade performance. It is highly recommended to run these blocking operations inside tokio::task::spawn_blocking.
async fn api_kill_session(
State(st): State<WebState>,
Path(id): Path<u64>,
) -> impl IntoResponse {
let Some(base) = st.cfg.http.clone() else {
return (StatusCode::BAD_REQUEST, "monitor started without --http").into_response();
};
let res = tokio::task::spawn_blocking(move || {
let client = HttpClient::new(&base);
client.kill_session(id).map(|_| {
if let Ok(mut state) = st.poll.write() {
state.refresh(&st.cfg);
if let Ok(mut out) = st.snap.write() {
*out = state.to_snapshot(&st.cfg);
}
}
})
})
.await;
match res {
Ok(Ok(())) => StatusCode::OK.into_response(),
Ok(Err(e)) => (StatusCode::BAD_GATEWAY, e.to_string()).into_response(),
Err(e) => (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()).into_response(),
}
}| let len = u32::from_be_bytes([header[0], header[1], header[2], header[3]]) as usize; | ||
| let opcode = header[4]; | ||
| let mut payload = vec![0u8; len]; |
There was a problem hiding this comment.
The frame length len is read directly from the network as a u32 and used to allocate a Vec of that size. If a corrupted or malicious server sends a very large length, this can cause an out-of-memory crash (OOM panic) on the client. Adding a reasonable upper limit (e.g., 16MB) before allocating the payload buffer prevents this vulnerability.
let len = u32::from_be_bytes([header[0], header[1], header[2], header[3]]) as usize;
if len > 16 * 1024 * 1024 {
return Err(anyhow::anyhow!("frame length {} exceeds maximum limit of 16MB", len));
}
let opcode = header[4];
let mut payload = vec![0u8; len];| match self.client.get(format!("{}/api/health", self.base)).send() { | ||
| Ok(r) if r.status().is_success() => { | ||
| snap.health = r.json().ok(); | ||
| } | ||
| Ok(r) => snap.error = Some(format!("health HTTP {}", r.status())), | ||
| Err(e) => snap.error = Some(format!("health: {e}")), | ||
| } |
There was a problem hiding this comment.
If r.json().ok() returns None due to a JSON deserialization failure, the error is silently ignored, and snap.health remains None without any error message. It is better to capture the deserialization error and store it in snap.error to assist with debugging.
match self.client.get(format!("{}/api/health", self.base)).send() {
Ok(r) if r.status().is_success() => {
match r.json::<Health>() {
Ok(h) => snap.health = Some(h),
Err(e) => snap.error = Some(format!("health JSON decode: {e}")),
}
}
Ok(r) => snap.error = Some(format!("health HTTP {}", r.status())),
Err(e) => snap.error = Some(format!("health: {e}")),
}| impl TuiState { | ||
| fn refresh(&mut self, cfg: &MonitorConfig) { | ||
| self.inner.refresh(cfg); | ||
| self.last_refresh = Instant::now(); | ||
| } | ||
| } |
There was a problem hiding this comment.
If the number of active HTTP sessions shrinks after a refresh, app.selected_session can become out of bounds. Clamping selected_session to the new session list length inside TuiState::refresh ensures the selection highlight remains valid and visible.
impl TuiState {
fn refresh(&mut self, cfg: &MonitorConfig) {
self.inner.refresh(cfg);
self.last_refresh = Instant::now();
let len = self.inner.http.sessions.len();
if self.selected_session >= len {
self.selected_session = len.saturating_sub(1);
}
}
}Wire Mg telemetry + Studio HTTP sessions. See tools/openmonitor/README.md. Co-Authored-By: Grok Build (colab) <noreply@x.ai>
TUI terminal restore on error, spawn_blocking for web kill, 16MB wire frame cap, explicit JSON/reqwest errors, clamp selected_session. Co-Authored-By: Grok Build (colab) <noreply@x.ai>
Summary
Adds
tools/openmonitor/, a Rust lab/monitoring tool for a runningopenads_serverdinstance.Features
MgRequest/MgSnapshot(uptime, connections, open tables, packet counters)/api/server/sessions, kill session from TUI--web, default:9850)--onceprints a text snapshot and exitsBuild
cd tools/openmonitor cargo build --releaseWindows:
build.bat(requirescargoon PATH).Usage
See
tools/openmonitor/README.mdfor full CLI reference.Notes
tools/bench.openads-wire-lab); this PR only adds the monitor tool itself.16262(lab-friendly; avoids clashing with legacy default 6262).Co-Authored-By: Grok Build (colab) noreply@x.ai