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(inputs.infiniband): Add support for RDMA counters #16336

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(inputs.infiniband): Add support for RDMA counters #16336

wants to merge 1 commit into from

Conversation

izekr
Copy link

@izekr izekr commented Dec 19, 2024 •
edited
Loading

Summary

This PR introduces a RDMA counters (in a Linux environment) to the exiting InfiniBand input plugin. The new RDMA counters gathers statistics from the hardware counters located under (for example):
/sys/class/infiniband/mlx5_0/ports/1/hw_counters/

Checklist

  • No AI generated code was used in this PR

Related issues

resolves #16334
superseeds #16335

@telegraf-tiger
Copy link
Contributor

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

@telegraf-tiger telegraf-tiger bot added feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins labels Dec 19, 2024
@srebhan
Copy link
Member

srebhan commented Dec 20, 2024

@izekr why opening a new PR? Anyway, the comment here on why this cannot be done in the infiniband plugin remains...

@srebhan srebhan self-assigned this Dec 20, 2024
@izekr
Copy link
Author

izekr commented Dec 20, 2024

@izekr why opening a new PR? Anyway, the comment here on why this cannot be done in the infiniband plugin remains...

@srebhan,
Following your recommendations regarding infinibnad input plugin, I will update this PR to include InfiniBand hardware statistics (RDMA diagnostics) in the infinibnad input plugin.

Note
As for #16335,
I thought that I had some issues with #16335, and that's the reason why I closed it and opened #16336.

@srebhan
Copy link
Member

srebhan commented Dec 20, 2024

Thanks for looking into merging the feature into the existing plugin!

@srebhan
Copy link
Member

srebhan commented Jan 15, 2025

@izekr any news on this PR?

@srebhan srebhan added the waiting for response waiting for response from contributor label Jan 15, 2025
@izekr
Copy link
Author

izekr commented Jan 16, 2025 •
edited
Loading

@izekr any news on this PR?

@srebhan
Will return to work on it in the upcoming week

@telegraf-tiger telegraf-tiger bot removed the waiting for response waiting for response from contributor label Jan 16, 2025
@izekr izekr closed this Jan 23, 2025
@izekr izekr reopened this Jan 23, 2025
@izekr izekr force-pushed the master branch 2 times, most recently from 97e31b8 to 652fd89 Compare January 23, 2025 18:21
@izekr izekr changed the title feat(inputs.infiniband_hw): add a new input plugin for Infiniband card/ports HW statistics feat(inputs.infiniband_hw): add support for RDMA counters Jan 23, 2025
@srebhan
Copy link
Member

srebhan commented Jan 27, 2025

!signed-cla

@srebhan
Copy link
Member

srebhan commented Jan 27, 2025

@izekr could you please sign the CLA so I can give this PR a review!?

@telegraf-tiger
Copy link
Contributor

Download PR build artifacts for , , and .
Downloads for additional architectures and packages are available below.

🥳 This pull request decreases the Telegraf binary size by -100.00 % for linux amd64 (new size: 0 B, nightly size 280.6 MB)

📦 Click here to get additional PR build artifacts

Artifact URLs

DEB RPM TAR GZ
i386.deb i386.rpm freebsd_i386.tar.gz
linux_i386.tar.gz

@izekr
Copy link
Author

izekr commented Jan 27, 2025

!signed-cla

@izekr
Copy link
Author

izekr commented Jan 27, 2025 •
edited
Loading

#16336 (comment)
@srebhan
Done!

@izekr izekr mentioned this pull request Jan 27, 2025
Open
srebhan
srebhan reviewed Jan 29, 2025
View reviewed changes
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 @izekr for the update. Just some working comments...

plugins/inputs/infiniband/README.md
@@ -22,22 +24,30 @@ See the [CONFIGURATION.md][CONFIGURATION.md] for more details.
# This plugin ONLY supports Linux
[[inputs.infiniband]]
# no configuration

## By default, don't gather RDMA counters
# rdma = false
Copy link
Member

Choose a reason for hiding this comment

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

How about naming this gather_rdma to make it more clear?

plugins/inputs/infiniband/infiniband.go
Comment on lines +15 to +16
Log telegraf.Logger `toml:"-"`
RDMA bool `toml:"rdma" default:"false"`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Log telegraf.Logger `toml:"-"`
RDMA bool `toml:"rdma" default:"false"`
RDMA bool `toml:"gather_rdma"`
Log telegraf.Logger `toml:"-"`

plugins/inputs/infiniband/sample.conf
Comment on lines +6 to +7
## By default, don't gather RDMA counters
# rdma = false
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## By default, don't gather RDMA counters
# rdma = false
## Collect RDMA counters
# gather_rdma = false

@srebhan srebhan changed the title feat(inputs.infiniband_hw): add support for RDMA counters feat(inputs.infiniband): Add support for RDMA counters Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srebhan srebhan srebhan left review comments

Assignees

@srebhan srebhan

Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for RDMA counters for Infinibnad input plugin
3 participants
@izekr @srebhan @izekri