Skip to content

feat(inputs.nsdp): Add plugin#16392

Merged
mstrandboge merged 8 commits into
influxdata:masterfrom
hdecarne:nsdp-plugin
Mar 5, 2025
Merged

feat(inputs.nsdp): Add plugin#16392
mstrandboge merged 8 commits into
influxdata:masterfrom
hdecarne:nsdp-plugin

Conversation

@hdecarne

@hdecarne hdecarne commented Jan 11, 2025

Copy link
Copy Markdown
Contributor

Summary

This PR provides the inputs.nsdp plugin for collecting statistics from from NSDP
(Netgear Switch Discovery Protocol) capable devices.

I previously contributed this plugin as an external one. As part of the latest reworks I also converted it into an internal one.

Checklist

  • No AI generated code was used in this PR

Related issues

resolves #16391

@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 Jan 11, 2025
srebhan
srebhan reviewed Feb 14, 2025
View reviewed changes

@srebhan srebhan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @hdecarne for your contribution. I do have some comments in the code. For your other PR of the same structure, please also apply the comments there (if applicable) to save some review rounds!

Comment thread plugins/inputs/nsdp/README.md Outdated
Comment thread plugins/inputs/nsdp/README.md Outdated
Comment thread plugins/inputs/nsdp/README.md Outdated
Comment thread plugins/inputs/nsdp/README.md Outdated
Comment thread plugins/inputs/nsdp/README.md Outdated
Comment thread plugins/inputs/nsdp/nsdp.go Outdated
Comment thread plugins/inputs/nsdp/nsdp.go Outdated
Comment thread plugins/inputs/nsdp/nsdp_test.go Outdated
Comment thread plugins/inputs/nsdp/nsdp_test.go Outdated
Comment thread plugins/inputs/nsdp/nsdp_test.go Outdated
@srebhan srebhan self-assigned this Feb 14, 2025
@srebhan srebhan changed the title feat(inputs.nsdp): Make inputs.nsdp an internal plugin feat(inputs.nsdp): Add plugin Feb 14, 2025
@hdecarne

hdecarne commented Feb 15, 2025

Copy link
Copy Markdown
Contributor Author

@srebhan, I have incorporated your comments and also re-worked overall to match the comments from the other PR.

srebhan
srebhan reviewed Feb 17, 2025
View reviewed changes

@srebhan srebhan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the rework @hdecarne! Some smaller comments in the code...

Comment thread plugins/inputs/nsdp/README.md Outdated
Comment thread plugins/inputs/nsdp/nsdp.go Outdated
Comment thread plugins/inputs/nsdp/nsdp.go Outdated
Comment thread plugins/inputs/nsdp/nsdp.go Outdated
Comment thread plugins/inputs/nsdp/nsdp.go
Comment thread plugins/inputs/nsdp/nsdp.go Outdated
Comment thread plugins/inputs/nsdp/nsdp_test.go Outdated
Comment thread plugins/inputs/nsdp/nsdp_test.go Outdated
Comment thread plugins/inputs/nsdp/nsdp_test.go Outdated
srebhan
srebhan reviewed Feb 21, 2025
View reviewed changes

@srebhan srebhan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@hdecarne almost. :-) Two more minor comments in the code and the request to error out on invalid timeouts rather than overriding them.

Comment thread plugins/inputs/nsdp/nsdp.go Outdated
Comment thread plugins/inputs/nsdp/nsdp.go
@hdecarne

hdecarne commented Feb 22, 2025

Copy link
Copy Markdown
Contributor Author

@srebhan, updated as suggested.

srebhan
srebhan reviewed Feb 24, 2025
View reviewed changes

@srebhan srebhan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @hdecarne! A last round of formatting comments...

Comment thread plugins/inputs/nsdp/nsdp.go Outdated
Comment thread plugins/inputs/nsdp/nsdp.go Outdated
Comment thread plugins/inputs/nsdp/nsdp.go
srebhan
srebhan approved these changes Mar 3, 2025
View reviewed changes

@srebhan srebhan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks a lot @hdecarne!

@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 Mar 3, 2025
@srebhan srebhan assigned mstrandboge and unassigned srebhan Mar 3, 2025
mstrandboge
mstrandboge approved these changes Mar 5, 2025
View reviewed changes

@mstrandboge mstrandboge left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great, thanks for the PR @hdecarne!

@mstrandboge mstrandboge merged commit b122159 into influxdata:master Mar 5, 2025
@github-actions github-actions Bot added this to the v1.34.0 milestone Mar 5, 2025
asaharn pushed a commit to asaharn/telegraf that referenced this pull request Mar 10, 2025
@hdecarne @asaharn
feat(inputs.nsdp): Add plugin (influxdata#16392)
5c98e35
@hdecarne hdecarne deleted the nsdp-plugin branch March 23, 2025 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srebhan srebhan srebhan approved these changes
+1 more reviewer
@mstrandboge mstrandboge mstrandboge approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@mstrandboge mstrandboge

Labels

feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin new plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins 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

v1.34.0

Development

Successfully merging this pull request may close these issues.

Convert external plugin inputs.nsdp into an internal one

3 participants

@hdecarne @srebhan @mstrandboge