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

graphite parser, handle multiple templates empty filter #1871

Merged
merged 1 commit into from
Oct 11, 2016
Merged

Conversation

sparrc
Copy link
Contributor

@sparrc sparrc commented Oct 10, 2016

Required for all PRs:

  • CHANGELOG.md updated (we recommend not updating this until the PR has been approved by a maintainer)
  • Sign CLA (if not already signed)
  • README.md updated (if adding a new plugin)

Previously, the graphite parser would simply overwrite any template that
had an identical filter to a previous template. This included the empty
filter.

This change automatically creates a "*." filter for any template
specified with an empty filter. This allows users to specify only a
template, and Telegraf will auto-match based on the most specific
template.

closes #1731

@sparrc
Copy link
Contributor Author

sparrc commented Oct 10, 2016

cc @kostasb @dgnorton

stars = append(stars, "*")
}
filter = strings.Join(stars, ".")
}
Copy link
Contributor

@dgnorton dgnorton Oct 10, 2016

Choose a reason for hiding this comment

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

Could be shortened to:

if filter == "" {
   filter = strings.TrimRight(strings.Repeat("*.", strings.Count(template, ".")), ".")
}

or

if filter == "" {
   filter = strings.Repeat("*.", strings.Count(template, "."))
   filter = strings.TrimRight(filter, ".")
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

didn't know about strings.Repeat or strings.Count, thanks!

@sparrc
Copy link
Contributor Author

sparrc commented Oct 10, 2016

@dgnorton had to do a significant refactor of this, because the problem with adding a *. pattern for each template was that it fails with "over-specific" templates.

I don't like this behavior, but it's what we have, so I changed it to sort the templates instead, which keeps the current behavior of matching as much as possible on an "over-specific" template.

@sparrc sparrc force-pushed the cs1731 branch 3 times, most recently from 99ac51a to a209223 Compare October 11, 2016 14:16
Previously, the graphite parser would simply overwrite any template that
had an identical filter to a previous template. This included the empty
filter.

Now we will still overwrite, but first we will sort to make sure that
the most "specific" template always matches.

closes #1731
@sparrc sparrc merged commit e96f7a9 into master Oct 11, 2016
@sparrc sparrc deleted the cs1731 branch October 11, 2016 14:50
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.

Issues with tag matching in Graphite templates
2 participants