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

fix(logging): Clean up extra empty spaces when redirectLogger is used #16255

Merged
merged 2 commits into from
Dec 5, 2024

Conversation

zmyzheng
Copy link
Contributor

@zmyzheng zmyzheng commented Dec 3, 2024

Summary

This PR removes the extra spaces in logging when redirectLogger is used.

  • 3 spaces between the timestamp and log level
  • 3 or 4 spaces between the log level and log messages
  • 2 spaces between prefix and attrMsg

Example log contents:
image

The fix includes 3 aspects:

  1. Based on fmt.Println func's definition, spaces are always added between operands. Because of this, adding " " among ts.In(time.UTC).Format(time.RFC3339) , level.Indicator() and prefix + attrMsg will cause extra spaces to be generated (3 spaces rather than 1 space). This PR removes the " " among them.

image

  1. When prefix is not empty, a space is added behind ]:
image

At the same time, another space is added in front of (:
image

Because of this, 2 spaces are added between prefix and attrMsg:
image
This PR removes one space between them.

  1. When both prefix and attrMsg are empty, adding prefix + attrMsg into msg will cause the empty string of prefix + attrMsg being wrapped by 2 extra empty spaces:
    image

This PR updates the logic to only add prefix + attrMsg into msg if it is not empty

Note: the above issues only appears whenredirectLogger is used.

Checklist

  • No AI generated code was used in this PR

Related issues

resolves #16254

Sorry, something went wrong.

@telegraf-tiger telegraf-tiger bot added the fix pr to fix corresponding bug label Dec 3, 2024
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

@zmyzheng thanks for your contribution! I do have two small questions in the code...

@srebhan srebhan self-assigned this Dec 3, 2024
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution and your thorough explanation!

@srebhan srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Dec 4, 2024
@srebhan srebhan assigned DStrand1 and unassigned srebhan Dec 4, 2024
@DStrand1 DStrand1 merged commit 0c74b68 into influxdata:master Dec 5, 2024
26 checks passed
@github-actions github-actions bot added this to the v1.33.0 milestone Dec 5, 2024
justinwwhuang pushed a commit to justinwwhuang/telegraf_fork that referenced this pull request Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/logging fix pr to fix corresponding bug ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extra spaces are appended when redirectLogger is used
3 participants