-
Notifications
You must be signed in to change notification settings - Fork 107
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: add option to disable stats snapshots #799
Conversation
Add option to disable snapshots as an optimization
Add a missing semicolon
Corresponding PR in DefinitelyTyped: DefinitelyTyped/DefinitelyTyped#66220 |
Pull Request Test Coverage Report for Build 5718902301
💛 - Coveralls |
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.
This looks pretty good. can you please add a test for this
add tests
Thanks @lholmquist , just added some tests. Could you please give it a re-review? |
Also, would you like me to take care of the PR in DefinitelyTyped or is that something you would do when you release a new version? |
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.
Looks good - thank you for the contribution!
Hello! I had a small feature request
Motivation
As I understand it, every bucket_interval (1s by default), opossum emits a snapshot of the stats in the Status class
this[SNAPSHOT_INTERVAL] = setInterval(_ => this.emit('snapshot', this.stats), bucketInterval);
My app doesn't actually listen to these stats and the extra call to setInterval clogs up the memory
Solution
Could we have a configurable option to disable the stats snapshots, thereby removing the extra call to setInterval?
Issue here: #798