Skip to content

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

@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 mstrandboge and unassigned srebhan Dec 4, 2024
@mstrandboge mstrandboge 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.

3 participants