Migrate from logging to tracing #64
Conversation
examples/logging.rs
Outdated
|
|
||
| pub fn setup() { | ||
| tracing_subscriber::registry() | ||
| .with(EnvFilter::from_default_env()) |
There was a problem hiding this comment.
I believe just tracing_subscriber::fmt::init() is enough to do all of this. So you don't even need a separate module.
The default subscriber installed by init enables you to filter events at runtime using environment variables (using the EnvFilter).
At least that's how I always use it.
There was a problem hiding this comment.
Does the same apply to src/lib.rs:45 ?
|
One more thing that I'd like to see - which was actually initial motivation - is instrumentation of public API functions too, so that I could just do ops like let camera_file = fs.download(folder, filename).await?;
fs.delete_file(folder, filename).await?;without having to attach manual debug spans to each of those calls. As mentioned in the issue, it's probably going to be a bit tricky on tasks due to custom sync&async struct, but can be done by e.g. passing function name to If you want, I can do this myself at some point. |
I am not yet very familiar with the instrumentation part of tracing (sadly), if you could do this that would be very nice 🙏 |
Cool. I'm actually a bit busy this week and travelling the next 2, so it will take some time for me to get back to this, but it's not very urgent I suppose - happy to do it once I'm back :) |
|
Have fun on your holidays! I will also see if I can figure out this myself 😉 |
| HOOK_LOG_FUNCTION.call_once(|| unsafe { | ||
| libgphoto2_sys::gp_log_add_func(max_log_level, Some(log_function), std::ptr::null_mut()); | ||
| libgphoto2_sys::gp_log_add_func( | ||
| GPLogLevel::GP_LOG_DEBUG, |
There was a problem hiding this comment.
This should probably use https://docs.rs/tracing/latest/tracing/level_filters/constant.STATIC_MAX_LEVEL.html to match previous behaviour / optimisation that used log::STATIC_MAX_LEVEL.
| unsafe extern "C" fn log_function( | ||
| level: libgphoto2_sys::GPLogLevel, | ||
| domain: *const std::os::raw::c_char, | ||
| _domain: *const std::os::raw::c_char, |
There was a problem hiding this comment.
Wait dynamic domain also can't be passed to tracing? 😢
There was a problem hiding this comment.
I guess you could at least pass it as extra variable using tracing's structured logging like
tracing::event!(target: "gphoto2", Level::TRACE, scope, message)
|
tokio-rs/tracing#372 (comment) brings an interesting point: That would amount to reverting most of this PR except the |
There currently are some limitations by the tracing framework, most notably tokio-rs/tracing#372 which doesn't allow the creation of events that have a level or target set at runtime.