Skip to content

feat(crypto-ffi): Add per-level log methods to Logger callback interface - #6972

Open
morlinbrot wants to merge 2 commits into
matrix-org:mainfrom
morlinbrot:main
Open

feat(crypto-ffi): Add per-level log methods to Logger callback interface#6972
morlinbrot wants to merge 2 commits into
matrix-org:mainfrom
morlinbrot:main

Conversation

@morlinbrot

@morlinbrot morlinbrot commented Sep 3, 2026

Copy link
Copy Markdown

Closes #1759

The Logger trait previously only supported one catch-all log method which discarded the log level of tracing events. This adds dedicated per-level log methods to the trait.

LoggerWrapper has a new internal level field that tracks the log level, it's instantiated with Level::DEBUG as a default.

NOTE: This is a breaking change to the crate's public bindings API contract. I did not find any special instructions for cases like this but I wanted to note it prominently.

  • I've documented the public API changes in the appropriate changelog files (see Writing changelog entries).
  • This PR was made with the help of AI.

The Logger trait previously only supported one catch-all log method which discarded the log level of tracing events. This adds dedicated per-level log methods to the trait.

LoggerWrapper has a new internal level field that tracks the log level, it's instantiated with Level::DEBUG as a default

Signed-off-by: morlinbrot <morlinbrot@mailbox.org>
@morlinbrot
morlinbrot requested a review from a team as a code owner September 3, 2026 12:55
@morlinbrot
morlinbrot requested review from poljar and removed request for a team September 3, 2026 12:55
Signed-off-by: morlinbrot <morlinbrot@mailbox.org>
@codspeed-hq

codspeed-hq Bot commented Sep 3, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 50 untouched benchmarks


Comparing morlinbrot:main (050b02d) with main (4fbc1e8)

Open in CodSpeed

@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.23%. Comparing base (7a2a899) to head (050b02d).
⚠️ Report is 11 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6972      +/-   ##
==========================================
+ Coverage   90.22%   90.23%   +0.01%     
==========================================
  Files         408      408              
  Lines      117273   117273              
  Branches   117273   117273              
==========================================
+ Hits       105814   105826      +12     
+ Misses       7519     7498      -21     
- Partials     3940     3949       +9     

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

@morlinbrot

Copy link
Copy Markdown
Author

Pinging @Hywan since you reviewed a previously rejected PR and this implementation closely follows what you laid out in your comments there.

Two notes on decisions I made:

  • The above mentioned PR was criticized for removing the log method from Logger but that seems to be a legitimate change to me since its only previous user, LoggerWrapper's Write impl, doesn't use it anymore. And since we're already breaking the trait by splitting into multiple methods, I don't think it makes much sense to keep it for backwards compatibility either. Let me know if I'm overlooking something here.

  • The original ticket requests a data: String parameter be added to the logging methods. I deliberately left this out of the PR because the only current user of the logger is tracing_subscriber::fmt() which does not pass a data argument. Making use of one could be supported by a custom tracing_subscriber::Layer implementation but that felt like too big of a scope increase for this PR. Happy to look into this if it's wanted.

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.

Extend the logging callback interface in matrix-sdk-crypto-ffi

1 participant