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

major bug fix in BufferedFluentMetric.flush #28

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

angadsingh
Copy link

this wasted 2 days of my time assuming cloudwatch is dropping metrics when it was this bug here. it seems you haven't even tested your code with more than 20 metrics.

Issue #, if available:
when there are more than PAGE_SIZE metrics in the buffer, and BufferedFluentMetric.flush is called with send_partial=True, it will not send the remaining metrics because of a bug in the way "end" is calculated. It would only work if you have less than 20 metrics in the buffer. If there are 23 (or 63) for example, it would never send the remaining 3.

Description of changes:
end should be start + (len(buffer) % PAGE_SIZE) and not just (len(buffer) % PAGE_SIZE)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

this wasted 2 days of my time assuming cloudwatch is dropping metrics when it was this bug here. it seems you haven't even tested your code with more than 20 metrics.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant