PostHog stats for uploader #1198
Conversation
WalkthroughAdds posthog-rs and a patch override, initializes PostHog on startup when VITE_POSTHOG_KEY is set, introduces a PostHogEvent enum and async_capture_event helper, and instruments multipart/video upload flows to emit success/failure events with elapsed durations and error details. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as User
participant App as Desktop App (Tauri)
participant Telemetry as PostHog Module
participant PH as PostHog API
User->>App: Launch app
Note over App: read VITE_POSTHOG_KEY\nif present -> spawn init (background)
App->>Telemetry: init()
Telemetry->>PH: initialize client (async)
alt init ok
PH-->>Telemetry: success
else init error
Telemetry-->>App: log init error
end
sequenceDiagram
autonumber
participant UI as UI
participant Upload as Upload Flow
participant Telemetry as PostHog Module
participant PH as PostHog API
UI->>Upload: start upload (spawn)
Upload->>Upload: record start = Instant::now()
Upload->>Upload: run() (async)
alt success
Upload-->>UI: Ok
Upload->>Telemetry: async_capture_event(MultipartUploadComplete{duration})
Telemetry->>PH: capture event (async)
PH-->>Telemetry: resp
else failure
Upload-->>UI: Err(error)
Upload->>Telemetry: async_capture_event(MultipartUploadFailed{duration,error})
Telemetry->>PH: capture event (async)
PH-->>Telemetry: resp
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src-tauri/src/upload.rs (1)
213-219: Unusedstartvariable (will trip clippy).Remove or use it to report image upload timing; currently it’s dead code.
- let start = Instant::now();
🧹 Nitpick comments (3)
apps/desktop/src-tauri/Cargo.toml (1)
115-115: Make telemetry dependency optional (feature‑gated) to allow builds without analytics.Helps ship OSS/no‑telemetry builds and simplifies compliance toggles.
Within this file:
-posthog-rs = "0.3.7" +posthog-rs = { version = "0.3.7", optional = true }Elsewhere in this file (outside selected lines), add:
[features] default = [] telemetry = ["posthog-rs"]And gate usage sites with
#[cfg(feature = "telemetry")]as appropriate.apps/desktop/src-tauri/src/lib.rs (2)
2773-2780: Guard event capture behind a runtime toggle; avoid sending without consent.Early‑return if telemetry is disabled (env/config), and consider adding common props (e.g., app version).
Apply minimal guard:
pub fn async_capture_event(event: posthog_rs::Event) { - tokio::spawn(async move { + if std::env::var("CAP_TELEMETRY_ENABLED").unwrap_or_else(|_| "1".into()) == "0" { + return; + } + tokio::spawn(async move { posthog_rs::capture(event) .await .map_err(|err| error!("Error sending event to PostHog: {err:?}")) .ok(); }); }Also ensure codebase has a user‑visible setting persisted in GeneralSettings to control this toggle. Based on coding guidelines.
1947-1953: Refactor PostHog telemetry initialization
- Load the public
phc_…key from env/config (e.g.POSTHOG_KEY) instead of hard-coding.- Add explicit user opt-in/opt-out (with a global setting) before calling
init_global.- Disable telemetry in debug builds to avoid dev noise.
- Expose host/region config (e.g.
POSTHOG_HOST) and perform a shutdown/flush on exit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
apps/desktop/src-tauri/Cargo.toml(1 hunks)apps/desktop/src-tauri/src/lib.rs(2 hunks)apps/desktop/src-tauri/src/upload.rs(10 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/lib.rsapps/desktop/src-tauri/src/upload.rs
🧬 Code graph analysis (2)
apps/desktop/src-tauri/src/lib.rs (1)
apps/desktop/src-tauri/src/upload.rs (1)
spawn(366-403)
apps/desktop/src-tauri/src/upload.rs (3)
apps/desktop/src-tauri/src/lib.rs (8)
async_capture_event(2773-2780)run(1940-2518)app(1176-1177)app(2349-2349)app(2380-2380)app(2417-2417)app(2423-2423)app(2623-2624)crates/utils/src/lib.rs (1)
spawn_actor(6-13)apps/desktop/src-tauri/src/upload_legacy.rs (1)
run(620-803)
⏰ 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). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (1)
apps/desktop/src-tauri/src/upload.rs (1)
6-6: Import looks fine.No issues.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/desktop/src-tauri/src/upload.rs (2)
153-169: Castas_millis()tou64and consider thumbnail result for accurate metrics.Two issues:
Critical:
Duration::as_millis()returnsu128, which JSON/serde may not accept reliably. Cast tou64to ensure property insertion succeeds.Major: The "complete" event fires when video upload succeeds, even if the thumbnail upload fails (checked on line 170). This skews metrics.
Apply this diff to address both issues:
- async_capture_event(match &video_result { - Ok(()) => { + async_capture_event(match (&video_result, &thumbnail_result) { + (Ok(()), Ok(())) => { let mut e = posthog_rs::Event::new_anon("multipart_upload_complete"); - e.insert_prop("took", start.elapsed().as_millis()) + e.insert_prop("took", start.elapsed().as_millis() as u64) .map_err(|err| error!("Error adding PostHog property: {err:?}")) .ok(); e } - Err(err) => { + (Err(err), _) | (_, Err(err)) => { let mut e = posthog_rs::Event::new_anon("multipart_upload_failed"); e.insert_prop("error", err.to_string()) .map_err(|err| error!("Error adding PostHog property: {err:?}")) .ok(); e } });
374-402: Castas_millis()tou64to prevent property insertion failure.
Duration::as_millis()returnsu128, which JSON/serde may not handle reliably. Cast tou64for compatibility with PostHog's expected integer type.Ok(()) => { let mut e = posthog_rs::Event::new_anon("multipart_upload_complete"); - e.insert_prop("took", start.elapsed().as_millis()) + e.insert_prop("took", start.elapsed().as_millis() as u64) .map_err(|err| error!("Error adding PostHog property: {err:?}")) .ok(); e
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src-tauri/src/lib.rs(2 hunks)apps/desktop/src-tauri/src/upload.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/lib.rsapps/desktop/src-tauri/src/upload.rs
🧬 Code graph analysis (2)
apps/desktop/src-tauri/src/lib.rs (1)
apps/desktop/src-tauri/src/upload.rs (1)
spawn(366-404)
apps/desktop/src-tauri/src/upload.rs (2)
apps/desktop/src-tauri/src/lib.rs (7)
async_capture_event(2775-2784)app(1176-1177)app(2351-2351)app(2382-2382)app(2419-2419)app(2425-2425)app(2625-2626)crates/utils/src/lib.rs (1)
spawn_actor(6-13)
⏰ 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). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (3)
apps/desktop/src-tauri/src/lib.rs (2)
1947-1954: LGTM! PostHog initialization is non-blocking and well-handled.The startup task correctly spawns asynchronously, logs errors without panicking, and only initializes when the environment variable is present.
2775-2784: LGTM! Helper function is well-structured.The compile-time environment check and async spawn pattern are appropriate for fire-and-forget telemetry.
apps/desktop/src-tauri/src/upload.rs (1)
6-6: LGTM! Import is necessary for the telemetry integration.
apps/desktop/src-tauri/src/upload.rs
Outdated
| .map_err(Into::into); | ||
| } | ||
|
|
||
| let start = Instant::now(); |
There was a problem hiding this comment.
Remove unused start variable.
The start variable is captured but never used. This adds unnecessary overhead without providing value.
- let start = Instant::now();
let file_name = file_path📝 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.
| let start = Instant::now(); | |
| let file_name = file_path |
🤖 Prompt for AI Agents
In apps/desktop/src-tauri/src/upload.rs around line 213, the local variable
`start` is declared with Instant::now() but never used; remove the unused
`start` binding (delete the let start = Instant::now(); line) to eliminate the
dead variable and any unused import/unused variable warning.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src-tauri/src/lib.rs(3 hunks)apps/desktop/src-tauri/src/upload.rs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src-tauri/src/upload.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/lib.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/lib.rs (1)
apps/desktop/src-tauri/src/upload.rs (1)
spawn(358-389)
⏰ 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). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (3)
apps/desktop/src-tauri/src/lib.rs (3)
77-77: LGTM!The
Durationimport is correctly added and necessary for thePostHogEventenum.
2806-2815: LGTM with note on fire-and-forget pattern.The fire-and-forget async pattern is appropriate for telemetry—failures won't block the application. The conditional check ensures events are only sent when PostHog is configured.
Note: If the typo in
PostHogEventvariant names is fixed (see previous comment), ensure callers inupload.rsare updated accordingly.
1948-1955: Based on my investigation, I cannot find definitive documentation for theposthog-rscrate's behavior regarding event queuing before initialization. The crate is community-maintained and minimal (328 lines total), and available documentation doesn't explicitly address how pre-initialization events are handled. However, based on observable patterns:
- The posthog-rs client is not async-friendly due to using the blocking feature of the reqwest crate
- The code spawns
init_global()asynchronously without awaiting it- No explicit queueing mechanism is documented
The original review comment raises a plausible but unverified concern. Since I cannot definitively confirm or refute whether
posthog_rshandles pre-initialization events gracefully, I must recommend manual verification.Verify PostHog event handling before initialization completes.
The code spawns asynchronous initialization without awaiting, and
async_capture_event()calls (used in upload flows) may fire before initialization finishes. Whileposthog_rsis a minimal community-maintained crate, its behavior for pre-initialization event capture is not clearly documented. Recommend verifying in the actual crate implementation whether events sent beforeinit_global()completes are queued, silently dropped, or error-prone.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/desktop/src-tauri/src/upload.rs (1)
153-161: PostHog event only considers video result, not thumbnail result.The event emission at lines 153-161 only checks
video_result, meaning a "multipart_upload_complete" event fires even ifthumbnail_resultfails. Since line 163 will propagate the thumbnail error, the operation ultimately fails but metrics show success.This issue was previously flagged and should consider both results:
- async_capture_event(match &video_result { - Ok(()) => PostHogEvent::MultipartUploadComplete { + async_capture_event(match (&video_result, &thumbnail_result) { + (Ok(()), Ok(())) => PostHogEvent::MultipartUploadComplete { duration: start.elapsed(), }, - Err(err) => PostHogEvent::MultipartUploadFailed { + (Err(err), _) | (_, Err(err)) => PostHogEvent::MultipartUploadFailed { duration: start.elapsed(), error: err.to_string(), }, });
🧹 Nitpick comments (2)
apps/desktop/src-tauri/src/posthog.rs (2)
15-17: Error handling silently drops property insertion failures.The pattern
.map_err(|err| error!(...)).ok()logs insertion errors but continues, meaning events may be sent with missing properties. For telemetry this is acceptable, but consider whether partial events provide value or if failed insertions should skip the entire event.Also applies to: 22-27
45-54: Fire-and-forget telemetry may lose events on quick app shutdown.
async_capture_eventspawns a task without awaiting it, so if the application exits before the task completes, the event will be lost. This is acceptable for best-effort telemetry, but document this behavior if reliable event capture is ever required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src-tauri/src/lib.rs(2 hunks)apps/desktop/src-tauri/src/posthog.rs(1 hunks)apps/desktop/src-tauri/src/upload.rs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src-tauri/src/lib.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/upload.rsapps/desktop/src-tauri/src/posthog.rs
🧬 Code graph analysis (2)
apps/desktop/src-tauri/src/upload.rs (3)
apps/desktop/src-tauri/src/posthog.rs (1)
async_capture_event(45-54)crates/utils/src/lib.rs (1)
spawn_actor(6-13)apps/desktop/src-tauri/src/upload_legacy.rs (1)
run(620-803)
apps/desktop/src-tauri/src/posthog.rs (1)
apps/desktop/src-tauri/src/upload.rs (1)
spawn(358-389)
⏰ 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). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (1)
apps/desktop/src-tauri/src/upload.rs (1)
366-387: LGTM!The telemetry instrumentation in
spawncorrectly captures timing for the entire upload operation and emits appropriate events on both success and failure.
2 participants
This will allow us to get a better idea of what the average user feels when using Cap.
Summary by CodeRabbit
New Features
Chores