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

Aggregate multiple Stackdriver TimeSeries in requests #5407

Merged

Conversation

Legogris
Copy link
Contributor

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

This is a first attempt at #5404. This is not yet perfect, as it just does an iteration over the metrics/fields, adding TimeSeries until adding the next one would mean a duplicate, in which case it sends off the batch so far and then proceeds with a new request.
This should still be an improvement over the current situation (it is not yet properly tested in practice, though)

@danielnelson danielnelson added this to the 1.10.0 milestone Feb 11, 2019
@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Feb 11, 2019
@nicbaz nicbaz force-pushed the 5404-stackdriver-aggregate-requests branch 2 times, most recently from db55582 to cfbaa94 Compare February 14, 2019 10:29
@nicbaz
Copy link
Contributor

nicbaz commented Feb 14, 2019

hello @danielnelson,

Referring to #5404 what are your thoughts on this implementation?

@nicbaz nicbaz force-pushed the 5404-stackdriver-aggregate-requests branch from cfbaa94 to db3c13a Compare February 17, 2019 20:59
@nicbaz
Copy link
Contributor

nicbaz commented Feb 19, 2019

@danielnelson fixed 👍

@danielnelson
Copy link
Contributor

@Legogris I should have checked this already, but I don't have you listed as having signed our CLA. Could you fill it out, if you already did it once could you do it again as something must have gone wrong on our end.

https://www.influxdata.com/legal/cla/

@Legogris
Copy link
Contributor Author

@Legogris I should have checked this already, but I don't have you listed as having signed our CLA. Could you fill it out, if you already did it once could you do it again as something must have gone wrong on our end.

https://www.influxdata.com/legal/cla/

@danielnelson Signed (again, I think)! 👍

@danielnelson danielnelson merged commit 5823fef into influxdata:master Feb 20, 2019
@nicbaz nicbaz mentioned this pull request Feb 21, 2019
3 tasks
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
bitcharmer pushed a commit to bitcharmer/telegraf that referenced this pull request Oct 18, 2019
athoune pushed a commit to bearstech/telegraf that referenced this pull request Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants