Skip to content
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

Native Logging with tracing for Android and iOS #1372

Merged
merged 1 commit into from
Dec 4, 2024
Merged

Conversation

insipx
Copy link
Contributor

@insipx insipx commented Dec 4, 2024

Replaces FfiLogger with native bindings to Android and iOS/Mac Log utilities. This removes the need to pass a logger into xmtpv3 bindings, since it interacts directly with the android/ios libraries.

This also makes it possible to create benchmarks in bindings_ffi, and generate flamegraphs profiling the performance (the main reason behind opening this PR now is to profile create_client).

Future work maybe interested in adding a non-blocking appender layer to write logs to a file and send them somewhere once a fork is detected.

@insipx insipx requested a review from a team as a code owner December 4, 2024 16:34
@insipx insipx force-pushed the insipx/ffi-tracing branch from f398487 to bdfb40e Compare December 4, 2024 16:37
@insipx insipx changed the title Native Logging with tracing for Android and Ios Native Logging with tracing for Android and iOS Dec 4, 2024
@@ -142,7 +140,6 @@ pub async fn create_client(
#[allow(unused)]
#[uniffi::export(async_runtime = "tokio")]
pub async fn get_inbox_id_for_address(
logger: Box<dyn FfiLogger>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh sorry just read you description so it should just work now with out this! Nice

Copy link
Contributor Author

@insipx insipx Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup just tested with android, we actually get way more info now I think:

Screenshot 2024-12-04 at 11 47 46 AM

@insipx insipx enabled auto-merge (squash) December 4, 2024 16:48
@insipx insipx force-pushed the insipx/ffi-tracing branch from bdfb40e to 8531d74 Compare December 4, 2024 17:01
@insipx insipx force-pushed the insipx/ffi-tracing branch from 8531d74 to f6b821a Compare December 4, 2024 17:32
@insipx insipx merged commit 34427c9 into main Dec 4, 2024
13 checks passed
@insipx insipx deleted the insipx/ffi-tracing branch December 4, 2024 17:44
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