Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

subscriber: skip padding when skipping log.* fields in DefaultVisitor #2980

Merged
merged 6 commits into from
Dec 2, 2024

Conversation

Porges
Copy link
Contributor

@Porges Porges commented May 23, 2024

Motivation

Closes #2979.

The current behaviour of DefaultVisitor is that it will write padding even if it is going to skip writing a value, which results in extraneous padding being added when values are skipped by the tracing-log integration.

Solution

With this change, DefaultVisitor will only insert padding if it is actually going to write a value.

@Porges Porges requested review from hawkw, davidbarsky and a team as code owners May 23, 2024 00:20
Copy link
Contributor

@hds hds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about moving the check for log metadata to before the match statement?

@Porges Porges force-pushed the only-pad-if-writing branch from 03a6f54 to b8f6828 Compare May 27, 2024 21:01
Copy link
Contributor

@hds hds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your change!

@hds hds changed the title Only pad if actually writing a value in DefaultVisitor subscriber: skip padding when skipping log.* fields in DefaultVisitor Jun 4, 2024
@taikulawo
Copy link

same here

@hds hds merged commit b02a700 into tokio-rs:master Dec 2, 2024
55 checks passed
@xxchan
Copy link

xxchan commented Jan 24, 2025

Hi @hds, it's been a while since tracing-log 2.0. Would you mind publishing a new version?

@Porges Porges deleted the only-pad-if-writing branch February 2, 2025 23:31
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

All log/tracing-log outputs have 4 spaces on the end
4 participants