-
Notifications
You must be signed in to change notification settings - Fork 0
add methods to gauge-track the in-flight count of functions and promises #46
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
base: master
Are you sure you want to change the base?
Conversation
Change-type: minor Signed-off-by: dt-rush <[email protected]>
|
Odd that this passed on my local: |
| setTimeout(() => { | ||
| output = metrics.output(); | ||
| expect(/promise_running_gauge 0/.test(output)).to.be.true; | ||
| }, 200); |
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.
Can you change this to
| setTimeout(() => { | |
| output = metrics.output(); | |
| expect(/promise_running_gauge 0/.test(output)).to.be.true; | |
| }, 200); | |
| await Bluebird.delay(200); | |
| output = metrics.output(); | |
| expect(/promise_running_gauge 0/.test(output)).to.be.true; |
as it's clearer and also will mean that if the expect(/promise_running_gauge 0/.test(output)).to.be.true fails then it will be handled properly. The same goes for other setTimeouts
| promise.finally(() => { | ||
| this.inc(name, -1, labels); | ||
| }); |
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 is better as
| promise.finally(() => { | |
| this.inc(name, -1, labels); | |
| }); | |
| try { | |
| await promise | |
| } finally { | |
| this.inc(name, -1, labels); | |
| } |
because it should also handle promises that don't support .finally
| await func(); | ||
| this.inc(name, -1, labels); |
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.
Matching the other function this should be
| await func(); | |
| this.inc(name, -1, labels); | |
| try { | |
| await func(); | |
| } finally { | |
| this.inc(name, -1, labels); | |
| } |
and make sure we don't drop decrements
As suggested @ https://github.com/balena-io/balena-builder/pull/707#discussion_r388613618
Change-type: minor
Signed-off-by: dt-rush [email protected]