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

[telegraf] Add new telegraf plugin #3581

Merged
merged 1 commit into from
Mar 26, 2024

Conversation

wilkmar
Copy link

@wilkmar wilkmar commented Mar 22, 2024


Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname [email protected]?
  • Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?

Copy link

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/sosreport-sos-3581
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

Copy link
Member

@arif-ali arif-ali left a comment

Choose a reason for hiding this comment

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

We should also look out for service status. You could potentially add

services = ('telegraf',)

under the files on line 20

sos/report/plugins/telegraf.py Show resolved Hide resolved
@arif-ali arif-ali added the Kind/Good First Issue This is a good issue for a first time contributor label Mar 22, 2024
@wilkmar wilkmar force-pushed the feature/telegraf-plugin branch from ac732ab to ade3ae1 Compare March 22, 2024 16:08
@wilkmar
Copy link
Author

wilkmar commented Mar 22, 2024

@arif-ali, added the 'services' line.

@pmoravec
Copy link
Contributor

Per https://github.com/influxdata/telegraf/blob/master/docs/CONFIGURATION.md , there can be passwords in the main config file (and also in /etc/default/telegraf file that is automatically collected here). Please obfuscate them (if unsure how, here is a howto).

@wilkmar wilkmar force-pushed the feature/telegraf-plugin branch 2 times, most recently from 8771841 to 15f76af Compare March 25, 2024 08:12
@wilkmar
Copy link
Author

wilkmar commented Mar 25, 2024

Hey @pmoravec. Thanks for your comments. Indeed I missed the password obfuscation part. Please have a look at my latest patch where I added password and certificate obfuscation.
Marcin

@wilkmar wilkmar force-pushed the feature/telegraf-plugin branch from 15f76af to 79d727a Compare March 25, 2024 13:44
Copy link
Contributor

@pmoravec pmoravec left a comment

Choose a reason for hiding this comment

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

Great. Thanks for prompt feedback and first PR (to be merged soon)!

sos/report/plugins/telegraf.py Outdated Show resolved Hide resolved
sos/report/plugins/telegraf.py Outdated Show resolved Hide resolved
@wilkmar wilkmar force-pushed the feature/telegraf-plugin branch from 79d727a to 585f5b2 Compare March 25, 2024 17:06
@arif-ali arif-ali merged commit b9a3ca6 into sosreport:main Mar 26, 2024
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Kind/Good First Issue This is a good issue for a first time contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants