-
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
Update default value for Cloudwatch rate limit #2520
Update default value for Cloudwatch rate limit #2520
Conversation
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.
LGTM, thanks!
please update the changelog
PS: also please put that documentation link in the SampleConfig |
fc02b38
to
3fbf706
Compare
3fbf706
to
167aea5
Compare
@sparrc Done that and squashed. thanks! |
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.
Lgtm
If we're saying cloudwatch has a limit of 400 requests per second, do we really want to default to telegraf using all of it? This means that if telegraf has it maxed out, and someone simply goes to the AWS dashboard and tries to pull up cloudwatch metrics, their browser is going to push it over the limit. Ditto if they have any other system which also uses cloudwatch metrics. Should we not choose a safer default, and let people increase it to the max if the default isn't enough? |
it's probably time revive this PR #1795 so that telegraf can pull multiple periods of data in 1 request thus saving the API limit. |
@phemmer makes a good point....@AntoineAugusti can you lower the default to ~200? |
@sparrc Sure, done |
Follow up of: #2298
New documentation: http://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/cloudwatch_limits.html
Rate limit is used for the
GetMetricStatistics
endpoint.Required for all PRs: