fix(ffi): ensure there's always a global directive for logs #4485
Conversation
If it's present, we just let it untouched. Otherwise, we set it to `error` if it's missing. See code comment explaining why we need this.
| // If there's no global directive, and a non-empty set of module/crate | ||
| // directives, then logs from modules and crates *not* appearing in the list | ||
| // of directives will be silently swallowed. This is bad, especially at the | ||
| // `error` level, for which we always want to see logs (notably because the |
There was a problem hiding this comment.
The original reason for removing the global directive is to take zero chances that a 3rd party crate would log sensitive data, which would be hard to find and even harder to clean up.
There was a problem hiding this comment.
Yeah, that's a good point. I've implemented your suggestion to restrict this to only adding panic=error to the log filter line, since it's a more cautious choice.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4485 +/- ##
==========================================
- Coverage 85.12% 85.10% -0.03%
==========================================
Files 283 283
Lines 31775 31775
==========================================
- Hits 27048 27041 -7
- Misses 4727 4734 +7 ☔ View full report in Codecov by Sentry. |
|
|
||
| #[test] | ||
| fn test_do_nothing_when_provided() { | ||
| let mut filter = String::from("a=b,panic=error,c=d"); |
There was a problem hiding this comment.
Might be better if this test said panic=info or something, if that is allowed.
There was a problem hiding this comment.
Indeed, will adjust, thanks!
…ly enabling error
bnjbvr
force-pushed
the
bnjbvr/tweak-log-filter-ffi
branch
from
83fb579 to
353edfc
Compare
If it's present, we just let it untouched. Otherwise, we set it to
errorif it's missing. See code comment explaining why we need this.This makes sure we log panics at the FFI layer, since the log_panics crate will use an error level, with a target that's likely unknown to the user providing the log filter configuration.