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

consistent logging to enable better debugging #1096

Merged
merged 9 commits into from
Sep 30, 2024
Merged

Conversation

insipx
Copy link
Contributor

@insipx insipx commented Sep 30, 2024

Makes logging more consistent by only using the tracing crate wherever possible. the tracing crate provides contextual tracing useful for debugging async code, but can also fallback to emitting log events if no subscriber is set.

  • removes tracing-test crate preferring a small custom closure to capture logs from tests instead
    • this closure only works with tokios current_thread runtime. It can be made to work with a multi-thread environment, but not needed since its only used in a couple places so far. could make use of tracing-subscriber reload handle with rusts LazyLock global initializer to temporarily switch out a custom global subscriber that logs to a internal memory buffer only in the closure.
    • removing this allows us to avoid setting the traced-test global subscriber in ctor, giving us the ability to filter logs during debugging
  • Default log level for tests is still INFO
  • Logs in tests can be filtered on module again, which is useful for debugging
    • EX: if i'm debugging something related to sync, I can do RUST_LOG=xmtp_mls::groups=DEBUG to only get logs from xmtp_mls/src/groups/mod.rs
  • tracing also has useful adapters for debugging WebAssembly code
  • removes env_logger from mls_validation service preferring tracing-subscriber

bindings_ffi still uses the log crate. The FfiLogger trait doesn't fit well into the tracing::Subscriber trait -- enabling tracing there could be a different issue/PR but for now the benefit would be small to switch it out

@insipx insipx requested a review from a team as a code owner September 30, 2024 15:24
@insipx insipx requested review from codabrink and removed request for a team and codabrink September 30, 2024 15:26
@insipx insipx marked this pull request as draft September 30, 2024 15:28
Copy link

graphite-app bot commented Sep 30, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@insipx insipx marked this pull request as ready for review September 30, 2024 15:37
@insipx insipx requested review from a team and codabrink September 30, 2024 15:40
@insipx insipx changed the title Making logging more consistent to enable better debugging consistent logging to enable better debugging Sep 30, 2024
@codabrink
Copy link
Contributor

Thank you!

@insipx insipx merged commit 92e7b71 into main Sep 30, 2024
9 checks passed
@insipx insipx deleted the insipx/remove-log branch September 30, 2024 22:13
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