Support for user defined log formatting#8374
Support for user defined log formatting#8374eerii wants to merge 5 commits intobevyengine:mainfrom eerii:main
Conversation
|
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
hymm
left a comment
There was a problem hiding this comment.
The boxed function is unfortunate, but makes sense when the plugin needs to be immutable. Another option could be to use some interior mutability pattern, but you'd lose the ability to reuse the plugin.
|
Sorry about that, when I made the first change I accidentally moved adding the fmt layer from inside a block that only ran if it was not wasm. There are some more changes since fmt_layer has to be the first one to be added to the subscriber to satisfy trait bounds. If you could review it again, now it should (hopefully) work. |
|
Am I understanding correctly that custom layers aren't supported on android and wasm32? If so, this should at least be documented. |
|
I have looked a bit more into this and the issue is that tracing doesn't support wasm/android (tokio-rs/tracing#642), mainly because time panicks, (rust-lang/rust#48564). Bevy is using tracing-wasm and android_log-sys as workarounds for those special cases, passing We could go about it two ways. Either as it is now, where the user can only specify the layer for all platforms but those two, or we make it so the user has to specify all layers. That could be done with relative ease as we already need to use a function that returns a box due to the plugin mutability requirements, where we could have something like: LogPlugin {
layer: Box::new(|| {
#[cfg(target_arch = "wasm32")]
return Box::new(tracing_wasm::WASMLayer::new(tracing_wasm::WASMLayerConfig::default()));
#[cfg(target_os = "android")]
return Box::new(android_tracing::AndroidLayer::default());
Box::new(tracing_subscriber::fmt::Layer::default())
}),
..default()
}This would be great in terms of customizability, but it would be difficult to use as an user. Another option would be to have three parameters, How do you think we should proceed? |
I dislike these options, as they require the user to reason about platforms they may not care about. So the option shown by your example gets my vote (with docs). |
# Objective Adopted bevyengine#3778. This pr allows to configure how the bevy log is formatted. The default bevy log can be a bit verbose for some projects, specially the timestamp. The objective is to allow developers to select the format that best suits them. ``` Example of original bevy log: 2023-04-13T14:19:53.674653Z INFO bevy_render::renderer: [important text!] ``` ## Solution Using `tracing_subscriber::fmt::Subscriber`, adds the attribute `layer` to `LogPlugin`. This allows to pass a `tracing_subscriber::fmt::Layer` when creating the plugin and modifying the output. For example this ```rust App::new() .add_plugins(DefaultPlugins.set(bevy::log::LogPlugin { layer: Box::new(|| Box::new(bevy::log::tracing_subscriber::fmt::layer().pretty().without_time())) ..default() })) .run() ``` prints ``` INFO bevy_render::renderer: [important text!] ``` The main idea comes from @SonicZentropy and their pull request bevyengine#3778, but it was stale since may. This has all the changes and a fix since now `LogSettings` are included in the plugin and not in a resource. Since plugin builders are stateless and not mutable (bevyengine#8101), the old solution of using an `Option<Box<Layer>>` no longer works. One solution to fix this is to use a `Box<Fn() -> Box<Layer>>`. I am unaware of another more idiomatic fix, so if you have something better please revise it. Should plugins builders be mutable in the future, this can be changed back to the original approach. --- ## Changelog - Adds field `layer` to `LogPlugin` that takes a function returning a `tracing_subscriber` custom layer. - Reexports `tracing_subscriber` from log so it can be used to configure the plugin. - Modifies the `logs.rs` and `system_piping.rs` examples to include the new configuration. ## Migration Guide No breaking changes
# Objective Adopted bevyengine#3778. This pr allows to configure how the bevy log is formatted. The default bevy log can be a bit verbose for some projects, specially the timestamp. The objective is to allow developers to select the format that best suits them. ``` Example of original bevy log: 2023-04-13T14:19:53.674653Z INFO bevy_render::renderer: [important text!] ``` ## Solution Using `tracing_subscriber::fmt::Subscriber`, adds the attribute `layer` to `LogPlugin`. This allows to pass a `tracing_subscriber::fmt::Layer` when creating the plugin and modifying the output. For example this ```rust App::new() .add_plugins(DefaultPlugins.set(bevy::log::LogPlugin { layer: Box::new(|| Box::new(bevy::log::tracing_subscriber::fmt::layer().pretty().without_time())) ..default() })) .run() ``` prints ``` INFO bevy_render::renderer: [important text!] ``` The main idea comes from @SonicZentropy and their pull request bevyengine#3778, but it was stale since may. This has all the changes and a fix since now `LogSettings` are included in the plugin and not in a resource. Since plugin builders are stateless and not mutable (bevyengine#8101), the old solution of using an `Option<Box<Layer>>` no longer works. One solution to fix this is to use a `Box<Fn() -> Box<Layer>>`. I am unaware of another more idiomatic fix, so if you have something better please revise it. Should plugins builders be mutable in the future, this can be changed back to the original approach. --- ## Changelog - Adds field `layer` to `LogPlugin` that takes a function returning a `tracing_subscriber` custom layer. - Reexports `tracing_subscriber` from log so it can be used to configure the plugin. - Modifies the `logs.rs` and `system_piping.rs` examples to include the new configuration. ## Migration Guide No breaking changes
Objective
Adopted #3778.
This pr allows to configure how the bevy log is formatted. The default bevy log can be a bit verbose for some projects, specially the timestamp. The objective is to allow developers to select the format that best suits them.
Solution
Using
tracing_subscriber::fmt::Subscriber, adds the attributelayertoLogPlugin. This allows to pass atracing_subscriber::fmt::Layerwhen creating the plugin and modifying the output.For example this
prints
The main idea comes from @SonicZentropy and their pull request #3778, but it was stale since may. This has all the changes and a fix since now
LogSettingsare included in the plugin and not in a resource.Since plugin builders are stateless and not mutable (#8101), the old solution of using an
Option<Box<Layer>>no longer works. One solution to fix this is to use aBox<Fn() -> Box<Layer>>. I am unaware of another more idiomatic fix, so if you have something better please revise it. Should plugins builders be mutable in the future, this can be changed back to the original approach.Changelog
layertoLogPluginthat takes a function returning atracing_subscribercustom layer.tracing_subscriberfrom log so it can be used to configure the plugin.logs.rsandsystem_piping.rsexamples to include the new configuration.Migration Guide
No breaking changes