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

Counter never reset after flush, keeps incrementing #52

Open
michaelnatkin opened this issue Feb 2, 2017 · 5 comments
Open

Counter never reset after flush, keeps incrementing #52

michaelnatkin opened this issue Feb 2, 2017 · 5 comments

Comments

@michaelnatkin
Copy link

  • increment a counter once
  • leave your app running
  • watch librato dashboard, see it keep climbing

I added a little console.log and reduced the flush period just to demonstrate the problem:

{"name":"chefsteps-chatbot-facebook-messenger","hostname":"cs-mbp-11-4.local","pid":71247,"level":30,"methodName":"increment","args":["develop.chatbot-fb-messenger.message-sent"],"msg":"Librato debugging","time":"2017-02-02T23:53:15.924Z","v":0}
****** SENDING TO LIBRATO: {"counters":[{"name":"develop.chatbot-fb-messenger.message-received","value":1},{"name":"develop.chatbot-fb-messenger.message-sent","value":1}],"gauges":[{"value":1,"name":"develop.chatbot-fb-messenger.score"}]}
****** SENDING TO LIBRATO: {"counters":[{"name":"develop.chatbot-fb-messenger.message-received","value":1},{"name":"develop.chatbot-fb-messenger.message-sent","value":1}],"gauges":[]}
****** SENDING TO LIBRATO: {"counters":[{"name":"develop.chatbot-fb-messenger.message-received","value":1},{"name":"develop.chatbot-fb-messenger.message-sent","value":1}],"gauges":[]}
****** SENDING TO LIBRATO: {"counters":[{"name":"develop.chatbot-fb-messenger.message-received","value":1},{"name":"develop.chatbot-fb-messenger.message-sent","value":1}],"gauges":[]}
****** SENDING TO LIBRATO: {"counters":[{"name":"develop.chatbot-fb-messenger.message-received","value":1},{"name":"develop.chatbot-fb-messenger.message-sent","value":1}],"gauges":[]}
@michaelnatkin
Copy link
Author

Fix is to add @cache = {} to CounterCache#flushTo

@michaelnatkin
Copy link
Author

@bobzoller see above

@michaelnatkin
Copy link
Author

... or maybe it should have remained as a gauge?

@michaelnatkin
Copy link
Author

Yes, I think the correct thing is it should have stayed a gauge. Although it sounds right, increment shouldn't use counters, because counters are meant only for tracking the derivative of a monotonically increasing value (which you've done) and can't be used with server-side aggregation (which makes them useless for the normal increment case).

https://www.librato.com/docs/kb/faq/glossary/whats_a_counter/
https://www.librato.com/docs/kb/faq/app_questions/count_events/
librato/librato-metrics#105

@bobzoller
Copy link
Contributor

we went around on this back in #27 ... my intention is to treat librato-rack as the best practice, which is why increment was changed to use counters. at the time, I also bought into the case that was made for not deleting/resetting counters. FWIW, despite a somewhat painful migration, this has worked fine for our use cases.

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

No branches or pull requests

2 participants