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

Bug/Improvement: hostname resolution very slow - maybe cache it #130

Closed
DarkLiKally opened this issue Feb 4, 2022 · 3 comments
Closed
Labels
bug Something isn't working

Comments

@DarkLiKally
Copy link

Describe the bug
I'm using tslog in a AWS lambda envrionment - but the thing is also occuring locally. When you log many things the most of the execution time of our function is dedicated to the getHostname / hostname() function from the node:os package.

To Reproduce
Steps to reproduce the behavior:

  1. setup tslog in a default configuration
  2. log many messages
  3. attach a profiler to the node process (--inspect -> chrome://inspect -> profile)
  4. see the results

The hostname is resolved again and again for each log obejct in the _buildLogObject function

Expected behavior
Maybe it is better to cache the hostname for each logger instance or maybe refresh it in time frames or maybe add an option to enable/disable caching of it. And not evaluate it always again since the node os api for it is pretty slow compared to all other code thats run. It makes up 80 to 90 % of cpu time in my profiler runs for about 10.000 log messages.

Node.js Version
16

OS incl. Version
windows 10 x64 / ubuntu, wsl 8 / aws linux for lambda

@DarkLiKally DarkLiKally added the bug Something isn't working label Feb 4, 2022
@terehov
Copy link
Contributor

terehov commented Feb 6, 2022

Thanks, that’s a really good finding! I’ll fix it soon. 👍🏻

@DarkLiKally
Copy link
Author

DarkLiKally commented Feb 6, 2022

@terehov thanks :) another quick note:
The Logger constructor also resolves the hostname - when creating child loggers for each request in services injected via DI (we have many scoped services) there's also a good amount of requests to the os.hostname().
Maybe that's also good to cache.
For performance that is not a super deal in this case, it's not as much as for each log message but it could improve about 3 - 5 milliseconds in our situation per request (we would come down from ~15 ms to about 8 - 11 ms).

We could of course switch to some cached loggers, or a static instance or something, but we wanted to make use of the inherited settings with the requestId.

@terehov terehov closed this as completed in 130a7be Feb 8, 2022
@terehov
Copy link
Contributor

terehov commented Feb 8, 2022

@DarkLiKally Here you go, give it a try :-)

https://github.com/fullstack-build/tslog/releases/tag/v3.3.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants