Replace log with tracing

[neuronull]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-27519

📔 Objective

Cascading the standardization of tracing crate from clients repo into the SDK, that was started per bitwarden/clients#16321

, this change updates the remaining crates in the SDK to use tracing.

🚨 Breaking Changes

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation
  team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed
  issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[github-actions]

Checkmarx One – Scan Summary & Details – b01dcfff-99e3-4182-b62a-2dcc2d6bae69

Great job! No new security vulnerabilities introduced in this pull request

[github-actions]

🔍 SDK Breaking Change Detection Results

SDK Version: autofill/pm-27519/replace-log-with-tracing (788d3d8)
Completed: 2025-11-21 22:19:07 UTC
Total Time: 246s

Client	Status	Details
typescript	✅ No breaking changes detected	TypeScript compilation passed with new SDK version - View Details

---

Breaking change detection completed. View SDK workflow

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[codecov]

Codecov Report

❌ Patch coverage is 9.09091% with 100 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.51%. Comparing base (4921d8a) to head (788d3d8).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/bitwarden-uniffi/src/lib.rs	0.00%	54 Missing ⚠️
crates/bw/src/main.rs	0.00%	18 Missing ⚠️
crates/bitwarden-fido/src/authenticator.rs	0.00%	8 Missing ⚠️
crates/bw/src/auth/login.rs	0.00%	7 Missing ⚠️
crates/bitwarden-ipc/src/ipc_client.rs	44.44%	5 Missing ⚠️
crates/bitwarden-fido/src/crypto.rs	0.00%	3 Missing ⚠️
...den-core/src/auth/api/request/api_token_request.rs	0.00%	1 Missing ⚠️
...src/auth/api/request/auth_request_token_request.rs	0.00%	1 Missing ⚠️
...ore/src/auth/api/request/password_token_request.rs	0.00%	1 Missing ⚠️
...es/bitwarden-core/src/platform/get_user_api_key.rs	0.00%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #573      +/-   ##
==========================================
- Coverage   79.63%   79.51%   -0.13%     
==========================================
  Files         302      302              
  Lines       32256    32311      +55     
==========================================
+ Hits        25688    25691       +3     
- Misses       6568     6620      +52     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Claude finished @neuronull's task —— View job

---

Code Review: Replace log with tracing

Summary

This PR successfully migrates from the log crate to tracing across 27 files in multiple crates. The migration is generally well-executed with consistent patterns and improved structured logging. The changes align with the broader standardization effort started in the clients repo.

Changes since last review: N/A (first review)

Critical Issues

Finding 1: Missing test coverage for new tracing initialization code

❌ Codecov reports only 17.85% patch coverage with 46 lines missing coverage, primarily in:

• crates/bw/src/main.rs (18 lines uncovered) - tracing initialization logic
• crates/bitwarden-uniffi/src/lib.rs (likely similar)
• Various error logging paths in bitwarden-fido and bitwarden-ipc

Impact: The new tracing setup code in init_logger() and main.rs is untested, particularly the platform-specific branches for Android/iOS.

Recommendation: While testing logging infrastructure can be challenging, consider:

1. Adding integration tests that verify the logger initializes without panicking
2. Testing that log level filtering works as expected
3. Verifying the Once guard in uniffi prevents re-initialization

Location: crates/bitwarden-uniffi/src/lib.rs:114-171, crates/bw/src/main.rs:23-42

---

Finding 2: Potential secret logging in debug statements

⚠️ Per CLAUDE.md guidelines: "Do not log keys, passwords, or vault data in logs or error paths." Several debug statements log entire request objects that may contain sensitive data:

Affected locations

// crates/bitwarden-core/src/platform/get_user_api_key.rs:51
debug!(?input);  // SecretVerificationRequest may contain passwords

// crates/bw/src/auth/login.rs:27-30
error!(?two_factor);
debug!(?tf);
// These may contain tokens or sensitive authentication data

// crates/bw/src/auth/login.rs:73, 80, 85
debug!(?result);  
info!(?res);
// Login results and sync responses may contain vault data

Recommendation: Review each debug statement to ensure it doesn't log:

• Master passwords or password hashes
• API keys or tokens
• User vault data
• Private keys or encryption keys

Consider using custom Debug implementations or redacting sensitive fields.

Location: Multiple files listed above

---

Suggested Improvements

Finding 3: Inconsistent structured logging patterns

🎨 The migration shows inconsistent approaches to structured logging:

Pattern A (recommended - structured):

error!(%error, "Error handling RPC request");  // Display formatting
error!(?error, "Error finding credentials.");  // Debug formatting

Pattern B (less structured):

info!("password logging in");  // No structured fields

Pattern C (old-style string formatting):

debug!(?obj, "initializing");  // Context unclear without field name

Recommendation: Standardize on Pattern A with clear field names:

info!(user_id = ?user_id, "Password login initiated");
error!(%error, "Failed to save credential");

Location: Throughout changed files

---

Finding 4: uniffi logger initialization uses Once but doesn't handle failure

💭 The init_logger() function uses Once::call_once() which panics on initialization failure inside the closure:

.parse()
.expect("should provide valid log level at compile time.")

Question: Should initialization failures be handled more gracefully? If RUST_LOG is malformed at compile time, this will panic. Consider:

1. Using unwrap_or("info".parse().unwrap()) for safety
2. Documenting that RUST_LOG must be valid at compile-time
3. Logging a warning and falling back to INFO level

Location: crates/bitwarden-uniffi/src/lib.rs:124-127

---

Finding 5: Verbose formatting in uniffi may impact performance

💭 The uniffi tracing setup enables extensive formatting:

let fmtlayer = tracing_subscriber::fmt::layer()
    .with_ansi(true)
    .with_file(true)
    .with_line_number(true)
    .with_target(true)
    .pretty();

Question: Is this level of detail needed for production mobile apps? File/line/target information increases log verbosity. Consider:

1. Making this configurable based on build type (debug vs release)
2. Disabling .pretty() in release builds for performance
3. Documenting the performance trade-off

Location: crates/bitwarden-uniffi/src/lib.rs:133-138

---

Finding 6: Removed log workspace dependency but crates may still transitively depend on it

💭 The PR removes log from workspace dependencies, but many crates in the dependency tree likely still use log (e.g., reqwest, rustls). Consider:

1. Verifying that tracing-log bridge is included where needed
2. Checking if any direct dependencies still expose log types in their APIs
3. Documenting the migration strategy for transitive dependencies

Location: Cargo.toml:61

---

Good Practices Observed

• Consistent migration pattern using %error for Display and ?error for Debug
• Platform-specific logging implementations for iOS (oslog) and Android (logcat)
• Proper use of EnvFilter for runtime configuration
• Removal of unused dependencies (env_logger, android_logger, oslog)

---

Action Items

For Author:

1. ❌ Required: Address test coverage gap - at minimum add smoke tests for logger initialization
2. ⚠️ Required: Audit all debug/info statements for potential secret logging (Finding 2)
3. 🎨 Optional: Consider standardizing structured logging patterns across the codebase
4. 💭 Respond to questions about error handling (Finding 4) and verbosity (Finding 5)

PR Metadata:

• ✅ Title is clear and specific
• ✅ Objective explains what and why
• ✅ Jira reference present
• ⚠️ Test plan: PR description only states "Written new unit and / or integration tests where applicable" but codecov shows low patch coverage - please clarify testing approach

---