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.

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

fix logic in tags/names/fields filtering #3036

Merged
merged 1 commit into from
Jul 21, 2017

Conversation

dsalbert
Copy link
Contributor

This MR is the result of #2860 discussion/issue.

  • Adjust logic in functions responsible for passing metrics (should Field/Name/Tags Pass) in order to be able to process them correctly in case where pass and drop are defined together.
  • Change documentation to reflect this change
  • Create additional Unit Tests to cover shouldFieldPass and shouldNamePass.
  • Small refactor

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

Adjust logic in functions responsible for passing metrics (should Field/Name/Tags Pass) in order to be able to process them correctly in case where pass and drop are defined together.
Change documentation to reflect this change
Create additional tests to cover it.
@danielnelson danielnelson added this to the 1.4.0 milestone Jul 21, 2017
@danielnelson danielnelson merged commit d903a91 into influxdata:master Jul 21, 2017
vlamug pushed a commit to vlamug/telegraf that referenced this pull request Jul 24, 2017
Adjust logic in functions responsible for passing metrics in order to be able
to process them correctly in case where pass and drop are defined together.
jeichorn pushed a commit to jeichorn/telegraf that referenced this pull request Jul 24, 2017
Adjust logic in functions responsible for passing metrics in order to be able
to process them correctly in case where pass and drop are defined together.
maxunt pushed a commit that referenced this pull request Jun 26, 2018
Adjust logic in functions responsible for passing metrics in order to be able
to process them correctly in case where pass and drop are defined together.
}

if f.namePass != nil && f.nameDrop != nil {
return pass(f) && drop(f)
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's an old PR, but whenever I look at this code (because the docs aren't specific enough) I find this sequence confusing. The definition of drop(f) means "the drop filter says 'pass'", which inverts the prosaic sense of the code. if pass and drop just doesn't smell good. Would it be clearer to remove the negation inside drop , and use these?

  return pass(f) && ! drop(f)
  return pass(f)
  return ! drop(f)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants