-
Notifications
You must be signed in to change notification settings - Fork 366
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
MON-2895: toggle netlink implementation of netclass collector #1894
MON-2895: toggle netlink implementation of netclass collector #1894
Conversation
@raptorsun: This pull request references MON-2895 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
4d96dfb
to
e562835
Compare
/hold |
/retest-required |
e562835
to
25aaae5
Compare
/test e2e-agnostic-operator |
25aaae5
to
d8910a5
Compare
/test e2e-agnostic-operator |
1 similar comment
/test e2e-agnostic-operator |
3d5729b
to
11739c3
Compare
/test e2e-aws-single-node |
/retest-required |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/label px-approved |
The PR #1893 has been merged. |
Hello @Tai-RedHat @bburt-rh , the PR is ready for review, could you please have a look? 🙋♂️ |
tested PR with cluster-bot, new feature implemented. |
Documentation/api.md
Outdated
| Property | Type | Description | | ||
| -------- | ---- | ----------- | | ||
| enabled | bool | A Boolean flag that enables or disables the `netclass` collector. | | ||
| useNetlink | bool | A Boolean flag that activates the netlink implementation of the `netclass` collector. This improves performance of `netclass` collector while omitting these metrics: `node_network_address_assign_type`, `node_network_name_assign_type`, `node_network_device_id`. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| useNetlink | bool | A Boolean flag that activates the netlink implementation of the `netclass` collector. This improves performance of `netclass` collector while omitting these metrics: `node_network_address_assign_type`, `node_network_name_assign_type`, `node_network_device_id`. | | |
| useNetlink | bool | A Boolean flag that activates the `netlink` implementation of the `netclass` collector. This implementation improves the performance of the `netclass` collector by omitting these metrics: `node_network_address_assign_type`, `node_network_name_assign_type`, `node_network_device_id`. | |
- change to "by omitting these metrics" if the performance improvement is a result of omitting the metrics.
| Property | Type | Description | ||
|enabled|bool|A Boolean flag that enables or disables the `netclass` collector. | ||
|
||
|useNetlink|bool|A Boolean flag that activates the netlink implementation of the `netclass` collector. This improves performance of `netclass` collector while omitting these metrics: `node_network_address_assign_type`, `node_network_name_assign_type`, `node_network_device_id`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|useNetlink|bool|A Boolean flag that activates the netlink implementation of the `netclass` collector. This improves performance of `netclass` collector while omitting these metrics: `node_network_address_assign_type`, `node_network_name_assign_type`, `node_network_device_id`. | |
|useNetlink|bool|A Boolean flag that activates the `netlink` implementation of the `netclass` collector. This implementation improves the performance of the `netclass` collector by omitting these metrics: `node_network_address_assign_type`, `node_network_name_assign_type`, `node_network_device_id`. |
pkg/manifests/types.go
Outdated
type NodeExporterCollectorNetClassConfig struct { | ||
// A Boolean flag that enables or disables the `netclass` collector. | ||
Enabled bool `json:"enabled,omitempty"` | ||
// A Boolean flag that activates the netlink implementation of the `netclass` collector. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// A Boolean flag that activates the netlink implementation of the `netclass` collector. | |
// A Boolean flag that activates the `netlink` implementation of the `netclass` collector. |
pkg/manifests/types.go
Outdated
// A Boolean flag that enables or disables the `netclass` collector. | ||
Enabled bool `json:"enabled,omitempty"` | ||
// A Boolean flag that activates the netlink implementation of the `netclass` collector. | ||
// This improves performance of `netclass` collector while omitting these metrics: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// This improves performance of `netclass` collector while omitting these metrics: | |
// This implementation improves the performance of the `netclass` collector by omitting these metrics: |
@raptorsun Added a few suggested changes, but otherwise LGTM. |
11739c3
to
f5c760e
Compare
Thank you for the review :D |
/label docs-approved |
/retest-required |
@raptorsun: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoaoBraveCoding, raptorsun, simonpasquier The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR follows #1893 . It should be merged after that one.