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.

Sign up for GitHub

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

Jump to bottom

Invalid additional space when using without_time #1354

Closed
ririsoft opened this issue Apr 13, 2021 · 1 comment · Fixed by #1355
Closed

Invalid additional space when using without_time #1354

ririsoft opened this issue Apr 13, 2021 · 1 comment · Fixed by #1355
Labels
crate/subscriber Related to the `tracing-subscriber` crate kind/bug Something isn't working

Comments

@ririsoft
Copy link

Bug Report

Version

tracing v0.1.25
tracing-subscriber v0.2.17

Platform

Windows 64

Description

When using tracing-subscriber::fmt::SubscriberBuilder::without_time an unexpected additional space character is added at the beginning of the output.

I tried this code:

use tracing::info;

fn main() {
    tracing_subscriber::fmt()
        .without_time()
        .with_level(false)
        .with_target(false)
        .init();
    info!("Hello, world!");
}

I expected to see this happen: no space at the beginning of the line

❯ cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
     Running `target\debug\proximum.exe`
Hello, world!

Instead, this happened: space at the beginning of the line

❯ cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
     Running `target\debug\proximum.exe`
 Hello, world!

Am I missing something or this is a bug ?
Cheers !
@hawkw
Copy link
Member

hawkw commented Apr 13, 2021

Yup, that's definitely a bug, and we should fix that. Thanks for the report!

@hawkw hawkw added crate/subscriber Related to the `tracing-subscriber` crate kind/bug Something isn't working labels Apr 13, 2021
hawkw added a commit that referenced this issue Apr 13, 2021
@hawkw
subscriber: add test reproducing #1354
537e4da 
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this issue Apr 13, 2021
@hawkw
subscriber: remove space when timestamps are disabled
17947c3 
Currently, the default `Compact` and `Full` formatters in
`tracing-subscriber` will prefix log lines with a single space when
timestamps are disabled. The space is emitted in order to pad the
timestamp from the rest of the log line, but it shouldn't be emitted
when timestamps are turned off. This should be fixed.

This branch fixes the issue by skipping `time::write` entirely when
timestamps are disabled. This is done by tracking an additional boolean
flag for disabling timestamps.

Incidentally, this now means that span lifecycle timing can be enabled
even if event timestamps are disabled, like this:
```rust
use tracing_subscriber::fmt;
let subscriber = fmt::subscriber()
    .without_time()
    .with_timer(SystemTime::now)
    .with_span_events(fmt::FmtSpan::FULL);
```
or similar.

Closes #1354
@hawkw hawkw mentioned this issue Apr 13, 2021
Merged
hawkw added a commit that referenced this issue Apr 15, 2021
@hawkw
subscriber: remove space when timestamps are disabled (#1355)
9b1e113 
## Motivation

Currently, the default `Compact` and `Full` formatters in
`tracing-subscriber` will prefix log lines with a single space when
timestamps are disabled. The space is emitted in order to pad the
timestamp from the rest of the log line, but it shouldn't be emitted
when timestamps are turned off. This should be fixed.

## Solution

This branch fixes the issue by skipping `time::write` entirely when
timestamps are disabled. This is done by tracking an additional boolean
flag for disabling timestamps.

Incidentally, this now means that span lifecycle timing can be enabled
even if event timestamps are disabled, like this:
```rust
use tracing_subscriber::fmt;
let subscriber = fmt::subscriber()
    .without_time()
    .with_timer(SystemTime::now)
    .with_span_events(fmt::FmtSpan::FULL);
```
or similar.

I also added a new test reproducing the issue, and did a little
refactoring to try and clean up the timestamp formatting code a bit.

Closes #1354
hawkw added a commit that referenced this issue Apr 16, 2021
@hawkw
subscriber: remove space when timestamps are disabled (#1355)
3b52afc 
## Motivation

Currently, the default `Compact` and `Full` formatters in
`tracing-subscriber` will prefix log lines with a single space when
timestamps are disabled. The space is emitted in order to pad the
timestamp from the rest of the log line, but it shouldn't be emitted
when timestamps are turned off. This should be fixed.

## Solution

This branch fixes the issue by skipping `time::write` entirely when
timestamps are disabled. This is done by tracking an additional boolean
flag for disabling timestamps.

Incidentally, this now means that span lifecycle timing can be enabled
even if event timestamps are disabled, like this:
```rust
use tracing_subscriber::fmt;
let subscriber = fmt::subscriber()
    .without_time()
    .with_timer(SystemTime::now)
    .with_span_events(fmt::FmtSpan::FULL);
```
or similar.

I also added a new test reproducing the issue, and did a little
refactoring to try and clean up the timestamp formatting code a bit.

Closes #1354
hawkw added a commit that referenced this issue Apr 16, 2021
@hawkw
subscriber: remove space when timestamps are disabled (#1355)
3f2f022 
## Motivation

Currently, the default `Compact` and `Full` formatters in
`tracing-subscriber` will prefix log lines with a single space when
timestamps are disabled. The space is emitted in order to pad the
timestamp from the rest of the log line, but it shouldn't be emitted
when timestamps are turned off. This should be fixed.

## Solution

This branch fixes the issue by skipping `time::write` entirely when
timestamps are disabled. This is done by tracking an additional boolean
flag for disabling timestamps.

Incidentally, this now means that span lifecycle timing can be enabled
even if event timestamps are disabled, like this:
```rust
use tracing_subscriber::fmt;
let subscriber = fmt::subscriber()
    .without_time()
    .with_timer(SystemTime::now)
    .with_span_events(fmt::FmtSpan::FULL);
```
or similar.

I also added a new test reproducing the issue, and did a little
refactoring to try and clean up the timestamp formatting code a bit.

Closes #1354
hawkw added a commit that referenced this issue Apr 17, 2021
@hawkw
subscriber: remove space when timestamps are disabled (#1355)
eea2bb8 
## Motivation

Currently, the default `Compact` and `Full` formatters in
`tracing-subscriber` will prefix log lines with a single space when
timestamps are disabled. The space is emitted in order to pad the
timestamp from the rest of the log line, but it shouldn't be emitted
when timestamps are turned off. This should be fixed.

## Solution

This branch fixes the issue by skipping `time::write` entirely when
timestamps are disabled. This is done by tracking an additional boolean
flag for disabling timestamps.

Incidentally, this now means that span lifecycle timing can be enabled
even if event timestamps are disabled, like this:
```rust
use tracing_subscriber::fmt;
let subscriber = fmt::subscriber()
    .without_time()
    .with_timer(SystemTime::now)
    .with_span_events(fmt::FmtSpan::FULL);
```
or similar.

I also added a new test reproducing the issue, and did a little
refactoring to try and clean up the timestamp formatting code a bit.

Closes #1354
kaffarell pushed a commit to kaffarell/tracing that referenced this issue May 22, 2024
@hawkw @kaffarell
subscriber: remove space when timestamps are disabled (tokio-rs#1355)
7170051 
## Motivation

Currently, the default `Compact` and `Full` formatters in
`tracing-subscriber` will prefix log lines with a single space when
timestamps are disabled. The space is emitted in order to pad the
timestamp from the rest of the log line, but it shouldn't be emitted
when timestamps are turned off. This should be fixed.

## Solution

This branch fixes the issue by skipping `time::write` entirely when
timestamps are disabled. This is done by tracking an additional boolean
flag for disabling timestamps.

Incidentally, this now means that span lifecycle timing can be enabled
even if event timestamps are disabled, like this:
```rust
use tracing_subscriber::fmt;
let subscriber = fmt::subscriber()
    .without_time()
    .with_timer(SystemTime::now)
    .with_span_events(fmt::FmtSpan::FULL);
```
or similar.

I also added a new test reproducing the issue, and did a little
refactoring to try and clean up the timestamp formatting code a bit.

Closes tokio-rs#1354
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
crate/subscriber Related to the `tracing-subscriber` crate kind/bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

subscriber: remove space when timestamps are disabled tokio-rs/tracing
2 participants
@hawkw @ririsoft