-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Add set, and histogram reporting to aerospike telegraf plugin #8025
Add set, and histogram reporting to aerospike telegraf plugin #8025
Conversation
refactored to allow non integration tests fix tests to work with aerospike 4.9.0.11
Thanks! Looks like github.com/influxdata/telegraf/plugins/inputs/net_response failed, but only on darwin. Going to rerun it to see if it was just an intermittent fluke. Will review |
I attempted to make it backwards compatible, where all new features are opt-in. |
Hi, |
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.
Looks good. I have 2 comments you can optional address. I'll merge when you're ready.
a.NumberHistogramBuckets = 100 | ||
} else if a.NumberHistogramBuckets < 1 { | ||
a.NumberHistogramBuckets = 10 | ||
} |
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.
You inherited this, but there's an opportunity to clean it up. plugins can have an Init() function that allows you to do init and config related things that runs once and before the first Gather call.
Just add func (a *Aerospike) Init() error {
and you can move all this setup-related stuff there.
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.
maybe next time we do more work, we'll update this. Did not have time this iteration
I'll add the init function, Didn't know about that. |
All changes we are going to do this time are pushed up. If you are good with this PR, feel free to merge. |
Thanks, Josh :) |
…data#8025) Co-authored-by: Joshua Gross <[email protected]>
…data#8025) Co-authored-by: Joshua Gross <[email protected]>
refactored to allow non integration tests
fix tests to work with aerospike 4.9.0.11
Required for all PRs: