feat(inputs.dpdk): Add options to customize error-behavior and metric layout #14308

PanKaker · 2023-11-16T10:17:34Z

Required for all PRs

Updated associated README.md.
Wrote appropriate unit tests.
Pull request title or commits are in conventional commit format

Contribution to inputs.dpdk plugin.

New version of the plugin with:

Supporting new ethdev metric: ethdev/info
Possibility to expose fields: pid and version of DPDK
Supporting to read from multiple in-memory sockets
Exposing numerical state of /ethdev/link_status. (0 for "DOWN", and 1 for "UP")
Supporting new configuration option unreachable_socket_behavior

More info in the README.MD

resolves #14307

Hipska

Did only a small review on the docs..

plugins/inputs/dpdk/README.md

plugins/inputs/dpdk/sample.conf

plugins/inputs/dpdk/README.md

srebhan

Thank you for your contribution @PanKaker! I do have some comments regarding the plugin/function structure etc... Please take a look!

plugins/inputs/dpdk/README.md

plugins/inputs/dpdk/dpdk.go

plugins/inputs/dpdk/dpdk_cmds.go

plugins/inputs/dpdk/dpdk_connector.go

plugins/inputs/dpdk/dpdk_utils.go

PanKaker · 2023-11-16T14:42:18Z

Hi, @srebhan. I've addressed almost all comments.

I will need to think about renaming the configuration options (dpdk_*) and I will back with the answer under the discussions

PanKaker · 2023-11-17T13:10:53Z

All comments are addressed.
Waiting for feedback about renaming status_i field.

plugins/inputs/dpdk/README.md

plugins/inputs/dpdk/dpdk.go

PanKaker · 2023-11-23T17:48:20Z

Hi, @Hipska, @srebhan.

I can see that 1 ci is failing (ci/circleci: test-go-windows), I think it's not connected with the changes, could you please confirm this and rerun this step?

I'm also assuming that you will release a new release at the beginning of December. Could you please confirm, that we will be able to merge it until this time?

Thank you.

srebhan · 2023-11-24T10:43:48Z

@PanKaker I commented on some of your comments. My punchline is that splitting code too much (especially if the factored-out functions are only called once) should be avoided. Having to much fragmentation disturbs reviews and debugging later so please do not wrap single function calls with a function. Furthermore, think of functions as a functional unit that is ingested easier if in one place. There are of course exceptions for very complicated or long functions...

…tifySocket function. Remove prepareGlob func. Remove getArrayDiff func. Update tests

PanKaker · 2023-11-27T14:13:52Z

Hi, @srebhan.
All comments were addressed. Waiting for your feedback

srebhan

Thanks for the update @PanKaker! I do have some more minor comments but we are getting close.

plugins/inputs/dpdk/dpdk.go

plugins/inputs/dpdk/dpdk_connector.go

plugins/inputs/dpdk/dpdk_utils.go

…inor fixes

PanKaker · 2023-11-29T17:40:53Z

Hi, @srebhan.

All comments were addressed. Waiting for your feedback.

P.S. I've noticed that "Lint plugin readmes" is not passing, but it's not connected with this PR:

plugins/inputs/webhooks/artifactory/README.md:1: "Artifactory Webhook" does not match regexp " Plugin$"
plugins/inputs/webhooks/artifactory/README.md:1: missing required section "Configuration"
plugins/inputs/webhooks/artifactory/README.md:1: missing required section "Example Output"
plugins/inputs/webhooks/artifactory/README.md:1: missing required section "Metrics"
plugins/inputs/webhooks/artifactory/README.md:1: missing required section "Global configuration options"

…b func to proper file.

srebhan

Thanks for the update @PanKaker! Two more small comments and we are good to go from my side...

plugins/inputs/dpdk/dpdk_utils_test.go

plugins/inputs/dpdk/dpdk.go

PanKaker · 2023-11-30T21:37:24Z

Hi, @srebhan! Comments are addressed.

P.S. There is still issue wtih CI step Lint plugin readmes, but it's not connected with this PR:

plugins/inputs/webhooks/artifactory/README.md:1: "Artifactory Webhook" does not match regexp " Plugin$"
plugins/inputs/webhooks/artifactory/README.md:1: missing required section "Configuration"
plugins/inputs/webhooks/artifactory/README.md:1: missing required section "Example Output"
plugins/inputs/webhooks/artifactory/README.md:1: missing required section "Metrics"
plugins/inputs/webhooks/artifactory/README.md:1: missing required section "Global configuration options"

srebhan

Thanks @PanKaker! Don't worry about the Webhook linter error above, I'm aware that it is not introduced by your PR. :-)

I did check the error-messages once again and found some without parameters (where we should use errors.New) and one with a non-consistent style. I can just submit the suggestions or you do it. Whatever you prefer...

plugins/inputs/dpdk/dpdk.go

…age. Fix tests

srebhan

Thanks a lot @PanKaker!

PanKaker · 2023-12-01T12:24:34Z

Hi, @srebhan! Thank you for the feedback.

I've changed the fmt.Errorf to errors.New in addressed comments + in a few more places.

powersj

Thanks for the PR - I do have one question around the metadata fields option.

plugins/inputs/dpdk/README.md

…ields

powersj

Thank you for the time you put into this PR!

telegraf-tiger · 2023-12-04T19:02:49Z

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_arm64.zip
armel.deb	armv6hl.rpm	freebsd_amd64.tar.gz	windows_i386.zip
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

PanKaker added 2 commits November 16, 2023 11:06

feat(inputs.dpdk): update plugin

214c0ee

feat(inputs.dpdk): update readme

a153222

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 Nov 16, 2023

feat(inputs.dpdk): add proper build tags

df870cd

Hipska suggested changes Nov 16, 2023

View reviewed changes

plugins/inputs/dpdk/README.md Outdated Show resolved Hide resolved

plugins/inputs/dpdk/sample.conf Outdated Show resolved Hide resolved

plugins/inputs/dpdk/sample.conf Outdated Show resolved Hide resolved

plugins/inputs/dpdk/README.md Outdated Show resolved Hide resolved

PanKaker added 2 commits November 16, 2023 13:16

feat(inputs.dpdk): update README

39de600

feat(inputs.dpdk): update README

aaabcfd

srebhan reviewed Nov 16, 2023

View reviewed changes

powersj assigned srebhan Nov 16, 2023

feat(inputs.dpdk): addressing comments from community

87b8209

feat(inputs.dpdk): renaming dpdk_pid and dpdk_version

c1807a5

PanKaker added 3 commits November 20, 2023 16:10

feat(inputs.dpdk): rename field 'status_i' to 'link_status'

4260bfd

feat(inputs.dpdk): fix linter issue

d8c3c78

feat(inputs.dpdk): fix README

b4a7a20

Hipska suggested changes Nov 20, 2023

View reviewed changes

plugins/inputs/dpdk/README.md Outdated Show resolved Hide resolved

plugins/inputs/dpdk/dpdk.go Outdated Show resolved Hide resolved

Hipska approved these changes Nov 20, 2023

View reviewed changes

PanKaker requested a review from srebhan November 21, 2023 18:38

This comment was marked as outdated.

Sign in to view

feat(inputs.dpdk): move processCommand to dpdk_connector. Update iden…

e65d919

…tifySocket function. Remove prepareGlob func. Remove getArrayDiff func. Update tests

srebhan reviewed Nov 29, 2023

View reviewed changes

feat(inputs.dpdk): merging identifySockets and maintainConnections. M…

594fecb

…inor fixes

feat(inputs.dpdk): maintainConnections is simplified, move prepareGlo…

45611ab

…b func to proper file.

srebhan reviewed Nov 30, 2023

View reviewed changes

plugins/inputs/dpdk/dpdk_utils_test.go Outdated Show resolved Hide resolved

plugins/inputs/dpdk/dpdk.go Outdated Show resolved Hide resolved

srebhan changed the title ~~feat(inputs.dpdk): update plugin, new options and metrics~~ feat(inputs.dpdk): Add options to customize error-behavior and metric layout Nov 30, 2023

srebhan reviewed Dec 1, 2023

View reviewed changes

PanKaker added 2 commits December 1, 2023 12:58

feat(inputs.dpdk): change fmt.Errorf to errors.New. Update error mess…

59087ba

…age. Fix tests

feat(inputs.dpdk): change fmt.Errorf to errors.New

869197a

srebhan approved these changes Dec 1, 2023

View reviewed changes

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 1, 2023

srebhan assigned powersj and unassigned srebhan Dec 1, 2023

powersj reviewed Dec 4, 2023

View reviewed changes

plugins/inputs/dpdk/README.md Outdated Show resolved Hide resolved

feat(inputs.dpdk): update config: add 'version' to default metadata_f…

7269dbf

…ields

powersj approved these changes Dec 4, 2023

View reviewed changes

powersj merged commit f654d92 into influxdata:master Dec 4, 2023
3 of 4 checks passed

github-actions bot added this to the v1.29.0 milestone Dec 4, 2023

Porkepix mentioned this pull request Dec 11, 2023

telegraf 1.29.0 Homebrew/homebrew-core#156969

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(inputs.dpdk): Add options to customize error-behavior and metric layout #14308

feat(inputs.dpdk): Add options to customize error-behavior and metric layout #14308

PanKaker commented Nov 16, 2023 •

edited

Loading

Hipska left a comment

srebhan left a comment

PanKaker commented Nov 16, 2023 •

edited

Loading

PanKaker commented Nov 17, 2023

PanKaker commented Nov 23, 2023 •

edited

Loading

This comment was marked as outdated.

srebhan commented Nov 24, 2023

PanKaker commented Nov 27, 2023

srebhan left a comment

PanKaker commented Nov 29, 2023

srebhan left a comment

PanKaker commented Nov 30, 2023 •

edited

Loading

srebhan left a comment

srebhan left a comment

PanKaker commented Dec 1, 2023

powersj left a comment

powersj left a comment

telegraf-tiger bot commented Dec 4, 2023

Artifact URLs

feat(inputs.dpdk): Add options to customize error-behavior and metric layout #14308

feat(inputs.dpdk): Add options to customize error-behavior and metric layout #14308

Conversation

PanKaker commented Nov 16, 2023 • edited Loading

Required for all PRs

Hipska left a comment

Choose a reason for hiding this comment

srebhan left a comment

Choose a reason for hiding this comment

PanKaker commented Nov 16, 2023 • edited Loading

PanKaker commented Nov 17, 2023

PanKaker commented Nov 23, 2023 • edited Loading

This comment was marked as outdated.

srebhan commented Nov 24, 2023

PanKaker commented Nov 27, 2023

srebhan left a comment

Choose a reason for hiding this comment

PanKaker commented Nov 29, 2023

srebhan left a comment

Choose a reason for hiding this comment

PanKaker commented Nov 30, 2023 • edited Loading

srebhan left a comment

Choose a reason for hiding this comment

srebhan left a comment

Choose a reason for hiding this comment

PanKaker commented Dec 1, 2023

powersj left a comment

Choose a reason for hiding this comment

powersj left a comment

Choose a reason for hiding this comment

telegraf-tiger bot commented Dec 4, 2023

Artifact URLs

PanKaker commented Nov 16, 2023 •

edited

Loading

PanKaker commented Nov 16, 2023 •

edited

Loading

PanKaker commented Nov 23, 2023 •

edited

Loading

PanKaker commented Nov 30, 2023 •

edited

Loading