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

Rename processor #4528

Merged
merged 2 commits into from
Aug 13, 2018
Merged

Rename processor #4528

merged 2 commits into from
Aug 13, 2018

Conversation

goldibex
Copy link
Contributor

@goldibex goldibex commented Aug 7, 2018

Required for all PRs:

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

This PR adds a processor plugin called "rename", which simply renames metrics, tags, or fields according to its configuration. It has unit coverage and the documentation has been updated with instructions on how to use it.

We need this functionality because both the statsd input and cloudwatch output have hardcoded mutually inconsistent metric field names that we needed a simple way to harmonize.

Copy link
Contributor

@danielnelson danielnelson left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, this will be very useful. I just have one comment on how we can improve the performance:

if newMeasurementName, ok := r.measurements[point.Name()]; ok {
point.SetName(newMeasurementName)
}
for oldTagName, tagValue := range point.Tags() {
Copy link
Contributor

Choose a reason for hiding this comment

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

We would want to use TagList() here instead of Tags() because it doesn't allocate, but what might be even better is if we skipped the initialization completely and just directly used the renamer types:

for _, rule := range r.Tag {
	value, ok := point.GetTag(rule.From)
		if ok {
			point.AddTag(rule.To, value)
		}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do!

@glinton glinton added this to the 1.8.0 milestone Aug 13, 2018
@glinton glinton merged commit 7ca7f22 into influxdata:master Aug 13, 2018
rgitzel pushed a commit to rgitzel/telegraf that referenced this pull request Oct 17, 2018
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
dupondje pushed a commit to dupondje/telegraf that referenced this pull request Apr 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants