-
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
AWS CloudWatch Output Plugin #553
Conversation
|
||
## Amazon Authentication | ||
|
||
This plugin uses a credential chain for Authentication with the Kinesis API endpoint. In the following order the plugin |
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.
s/Kinesis/CloudWatch/g
"github.com/meirf/gopart" | ||
) | ||
|
||
type CloudWatchOutput struct { |
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.
rename struct to just CloudWatch
Also fix CloudWatch.Close() to return nil instead of error
} | ||
|
||
// Make a list of Dimensions by using a Point's tags. | ||
func buildDimensions(ptTags map[string]string) []*cloudwatch.Dimension { |
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.
There's actually a bug here. CloudWatch only supports up to 10 dimensions per datum. Some points have more than 10 tags. I'll fix this but I'm not sure the best approach here. I'm thinking of sorting and keeping the first 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.
hmm your solution seems reasonable enough, I'm not sure the best way to get around that either, some tags will have to be dropped
|
||
const maxDatumsPerCall = 20 // PutMetricData only supports up to 20 data points per call | ||
|
||
for idxRange := range gopart.Partition(len(datums), maxDatumsPerCall) { |
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.
I agree that using gopart
here makes it elegant, but I don't think it justifies adding another 3rd-party dependency to Telegraf. Could you add a little extra code and do this yourself? One option that comes to mind would be to have buildMetricDatum
do the partitioning itself and return [][]*cloudwatch.MetricDatum
this looks great, thanks @skwong2! please address my comment and write some unit tests and it will be ready to merge 👍 |
|
||
// Make a MetricDatum for each field in a Point. Only fields with values that can be | ||
// converted to float64 are supported. Non-supported fields are skipped. | ||
func buildMetricDatum(point *client.Point) []*cloudwatch.MetricDatum { |
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.
this function can be unit tested
@sparrc Not sure why the tests for other plugins failed there. Anyway I can kick off the build process again? |
@skwong2 I'll kick it |
thanks a bunch @skwong2! |
@sparrc Thanks! Does Telegraf have a nightly build? Or is there a date for the next release? |
No nightly builds yet 😐 I'll release 0.10.1 on Tuesday |
I added an output plugin for sending metrics into AWS CloudWatch.