[WIP] lib: added logger api in node core

[mertcanaltin]

for: https://github.com/nodejs/node/issues/49296#issuecomment-3457576883

I try a draft development for new logger api, and i try create some benchmark for pino and node:logger package

[nodejs-github-bot]

Review requested:

• [ ] @nodejs/performance

[mertcanaltin]

thats my first performance results:

I think need improvement for child logger area, but ı'm so happy because other results it looks nice. cc @mcollina @Qard @trentm @jsumners

node:logger vs pino

➜ node git:(mert/create-logger-api/node-core) ✗ ./node benchmark/logger/vs-pino.js n=100000
logger/vs-pino.js scenario="simple" logger="node-logger" n=100000: 5,240,540.823813018
logger/vs-pino.js scenario="child" logger="node-logger" n=100000: 2,635,847.7027229806
logger/vs-pino.js scenario="disabled" logger="node-logger" n=100000: 159,436,487.67795104
logger/vs-pino.js scenario="fields" logger="node-logger" n=100000: 3,619,336.304205216
logger/vs-pino.js scenario="simple" logger="pino" n=100000: 3,398,489.9761368227
logger/vs-pino.js scenario="child" logger="pino" n=100000: 4,489,799.803418606
logger/vs-pino.js scenario="disabled" logger="pino" n=100000: 119,772,384.56038144
logger/vs-pino.js scenario="fields" logger="pino" n=100000: 1,257,930.8609750536

[mertcanaltin]

I now learn this feat in Pino

https://github.com/pinojs/pino/pull/2281

I will try add in node:logger

[mcollina]

This will require support for serializers

[mertcanaltin]

This will require support for serializers

I wonder, should we name them like Pino does (built-in serializers), or go with something like standardSerializers ?

[mcollina]

This will require support for serializers

I wonder, should we name them like Pino does (built-in serializers), or go with something like standardSerializers ?

Follow pino and we’ll change it

[mertcanaltin]

This will require support for serializers

I wonder, should we name them like Pino does (built-in serializers), or go with something like standardSerializers ?

Follow pino and we’ll change it

I tryed serializer implement for logger, and some bench result repaired

and fields and simple are experiencing a decline; I will try to resolve these

fields: 3.62M → 2.16M (-40%) simple: 5.24M → 4.87M (-7%)

previously, the results for the child logger were quite slow at around 70%, but the new results have dropped to 18% and have actually improved significantly.

I continue to try new methods.

➜  node git:(mert/create-logger-api/node-core) ✗ ./node benchmark/logger/vs-pino.js n=100000
logger/vs-pino.js scenario="simple" logger="node-logger" n=100000: 4,868,164.032787085
logger/vs-pino.js scenario="child" logger="node-logger" n=100000: 3,894,327.425314102
logger/vs-pino.js scenario="disabled" logger="node-logger" n=100000: 160,503,080.85663706
logger/vs-pino.js scenario="fields" logger="node-logger" n=100000: 2,157,462.3927336666
logger/vs-pino.js scenario="simple" logger="pino" n=100000: 3,424,706.4418693925
logger/vs-pino.js scenario="child" logger="pino" n=100000: 4,753,595.477010947
logger/vs-pino.js scenario="disabled" logger="pino" n=100000: 122,100,122.10012211
logger/vs-pino.js scenario="fields" logger="pino" n=100000: 1,411,215.99189962
➜  node git:(mert/create-logger-api/node-core) ✗ 

[mertcanaltin]

This will require support for serializers

I wonder, should we name them like Pino does (built-in serializers), or go with something like standardSerializers ?

Follow pino and we’ll change it

I tryed serializer implement for logger, and some bench result repaired

and fields and simple are experiencing a decline; I will try to resolve these

fields: 3.62M → 2.16M (-40%) simple: 5.24M → 4.87M (-7%)

previously, the results for the child logger were quite slow at around 70%, but the new results have dropped to 18% and have actually improved significantly.

I continue to try new methods.

➜  node git:(mert/create-logger-api/node-core) ✗ ./node benchmark/logger/vs-pino.js n=100000
logger/vs-pino.js scenario="simple" logger="node-logger" n=100000: 4,868,164.032787085
logger/vs-pino.js scenario="child" logger="node-logger" n=100000: 3,894,327.425314102
logger/vs-pino.js scenario="disabled" logger="node-logger" n=100000: 160,503,080.85663706
logger/vs-pino.js scenario="fields" logger="node-logger" n=100000: 2,157,462.3927336666
logger/vs-pino.js scenario="simple" logger="pino" n=100000: 3,424,706.4418693925
logger/vs-pino.js scenario="child" logger="pino" n=100000: 4,753,595.477010947
logger/vs-pino.js scenario="disabled" logger="pino" n=100000: 122,100,122.10012211
logger/vs-pino.js scenario="fields" logger="pino" n=100000: 1,411,215.99189962
➜  node git:(mert/create-logger-api/node-core) ✗ 

I inspected pino and I learn some patterns, than I applied this commit and new results! https://github.com/nodejs/node/pull/60468/commits/1ededc779f061340ef07b2682c96e2e1c594436b

I used this patterns

removed the cost of serializing bindings in each log for the child logger, skipped unnecessary serializer checks, used direct string concatenation instead of object merging, sped up type checking

simple: 6.06M vs 3.48M ops/s (+74% faster) child: 5.76M vs 4.41M ops/s (+31% faster) disabled: 174M vs 146M ops/s (+19% faster) fields: 2.13M vs 1.36M ops/s (+56% faster)

➜  node git:(mert/create-logger-api/node-core) ✗ ./node benchmark/logger/vs-pino.js n=100000
logger/vs-pino.js scenario="simple" logger="node-logger" n=100000: 6,062,182.962986493
logger/vs-pino.js scenario="child" logger="node-logger" n=100000: 5,758,903.394222795
logger/vs-pino.js scenario="disabled" logger="node-logger" n=100000: 174,026,539.04720467
logger/vs-pino.js scenario="fields" logger="node-logger" n=100000: 2,126,059.37552321
logger/vs-pino.js scenario="simple" logger="pino" n=100000: 3,477,918.037575009
logger/vs-pino.js scenario="child" logger="pino" n=100000: 4,407,389.658686015
logger/vs-pino.js scenario="disabled" logger="pino" n=100000: 145,551,509.22359914
logger/vs-pino.js scenario="fields" logger="pino" n=100000: 1,363,125.197883181
➜  node git:(mert/create-logger-api/node-core) ✗ 

[mertcanaltin]

hello @mcollina do you any comments or suggestions for this end commits, I'm curious 🙏 .

[codecov[bot]]

Codecov Report

:x: Patch coverage is 88.21752% with 78 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 88.51%. Comparing base (6274eb7) to head (83a12b7). :warning: Report is 64 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/logger.js	87.60%	73 Missing :warning:
lib/internal/logger/serializers.js	93.15%	5 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #60468      +/-   ##
==========================================
- Coverage   88.55%   88.51%   -0.04%     
==========================================
  Files         703      705       +2     
  Lines      208291   209158     +867     
  Branches    40170    40310     +140     
==========================================
+ Hits       184443   185135     +692     
- Misses      15874    16041     +167     
- Partials     7974     7982       +8     

Files with missing lines	Coverage Δ
lib/internal/logger/serializers.js	93.15% <93.15%> (ø)
lib/logger.js	87.60% <87.60%> (ø)

... and 44 files with indirect coverage changes

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mertcanaltin]

I will today inspect this error

Error: --- stderr ---
node:internal/logger/serializers:12
  if (!ErrorIsError(error)) {
       ^

TypeError: ErrorIsError is not a function
    at Object.serializeErr [as err] (node:internal/logger/serializers:12:8)
    at #log (node:logger:481:29)
    at Logger.error (node:logger:558:14)
    at Object.<anonymous> (/home/runner/work/node/node/node/test/parallel/test-log-basic.js:288:10)
    at Module._compile (node:internal/modules/cjs/loader:1760:14)
    at Object..js (node:internal/modules/cjs/loader:1891:10)
    at Module.load (node:internal/modules/cjs/loader:1480:32)
    at Module._load (node:internal/modules/cjs/loader:1299:12)
    at TracingChannel.traceSync (node:diagnostics_channel:328:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:245:24)

Node.js v26.0.0-pre
Command: out/Release/node /home/runner/work/node/node/node/test/parallel/test-log-basic.js

Update: we solved this problem, used isNativeError

[mertcanaltin]

I tried running parallel/test-debugger-backtrace test file locally and succes