-
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
feat(inputs.influxdb_v2_listener): Add support for rate limiting #15361
feat(inputs.influxdb_v2_listener): Add support for rate limiting #15361
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.
Thanks for putting this up. Played with this locally this morning.
There is a footgun if the max_undelivered_metrics size is greater than the number of metrics sent in a batch to Telegraf the batch would never get accepted :\ That's a little different from the other plugins with tracking metrics as they pull/consume as they can.
Good catch! It should be logged as a warning(?). I'll update the code tomorrow. |
Yep! 413 sounds like the right return code.
Hmm a warning, an error, or ignored? We usually error when we cannot parse data. It opens the question if we should do the same for the 429 |
…r than max_undelivered_metrics
I chose to ignore it. It is not really an error in the program, just user error. I did add a debug entry, since that might help the user pin down why batches are being rejected. |
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
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.
Thanks @LarsStegman!
@powersj assigning it back to you as the code was changed since your accept. Feel free to merge if you are still OK with the current state. ;-) |
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.
Thanks for the update!
Summary
As discussed in #15340 it can be beneficial for the Influx listener to rate limit any client pushing data. This change is compatible with the API spec described here: https://docs.influxdata.com/influxdb/v2/api/#operation/PostWrite
Checklist
Related issues
resolves #15340