Skip to content

Add support for adding custom log layers#7682

Closed
johanhelsing wants to merge 5 commits intobevyengine:mainfrom
johanhelsing:log-layers
Closed

Add support for adding custom log layers#7682
johanhelsing wants to merge 5 commits intobevyengine:mainfrom
johanhelsing:log-layers

Conversation

@johanhelsing
Copy link
Contributor

Objective

  • Currently, custom log layers can not co-exist with LogPlugin
  • This makes it hard to integrate with other error diagnostics solutions like automatic error reporting, saving logs to files, etc. without also losing the normal functionality of bevy's logging
  • Enabler for Bevy Log Output to Files #5233, alternative to Log to file #5342

Solution

  • Add a number of user-specified, boxed Subscribers to LogPlugin's settings

Additional notes

Draft PR because how extra_layers is passed in (Arc<Mutex<Option<Vec>>>) is pretty ugly. I'm not sure what the current idiom of letting Plugin::build take ownership of something is? Should it be in a resource instead?

@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Diagnostics Logging, crash handling, error reporting and performance analysis labels Feb 15, 2023
@geieredgar
Copy link
Contributor

After this comment #8101 (comment) for my PR

note this potentially goes would go against cart's comment here that plugin build should be stateless.

#4212 (comment)

Something else worth calling out: consuming self would mean that the Plugin's init isn't a repeatable process, which might come in handy for things like editors, hot reloading, etc.
I think we should be moving in the direction of "plugins are completely stateless by default and configured with an instance of a specific type (ideally with a default impl)"

I think that the best approach would be to use Box<dyn Fn() -> Vec<_>> as type for extra_layers with Box::new(|| Vec::new()) as default value. And then branching on empty/non-empty Vec. Would that work for you?

@alice-i-cecile
Copy link
Member

Closing; similar work was completed in #10822.

github-merge-queue bot pushed a commit that referenced this pull request Jan 15, 2024
# Objective

This PR is heavily inspired by
#7682
It aims to solve the same problem: allowing the user to extend the
tracing subscriber with extra layers.

(in my case, I'd like to use `use
metrics_tracing_context::{MetricsLayer, TracingContextLayer};`)


## Solution

I'm proposing a different api where the user has the opportunity to take
the existing `subscriber` and apply any transformations on it.

---

## Changelog

- Added a `update_subscriber` option on the `LogPlugin` that lets the
user modify the `subscriber` (for example to extend it with more tracing
`Layers`


## Migration Guide

> This section is optional. If there are no breaking changes, you can
delete this section.

- Added a new field `update_subscriber` in the `LogPlugin`

---------

Co-authored-by: Charles Bournhonesque <cbournhonesque@snapchat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Diagnostics Logging, crash handling, error reporting and performance analysis C-Usability A targeted quality-of-life change that makes Bevy easier to use

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants