Skip to content

Add include_nodes filter for jenkins#9351

Merged
MyaLongmire merged 3 commits intoinfluxdata:masterfrom
akrantz01:9047-jenkins-include-nodes
Jul 1, 2021
Merged

Add include_nodes filter for jenkins#9351
MyaLongmire merged 3 commits intoinfluxdata:masterfrom
akrantz01:9047-jenkins-include-nodes

Conversation

@akrantz01
Copy link
Copy Markdown
Contributor

Required for all PRs:

  • Updated associated README.md.
  • Wrote appropriate unit tests.

resolves #9047

Adds the ability to explicitly include nodes for the jenkins_node metric. Previously, you could only exclude nodes from being collected.

Like the job_exclude and job_include filters, node_exclude takes precedence over node_include if they are both present.

@akrantz01 akrantz01 added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Jun 10, 2021
@akrantz01 akrantz01 force-pushed the 9047-jenkins-include-nodes branch from e55b232 to e92d900 Compare June 10, 2021 21:28
@akrantz01
Copy link
Copy Markdown
Contributor Author

@G0lang Would you be willing to test this?

Copy link
Copy Markdown
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.

@akrantz01 thanks for this PR, it makes totally sense. However, you probably should use NewIncludeExcludeFilter() of the filter package which does the include/exclude handling for you and reduces the amount of code.

@srebhan srebhan self-assigned this Jun 28, 2021
@srebhan srebhan added the plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins label Jun 28, 2021
Copy link
Copy Markdown
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.

Looks good to me. Thanks for also tackling jobFilter! I have just one minor comment in the code, but it is optional.

Copy link
Copy Markdown
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.

Perfect, thank you! LGTM.

@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 Jun 30, 2021
@MyaLongmire MyaLongmire merged commit 17e86ab into influxdata:master Jul 1, 2021
bhsu-ms pushed a commit to bhsu-ms/telegraf that referenced this pull request Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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 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

Development

Successfully merging this pull request may close these issues.

support include nodes for jenkins plugin

3 participants