Skip to content

Conversation

oscartbeaumont
Copy link
Member

@oscartbeaumont oscartbeaumont commented Oct 17, 2025

Summary by CodeRabbit

  • Refactor
    • Enhanced internal API instrumentation for upload operations to improve system observability and maintainability.

Copy link
Contributor

coderabbitai bot commented Oct 17, 2025

Walkthrough

Updated instrumentation attributes on async API functions in the Tauri desktop app and added app: &AppHandle parameter to method signatures for upload operations, including multipart initialization, part presigning, signing, progress tracking, and completion.

Changes

Cohort / File(s) Summary
API Instrumentation and Signature Updates
apps/desktop/src-tauri/src/api.rs
Adjusted #[instrument] attributes on five public async functions to skip the app parameter; updated method signatures for upload_multipart_initiate, upload_multipart_presign_part, upload_signed, desktop_video_progress, and upload_multipart_complete to accept app: &AppHandle as first parameter

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Poem

🐰 Hops with glee through methods bright,
App parameters now in sight!
Skip and instrument with care,
Upload functions floating fair!
Tauri magic, code so neat,
Makes this rabbit's heart skip beat! 🚀

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Ignore app from more traces" directly and accurately describes the main changes in the changeset. The modifications center on adjusting instrumentation attributes on public async API functions to skip the app parameter from traces, such as changing #[instrument] to #[instrument(skip(app))] and updating existing skip attributes to include app. The title is concise, specific, and clearly communicates the intent of the changes without unnecessary noise. A teammate scanning the repository history would readily understand that this PR addresses tracing instrumentation adjustments related to the app parameter.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ignore-app-from-more-traces

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 47cb080 and d94ad26.

📒 Files selected for processing (1)
  • apps/desktop/src-tauri/src/api.rs (4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Format Rust code using rustfmt and 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/api.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/api.rs (7)
apps/desktop/src-tauri/src/lib.rs (6)
  • app (1171-1172)
  • app (2340-2340)
  • app (2371-2371)
  • app (2408-2408)
  • app (2414-2414)
  • app (2614-2615)
apps/web/app/api/upload/[...route]/multipart.ts (1)
  • app (26-26)
apps/web/app/api/desktop/[...route]/video.ts (1)
  • app (31-31)
apps/desktop/src-tauri/src/fake_window.rs (1)
  • app (52-52)
apps/desktop/src-tauri/src/hotkeys.rs (2)
  • app (103-103)
  • app (187-187)
apps/desktop/src-tauri/src/windows.rs (4)
  • app (214-214)
  • app (366-366)
  • app (456-456)
  • app (771-771)
apps/desktop/src-tauri/src/deeplink_actions.rs (1)
  • app (117-117)
⏰ 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: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Analyze (rust)
🔇 Additional comments (5)
apps/desktop/src-tauri/src/api.rs (5)

11-15: LGTM! Instrumentation correctly skips app.

The #[instrument(skip(app))] attribute appropriately excludes the AppHandle from traces while preserving video_id for observability.


49-55: LGTM! Instrumentation correctly skips sensitive parameters.

The attribute appropriately skips both app and upload_id while preserving video_id and part_number for trace correlation.


111-118: Verify skip_all vs. selective skipping.

The instrumentation uses skip_all which hides all parameters from traces, including potentially useful identifiers like video_id. This appears inconsistent with:

  1. The AI-generated summary that indicates this should be #[instrument(skip(app))]
  2. The PR objective "Ignore app from more traces" which suggests selective skipping
  3. Other functions in this file that use selective skipping (e.g., skip(app) or skip(app, upload_id))

If skip_all is intentional due to the size of the parts array, consider #[instrument(skip(app, parts, meta))] to preserve video_id and upload_id for trace correlation.


183-187: LGTM! Instrumentation correctly skips app.

The attribute appropriately excludes the AppHandle while preserving the body parameter for trace visibility.


221-227: LGTM! Instrumentation correctly skips app.

The attribute appropriately excludes the AppHandle while preserving progress tracking parameters (video_id, uploaded, total) for observability.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Brendonovich Brendonovich merged commit 6ecdf9e into main Oct 17, 2025
15 checks passed
@oscartbeaumont oscartbeaumont deleted the ignore-app-from-more-traces branch October 17, 2025 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants