bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
36.24k stars 3.57k forks source link

LogPlugin's `update_subscriber` is broken when adding extra FmtLayers #12597

Closed cBournhonesque closed 6 months ago

cBournhonesque commented 7 months ago

Bevy version

The LogPlugin has an extra field update_subscriber that is used to attach extra tracing Layers to the subscriber: https://github.com/bevyengine/bevy/blob/main/crates/bevy_log/src/lib.rs#L115

There's an example showcasing this behaviour: https://github.com/bevyengine/bevy/blob/main/examples/app/log_layers.rs#L27, where we create a custom layer that.

The issue is that we provide as input a type-erased: Box<dyn Subscriber>, and some common Layers required additional bounds on the Subscriber. For example the tracing_subscriber::fmt::Layer requires the extra bound for<'a> LookupSpan<'a> to be a proper layer: https://github.com/tokio-rs/tracing/blob/tracing-subscriber-0.3.18/tracing-subscriber/src/fmt/fmt_layer.rs#L806

This means that examples such as:

impl Plugin for DebugPlugin {
    fn build(&self, app: &mut App) {
        app.add_plugins(LogPlugin {
            update_subscriber: Some(update_subscriber),
            ..default()
        });
    }
}

fn update_subscriber(subscriber: BoxedSubscriber) -> BoxedSubscriber {
    Box::new(subscriber.with(tracing_subscriber::fmt::layer().with_file(true)))
}

Are broken, with the error the trait bound `for<'a> std::boxed::Box<dyn bevy::utils::tracing::Subscriber + Send + Sync>: LookupSpan<'a>` is not satisfied note: required for `bevy::log::tracing_subscriber::fmt::Layer<std::boxed::Box<dyn bevy::utils::tracing::Subscriber + Send + Sync>>` to implement `__tracing_subscriber_Layer<std::boxed::Box<dyn bevy::utils::tracing::Subscriber + Send + Sync>>

Potential solutions

Here is more info about registering Layers at runtime: https://docs.rs/tracing-subscriber/latest/tracing_subscriber/layer/index.html#runtime-configuration-with-layers

Other relevant info:

philpax commented 6 months ago

I'm also experiencing this issue, with the same use case: I'd like to extend the subscriber to also output to a file using fmt::layer().with_writer, but I can't actually find a way to do that with the current API as the subscriber passed to update_subscriber does not implement LookupSpan.

For now, I've duplicated the original LogPlugin and replicated the original format with output to a file (tracing_log is required as a dependency, Bevy doesn't export this):

use std::path::PathBuf;

use bevy::{
    app::{App, Plugin},
    log::{
        tracing_subscriber::{fmt, layer::SubscriberExt, EnvFilter, Registry},
        Level,
    },
};
use tracing_log::LogTracer;

/// Copy of the `LogPlugin` from Bevy, but with file output.
pub struct LogPlugin {
    /// Filters logs using the [`EnvFilter`] format
    pub filter: String,

    /// Filters out logs that are "less than" the given level.
    /// This can be further filtered using the `filter` setting.
    pub level: Level,

    /// File to write logs to.
    pub file_path: PathBuf,
}

impl Plugin for LogPlugin {
    fn build(&self, _app: &mut App) {
        let default_filter = { format!("{},{}", self.level, self.filter) };

        let filter_layer = EnvFilter::try_from_default_env()
            .or_else(|_| EnvFilter::try_new(&default_filter))
            .unwrap();
        let stdout_fmt_layer = fmt::Layer::default().with_writer(std::io::stderr);
        let file_layer = fmt::layer()
            .fmt_fields(fmt::format::DefaultFields::new())
            .event_format(fmt::format::Format::default().with_ansi(false))
            .with_writer(std::fs::File::create(&self.file_path).unwrap());

        let subscriber = Registry::default()
            .with(filter_layer)
            .with(stdout_fmt_layer)
            .with(file_layer);

        LogTracer::init().unwrap();
        bevy::utils::tracing::subscriber::set_global_default(subscriber).unwrap();
    }
}

but it'd be nice to avoid this if possible!

AlexAegis commented 6 months ago

It would also be really nice to add more subscribers after the log plugin has been initialized. (I made a little plugin that forwards some logs to an egui window so I see them in-game, it was kind of awkward to use since it had to replace the base logplugin, if I had two of these custom log targets, I'd have a bad time implementing it)

cBournhonesque commented 6 months ago

@AlexAegis this could probably be achieved by adding a layer on the existing main subscriber no?

Maybe the best long-term solution is to have a Resource wrapping a reload_handle (as shown here: https://github.com/tokio-rs/tracing/issues/2499#issuecomment-1462542365) and the users can dynamically choose to add subscribers/layers and reload everything.

The tracing docs themselves say:

Finally, if the number of layers changes at runtime, a Vec of subscribers can be used alongside the reload module to add or remove subscribers dynamically at runtime.

cBournhonesque commented 6 months ago

Actually I think I found a good solution https://github.com/bevyengine/bevy/pull/13159