Skip to content
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

Lossy parse log filter if it comes from env variable #13918

Open
LucDrenth opened this issue Jun 18, 2024 · 0 comments
Open

Lossy parse log filter if it comes from env variable #13918

LucDrenth opened this issue Jun 18, 2024 · 0 comments
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis C-Usability A simple quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Uncontroversial This work is generally agreed upon

Comments

@LucDrenth
Copy link
Contributor

LucDrenth commented Jun 18, 2024

What problem does this solve or what need does it fill?

When setting a log filter through an environment variable RUST_LOG, use lossy parsing to go on with the directives that do work. This will make it consistent with the lossy parsing that happens when the filter comes from LogPlugin.filter.

What solution would you like?

We can reuse tracing-subscriber their env_var_name method (see tracing-subscriber-0.3.18/src/filter/env/builder.rs) after they expose it as public to reuse their code. An issue has been made on their repo for this: tokio-rs/tracing#3009.

After this is done, we can lossy parse the env variable if its set (which will output a log if there's a faulty directive). If the environment variable is not set, default to the LogPlugin.filter as it currently does.

In the solution, running an app with RUST_LOG="my_package=invalid_log_level,naga=warn" cargo run should result in an error being printed for the my_package=invalid_log_level directive while the valid naga=warn will be used.

What alternative(s) have you considered?

We can work around it by using tracing-subscriber its EnvFilter::DEFAULT_ENV but ideally we'd reuse their code. This is better so that one we want to have the option to overwrite the environment variable that is used, we could leverage their builder instead of providing our own logic.

Additional context

This is a follow-up (and dependent on) #13897.

@LucDrenth LucDrenth added C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled labels Jun 18, 2024
@alice-i-cecile alice-i-cecile added C-Usability A simple quality-of-life change that makes Bevy easier to use A-Diagnostics Logging, crash handling, error reporting and performance analysis and removed C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled labels Jun 18, 2024
@janhohenheim janhohenheim added S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Jun 26, 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 C-Usability A simple quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

No branches or pull requests

3 participants