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

log feature emits logs when the log crate doesn't, ignoring log::max_level() #856

Closed
Florob opened this issue Jul 28, 2020 · 1 comment · Fixed by #870
Closed

log feature emits logs when the log crate doesn't, ignoring log::max_level() #856

Florob opened this issue Jul 28, 2020 · 1 comment · Fixed by #870
Labels
crate/tracing Related to the `tracing` crate kind/bug Something isn't working

Comments

@Florob
Copy link

Florob commented Jul 28, 2020

Bug Report

Version

0.1.16, seems to be no different on master

Platform

Linux 4.19.0-xilinx #2 SMP PREEMPT Wed Jul 22 19:10:58 CEST 2020 armv7l GNU/Linux

Description

I was surprised by hyper emitting Trace level logs in conjunction with the syslog crate, while the level filter was set to Info. This turned out to be caused by hyper using the tracing crate instead of the log crate directly. In particular log::max_level() seems to be ignored.

In tracing/macros.rs tracing_log!() computes whether to log differently from the log crate.
While the log crate checks for lvl <= log::STATIC_MAX_LEVEL && lvl <= log::max_level(), tracing instead checks for lvl <= log::STATIC_MAX_LEVEL and later the output of logger.enabled(). This causes differences in what is being logged including unexpected log output.

@hawkw hawkw added the kind/bug Something isn't working label Jul 28, 2020
@hawkw
Copy link
Member

hawkw commented Jul 28, 2020

Yup, that's definitely a bug, thanks for reporting it!

I think we haven't noticed this in the past, since most people seem to be using env_logger, which disables these logs from Log::enabled.

@hawkw hawkw added the crate/tracing Related to the `tracing` crate label Jul 29, 2020
hawkw added a commit that referenced this issue Jul 31, 2020
In `tracing/macros.rs` `tracing_log!()` computes whether to log
differently from the log crate.

While the log crate checks for `lvl <= log::STATIC_MAX_LEVEL && lvl <=
log::max_level()`, tracing instead checks for `lvl <=
log::STATIC_MAX_LEVEL` and later the output of `logger.enabled()`. This
causes differences in what is being logged including unexpected log
output.

Since most of our testing has been with `env_logger`, we missed this
issue, since `env_logger` will also return `false` from its `enabled`
method for any records below the max level. However, other loggers may
not. Also, `enabled` is more expensive than checking the max level, so
this should result in better filtering performance for `log` records
emitted by tracing.

Fixes #856

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this issue Jul 31, 2020
## Motivation

In `tracing/macros.rs` `tracing_log!()` computes whether to log
differently from the log crate.

While the log crate checks for `lvl <= log::STATIC_MAX_LEVEL && lvl <=
log::max_level()`, tracing instead checks for `lvl <=
log::STATIC_MAX_LEVEL` and later the output of `logger.enabled()`. This
causes differences in what is being logged including unexpected log
output.

Since most of our testing has been with `env_logger`, we missed this
issue, since `env_logger` will also return `false` from its `enabled`
method for any records below the max level. However, other loggers may
not. Also, `enabled` is more expensive than checking the max level, so
this should result in better filtering performance for `log` records
emitted by tracing.

## Solution

This commit...fixes that.

Fixes #856

Signed-off-by: Eliza Weisman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/tracing Related to the `tracing` crate kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants