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

feat: add ANSI color removal for tail input plugin #10880

Merged
merged 1 commit into from
Mar 30, 2022
Merged

feat: add ANSI color removal for tail input plugin #10880

merged 1 commit into from
Mar 30, 2022

Conversation

gmlexx
Copy link
Contributor

@gmlexx gmlexx commented Mar 24, 2022

Required for all PRs:

Adds an ability to read a colorized log files for the input tail plugin

@telegraf-tiger telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Mar 24, 2022
@gmlexx gmlexx force-pushed the master branch 3 times, most recently from fa101ee to 8fa725d Compare March 24, 2022 10:24
MyaLongmire
MyaLongmire suggested changes Mar 24, 2022
View reviewed changes
Copy link
Contributor

@MyaLongmire MyaLongmire 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 so much for this pr! Just a few minor comments :)

plugins/inputs/tail/README.md Outdated
Comment on lines 70 to 73
## Remove ANSI colors
# remove_colors = false

Copy link
Contributor

Choose a reason for hiding this comment

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

We try to avoid booleans in the config as it creates some stumbling blocks down the road if we need to expand on it. Maybe something like remove = ["colors"]?

plugins/inputs/tail/tail.go Outdated Show resolved Hide resolved
@gmlexx gmlexx force-pushed the master branch 2 times, most recently from 6353792 to 47fd418 Compare March 24, 2022 13:46
@gmlexx gmlexx requested a review from MyaLongmire March 24, 2022 13:47
MyaLongmire
MyaLongmire approved these changes Mar 24, 2022
View reviewed changes
Copy link
Contributor

@MyaLongmire MyaLongmire left a comment

Choose a reason for hiding this comment

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

lgtm! This pr will need to be approved by one more team member before it can be merged

@MyaLongmire MyaLongmire 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 Mar 24, 2022
reimda
reimda suggested changes Mar 29, 2022
View reviewed changes
plugins/inputs/tail/README.md Outdated Show resolved Hide resolved
reimda
reimda approved these changes Mar 30, 2022
View reviewed changes
Copy link
Contributor

@reimda reimda left a comment

Choose a reason for hiding this comment

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

Thanks!

@telegraf-tiger
Copy link
Contributor

Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip.
Downloads for additional architectures and packages are available below.

☺️ This pull request doesn't significantly change the Telegraf binary size (less than 1%)

📦 Click here to get additional PR build artifacts

Artifact URLs

DEB RPM TAR GZ ZIP
amd64.deb aarch64.rpm darwin_amd64.tar.gz windows_amd64.zip
arm64.deb armel.rpm darwin_arm64.tar.gz windows_i386.zip
armel.deb armv6hl.rpm freebsd_amd64.tar.gz
armhf.deb i386.rpm freebsd_armv7.tar.gz
i386.deb ppc64le.rpm freebsd_i386.tar.gz
mips.deb riscv64.rpm linux_amd64.tar.gz
mipsel.deb s390x.rpm linux_arm64.tar.gz
ppc64el.deb x86_64.rpm linux_armel.tar.gz
riscv64.deb linux_armhf.tar.gz
s390x.deb linux_i386.tar.gz
linux_mips.tar.gz
linux_mipsel.tar.gz
linux_ppc64le.tar.gz
linux_riscv64.tar.gz
linux_s390x.tar.gz
static_linux_amd64.tar.gz

@gmlexx
Copy link
Contributor Author

gmlexx commented Mar 30, 2022

@reimda Could you merge the PR, please?

@reimda
Copy link
Contributor

reimda commented Mar 30, 2022

@reimda Could you merge the PR, please?

I was waiting for CI to complete. Maybe I should have waited for it to complete before approving :)

@reimda reimda merged commit 9d3d5b9 into influxdata:master Mar 30, 2022
@sspaink sspaink mentioned this pull request Apr 5, 2022
Open
@sjwang90 sjwang90 mentioned this pull request Jun 9, 2022
Closed
MyaLongmire pushed a commit that referenced this pull request Jul 6, 2022
@gmlexx @MyaLongmire
feat: add ANSI color filter for tail input plugin (#10880)
704a61e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@powersj powersj powersj left review comments

@MyaLongmire MyaLongmire MyaLongmire approved these changes

@reimda reimda reimda approved these changes

Assignees
No one assigned
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gmlexx @reimda @powersj @MyaLongmire