-
-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
perf(common): In case of an expensive log, allow to pass a function #11281
Conversation
In certain case, it is very expensive to log some values. Instead of checking if certain level is enabled, allow the users to pass in a function that is only called if we need to stringify the log value. This keeps the API small, while still allowing expensive logging to be called when needed, and skipped when not. e.g. instead of ```javascript // This check is technically done twice. if (this.logger.isLevelEnabled('debug')) { this.logger.debug(this.generateAsciiArtFromJpeg()); } ``` the customer could use: ```javascript this.logger.debug(() => this.generateAsciiArtFromJpeg()); ```
Pull Request Test Coverage Report for Build 05c156b5-edfc-4006-829e-036c213cca17
💛 - Coveralls |
Hmm this one's odd. Using |
I can at least speak to the idea of it, I'll try to make a repo for this later if hansl doesn't. This would be like if someone wanted to optionally print out a value from a function, and the function took a lot of time or resources to complete, but only call the function if in this.logger.debug(someComputationallyHeavyMethod()) To get this to optionally print and not take up those computation cycles, right now we'd have to do this this.logger.debug(() => someComputationallyHeavyMethod()) In this case, the heavy computation method would only be evaluated, and its result printed, when the |
My specific use case was that I have some large ArrayBuffer that I want to debug in hexadecimal (think multi kb requests and responses). This process is too heavy to do every time on every requests, and since prod doesn't use debug logging it doesn't need to perform those operations. @jmcdo29 100% described it better than I did in the PR description. Thanks! |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
In certain case, it is very expensive to log some values. Instead of checking if certain level is enabled, allow the users to pass in a function that is only called if we need to stringify the log value.
This keeps the API small, while still allowing expensive logging to be called when needed, and skipped when not.
e.g. instead of
the customer could use:
Does this PR introduce a breaking change?
Other information