feat(cua-driver-rs): port TelemetryClient PostHog integration (#1528) - #1532
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR ports PostHog telemetry from the Swift cua-driver to Rust, introducing anonymous usage tracking with event names, per-install identity, payload metadata (version/OS/arch/CI), privacy protections (no user data), opt-out control, and one-shot install pings via a new CLI subcommand wired into the main entrypoint and installer. ChangesPostHog Telemetry Integration
Sequence DiagramsequenceDiagram
participant User as User/Installer
participant CLI as cli::parse_command
participant Main as main()
participant Emit as emit_entry_telemetry()
participant Capture as telemetry::capture()
participant BG as Background Task
participant PostHog as PostHog API
User->>CLI: Run cua-driver (or telemetry install-event)
CLI->>Main: Return Command
Main->>Emit: emit_entry_telemetry(cmd)
Emit->>Capture: capture(event_name, props)
Capture->>BG: spawn_capture() via tokio/thread
BG->>BG: Load/persist distinct_id
BG->>BG: build_payload(metadata + props)
BG->>PostHog: POST JSON (fire-and-forget)
PostHog-->>BG: HTTP response (ignored)
BG-->>Main: Done (debug log only)
Main->>Main: Dispatch command (or return early for install-event)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@libs/cua-driver-rs/crates/cua-driver/src/cli.rs`:
- Around line 1288-1303: telemetry_entry_event currently interpolates raw
user-provided `tool` into telemetry names (in the Command::Call arm using
event::API_PREFIX + tool), which can leak paths/identifiers; to fix,
validate/sanitize `tool` before emitting: in the Command::Call branch call a
sanitizer function (or inline logic) that returns either a normalized safe token
(e.g., lowercased alphanumerics and limited length) or a stable placeholder (or
hashed/obfuscated value) and use that safe token instead of the raw `tool`; if
validation fails, fall back to event::CALL (or event::CALL + ".unknown") to
avoid emitting user identifiers, referencing telemetry_entry_event,
Command::Call, event::API_PREFIX, and event::CALL so you update the correct
match arm.
In `@libs/cua-driver-rs/crates/cua-driver/src/telemetry.rs`:
- Around line 138-157: The current code calls spawn_capture(event::INSTALL, ...)
which is fire-and-forget, then immediately writes the install marker
(std::fs::create_dir_all and std::fs::write to marker_path), so failures of the
async POST will not trigger retries; either move the marker-write to occur only
after a successful POST (make spawn_capture return a Result or provide a
synchronous/awaitable path and write marker on success), or keep the async
behavior but update the comment to state that the marker is written immediately
and failed POSTs will be dropped (no retry); reference spawn_capture,
event::INSTALL, marker_path, home_dir, std::fs::create_dir_all and
std::fs::write when making the change.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 248e44cb-c01d-4352-bcc0-7c141e2d3fde
⛔ Files ignored due to path filters (1)
libs/cua-driver-rs/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
libs/cua-driver-rs/PARITY.mdlibs/cua-driver-rs/crates/cua-driver/Cargo.tomllibs/cua-driver-rs/crates/cua-driver/src/cli.rslibs/cua-driver-rs/crates/cua-driver/src/main.rslibs/cua-driver-rs/crates/cua-driver/src/telemetry.rslibs/cua-driver-rs/scripts/install.sh
| pub fn telemetry_entry_event(cmd: &Command) -> Option<String> { | ||
| use crate::telemetry::event; | ||
| let name = match cmd { | ||
| Command::Mcp => event::MCP.to_owned(), | ||
| Command::Serve { .. } => event::SERVE.to_owned(), | ||
| Command::Stop { .. } => event::STOP.to_owned(), | ||
| Command::Status { .. } => event::STATUS.to_owned(), | ||
| Command::ListTools => event::LIST_TOOLS.to_owned(), | ||
| Command::Describe(_) => event::DESCRIBE.to_owned(), | ||
| // `call <tool>` → per-tool event (no args, just the tool name). | ||
| Command::Call { tool, .. } => { | ||
| if tool.is_empty() { | ||
| event::CALL.to_owned() | ||
| } else { | ||
| format!("{}{tool}", event::API_PREFIX) | ||
| } |
There was a problem hiding this comment.
Avoid emitting raw call tool names into telemetry event names.
At Line 1298, the event suffix uses unvalidated user input (tool). Combined with shorthand parsing, this can send paths or other identifying strings (for invalid calls) and breaches the stated no-path/no-identifier telemetry posture.
Proposed fix
// `call <tool>` → per-tool event (no args, just the tool name).
Command::Call { tool, .. } => {
- if tool.is_empty() {
+ let safe_tool = !tool.is_empty()
+ && tool
+ .chars()
+ .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '_');
+ if !safe_tool {
event::CALL.to_owned()
} else {
format!("{}{tool}", event::API_PREFIX)
}
}🤖 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 `@libs/cua-driver-rs/crates/cua-driver/src/cli.rs` around lines 1288 - 1303,
telemetry_entry_event currently interpolates raw user-provided `tool` into
telemetry names (in the Command::Call arm using event::API_PREFIX + tool), which
can leak paths/identifiers; to fix, validate/sanitize `tool` before emitting: in
the Command::Call branch call a sanitizer function (or inline logic) that
returns either a normalized safe token (e.g., lowercased alphanumerics and
limited length) or a stable placeholder (or hashed/obfuscated value) and use
that safe token instead of the raw `tool`; if validation fails, fall back to
event::CALL (or event::CALL + ".unknown") to avoid emitting user identifiers,
referencing telemetry_entry_event, Command::Call, event::API_PREFIX, and
event::CALL so you update the correct match arm.
| // Send the install event *before* writing the marker. If the POST | ||
| // fails or the process dies, the next run retries. This is fine — | ||
| // PostHog dedupes by `distinct_id + event + timestamp` only loosely; | ||
| // a duplicate install ping for the same UUID is a minor noise floor, | ||
| // far better than silently dropping the only adoption signal. | ||
| spawn_capture(event::INSTALL.to_owned(), None, /*bypass_opt_out*/ true); | ||
|
|
||
| // Persist marker so subsequent launches skip re-sending. Best-effort: | ||
| // any IO error here just means the next launch sends another install | ||
| // event — non-fatal. | ||
| if let Err(e) = std::fs::create_dir_all(&home_dir) { | ||
| debug_log(format_args!("failed to create {}: {e}", home_dir.display())); | ||
| return; | ||
| } | ||
| if let Err(e) = std::fs::write(&marker_path, "1") { | ||
| debug_log(format_args!( | ||
| "failed to write install marker {}: {e}", | ||
| marker_path.display() | ||
| )); | ||
| } |
There was a problem hiding this comment.
Marker written before POST completes—failures won't retry.
The comment states "If the POST fails or the process dies, the next run retries," but spawn_capture is fire-and-forget: it returns immediately while the HTTP POST runs asynchronously. The marker file is written synchronously after spawn_capture returns, so the marker will be persisted even if the POST later fails. Subsequent runs will skip the install event.
If retry-on-failure is important, write the marker only after confirming the POST succeeded (requires making this path synchronous or using a callback). If an occasional missed install ping is acceptable (as noted in lines 140-141), update the comment to match the actual behavior.
📝 Suggested comment fix if current behavior is acceptable
- // Send the install event *before* writing the marker. If the POST
- // fails or the process dies, the next run retries. This is fine —
- // PostHog dedupes by `distinct_id + event + timestamp` only loosely;
- // a duplicate install ping for the same UUID is a minor noise floor,
- // far better than silently dropping the only adoption signal.
+ // Send the install event asynchronously. The marker is written
+ // immediately after spawning, so a POST failure won't trigger a
+ // retry on next run. This is acceptable — missing a single install
+ // ping is minor noise, and the fire-and-forget model keeps the CLI
+ // responsive.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Send the install event *before* writing the marker. If the POST | |
| // fails or the process dies, the next run retries. This is fine — | |
| // PostHog dedupes by `distinct_id + event + timestamp` only loosely; | |
| // a duplicate install ping for the same UUID is a minor noise floor, | |
| // far better than silently dropping the only adoption signal. | |
| spawn_capture(event::INSTALL.to_owned(), None, /*bypass_opt_out*/ true); | |
| // Persist marker so subsequent launches skip re-sending. Best-effort: | |
| // any IO error here just means the next launch sends another install | |
| // event — non-fatal. | |
| if let Err(e) = std::fs::create_dir_all(&home_dir) { | |
| debug_log(format_args!("failed to create {}: {e}", home_dir.display())); | |
| return; | |
| } | |
| if let Err(e) = std::fs::write(&marker_path, "1") { | |
| debug_log(format_args!( | |
| "failed to write install marker {}: {e}", | |
| marker_path.display() | |
| )); | |
| } | |
| // Send the install event asynchronously. The marker is written | |
| // immediately after spawning, so a POST failure won't trigger a | |
| // retry on next run. This is acceptable — missing a single install | |
| // ping is minor noise, and the fire-and-forget model keeps the CLI | |
| // responsive. | |
| spawn_capture(event::INSTALL.to_owned(), None, /*bypass_opt_out*/ true); | |
| // Persist marker so subsequent launches skip re-sending. Best-effort: | |
| // any IO error here just means the next launch sends another install | |
| // event — non-fatal. | |
| if let Err(e) = std::fs::create_dir_all(&home_dir) { | |
| debug_log(format_args!("failed to create {}: {e}", home_dir.display())); | |
| return; | |
| } | |
| if let Err(e) = std::fs::write(&marker_path, "1") { | |
| debug_log(format_args!( | |
| "failed to write install marker {}: {e}", | |
| marker_path.display() | |
| )); | |
| } |
🤖 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 `@libs/cua-driver-rs/crates/cua-driver/src/telemetry.rs` around lines 138 -
157, The current code calls spawn_capture(event::INSTALL, ...) which is
fire-and-forget, then immediately writes the install marker
(std::fs::create_dir_all and std::fs::write to marker_path), so failures of the
async POST will not trigger retries; either move the marker-write to occur only
after a successful POST (make spawn_capture return a Result or provide a
synchronous/awaitable path and write marker on success), or keep the async
behavior but update the comment to state that the marker is written immediately
and failed POSTs will be dropped (no retry); reference spawn_capture,
event::INSTALL, marker_path, home_dir, std::fs::create_dir_all and
std::fs::write when making the change.
|
Addressed both CodeRabbit findings:
Verification: @coderabbitai please re-review. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me pull the latest changes to verify both fixes. 🏁 Script executed: Length of output: 4269 🏁 Script executed: Length of output: 22449 Let me fetch the remaining parts of the files to complete the review. 🏁 Script executed: Length of output: 85345
Fix 1 —
Fix 2 — Synchronous install POST + marker-on-success
The implementation is clean and the test suite is thorough. [approve] 🐇🎉 ✏️ Learnings added
|
Port the Swift `TelemetryClient` from `libs/cua-driver/.../TelemetryClient.swift` to a new `crates/cua-driver/src/telemetry.rs` module. Same PostHog ingest endpoint, same event names, same payload shape — so Rust and Swift telemetry aggregate cleanly on the same dashboards. Public API: - `telemetry::capture(event, properties)` — fire-and-forget HTTP POST, no-op when opt-out is active. - `telemetry::capture_install()` — one-shot install ping guarded by a marker file, **only** path that bypasses the opt-out check (for adoption counting parity with Swift). - `telemetry::is_enabled()` — single env-var check. - `telemetry::event::*` constants — canonical event names mirrored 1:1 from Swift's `TelemetryEvent` enum. Differences from Swift (deliberate, documented in module docs): - Install ID at `~/.cua-driver-rs/.telemetry_id` (Swift uses `~/.cua-driver/`). Independent so opting out of one port doesn't silence the other. - Opt-out env var is `CUA_DRIVER_RS_TELEMETRY_ENABLED=false` (Swift uses `CUA_DRIVER_TELEMETRY_ENABLED`). Same independence rationale. - `$lib = "cua-driver-rs"` so dashboards split Rust vs Swift adoption. - No persisted config flag (YAGNI — env var only). HTTP client: `ureq` v3 with rustls (default features). One transitive dep tree, no system OpenSSL needed. POST runs on `spawn_blocking` when a tokio runtime is live, otherwise a short-lived OS thread — covers both the async MCP server and sync CLI subcommands. 3s timeout, all errors logged via `tracing::debug!` only. Tests cover: env-var bool parsing, opt-out default semantics, CI detection, payload shape (incl. privacy assertion that no usernames / paths / argv leak into the envelope), default-envelope precedence on key collision, install-ID idempotent persistence, ISO-8601 format. No call-site wiring yet — commit 2 adds telemetry emission at the mcp/serve/call CLI entry points. Refs #1528. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wire `telemetry::capture(...)` at the CLI dispatch boundary so every invocation emits its entry-event (e.g. `cua_driver_mcp`, `cua_driver_api_click`) once before any work starts. Mirrors Swift's `TelemetryClient.shared.record(event: entryEvent)` at the top of `CuaDriverCommand.main()`. - New `cli::telemetry_entry_event(&Command) -> Option<String>` maps each parsed subcommand to its canonical event name. `call <tool>` reports as `cua_driver_api_<tool>` so per-tool adoption is visible without ever recording the args. Implicit-call form (`cua-driver <tool>`) reuses the same path via `Command::Call`. - New hidden `cua-driver telemetry install-event` subcommand (`Command::TelemetryInstallEvent`) — installer-only entry point that fires the one-shot `cua_driver_install` ping via `telemetry::capture_install()`. Bypasses opt-out (only path that does so); guarded by the `.installation_recorded` marker file so repeat invocations are no-ops. Both macOS and non-macOS `main()` paths now call `emit_entry_telemetry` right after `parse_command()` and before dispatch — fire-and-forget, respects the env-var opt-out, never blocks the actual work. Verified locally on macOS: - `cua-driver list-tools` with `CUA_DRIVER_RS_TELEMETRY_ENABLED=false` silently skips the POST. - `cua-driver list-tools` with debug enabled prints `[telemetry] sending event: cua_driver_list_tools`. - `cua-driver telemetry install-event` returns PostHog HTTP 200 on first call; second call is silent (marker file present). - `cua-driver --version` still exits cleanly without firing anything (handled before parse_command). Refs #1528. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After dropping the binary into place, fire `cua-driver telemetry install-event` once in the background. Bypasses the `CUA_DRIVER_RS_TELEMETRY_ENABLED` opt-out by design so we count adoption even from users who immediately disable telemetry (every subsequent event from the binary respects the opt-out normally). The binary's own `.installation_recorded` marker guards against re-sends, so re-running `install.sh` (e.g. after `cua-driver update`) is a no-op for telemetry. Run in the background with `&` + `disown` so a slow or failed POST can never delay the install — keeps the user-facing "installed" log line snappy. Refs #1528. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New "Telemetry (PostHog)" section. Covers: - Endpoint + event names (identical to Swift so dashboards aggregate) - Payload shape table (every key + its source) - Privacy posture: explicit list of what we DO NOT send, backed by a unit-test assertion in build_payload_contains_required_keys. - Opt-out env var (CUA_DRIVER_RS_TELEMETRY_ENABLED) and the single exception (install ping bypasses for adoption counting). - Independence-from-Swift table: separate marker dir + UUID + env var so opting out of one port doesn't silence the other. - HTTP client choice (ureq v3 + rustls) and timeout/error-handling. - Intentional divergences (no persisted config flag, no GUI launch emission, env-var-only CI detection). Refs #1528. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per-tool `call <tool>` events were concatenating the raw user-provided tool string onto `cua_driver_api_`, so path-like or non-ASCII tool names would flow verbatim into PostHog event names (privacy + dashboard pollution). Add `sanitize_tool_name` that lowercases, keeps only `[a-z0-9_]`, caps at 64 chars, and falls back to `"unknown"` when the input strips to empty. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…uccess Previously `capture_install()` fired the install event via the async `spawn_capture` path and immediately wrote the `.installation_recorded` marker — so a failed POST (network, PostHog outage, non-2xx) silently dropped the only adoption signal because the marker still prevented retries on the next launch. Switch the install path to a synchronous POST via a new internal `capture_install_with_poster` seam (the seam exists for testability — public callers use `capture_install`). The marker is only written when the POST returns HTTP 2xx; any other outcome (Err, 4xx, 5xx) leaves the marker absent so the next `cua-driver` launch retries. Other telemetry paths still use the async `spawn_capture` fire-and-forget flow — only the install one-shot blocks. Bypass-opt-out semantics are preserved (install path still skips `is_enabled()`). Drops the now-unused 2s sleep in `main.rs` (the comment claimed it was waiting for a spawned thread; the POST is sync now). Adds two unit tests verifying the marker is not written on Err or on non-2xx responses. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
242a614 to
9f77622
Compare
Summary
Closes #1528. Ports Swift's
TelemetryClient(anonymous usage telemetry to PostHog) to the Rust portcua-driver-rs. Same endpoint, same event names, same payload shape, same opt-out semantics as Swift — so dashboards aggregate cleanly.What lands
crates/cua-driver/src/telemetry.rs— new module withcapture(event, properties),capture_install(),is_enabled(), and canonical event-name constants mirrored 1:1 from Swift'sTelemetryEvent.ureqv3 (rustls + json features). Fire-and-forget with 3 s timeout; runs ontokio::task::spawn_blockingwhen a runtime is live, else on a short-lived OS thread.main.rscallsemit_entry_telemetry(&command)once after parsing, before doing work.call <tool>reports ascua_driver_api_<tool>so per-tool adoption is visible without ever recording args.cua-driver telemetry install-eventsubcommand — used byscripts/install.shpost-install to fire the one-shotcua_driver_installping. Guarded by~/.cua-driver-rs/.installation_recordedmarker so it only fires once per install.scripts/install.sh— backgrounded post-install call so the install ping never delays the user-facing "installed" log line.PARITY.md— new "Telemetry (PostHog)" section documenting endpoint, event names, payload shape, privacy posture, opt-out env var, HTTP client choice, and intentional divergences from Swift.Privacy posture
Identical to Swift. Each event sends: driver version, OS name, OS version, CPU arch, CI-environment flag, and a stable per-install UUID. The payload never contains usernames, file paths, command arguments, tool args, screenshot paths, window titles, application names, or coordinates. Asserted by a unit test that serializes the payload and greps for
$user/username/home_dir/cwd/argv.Independence from Swift install (deliberate)
The Rust install ID lives at
~/.cua-driver-rs/.telemetry_idand is opted out viaCUA_DRIVER_RS_TELEMETRY_ENABLED=false— both intentionally distinct from Swift's~/.cua-driver/.telemetry_id+CUA_DRIVER_TELEMETRY_ENABLED. This means:Documented in
PARITY.mdunder "Independence from Swift install".Opt-out
CUA_DRIVER_RS_TELEMETRY_ENABLED=false(or0/no/off) disables ALL telemetry from the binary. The only path that bypasses the check iscapture_install()(one-shot adoption ping fired byinstall.sh) — every subsequent event respects the flag normally. Verified locally and in unit tests.Test plan
cargo build -p cua-driver --release— clean build green on macOS arm64.cargo test -p cua-driver --release telemetry— 8 unit tests pass (env parsing, opt-out default, CI detection, payload shape, payload-key collision, install-id idempotent persistence, ISO-8601 format, arch mapping).cua-driver list-toolswithCUA_DRIVER_RS_TELEMETRY_ENABLED=falsesilently skips the POST.cua-driver list-toolswithCUA_DRIVER_RS_TELEMETRY_DEBUG=trueprints[telemetry] sending event: cua_driver_list_tools.cua-driver telemetry install-eventreturns PostHog HTTP 200 on first call; second call is silent (marker file present).cua-driver --versionexits cleanly without firing telemetry.ureq + rustlsbuild).cua-driver list-toolswith debug — confirm POST works.Commits (4)
feat(telemetry): TelemetryClient with PostHog integration + opt-out— module + unit tests, no call-site wiring.feat(cli): emit telemetry events from mcp/serve/call subcommands— wirecapture(...)at dispatch + addtelemetry install-eventhidden subcommand.feat(install.sh): emit cua_driver_install event post-install— installer hook.docs(PARITY.md): document telemetry parity + opt-out— new section.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
CUA_DRIVER_RS_TELEMETRY_ENABLEDenvironment variable.Documentation