-
Notifications
You must be signed in to change notification settings - Fork 24
feat(uniffi): add logging callback for mobile observability #627
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| name: Free Disk Space | ||
| description: Remove unused toolchains and packages to free disk space on ubuntu runners | ||
| runs: | ||
| using: "composite" | ||
| steps: | ||
| - name: Free Disk Space | ||
| shell: bash | ||
| run: | | ||
| # Function to measure and remove a directory | ||
| remove_and_log() { | ||
| local path=$1 | ||
| local name=$2 | ||
| if [ -e "$path" ]; then | ||
| local size_kb=$(du -sk "$path" 2>/dev/null | cut -f1) | ||
| local size_mb=$((size_kb / 1024)) | ||
| sudo rm -rf "$path" | ||
| echo "Removed $name, freeing ${size_mb}MB" | ||
| fi | ||
| } | ||
| # Capture initial disk space | ||
| initial_avail=$(df / | awk 'NR==2 {print $4}') | ||
| echo "=== Disk Cleanup Starting ===" | ||
| echo "Available space before cleanup: $(df -h / | awk 'NR==2 {print $4}')" | ||
| echo "" | ||
| # Remove directories and track space freed | ||
| remove_and_log "/usr/share/dotnet" ".NET SDKs" | ||
| remove_and_log "/usr/share/swift" "Swift toolchain" | ||
| remove_and_log "/usr/local/.ghcup" "Haskell (ghcup)" | ||
| remove_and_log "/usr/share/miniconda" "Miniconda" | ||
| remove_and_log "/usr/local/aws-cli" "AWS CLI v1" | ||
| remove_and_log "/usr/local/aws-sam-cli" "AWS SAM CLI" | ||
| remove_and_log "/usr/local/lib/android" "Android SDK" | ||
| remove_and_log "/usr/lib/google-cloud-sdk" "Google Cloud SDK" | ||
| remove_and_log "/usr/lib/jvm" "Java JDKs" | ||
| remove_and_log "/usr/local/share/powershell" "PowerShell" | ||
| remove_and_log "/opt/hostedtoolcache" "Hosted tool cache" | ||
| # Calculate and display results | ||
| final_avail=$(df / | awk 'NR==2 {print $4}') | ||
| space_freed=$((final_avail - initial_avail)) | ||
| space_freed_mb=$((space_freed / 1024)) | ||
| echo "" | ||
| echo "=== Cleanup Complete ===" | ||
| echo "Available space after cleanup: $(df -h / | awk 'NR==2 {print $4}')" | ||
| echo "Total space freed: ${space_freed_mb}MB" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,164 @@ | ||
| # SDK Logging Callback Guide | ||
|
|
||
| ## Overview | ||
|
|
||
| The Bitwarden SDK provides an optional logging callback interface that enables mobile applications | ||
| to receive trace logs from the SDK and forward them to observability systems like Flight Recorder. | ||
| This document explains the design, integration patterns, and best practices for using the logging | ||
| callback feature. | ||
|
|
||
| ## Purpose and Use Case | ||
|
|
||
| The logging callback addresses mobile teams' requirements for: | ||
|
|
||
| - **Observability**: Collecting SDK trace events in centralized monitoring systems | ||
| - **Debugging**: Capturing SDK behavior for troubleshooting production issues | ||
| - **Flight Recorder Integration**: Feeding SDK logs into mobile observability platforms | ||
|
|
||
| The callback is entirely optional. Platform-specific loggers (oslog on iOS, android_logger on | ||
| Android) continue functioning independently whether or not a callback is registered. | ||
|
|
||
| ## Architecture | ||
|
|
||
| ### Design Principles | ||
|
|
||
| 1. **Non-intrusive**: The SDK operates identically with or without a callback registered | ||
| 2. **Thread-safe**: Multiple SDK threads may invoke the callback concurrently | ||
| 3. **Error-resilient**: Mobile callback failures do not crash the SDK | ||
| 4. **Simple contract**: Mobile teams receive level, target, and message - all other decisions are | ||
| theirs | ||
|
|
||
| ### Data Flow | ||
|
|
||
| ```mermaid | ||
| graph TB | ||
| A[SDK Code] -->|tracing::info!| B[Tracing Subscriber] | ||
| B --> C[tracing-subscriber Registry] | ||
| C --> D[CallbackLayer] | ||
| C --> E[Platform Loggers] | ||
| D --> F{Callback<br/>Registered?} | ||
| F -->|Yes| G[UNIFFI Callback] | ||
| F -->|No| H[No-op] | ||
| G -->|FFI Boundary| I[Mobile Implementation] | ||
| I --> J[Flight Recorder] | ||
| E --> K[oslog iOS] | ||
| E --> L[android_logger] | ||
| E --> M[env_logger Other] | ||
| style D fill:#e1f5ff | ||
| style G fill:#ffe1e1 | ||
| style I fill:#fff4e1 | ||
| style J fill:#e1ffe1 | ||
| ``` | ||
|
|
||
| Platform loggers (oslog/android_logger) operate in parallel, receiving the same events | ||
| independently. | ||
|
|
||
| ### Callback Invocation Flow | ||
|
|
||
| ```mermaid | ||
| sequenceDiagram | ||
| participant SDK as SDK Code | ||
| participant Tracing as Tracing Layer | ||
| participant Callback as CallbackLayer | ||
| participant UNIFFI as UNIFFI Bridge | ||
| participant Mobile as Mobile Client | ||
| participant FR as Flight Recorder | ||
| SDK->>Tracing: tracing::info!("message") | ||
| Tracing->>Callback: on_event() | ||
| Callback->>Callback: Extract level, target, message | ||
| Callback->>UNIFFI: callback.on_log(...) | ||
| UNIFFI->>Mobile: onLog(...) [FFI] | ||
| Mobile-->>FR: Queue & Process | ||
| UNIFFI-->>Callback: Result<(), Error> | ||
| Note over Callback: If error: log to platform logger | ||
| Note over Mobile: Mobile controls batching, filtering | ||
| ``` | ||
|
|
||
| ## LogCallback Interface | ||
|
|
||
| ### Trait Definition | ||
|
|
||
| ```rust | ||
| pub trait LogCallback: Send + Sync { | ||
| /// Called when SDK emits a log entry | ||
| /// | ||
| /// # Parameters | ||
| /// - level: Log level string ("TRACE", "DEBUG", "INFO", "WARN", "ERROR") | ||
| /// - target: Module that emitted log (e.g., "bitwarden_core::auth") | ||
| /// - message: The formatted log message | ||
| /// | ||
| /// # Returns | ||
| /// Result<()> - Return errors rather than panicking | ||
| fn on_log(&self, level: String, target: String, message: String) -> Result<()>; | ||
| } | ||
| ``` | ||
|
|
||
| ### Thread Safety Requirements | ||
|
|
||
| The `Send + Sync` bounds are mandatory. The SDK invokes callbacks from arbitrary background threads, | ||
| potentially concurrently. Mobile implementations **must** use thread-safe patterns: | ||
|
|
||
| - **Kotlin**: `ConcurrentLinkedQueue`, synchronized blocks, or coroutine channels | ||
| - **Swift**: `DispatchQueue`, `OSAllocatedUnfairLock`, or actor isolation | ||
|
|
||
| **Critical**: Callbacks are invoked on SDK background threads, NOT the main/UI thread. Performing UI | ||
| updates directly in the callback will cause crashes. | ||
|
|
||
| ## Performance Considerations | ||
|
|
||
| ### Callback Execution Requirements | ||
|
|
||
| Callbacks should return quickly (ideally < 1ms). Blocking operations in the callback delay SDK | ||
| operations. Follow these patterns: | ||
|
|
||
| โ **Do:** | ||
|
|
||
| - Queue logs to thread-safe data structure immediately | ||
| - Process queued logs asynchronously in background | ||
| - Batch multiple logs per Flight Recorder API call | ||
| - Use timeouts for Flight Recorder network calls | ||
| - Handle errors gracefully (catch exceptions, return errors) | ||
|
|
||
| โ **Don't:** | ||
|
|
||
| - Make synchronous network calls in callback | ||
| - Perform expensive computation in callback | ||
| - Access shared state without synchronization | ||
| - Update UI directly (wrong thread!) | ||
| - Throw exceptions without catching | ||
|
|
||
| ### Filtering Strategy | ||
|
|
||
| The SDK sends all INFO+ logs to the callback. Mobile teams filter based on requirements: | ||
|
|
||
| ```kotlin | ||
| override fun onLog(level: String, target: String, message: String) { | ||
| // Example: Only forward WARN and ERROR to Flight Recorder | ||
| if (level == "WARN" || level == "ERROR") { | ||
| logQueue.offer(LogEntry(level, target, message)) | ||
| } | ||
| // INFO logs are ignored | ||
| } | ||
| ``` | ||
|
|
||
| ## Known Limitations | ||
|
|
||
addisonbeck marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| The current implementation has these characteristics: | ||
|
|
||
| 1. **Span Support**: Only individual log events are forwarded, not span lifecycle events | ||
| (enter/exit/close). Mobile teams receive logs without hierarchical operation context. | ||
|
|
||
| 2. **Structured Fields**: Log metadata (user IDs, request IDs, etc.) is flattened to strings. Mobile | ||
| teams cannot access structured key-value data without parsing message strings. | ||
|
|
||
| 3. **Dynamic Filtering**: Mobile teams cannot adjust the SDK's filter level at runtime. The callback | ||
| receives all INFO+ logs regardless of mobile interest. | ||
|
|
||
| 4. **Observability**: The callback mechanism itself does not emit metrics (success rate, invocation | ||
| latency, error frequency). Mobile teams implement monitoring in their callback implementations. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| //! Demonstration of SDK logging callback mechanism | ||
| //! | ||
| //! This example shows how mobile clients can register a callback to receive | ||
| //! SDK logs and forward them to Flight Recorder or other observability systems. | ||
|
|
||
| use std::sync::{Arc, Mutex}; | ||
|
|
||
| use bitwarden_core::client::internal::ClientManagedTokens; | ||
| use bitwarden_uniffi::{Client, LogCallback}; | ||
|
|
||
| /// Mock token provider for demo | ||
| #[derive(Debug)] | ||
| struct DemoTokenProvider; | ||
|
|
||
| #[async_trait::async_trait] | ||
| impl ClientManagedTokens for DemoTokenProvider { | ||
| async fn get_access_token(&self) -> Option<String> { | ||
| Some("demo_token".to_string()) | ||
| } | ||
| } | ||
|
|
||
| /// Demo callback that prints logs to stdout | ||
| struct DemoLogCallback { | ||
| logs: Arc<Mutex<Vec<(String, String, String)>>>, | ||
| } | ||
|
|
||
| impl LogCallback for DemoLogCallback { | ||
| fn on_log( | ||
| &self, | ||
| level: String, | ||
| target: String, | ||
| message: String, | ||
| ) -> Result<(), bitwarden_uniffi::error::BitwardenError> { | ||
| println!("๐ Callback received: [{}] {} - {}", level, target, message); | ||
| self.logs | ||
| .lock() | ||
| .expect("Failed to lock logs mutex") | ||
| .push((level, target, message)); | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| fn main() { | ||
| println!("๐ SDK Logging Callback Demonstration\n"); | ||
| println!("Creating SDK client with logging callback...\n"); | ||
|
|
||
| let logs = Arc::new(Mutex::new(Vec::new())); | ||
| let callback = Arc::new(DemoLogCallback { logs: logs.clone() }); | ||
|
|
||
| // Create client with callback | ||
| let _client = Client::new(Arc::new(DemoTokenProvider), None, Some(callback)); | ||
|
|
||
| println!("โ Client initialized with callback\n"); | ||
| println!("Emitting SDK logs at different levels...\n"); | ||
|
|
||
| // Emit logs that will be captured by callback | ||
| tracing::info!("User authentication started"); | ||
| tracing::warn!("API rate limit approaching"); | ||
| tracing::error!("Network request failed"); | ||
|
|
||
| println!("\n๐ Summary:"); | ||
| let captured = logs.lock().expect("Failed to lock logs mutex"); | ||
| println!(" Captured {} log events", captured.len()); | ||
| println!( | ||
| " Levels: {}", | ||
| captured | ||
| .iter() | ||
| .map(|(l, _, _)| l.as_str()) | ||
| .collect::<Vec<_>>() | ||
| .join(", ") | ||
| ); | ||
| println!("\nโจ Callback successfully forwarded all SDK logs!"); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,7 +93,16 @@ pub enum BitwardenError { | |
| SshGeneration(#[from] bitwarden_ssh::error::KeyGenerationError), | ||
| #[error(transparent)] | ||
| SshImport(#[from] bitwarden_ssh::error::SshKeyImportError), | ||
| #[error("Callback invocation failed")] | ||
| CallbackError, | ||
|
|
||
| #[error("A conversion error occurred: {0}")] | ||
| Conversion(String), | ||
| } | ||
| /// Required From implementation for UNIFFI callback error handling | ||
| /// Converts unexpected mobile exceptions into BitwardenError | ||
| impl From<uniffi::UnexpectedUniFFICallbackError> for BitwardenError { | ||
addisonbeck marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| fn from(_: uniffi::UnexpectedUniFFICallbackError) -> Self { | ||
addisonbeck marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Self::CallbackError | ||
|
Comment on lines
+104
to
+106
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Details and fixThe Current behavior: impl From<uniffi::UnexpectedUniFFICallbackError> for BitwardenError {
fn from(_: uniffi::UnexpectedUniFFICallbackError) -> Self {
Self::CallbackError // Loses error details
}
}Impact: When debugging callback failures, the platform logger at Recommended fix: impl From<uniffi::UnexpectedUniFFICallbackError> for BitwardenError {
fn from(e: uniffi::UnexpectedUniFFICallbackError) -> Self {
Self::Conversion(format!("Callback invocation failed: {}", e.reason))
}
}This preserves diagnostic information while maintaining security (the Alternative: If if let Err(e) = self.callback.on_log(level, target, message) {
tracing::error!(target: "bitwarden_uniffi::log_callback",
"Logging callback failed: {:?}", e); // This captures it
}The current code at line 48 already does this, so the information isn't completely lostโit just doesn't propagate through the error type.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll let mobile decide if they need this right now. |
||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.