feat: replace isLevelEnabled guards with Pino interpolation values
#4679
Conversation
Signed-off-by: nikolay <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
jasuwienas
left a comment
jasuwienas
left a comment
There was a problem hiding this comment.
That's pretty straightforward! I wasn't sure whether we should use %s for an array, but it matches the current behavior where we inject the values directly into the string, so it's fine (${['']} === [''].toString()).
That said, these changes will heavily conflict with the work I did for the Redis refactor. (and which I didn't merge because of the newman issue): #4623
Perhaps we can leave the updates to RedisCache.ts out of this PR and handle them in a separate task? wdyt?
…ing-interpolation-when-logging
natanasow
dismissed stale reviews from simzzz, konstantinabl, and jasuwienas
via
819aac1
…ing-interpolation-when-logging # Conflicts: # packages/relay/src/lib/services/rateLimiterService/RedisRateLimitStore.ts # packages/relay/tests/lib/services/rateLimiterService/RedisRateLimitStore.spec.ts
Signed-off-by: nikolay <[email protected]>
quiet-node
left a comment
quiet-node
left a comment
There was a problem hiding this comment.
Great work so far but I think many more guards can be removed or updated
packages/relay/src/lib/services/ethService/blockService/BlockService.ts
Outdated
Show resolved
Hide resolved
packages/relay/src/lib/services/ethService/blockService/BlockService.ts
Outdated
Show resolved
Hide resolved
packages/relay/src/lib/services/ethService/blockService/BlockService.ts
Outdated
Show resolved
Hide resolved
packages/relay/src/lib/services/ethService/contractService/ContractService.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: nikolay <[email protected]>
…ing-interpolation-when-logging # Conflicts: # packages/relay/src/lib/services/ethService/contractService/ContractService.ts
|
@quiet-node all comments were resolved, thank you for the feedback! 🙏 |
Signed-off-by: nikolay <[email protected]>
🚨 Memory Leak Detected 🚨A potential memory leak has been detected in the test titled Details📊 Memory Leak Detection Report 📊 GC Type: MarkSweepCompact Heap Statistics (before vs after executing the test):
Heap Space Statistics (before vs after executing the test):
RecommendationsPlease investigate the memory allocations in this test, focusing on objects that are not being properly deallocated. |
🚨 Memory Leak Detected 🚨A potential memory leak has been detected in the test titled Details📊 Memory Leak Detection Report 📊 GC Type: MarkSweepCompact Heap Statistics (before vs after executing the test):
Heap Space Statistics (before vs after executing the test):
RecommendationsPlease investigate the memory allocations in this test, focusing on objects that are not being properly deallocated. |
Signed-off-by: nikolay <[email protected]>
Signed-off-by: nikolay <[email protected]>
Signed-off-by: nikolay <[email protected]>
natanasow
deleted the
4080-use-interpolation-values-instead-of-string-interpolation-when-logging
branch
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #4679 +/- ##
==========================================
+ Coverage 95.59% 95.84% +0.25%
==========================================
Files 137 137
Lines 21475 21325 -150
Branches 1796 1718 -78
==========================================
- Hits 20528 20439 -89
+ Misses 929 866 -63
- Partials 18 20 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
Description
This PR implements a comprehensive logging performance optimization by replacing
isLevelEnabledguards with Pino's interpolation values (printf-style formatting) across the codebase. The current implementation uses JavaScript string interpolation within conditional guards, which causes performance overhead by building log messages even when the corresponding log level is disabled.Key improvements:
%sfor string interpolation and%ofor object interpolationRelated issue(s)
Fixes #4080
Testing Guide
Changes from original design (optional)
N/A
Additional work needed (optional)
N/A
Checklist