feat(node): add promise count #10149
Conversation
|
@akhil888binoy, thank you for the PR! What are the performance implications of this change? Is it possible for you to measure the overhead? |
| /** | ||
| * | ||
| */ |
There was a problem hiding this comment.
Removing of these lines gives eslint error on line 72 function
Missing JSDoc comment.eslintjsdoc/require-jsdoc
| * @param {Object} opts - Options for counting promises. | ||
| * @param {boolean} [opts.locations=false] - Whether to count promise locations. | ||
| * @param {boolean} [opts.continuation=false] - Whether to consider async continuation chains. | ||
| * @returns {function(): number | { [key: string]: number }} - A function to get the count of created and settled promises. |
There was a problem hiding this comment.
| * @returns {function(): number | { [key: string]: number }} - A function to get the count of created and settled promises. | |
| * @returns {() => number | Record<string, number>} - A function to get the count of created and settled promises. |
| @@ -1,7 +1,8 @@ | |||
| /* eslint-disable @typescript-eslint/no-unused-vars */ | |||
There was a problem hiding this comment.
Can we just disable in specific lines, rather than in all file?
As the unit test fails for the moment I am not able to provide performance implications and overhead |
Can you test this outside of the repo? Via a simple Node script? |
|
Hi @akhil888binoy, thank you so much for the contribution! We had some internal conversations on whether we want to move forward with this. We settled on the fact that this will add complexity to the SDK which we currently do not have the capacity to maintain. However, the feature is awesome and I would love to see it in the wild! I would like to encourage you to publish this as a standalone package as an integration that anybody could add to their Sentry setup. I hope this isn't too frustrating. We definitely appreciate the effort you have put in. |
I tried to solve the issue #9329 . Can you please review this PR and suggest me the changes ?