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

subscriber: fix excessive padding on log messages #3070

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kaffarell
Copy link
Contributor

@kaffarell kaffarell commented Sep 2, 2024

Don't add padding to log.* attributes (from log events) which are not outputted.

Problem

For example this config which automatically transforms log-events to tracing ones:

tracing_subscriber::fmt()
    .with_max_level(tracing::Level::TRACE)
    .init();

With the following log message:

log::debug!("this is a log line");

Prints something like:
"this is a log line \n"

Solution

Do something like in the PrettyVisitor: add a write helper which automatically pads every write.

Note: this would also need to be backported to v0.1.x!

@kaffarell kaffarell requested review from hawkw and a team as code owners September 2, 2024 09:08
@kaffarell
Copy link
Contributor Author

CI errors are unrelated, will be fixed with #3069. ping @hawkw @davidbarsky.

Don't add padding to `log.*` attributes (from `log` events) which are
not outputted.

## Problem
For example this config which automatically transforms `log`-events to
`tracing` ones:

```rust
tracing_subscriber::fmt()
    .with_max_level(tracing::Level::TRACE)
    .init();
```

With the following log message:

```rust
log::debug!("this is a log line");
```

Prints something like:
"this is a log line   \n"

## Solution
Do something like in the PrettyVisitor: add a write helper which
automatically pads every write.
@kaffarell kaffarell changed the base branch from v0.1.x to master September 3, 2024 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant