-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve tracing layer customization #13159
Merged
alice-i-cecile
merged 5 commits into
bevyengine:main
from
cBournhonesque:cb/layer-customization
May 12, 2024
Merged
Improve tracing layer customization #13159
alice-i-cecile
merged 5 commits into
bevyengine:main
from
cBournhonesque:cb/layer-customization
May 12, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BD103
reviewed
May 5, 2024
Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>
BD103
approved these changes
May 7, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general looks good! I'm a bit concerned about encouraging people to create Box<Vec<...>>
, but since the function itself only gets called once it's probably fine.
afonsolage
approved these changes
May 7, 2024
james7132
approved these changes
May 12, 2024
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
D-Domain-Agnostic
Can be tackled by anyone with generic programming or Rust skills
D-Modest
A "normal" level of difficulty; suitable for simple features or challenging fixes
S-Ready-For-Final-Review
This PR has been approved by the community. It's ready for a maintainer to consider merging it
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
update_subscriber
is broken when adding extra FmtLayers #12597The current tracing customization option (the
update_subscriber
field) was basically unusable because it provides adyn Subscriber
and most layers require aSubscriber
that also implementsfor<'a> LookupSpan<'a, Data=Data<'a>>
, so it was impossible to add a layer on top of thedyn Subscriber
.This PR provides an alternative way of adding additional tracing layers to the LogPlugin by instead creating an
Option<Layer>
.This is enough for most situations because
Option<Layer>
andVec<Layer>
both implementLayer
.Solution
update_subscriber
field ofLogPlugin
with acustom_layer
field which is function pointer returning anOption<BoxedLayer>
bevy::log::tracing_subscriber::fmt::layer().with_file(true)
(mentioned in the issue)Note that in the example this results in duplicate logs, since we have our own layer on top of the default
fmt_layer
added in the LogPlugin; maybe in the future we might want to provide a single one? Or to let the user customize the defaultfmt_layer
? I still think this change is an improvement upon the previous solution, which was basically broken.Changelog
LogPlugin
'supdate_subscriber
field has been replaced withcustom_layer
to allow the user to flexibly add atracing::Layer
to the layer stackMigration Guide
LogPlugin
'supdate_subscriber
field has been replaced withcustom_layer