-
-
Notifications
You must be signed in to change notification settings - Fork 34.2k
[WIP] lib: added logger api in node core #60468
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
Open
mertcanaltin
wants to merge
23
commits into
nodejs:main
Choose a base branch
from
mertcanaltin:mert/create-logger-api/node-core
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,723
−13
Open
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
429359b
lib: added logger api in node core
mertcanaltin 6d56e36
fix: validate log level in LogConsumer.enabled method
mertcanaltin 79faca4
fix: improve error serialization check in serializeErr function
mertcanaltin 1a76b7f
fix: update error serialization check and improve log level validation
mertcanaltin 61461e4
fix: replace ErrorIsError with isNativeError for better error seriali…
mertcanaltin c669174
fix: remove unused Error import in logger.js
mertcanaltin 3ec6c9f
fix: update logger module imports to use 'node:logger' for consistency
mertcanaltin 58cca8b
fix: remove unused spawnSyncAndAssert import in test-require-module-t…
mertcanaltin c5cb707
fix: add missing commas in spawnSync options for consistency
mertcanaltin 5dc2b2d
fix: update consumer end method to use callback for asynchronous log …
mertcanaltin a9e288f
fix: remove log file empty assertion message for clarity
mertcanaltin 8f88a71
fix: refactor test assertions to use common.mustSucceed for consistency
mertcanaltin 83a12b7
docs: add LogConsumer class and enabled method documentation
mertcanaltin 22ed15d
Update lib/internal/logger/serializers.js
mertcanaltin c38b9b1
Update lib/internal/logger/serializers.js
mertcanaltin 8a300fe
Update lib/internal/logger/serializers.js
mertcanaltin 68c520b
Update lib/logger.js
mertcanaltin 1f5b449
Update lib/logger.js
mertcanaltin e0ae4d6
Update lib/logger.js
mertcanaltin 8f4c641
Enhance JSDoc comments for serializers and logger methods
mertcanaltin 09f0ef2
Refactor JSDoc for HTTP request serializer and remove unused error code
mertcanaltin b23e669
add benchmark utility for isNativeError function
mertcanaltin 76b0742
Remove bytes submodule from commit
mertcanaltin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| 'use strict'; | ||
|
|
||
| const common = require('../common'); | ||
| const { createLogger, JSONHandler } = require('node:logger'); | ||
| const fs = require('node:fs'); | ||
|
|
||
| const bench = common.createBenchmark(main, { | ||
| n: [1e5], | ||
| level: ['info', 'debug'], | ||
| fields: [0, 5], | ||
| type: ['simple', 'child', 'disabled'], | ||
| }); | ||
|
|
||
| function main({ n, level, fields, type }) { | ||
| // Use /dev/null to avoid I/O overhead in benchmarks | ||
| const nullFd = fs.openSync('/dev/null', 'w'); | ||
| const handler = new JSONHandler({ stream: nullFd, level: 'info' }); | ||
| const logger = createLogger({ handler, level }); | ||
|
|
||
| // Create test data based on fields count | ||
| const logData = { msg: 'benchmark test message' }; | ||
| for (let i = 0; i < fields; i++) { | ||
| logData[`field${i}`] = `value${i}`; | ||
| } | ||
|
|
||
| let testLogger; | ||
| switch (type) { | ||
| case 'simple': | ||
| testLogger = logger; | ||
| break; | ||
| case 'child': | ||
| testLogger = logger.child({ requestId: 'bench-123', userId: 456 }); | ||
| break; | ||
| case 'disabled': { | ||
| // When level is debug and handler is info, logs will be disabled | ||
| const nullFd2 = fs.openSync('/dev/null', 'w'); | ||
|
|
||
| testLogger = createLogger({ | ||
| handler: new JSONHandler({ stream: nullFd2, level: 'warn' }), | ||
| level: 'debug', | ||
| }); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| bench.start(); | ||
| for (let i = 0; i < n; i++) { | ||
| testLogger.info(logData); | ||
| } | ||
| bench.end(n); | ||
|
|
||
| handler.end(); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| 'use strict'; | ||
|
|
||
| const common = require('../common'); | ||
| const fs = require('node:fs'); | ||
|
|
||
| const bench = common.createBenchmark(main, { | ||
| n: [1e5], | ||
| logger: ['node-logger', 'pino'], | ||
| scenario: ['simple', 'child', 'disabled', 'fields'], | ||
| }); | ||
|
|
||
| function main({ n, logger, scenario }) { | ||
| const nullFd = fs.openSync('/dev/null', 'w'); | ||
| let testLogger; | ||
| let consumer; | ||
|
|
||
| if (logger === 'node-logger') { | ||
| const { createLogger, JSONConsumer } = require('node:logger'); | ||
|
|
||
| switch (scenario) { | ||
| case 'simple': { | ||
| consumer = new JSONConsumer({ stream: nullFd, level: 'info' }); | ||
| consumer.attach(); | ||
| testLogger = createLogger({ level: 'info' }); | ||
|
|
||
| bench.start(); | ||
| for (let i = 0; i < n; i++) { | ||
| testLogger.info('benchmark test message'); | ||
| } | ||
| bench.end(n); | ||
| break; | ||
| } | ||
|
|
||
| case 'child': { | ||
| consumer = new JSONConsumer({ stream: nullFd, level: 'info' }); | ||
| consumer.attach(); | ||
| const baseLogger = createLogger({ level: 'info' }); | ||
| testLogger = baseLogger.child({ requestId: 'req-123', userId: 456 }); | ||
|
|
||
| bench.start(); | ||
| for (let i = 0; i < n; i++) { | ||
| testLogger.info('benchmark test message'); | ||
| } | ||
| bench.end(n); | ||
| break; | ||
| } | ||
|
|
||
| case 'disabled': { | ||
| consumer = new JSONConsumer({ stream: nullFd, level: 'warn' }); | ||
| consumer.attach(); | ||
| testLogger = createLogger({ level: 'warn' }); | ||
|
|
||
| bench.start(); | ||
| for (let i = 0; i < n; i++) { | ||
| testLogger.debug('benchmark test message'); | ||
| } | ||
| bench.end(n); | ||
| break; | ||
| } | ||
|
|
||
| case 'fields': { | ||
| consumer = new JSONConsumer({ stream: nullFd, level: 'info' }); | ||
| consumer.attach(); | ||
| testLogger = createLogger({ level: 'info' }); | ||
|
|
||
| bench.start(); | ||
| for (let i = 0; i < n; i++) { | ||
| testLogger.info('benchmark test message', { | ||
| field1: 'value1', | ||
| field2: 'value2', | ||
| field3: 'value3', | ||
| field4: 'value4', | ||
| field5: 'value5', | ||
| }); | ||
| } | ||
| bench.end(n); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (consumer) { | ||
| consumer.flushSync(); | ||
| } | ||
| fs.closeSync(nullFd); | ||
|
|
||
| } else if (logger === 'pino') { | ||
| const pino = require('pino'); | ||
| const destination = pino.destination({ dest: nullFd, sync: false }); | ||
|
|
||
| switch (scenario) { | ||
| case 'simple': { | ||
| testLogger = pino({ level: 'info' }, destination); | ||
|
|
||
| bench.start(); | ||
| for (let i = 0; i < n; i++) { | ||
| testLogger.info('benchmark test message'); | ||
| } | ||
| bench.end(n); | ||
| break; | ||
| } | ||
|
|
||
| case 'child': { | ||
| const baseLogger = pino({ level: 'info' }, destination); | ||
| testLogger = baseLogger.child({ requestId: 'req-123', userId: 456 }); | ||
|
|
||
| bench.start(); | ||
| for (let i = 0; i < n; i++) { | ||
| testLogger.info('benchmark test message'); | ||
| } | ||
| bench.end(n); | ||
| break; | ||
| } | ||
|
|
||
| case 'disabled': { | ||
| testLogger = pino({ level: 'warn' }, destination); | ||
|
|
||
| bench.start(); | ||
| for (let i = 0; i < n; i++) { | ||
| testLogger.debug('benchmark test message'); | ||
| } | ||
| bench.end(n); | ||
| break; | ||
| } | ||
|
|
||
| case 'fields': { | ||
| testLogger = pino({ level: 'info' }, destination); | ||
|
|
||
| bench.start(); | ||
| for (let i = 0; i < n; i++) { | ||
| testLogger.info({ | ||
| msg: 'benchmark test message', | ||
| field1: 'value1', | ||
| field2: 'value2', | ||
| field3: 'value3', | ||
| field4: 'value4', | ||
| field5: 'value5', | ||
| }); | ||
| } | ||
| bench.end(n); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| destination.flushSync(); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| 'use strict'; | ||
|
|
||
| const common = require('../common'); | ||
|
|
||
| const args = { | ||
| true: new Error('test'), | ||
| falsePrimitive: 42, | ||
| falseObject: { foo: 'bar' }, | ||
| }; | ||
|
|
||
| const bench = common.createBenchmark( | ||
| main, | ||
| { | ||
| argument: ['true', 'falsePrimitive', 'falseObject'], | ||
| version: ['native', 'js'], | ||
| n: [1e6], | ||
| }, | ||
| { | ||
| flags: ['--expose-internals', '--no-warnings'], | ||
| }, | ||
| ); | ||
|
|
||
| function main({ argument, version, n }) { | ||
| const util = common.binding('util'); | ||
| const types = require('internal/util/types'); | ||
|
|
||
| const func = { native: util, js: types }[version].isNativeError; | ||
| const arg = args[argument]; | ||
|
|
||
| bench.start(); | ||
| for (let iteration = 0; iteration < n; iteration++) { | ||
| func(arg); | ||
| } | ||
| bench.end(n); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| # Logger | ||
|
|
||
| <!--introduced_in=v26.0.0--> | ||
|
|
||
| > Stability: 1 - Experimental | ||
|
|
||
| <!-- source_link=lib/logger.js --> | ||
|
|
||
| The `node:logger` module provides structured logging capabilities for Node.js | ||
| applications. | ||
|
|
||
| ## Class: `LogConsumer` | ||
|
|
||
| ### `consumer.enabled(level)` | ||
|
|
||
| <!-- YAML | ||
| added: REPLACEME | ||
| --> | ||
|
|
||
| * `level` {string} The log level to check (e.g., `'debug'`, `'info'`, `'warn'`, | ||
| `'error'`, `'fatal'`). | ||
| * Returns: {boolean} `true` if the level is enabled, `false` otherwise. | ||
|
|
||
| Checks if a specific log level is enabled for this consumer. | ||
|
|
||
| This method returns `false` for unknown log levels without throwing an error. | ||
| Log levels are case-sensitive and must be one of the predefined levels: | ||
| `'trace'`, `'debug'`, `'info'`, `'warn'`, `'error'`, `'fatal'`. | ||
|
|
||
| ```js | ||
| const { LogConsumer } = require('node:logger'); | ||
|
|
||
| const consumer = new LogConsumer({ level: 'info' }); | ||
|
|
||
| console.log(consumer.enabled('debug')); // false (below threshold) | ||
| console.log(consumer.enabled('info')); // true | ||
| console.log(consumer.enabled('error')); // true | ||
| console.log(consumer.enabled('DEBUG')); // false (unknown level - case sensitive) | ||
| console.log(consumer.enabled('unknown')); // false (unknown level) | ||
| ``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| 'use strict'; | ||
| const { isNativeError } = require('internal/util/types'); | ||
|
|
||
| /** | ||
| * Serializes an Error object | ||
| * @param {Error} error | ||
| * @returns {{ type: string, message: string, stack: string, code?: any, cause?: any } | Error } | ||
| */ | ||
| function serializeErr(error) { | ||
| if (!isNativeError(error)) { | ||
| return error; | ||
| } | ||
|
|
||
| const obj = { | ||
| __proto__: null, | ||
| type: error.constructor.name, | ||
| message: error.message, | ||
| stack: error.stack, | ||
| }; | ||
|
|
||
| // Include additional error properties | ||
| for (const key in error) { | ||
| if (obj[key] === undefined) { | ||
| obj[key] = error[key]; | ||
| } | ||
| } | ||
|
|
||
| // Handle error code if present | ||
| if (error.code !== undefined) { | ||
| obj.code = error.code; | ||
| } | ||
|
|
||
| // Handle error cause recursively | ||
| if (error.cause !== undefined) { | ||
| obj.cause = typeof error.cause === 'object' && error.cause !== null ? | ||
| serializeErr(error.cause) : | ||
| error.cause; | ||
| } | ||
|
|
||
| return obj; | ||
| } | ||
|
|
||
| /** | ||
| * Serializes HTTP request object | ||
| * @param {{ method: string, url: string, headers: object, socket?: { | ||
| * remoteAddress?: string, remotePort?: string | ||
| * } }} req - HTTP request | ||
| * @returns {{ method: string, url: string, headers: object, remoteAddress?: string, remotePort?: string }} | ||
| */ | ||
| function req(req) { | ||
| return { | ||
| method: req.method, | ||
| url: req.url, | ||
| headers: req.headers, | ||
| remoteAddress: req.socket?.remoteAddress, | ||
| remotePort: req.socket?.remotePort, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Serializes HTTP response object | ||
| * @param {{ statusCode: number, getHeaders?: () => object, headers?: object }} res - HTTP response | ||
| * @returns {{ statusCode: number, headers: object }} | ||
| */ | ||
| function res(res) { | ||
| return { | ||
| statusCode: res.statusCode, | ||
| headers: res.getHeaders?.() ?? res.headers, | ||
| }; | ||
| } | ||
|
|
||
| module.exports = { | ||
| err: serializeErr, | ||
| req, | ||
| res, | ||
| }; | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have any benchmarks on
isNativeError?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, no. I'm trying to create one, but the first problem I encountered is that Benchmark I understand it's not possible to run it directly using internal modules. I'll look for a different solution,
Do you have any suggestions? @anonrig
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find a solution for expose-internals, and benchmark results for isNativeError :
b23e669
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you open a separate pull request for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I opened a new pull request #61180