From a2840c8153dffbbaf14abb7a2f3ce4a6500ac5bf Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 7 Apr 2021 11:49:26 +0200 Subject: [PATCH 01/63] Basic transport implemented on top of ThreadStream --- lib/transport.js | 17 +++++++++++++++++ package.json | 4 +++- pino.js | 2 ++ test/fixtures/to-file-transport.js | 12 ++++++++++++ test/transport.test.js | 29 +++++++++++++++++++++++++++++ 5 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 lib/transport.js create mode 100644 test/fixtures/to-file-transport.js create mode 100644 test/transport.test.js diff --git a/lib/transport.js b/lib/transport.js new file mode 100644 index 000000000..473ec0991 --- /dev/null +++ b/lib/transport.js @@ -0,0 +1,17 @@ +'use strict' + +const ThreadStream = require('thread-stream') + +function transport (filename, workerData) { + const stream = new ThreadStream({ + filename, + workerData, + sync: true // TODO should this be configurable? + }) + + stream.unref() + + return stream +} + +module.exports = transport diff --git a/package.json b/package.json index fc914fa83..864416ce3 100644 --- a/package.json +++ b/package.json @@ -93,8 +93,10 @@ "fast-redact": "^3.0.0", "fast-safe-stringify": "^2.0.7", "flatstr": "^1.0.12", + "get-caller-file": "^2.0.5", "pino-std-serializers": "^3.1.0", "quick-format-unescaped": "^4.0.3", - "sonic-boom": "^1.0.2" + "sonic-boom": "^1.0.2", + "thread-stream": "^0.7.1" } } diff --git a/pino.js b/pino.js index 0d0e3e33f..ea862655c 100644 --- a/pino.js +++ b/pino.js @@ -223,6 +223,8 @@ module.exports.destination = (dest = process.stdout.fd) => { } } +module.exports.transport = require('./lib/transport') + module.exports.final = final module.exports.levels = mappings() module.exports.stdSerializers = serializers diff --git a/test/fixtures/to-file-transport.js b/test/fixtures/to-file-transport.js new file mode 100644 index 000000000..9ceb636ef --- /dev/null +++ b/test/fixtures/to-file-transport.js @@ -0,0 +1,12 @@ +'use strict' + +const fs = require('fs') +const { once } = require('events') + +async function run (opts) { + const stream = fs.createWriteStream(opts.dest) + await once(stream, 'open') + return stream +} + +module.exports = run diff --git a/test/transport.test.js b/test/transport.test.js new file mode 100644 index 000000000..a6e281e93 --- /dev/null +++ b/test/transport.test.js @@ -0,0 +1,29 @@ +'use strict' + +const os = require('os') +const { join } = require('path') +const { readFile } = require('fs').promises +const { test } = require('tap') +const { watchFileCreated } = require('./helper') +const pino = require('../') + +const { pid } = process +const hostname = os.hostname() + +test('pino.transport with file', async ({ same }) => { + const dest = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const instance = pino(pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js'), { dest })) + instance.info('hello') + await watchFileCreated(dest) + const result = JSON.parse(await readFile(dest)) + delete result.time + same(result, { + pid, + hostname, + level: 30, + msg: 'hello' + }) +}) From b2404c9b9a3f58b24c35d646bd557c28bada5e4a Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 7 Apr 2021 12:29:06 +0200 Subject: [PATCH 02/63] Support module transports --- lib/transport.js | 9 ++++++ test/fixtures/transport/index.js | 12 +++++++ test/fixtures/transport/package.json | 5 +++ test/transport.test.js | 47 +++++++++++++++++++++++++++- 4 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/transport/index.js create mode 100644 test/fixtures/transport/package.json diff --git a/lib/transport.js b/lib/transport.js index 473ec0991..49332759b 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -1,8 +1,17 @@ 'use strict' +const { createRequire } = require('module') +const caller = require('get-caller-file') +const { isAbsolute } = require('path') + const ThreadStream = require('thread-stream') function transport (filename, workerData) { + if (!(isAbsolute(filename) || filename.indexOf('file://') === 0)) { + const callerRequire = createRequire(caller()) + filename = callerRequire.resolve(filename) + } + const stream = new ThreadStream({ filename, workerData, diff --git a/test/fixtures/transport/index.js b/test/fixtures/transport/index.js new file mode 100644 index 000000000..9ceb636ef --- /dev/null +++ b/test/fixtures/transport/index.js @@ -0,0 +1,12 @@ +'use strict' + +const fs = require('fs') +const { once } = require('events') + +async function run (opts) { + const stream = fs.createWriteStream(opts.dest) + await once(stream, 'open') + return stream +} + +module.exports = run diff --git a/test/fixtures/transport/package.json b/test/fixtures/transport/package.json new file mode 100644 index 000000000..26beeaaea --- /dev/null +++ b/test/fixtures/transport/package.json @@ -0,0 +1,5 @@ +{ + "name": "transport", + "version": "0.0.1", + "main": "./index.js" +} diff --git a/test/transport.test.js b/test/transport.test.js index a6e281e93..082729e7a 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -3,13 +3,22 @@ const os = require('os') const { join } = require('path') const { readFile } = require('fs').promises -const { test } = require('tap') +const { test, teardown, comment } = require('tap') +const { spawnSync } = require('child_process') const { watchFileCreated } = require('./helper') const pino = require('../') +const url = require('url') const { pid } = process const hostname = os.hostname() +comment('linkining commencing, might take a little while') +spawnSync('npm', ['link', join(__dirname, 'fixtures', 'transport')]) +teardown(() => { + comment('teardown commencing, might take a little while') + spawnSync('npm', ['unlink', 'transport']) +}) + test('pino.transport with file', async ({ same }) => { const dest = join( os.tmpdir(), @@ -27,3 +36,39 @@ test('pino.transport with file', async ({ same }) => { msg: 'hello' }) }) + +test('pino.transport with package', async ({ same }) => { + const dest = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const instance = pino(pino.transport('transport', { dest })) + instance.info('hello') + await watchFileCreated(dest) + const result = JSON.parse(await readFile(dest)) + delete result.time + same(result, { + pid, + hostname, + level: 30, + msg: 'hello' + }) +}) + +test('pino.transport with file URL', async ({ same }) => { + const dest = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const instance = pino(pino.transport(url.pathToFileURL(join(__dirname, 'fixtures', 'to-file-transport.js')).href, { dest })) + instance.info('hello') + await watchFileCreated(dest) + const result = JSON.parse(await readFile(dest)) + delete result.time + same(result, { + pid, + hostname, + level: 30, + msg: 'hello' + }) +}) From 6b7d7bb2db1e0dd9ed9ad20076bee42d2f58acb0 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 7 Apr 2021 12:36:46 +0200 Subject: [PATCH 03/63] Skip package test on win32 --- test/transport.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/transport.test.js b/test/transport.test.js index 082729e7a..12779d10b 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -37,7 +37,7 @@ test('pino.transport with file', async ({ same }) => { }) }) -test('pino.transport with package', async ({ same }) => { +test('pino.transport with package', { skip: process.platform === 'win32' }, async ({ same }) => { const dest = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) From b3b537a409633e3a4b9490a6db17bd39ab99ef5e Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 7 Apr 2021 14:51:47 +0200 Subject: [PATCH 04/63] Ire-enabled failing test on windows --- lib/transport.js | 1 + test/transport.test.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/transport.js b/lib/transport.js index 49332759b..8c47b1018 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -10,6 +10,7 @@ function transport (filename, workerData) { if (!(isAbsolute(filename) || filename.indexOf('file://') === 0)) { const callerRequire = createRequire(caller()) filename = callerRequire.resolve(filename) + console.log(filename) } const stream = new ThreadStream({ diff --git a/test/transport.test.js b/test/transport.test.js index 12779d10b..082729e7a 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -37,7 +37,7 @@ test('pino.transport with file', async ({ same }) => { }) }) -test('pino.transport with package', { skip: process.platform === 'win32' }, async ({ same }) => { +test('pino.transport with package', async ({ same }) => { const dest = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) From b3e4178be9c0bbb035b0a771784f5cc1dc460547 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 7 Apr 2021 14:55:13 +0200 Subject: [PATCH 05/63] full cov report --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 864416ce3..6f3320b50 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "browser-test": "airtap --local 8080 test/browser*test.js", "lint": "eslint .", "test": "npm run lint && tap --100 test/*test.js test/*/*test.js", - "test-ci": "npm run lint && tap test/*test.js test/*/*test.js --coverage-report=lcovonly", + "test-ci": "npm run lint && tap test/*test.js test/*/*test.js", "cov-ui": "tap --coverage-report=html test/*test.js test/*/*test.js", "bench": "node benchmarks/utils/runbench all", "bench-basic": "node benchmarks/utils/runbench basic", From ec34df7d4b36ffd30e0be701ba763c1feba50eb0 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 7 Apr 2021 14:58:15 +0200 Subject: [PATCH 06/63] Revert "full cov report" This reverts commit b3e4178be9c0bbb035b0a771784f5cc1dc460547. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6f3320b50..864416ce3 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "browser-test": "airtap --local 8080 test/browser*test.js", "lint": "eslint .", "test": "npm run lint && tap --100 test/*test.js test/*/*test.js", - "test-ci": "npm run lint && tap test/*test.js test/*/*test.js", + "test-ci": "npm run lint && tap test/*test.js test/*/*test.js --coverage-report=lcovonly", "cov-ui": "tap --coverage-report=html test/*test.js test/*/*test.js", "bench": "node benchmarks/utils/runbench all", "bench-basic": "node benchmarks/utils/runbench basic", From ed8cfd579f2b400c16ff1d600bc318ccad829206 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 7 Apr 2021 15:01:29 +0200 Subject: [PATCH 07/63] ignore lines unreacheable on Windows --- lib/transport.js | 3 ++- test/transport.test.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/transport.js b/lib/transport.js index 8c47b1018..89658b9a3 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -7,11 +7,12 @@ const { isAbsolute } = require('path') const ThreadStream = require('thread-stream') function transport (filename, workerData) { + /* istabul ignore start */ if (!(isAbsolute(filename) || filename.indexOf('file://') === 0)) { const callerRequire = createRequire(caller()) filename = callerRequire.resolve(filename) - console.log(filename) } + /* istabul ignore end */ const stream = new ThreadStream({ filename, diff --git a/test/transport.test.js b/test/transport.test.js index 082729e7a..12779d10b 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -37,7 +37,7 @@ test('pino.transport with file', async ({ same }) => { }) }) -test('pino.transport with package', async ({ same }) => { +test('pino.transport with package', { skip: process.platform === 'win32' }, async ({ same }) => { const dest = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) From e1c72b958ed7cd3981390a44669d2171064d0403 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 7 Apr 2021 18:04:09 +0200 Subject: [PATCH 08/63] Update lib/transport.js Co-authored-by: James Sumners --- lib/transport.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/transport.js b/lib/transport.js index 89658b9a3..53a7ff48d 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -12,7 +12,7 @@ function transport (filename, workerData) { const callerRequire = createRequire(caller()) filename = callerRequire.resolve(filename) } - /* istabul ignore end */ + /* istanbul ignore end */ const stream = new ThreadStream({ filename, From 44c150c6b520af042cfd1eb1a2068e71ed7c39cb Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 7 Apr 2021 18:04:15 +0200 Subject: [PATCH 09/63] Update lib/transport.js Co-authored-by: James Sumners --- lib/transport.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/transport.js b/lib/transport.js index 53a7ff48d..4c4c7b25e 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -7,7 +7,8 @@ const { isAbsolute } = require('path') const ThreadStream = require('thread-stream') function transport (filename, workerData) { - /* istabul ignore start */ + // Unreachable on Windows, so we ignore coverage here. + /* istanbul ignore start */ if (!(isAbsolute(filename) || filename.indexOf('file://') === 0)) { const callerRequire = createRequire(caller()) filename = callerRequire.resolve(filename) From 246b6f01988bb4905b5f1eb02d38af0b46ce80cb Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 7 Apr 2021 18:11:55 +0200 Subject: [PATCH 10/63] Revert "Revert "full cov report"" This reverts commit ec34df7d4b36ffd30e0be701ba763c1feba50eb0. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 864416ce3..6f3320b50 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "browser-test": "airtap --local 8080 test/browser*test.js", "lint": "eslint .", "test": "npm run lint && tap --100 test/*test.js test/*/*test.js", - "test-ci": "npm run lint && tap test/*test.js test/*/*test.js --coverage-report=lcovonly", + "test-ci": "npm run lint && tap test/*test.js test/*/*test.js", "cov-ui": "tap --coverage-report=html test/*test.js test/*/*test.js", "bench": "node benchmarks/utils/runbench all", "bench-basic": "node benchmarks/utils/runbench basic", From eeca49668fa8b9e92f160bb7e4a3e34cde506415 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 7 Apr 2021 18:17:27 +0200 Subject: [PATCH 11/63] try out different ignore strategies --- lib/transport.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/transport.js b/lib/transport.js index 4c4c7b25e..5868afe15 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -8,12 +8,11 @@ const ThreadStream = require('thread-stream') function transport (filename, workerData) { // Unreachable on Windows, so we ignore coverage here. - /* istanbul ignore start */ + /* istanbul ignore if */ if (!(isAbsolute(filename) || filename.indexOf('file://') === 0)) { const callerRequire = createRequire(caller()) filename = callerRequire.resolve(filename) } - /* istanbul ignore end */ const stream = new ThreadStream({ filename, From 661de3efc1f3f6ab1ec0c876b591f7853a8af1ff Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 7 Apr 2021 18:21:47 +0200 Subject: [PATCH 12/63] Revert "Revert "Revert "full cov report""" This reverts commit 246b6f01988bb4905b5f1eb02d38af0b46ce80cb. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6f3320b50..864416ce3 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "browser-test": "airtap --local 8080 test/browser*test.js", "lint": "eslint .", "test": "npm run lint && tap --100 test/*test.js test/*/*test.js", - "test-ci": "npm run lint && tap test/*test.js test/*/*test.js", + "test-ci": "npm run lint && tap test/*test.js test/*/*test.js --coverage-report=lcovonly", "cov-ui": "tap --coverage-report=html test/*test.js test/*/*test.js", "bench": "node benchmarks/utils/runbench all", "bench-basic": "node benchmarks/utils/runbench basic", From d78a30b7315f226153179a8a0ecd98c08ea43871 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 8 Apr 2021 10:40:51 +0200 Subject: [PATCH 13/63] Works on Windows --- lib/transport.js | 5 ++--- package.json | 3 ++- test/transport.test.js | 12 ++---------- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/lib/transport.js b/lib/transport.js index 5868afe15..1c3652803 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -7,10 +7,9 @@ const { isAbsolute } = require('path') const ThreadStream = require('thread-stream') function transport (filename, workerData) { - // Unreachable on Windows, so we ignore coverage here. - /* istanbul ignore if */ if (!(isAbsolute(filename) || filename.indexOf('file://') === 0)) { - const callerRequire = createRequire(caller()) + const callerFile = caller() + const callerRequire = createRequire(callerFile) filename = callerRequire.resolve(filename) } diff --git a/package.json b/package.json index 864416ce3..41a3cdb7e 100644 --- a/package.json +++ b/package.json @@ -87,7 +87,8 @@ "tap": "^15.0.1", "tape": "^5.0.0", "through2": "^4.0.0", - "winston": "^3.3.3" + "winston": "^3.3.3", + "transport": "./test/fixtures/transport" }, "dependencies": { "fast-redact": "^3.0.0", diff --git a/test/transport.test.js b/test/transport.test.js index 12779d10b..ceb412f82 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -3,8 +3,7 @@ const os = require('os') const { join } = require('path') const { readFile } = require('fs').promises -const { test, teardown, comment } = require('tap') -const { spawnSync } = require('child_process') +const { test } = require('tap') const { watchFileCreated } = require('./helper') const pino = require('../') const url = require('url') @@ -12,13 +11,6 @@ const url = require('url') const { pid } = process const hostname = os.hostname() -comment('linkining commencing, might take a little while') -spawnSync('npm', ['link', join(__dirname, 'fixtures', 'transport')]) -teardown(() => { - comment('teardown commencing, might take a little while') - spawnSync('npm', ['unlink', 'transport']) -}) - test('pino.transport with file', async ({ same }) => { const dest = join( os.tmpdir(), @@ -37,7 +29,7 @@ test('pino.transport with file', async ({ same }) => { }) }) -test('pino.transport with package', { skip: process.platform === 'win32' }, async ({ same }) => { +test('pino.transport with package', async ({ same }) => { const dest = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) From c54769e564c8281af99a389d67fac5590b95b797 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 8 Apr 2021 15:52:28 +0200 Subject: [PATCH 14/63] Emit 'error' if thread exits abruptly --- lib/transport.js | 4 +++- package.json | 4 ++-- test/transport.test.js | 9 +++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/transport.js b/lib/transport.js index 1c3652803..c90f50473 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -19,7 +19,9 @@ function transport (filename, workerData) { sync: true // TODO should this be configurable? }) - stream.unref() + stream.on('ready', function () { + stream.unref() + }) return stream } diff --git a/package.json b/package.json index 41a3cdb7e..6d7f7635c 100644 --- a/package.json +++ b/package.json @@ -87,8 +87,8 @@ "tap": "^15.0.1", "tape": "^5.0.0", "through2": "^4.0.0", - "winston": "^3.3.3", - "transport": "./test/fixtures/transport" + "transport": "./test/fixtures/transport", + "winston": "^3.3.3" }, "dependencies": { "fast-redact": "^3.0.0", diff --git a/test/transport.test.js b/test/transport.test.js index ceb412f82..7cdedf09e 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -64,3 +64,12 @@ test('pino.transport with file URL', async ({ same }) => { msg: 'hello' }) }) + +test('pino.transport errors if file does not exists', ({ plan, pass }) => { + plan(1) + const instance = pino.transport(join(__dirname, 'fixtures', 'non-existent-file')) + instance.on('error', function (err) { + console.log(err) + pass('error received') + }) +}) From 4c01c8951dade1adecd9754d1b3867a0355d9460 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 10 Apr 2021 23:53:23 +0200 Subject: [PATCH 15/63] Implement pass-through of WorkerThread options --- lib/transport.js | 3 ++- package.json | 2 +- test/transport.test.js | 9 ++++++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/transport.js b/lib/transport.js index c90f50473..d59c98954 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -6,7 +6,7 @@ const { isAbsolute } = require('path') const ThreadStream = require('thread-stream') -function transport (filename, workerData) { +function transport (filename, workerData, workerOpts = {}) { if (!(isAbsolute(filename) || filename.indexOf('file://') === 0)) { const callerFile = caller() const callerRequire = createRequire(callerFile) @@ -16,6 +16,7 @@ function transport (filename, workerData) { const stream = new ThreadStream({ filename, workerData, + workerOpts, sync: true // TODO should this be configurable? }) diff --git a/package.json b/package.json index 6d7f7635c..2bfad5325 100644 --- a/package.json +++ b/package.json @@ -98,6 +98,6 @@ "pino-std-serializers": "^3.1.0", "quick-format-unescaped": "^4.0.3", "sonic-boom": "^1.0.2", - "thread-stream": "^0.7.1" + "thread-stream": "^0.8.0" } } diff --git a/test/transport.test.js b/test/transport.test.js index 7cdedf09e..97dba1ec9 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -67,9 +67,12 @@ test('pino.transport with file URL', async ({ same }) => { test('pino.transport errors if file does not exists', ({ plan, pass }) => { plan(1) - const instance = pino.transport(join(__dirname, 'fixtures', 'non-existent-file')) - instance.on('error', function (err) { - console.log(err) + const instance = pino.transport(join(__dirname, 'fixtures', 'non-existent-file'), {}, { + stdin: true, + stdout: true, + stderr: true + }) + instance.on('error', function () { pass('error received') }) }) From 463af9d5bd344a96fb57935fcff87dc7409acb90 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 11 Apr 2021 11:55:57 +0200 Subject: [PATCH 16/63] Esm support with transport --- package.json | 1 + test/fixtures/to-file-transport.mjs | 8 ++++++++ test/transport.test.js | 18 ++++++++++++++++++ 3 files changed, 27 insertions(+) create mode 100644 test/fixtures/to-file-transport.mjs diff --git a/package.json b/package.json index 2bfad5325..dca496153 100644 --- a/package.json +++ b/package.json @@ -95,6 +95,7 @@ "fast-safe-stringify": "^2.0.7", "flatstr": "^1.0.12", "get-caller-file": "^2.0.5", + "pino-abstract-transport": "^0.1.0", "pino-std-serializers": "^3.1.0", "quick-format-unescaped": "^4.0.3", "sonic-boom": "^1.0.2", diff --git a/test/fixtures/to-file-transport.mjs b/test/fixtures/to-file-transport.mjs new file mode 100644 index 000000000..2fcfe2d14 --- /dev/null +++ b/test/fixtures/to-file-transport.mjs @@ -0,0 +1,8 @@ +import { createWriteStream } from 'fs' +import { once } from 'events' + +export default async function run (opts) { + const stream = createWriteStream(opts.dest) + await once(stream, 'open') + return stream +} diff --git a/test/transport.test.js b/test/transport.test.js index 97dba1ec9..e7667dd28 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -76,3 +76,21 @@ test('pino.transport errors if file does not exists', ({ plan, pass }) => { pass('error received') }) }) + +test('pino.transport with esm', async ({ same }) => { + const dest = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const instance = pino(pino.transport(join(__dirname, 'fixtures', 'to-file-transport.mjs'), { dest })) + instance.info('hello') + await watchFileCreated(dest) + const result = JSON.parse(await readFile(dest)) + delete result.time + same(result, { + pid, + hostname, + level: 30, + msg: 'hello' + }) +}) From aa28076967b5fa421e948fbb2155b23aaa4f5e70 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 11 Apr 2021 18:05:36 +0200 Subject: [PATCH 17/63] Basic multitransport code --- lib/multitransport/index.js | 8 ++++++ lib/multitransport/worker.js | 35 ++++++++++++++++++++++++ pino.js | 1 + test/multitransport.test.js | 52 ++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+) create mode 100644 lib/multitransport/index.js create mode 100644 lib/multitransport/worker.js create mode 100644 test/multitransport.test.js diff --git a/lib/multitransport/index.js b/lib/multitransport/index.js new file mode 100644 index 000000000..a8f67d8e9 --- /dev/null +++ b/lib/multitransport/index.js @@ -0,0 +1,8 @@ +'use strict' + +const transport = require('../transport') +const { join } = require('path') + +module.exports = function multitransport (definitions, workerOpts) { + return transport(join(__dirname, 'worker.js'), definitions, workerOpts) +} diff --git a/lib/multitransport/worker.js b/lib/multitransport/worker.js new file mode 100644 index 000000000..b3faaa484 --- /dev/null +++ b/lib/multitransport/worker.js @@ -0,0 +1,35 @@ +'use strict' + +const pino = require('../../pino.js') +const build = require('pino-abstract-transport') + +module.exports = async function ({ transports }) { + transports = await Promise.all(transports.map(async (t) => { + const stream = await (await import(t.module)).default(t.opts) + return { + level: t.level, + stream + } + })) + return build(process, { parse: 'lines', metadata: true }) + + function process (stream) { + const multi = pino.multistream(transports) + stream.on('data', function (chunk) { + const { lastTime, lastMsg, lastObj, lastLevel } = this + multi.lastLevel = lastLevel + multi.lastTime = lastTime + multi.lastMsg = lastMsg + multi.lastObj = lastObj + + // TODO handle backpressure + multi.write(chunk) + }) + + stream.on('end', function () { + for (const transport of transports) { + transport.stream.end() + } + }) + } +} diff --git a/pino.js b/pino.js index 9ea649e26..5d1ff4e82 100644 --- a/pino.js +++ b/pino.js @@ -225,6 +225,7 @@ module.exports.destination = (dest = process.stdout.fd) => { module.exports.transport = require('./lib/transport') module.exports.multistream = require('./lib/multistream') +module.exports.multitransport = require('./lib/multitransport') module.exports.final = final module.exports.levels = mappings() diff --git a/test/multitransport.test.js b/test/multitransport.test.js new file mode 100644 index 000000000..2072b4136 --- /dev/null +++ b/test/multitransport.test.js @@ -0,0 +1,52 @@ +'use strict' + +const os = require('os') +const { join } = require('path') +const { readFile } = require('fs').promises +const { test } = require('tap') +const { watchFileCreated } = require('./helper') +const pino = require('../') + +const { pid } = process +const hostname = os.hostname() + +test('pino.multitransport with two files', async ({ same }) => { + const dest1 = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const dest2 = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const transport = pino.multitransport({ + transports: [{ + level: 'info', + module: join(__dirname, 'fixtures', 'to-file-transport.js'), + opts: { dest: dest1 } + }, { + level: 'info', + module: join(__dirname, 'fixtures', 'to-file-transport.js'), + opts: { dest: dest2 } + }] + }) + const instance = pino(transport) + instance.info('hello') + await Promise.all([watchFileCreated(dest1), watchFileCreated(dest2)]) + const result1 = JSON.parse(await readFile(dest1)) + delete result1.time + same(result1, { + pid, + hostname, + level: 30, + msg: 'hello' + }) + const result2 = JSON.parse(await readFile(dest2)) + delete result2.time + same(result2, { + pid, + hostname, + level: 30, + msg: 'hello' + }) +}) From 2f9649f977458bd0ee426f73491fe9b747cb55c4 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 26 Apr 2021 20:09:12 +0200 Subject: [PATCH 18/63] multitransport example --- example-transports.js | 55 +++++++++++++++++++++++++++ lib/multitransport/worker.js | 38 +++++++++++++++++- lib/tools.js | 5 ++- test/multitransport.test.js | 74 ++++++++++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 example-transports.js diff --git a/example-transports.js b/example-transports.js new file mode 100644 index 000000000..b58e34f3a --- /dev/null +++ b/example-transports.js @@ -0,0 +1,55 @@ +'use strict' + +const pino = require('./') +const { tmpdir } = require('os') +const { join } = require('path') + +const file = join(tmpdir(), `pino-${process.pid}-example`) + +const logger = pino(pino.multitransport({ + transports: [{ + level: 'warn', + destination: file + }, { + level: 'info', + prettyPrint: true + }] +})) + +logger.info({ + file +}, 'logging destination') + +logger.info('hello world') +logger.error('this is at error level') +logger.info('the answer is %d', 42) +logger.info({ obj: 42 }, 'hello world') +logger.info({ obj: 42, b: 2 }, 'hello world') +logger.info({ nested: { obj: 42 } }, 'nested') +logger.warn('WARNING!') +setImmediate(() => { + logger.info('after setImmediate') +}) +logger.error(new Error('an error')) + +const child = logger.child({ a: 'property' }) +child.info('hello child!') + +const childsChild = child.child({ another: 'property' }) +childsChild.info('hello baby..') + +logger.debug('this should be mute') + +logger.level = 'trace' + +logger.debug('this is a debug statement') + +logger.child({ another: 'property' }).debug('this is a debug statement via child') +logger.trace('this is a trace statement') + +logger.debug('this is a "debug" statement with "') + +logger.info(new Error('kaboom')) +logger.info(null) + +logger.info(new Error('kaboom'), 'with', 'a', 'message') diff --git a/lib/multitransport/worker.js b/lib/multitransport/worker.js index b3faaa484..2f766bcae 100644 --- a/lib/multitransport/worker.js +++ b/lib/multitransport/worker.js @@ -1,11 +1,46 @@ 'use strict' const pino = require('../../pino.js') +const { Transform, pipeline } = require('stream') const build = require('pino-abstract-transport') +const { once } = require('events') + +// This file is not checked by the code coverage tool module.exports = async function ({ transports }) { transports = await Promise.all(transports.map(async (t) => { - const stream = await (await import(t.module)).default(t.opts) + let stream + if (t.module) { + stream = await (await import(t.module)).default(t.opts) + } else if (t.prettyPrint) { + const pretty = require('pino-pretty')(t.prettyPrint) + stream = new Transform({ + objectMode: true, + transform (chunk, enc, cb) { + const line = pretty(chunk.toString()) + if (line === undefined) return cb() + cb(null, line) + } + }) + + // TODO figure out why sync: false does not work here + const destination = pino.destination(t.destination || 0) + await once(destination, 'ready') + pipeline(stream, destination, () => {}) + } else { + // TODO remove the transform + stream = new Transform({ + objectMode: true, + transform (chunk, enc, cb) { + cb(null, chunk.toString() + '\n') + } + }) + + // TODO figure out why sync: false does not work here + const destination = pino.destination(t.destination || 0) + await once(destination, 'ready') + pipeline(stream, destination, () => {}) + } return { level: t.level, stream @@ -15,6 +50,7 @@ module.exports = async function ({ transports }) { function process (stream) { const multi = pino.multistream(transports) + // TODO manage backpressure stream.on('data', function (chunk) { const { lastTime, lastMsg, lastObj, lastLevel } = this multi.lastLevel = lastLevel diff --git a/lib/tools.js b/lib/tools.js index a7c9389a2..0e1392dc2 100644 --- a/lib/tools.js +++ b/lib/tools.js @@ -234,7 +234,10 @@ function prettifierMetaWrapper (pretty, dest, opts) { let time = this.lastTime - if (time.match(/^\d+/)) { + /* istanbul ignore next */ + if (typeof time === 'number') { + // do nothing! + } else if (time.match(/^\d+/)) { time = parseInt(time) } else { time = time.slice(1, -1) diff --git a/test/multitransport.test.js b/test/multitransport.test.js index 2072b4136..f4e67996b 100644 --- a/test/multitransport.test.js +++ b/test/multitransport.test.js @@ -6,6 +6,7 @@ const { readFile } = require('fs').promises const { test } = require('tap') const { watchFileCreated } = require('./helper') const pino = require('../') +const strip = require('strip-ansi') const { pid } = process const hostname = os.hostname() @@ -50,3 +51,76 @@ test('pino.multitransport with two files', async ({ same }) => { msg: 'hello' }) }) + +test('pino.multitransport with two files', async ({ same }) => { + const dest1 = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const dest2 = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const transport = pino.multitransport({ + transports: [{ + level: 'info', + destination: dest1 + }, { + level: 'info', + destination: dest2 + }] + }) + const instance = pino(transport) + instance.info('hello') + await Promise.all([watchFileCreated(dest1), watchFileCreated(dest2)]) + const result1 = JSON.parse(await readFile(dest1)) + delete result1.time + same(result1, { + pid, + hostname, + level: 30, + msg: 'hello' + }) + const result2 = JSON.parse(await readFile(dest2)) + delete result2.time + same(result2, { + pid, + hostname, + level: 30, + msg: 'hello' + }) +}) + +test('pino.multitransport with a prettyPrint destination', async ({ same, match }) => { + const dest1 = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const dest2 = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const transport = pino.multitransport({ + transports: [{ + level: 'info', + destination: dest1 + }, { + level: 'info', + prettyPrint: true, + destination: dest2 + }] + }) + const instance = pino(transport) + instance.info('hello') + await Promise.all([watchFileCreated(dest1), watchFileCreated(dest2)]) + const result1 = JSON.parse(await readFile(dest1)) + delete result1.time + same(result1, { + pid, + hostname, + level: 30, + msg: 'hello' + }) + const actual = (await readFile(dest2)).toString() + match(strip(actual), /\[.*\] INFO.*hello/) +}) From 6dd0627786956725e3c8ddd4cc6c0ad4c383da6f Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 27 Apr 2021 01:30:02 +0200 Subject: [PATCH 19/63] merged transport and multitransport --- lib/multitransport/index.js | 8 -- lib/transport.js | 12 ++- lib/{multitransport => }/worker.js | 4 +- pino.js | 1 - test/multitransport.test.js | 126 ----------------------------- test/transport.test.js | 109 +++++++++++++++++++++++++ 6 files changed, 121 insertions(+), 139 deletions(-) delete mode 100644 lib/multitransport/index.js rename lib/{multitransport => }/worker.js (95%) delete mode 100644 test/multitransport.test.js diff --git a/lib/multitransport/index.js b/lib/multitransport/index.js deleted file mode 100644 index a8f67d8e9..000000000 --- a/lib/multitransport/index.js +++ /dev/null @@ -1,8 +0,0 @@ -'use strict' - -const transport = require('../transport') -const { join } = require('path') - -module.exports = function multitransport (definitions, workerOpts) { - return transport(join(__dirname, 'worker.js'), definitions, workerOpts) -} diff --git a/lib/transport.js b/lib/transport.js index d59c98954..c685b9630 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -2,7 +2,7 @@ const { createRequire } = require('module') const caller = require('get-caller-file') -const { isAbsolute } = require('path') +const { isAbsolute, join } = require('path') const ThreadStream = require('thread-stream') @@ -27,4 +27,12 @@ function transport (filename, workerData, workerOpts = {}) { return stream } -module.exports = transport +function multitrasport (...args) { + if (Array.isArray(args[0])) { + return transport(join(__dirname, 'worker.js'), args[0], args[1]) + } + + return transport(...args) +} + +module.exports = multitrasport diff --git a/lib/multitransport/worker.js b/lib/worker.js similarity index 95% rename from lib/multitransport/worker.js rename to lib/worker.js index 2f766bcae..764d02fcf 100644 --- a/lib/multitransport/worker.js +++ b/lib/worker.js @@ -1,13 +1,13 @@ 'use strict' -const pino = require('../../pino.js') +const pino = require('../pino.js') const { Transform, pipeline } = require('stream') const build = require('pino-abstract-transport') const { once } = require('events') // This file is not checked by the code coverage tool -module.exports = async function ({ transports }) { +module.exports = async function (transports) { transports = await Promise.all(transports.map(async (t) => { let stream if (t.module) { diff --git a/pino.js b/pino.js index 5d1ff4e82..9ea649e26 100644 --- a/pino.js +++ b/pino.js @@ -225,7 +225,6 @@ module.exports.destination = (dest = process.stdout.fd) => { module.exports.transport = require('./lib/transport') module.exports.multistream = require('./lib/multistream') -module.exports.multitransport = require('./lib/multitransport') module.exports.final = final module.exports.levels = mappings() diff --git a/test/multitransport.test.js b/test/multitransport.test.js deleted file mode 100644 index f4e67996b..000000000 --- a/test/multitransport.test.js +++ /dev/null @@ -1,126 +0,0 @@ -'use strict' - -const os = require('os') -const { join } = require('path') -const { readFile } = require('fs').promises -const { test } = require('tap') -const { watchFileCreated } = require('./helper') -const pino = require('../') -const strip = require('strip-ansi') - -const { pid } = process -const hostname = os.hostname() - -test('pino.multitransport with two files', async ({ same }) => { - const dest1 = join( - os.tmpdir(), - '_' + Math.random().toString(36).substr(2, 9) - ) - const dest2 = join( - os.tmpdir(), - '_' + Math.random().toString(36).substr(2, 9) - ) - const transport = pino.multitransport({ - transports: [{ - level: 'info', - module: join(__dirname, 'fixtures', 'to-file-transport.js'), - opts: { dest: dest1 } - }, { - level: 'info', - module: join(__dirname, 'fixtures', 'to-file-transport.js'), - opts: { dest: dest2 } - }] - }) - const instance = pino(transport) - instance.info('hello') - await Promise.all([watchFileCreated(dest1), watchFileCreated(dest2)]) - const result1 = JSON.parse(await readFile(dest1)) - delete result1.time - same(result1, { - pid, - hostname, - level: 30, - msg: 'hello' - }) - const result2 = JSON.parse(await readFile(dest2)) - delete result2.time - same(result2, { - pid, - hostname, - level: 30, - msg: 'hello' - }) -}) - -test('pino.multitransport with two files', async ({ same }) => { - const dest1 = join( - os.tmpdir(), - '_' + Math.random().toString(36).substr(2, 9) - ) - const dest2 = join( - os.tmpdir(), - '_' + Math.random().toString(36).substr(2, 9) - ) - const transport = pino.multitransport({ - transports: [{ - level: 'info', - destination: dest1 - }, { - level: 'info', - destination: dest2 - }] - }) - const instance = pino(transport) - instance.info('hello') - await Promise.all([watchFileCreated(dest1), watchFileCreated(dest2)]) - const result1 = JSON.parse(await readFile(dest1)) - delete result1.time - same(result1, { - pid, - hostname, - level: 30, - msg: 'hello' - }) - const result2 = JSON.parse(await readFile(dest2)) - delete result2.time - same(result2, { - pid, - hostname, - level: 30, - msg: 'hello' - }) -}) - -test('pino.multitransport with a prettyPrint destination', async ({ same, match }) => { - const dest1 = join( - os.tmpdir(), - '_' + Math.random().toString(36).substr(2, 9) - ) - const dest2 = join( - os.tmpdir(), - '_' + Math.random().toString(36).substr(2, 9) - ) - const transport = pino.multitransport({ - transports: [{ - level: 'info', - destination: dest1 - }, { - level: 'info', - prettyPrint: true, - destination: dest2 - }] - }) - const instance = pino(transport) - instance.info('hello') - await Promise.all([watchFileCreated(dest1), watchFileCreated(dest2)]) - const result1 = JSON.parse(await readFile(dest1)) - delete result1.time - same(result1, { - pid, - hostname, - level: 30, - msg: 'hello' - }) - const actual = (await readFile(dest2)).toString() - match(strip(actual), /\[.*\] INFO.*hello/) -}) diff --git a/test/transport.test.js b/test/transport.test.js index e7667dd28..863e28f50 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -7,6 +7,7 @@ const { test } = require('tap') const { watchFileCreated } = require('./helper') const pino = require('../') const url = require('url') +const strip = require('strip-ansi') const { pid } = process const hostname = os.hostname() @@ -94,3 +95,111 @@ test('pino.transport with esm', async ({ same }) => { msg: 'hello' }) }) + +test('pino.transport with two files', async ({ same }) => { + const dest1 = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const dest2 = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const transport = pino.transport([{ + level: 'info', + module: join(__dirname, 'fixtures', 'to-file-transport.js'), + opts: { dest: dest1 } + }, { + level: 'info', + module: join(__dirname, 'fixtures', 'to-file-transport.js'), + opts: { dest: dest2 } + }]) + const instance = pino(transport) + instance.info('hello') + await Promise.all([watchFileCreated(dest1), watchFileCreated(dest2)]) + const result1 = JSON.parse(await readFile(dest1)) + delete result1.time + same(result1, { + pid, + hostname, + level: 30, + msg: 'hello' + }) + const result2 = JSON.parse(await readFile(dest2)) + delete result2.time + same(result2, { + pid, + hostname, + level: 30, + msg: 'hello' + }) +}) + +test('pino.transport with two files', async ({ same }) => { + const dest1 = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const dest2 = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const transport = pino.transport([{ + level: 'info', + destination: dest1 + }, { + level: 'info', + destination: dest2 + }]) + const instance = pino(transport) + instance.info('hello') + await Promise.all([watchFileCreated(dest1), watchFileCreated(dest2)]) + const result1 = JSON.parse(await readFile(dest1)) + delete result1.time + same(result1, { + pid, + hostname, + level: 30, + msg: 'hello' + }) + const result2 = JSON.parse(await readFile(dest2)) + delete result2.time + same(result2, { + pid, + hostname, + level: 30, + msg: 'hello' + }) +}) + +test('pino.transport with an array including a prettyPrint destination', async ({ same, match }) => { + const dest1 = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const dest2 = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const transport = pino.transport([{ + level: 'info', + destination: dest1 + }, { + level: 'info', + prettyPrint: true, + destination: dest2 + }]) + const instance = pino(transport) + instance.info('hello') + await Promise.all([watchFileCreated(dest1), watchFileCreated(dest2)]) + const result1 = JSON.parse(await readFile(dest1)) + delete result1.time + same(result1, { + pid, + hostname, + level: 30, + msg: 'hello' + }) + const actual = (await readFile(dest2)).toString() + match(strip(actual), /\[.*\] INFO.*hello/) +}) From 2ad77cefd50cdf4497a76ab8d87e003bbdf1bbf6 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 29 Apr 2021 21:05:47 +0200 Subject: [PATCH 20/63] Increased timeout --- test/helper.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/helper.js b/test/helper.js index dd25d9df8..3803a35f5 100644 --- a/test/helper.js +++ b/test/helper.js @@ -53,7 +53,7 @@ function sleep (ms) { function watchFileCreated (filename) { return new Promise((resolve, reject) => { - const TIMEOUT = 800 + const TIMEOUT = 2000 const INTERVAL = 100 const threshold = TIMEOUT / INTERVAL let counter = 0 From 465ed1736d83c597da591f7d6c6239e5d139546f Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 30 Apr 2021 21:45:21 +0200 Subject: [PATCH 21/63] Added node v16 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e7c5db403..b1658fa87 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,7 +16,7 @@ jobs: fail-fast: false matrix: os: [macOS-latest, windows-latest, ubuntu-latest] - node-version: [12, 14] + node-version: [12, 14, 16] steps: - uses: actions/checkout@v2 - name: Use Node.js ${{ matrix.node-version }} From 446a58165bab531d6a2135d876f42fcd600ca275 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 30 Apr 2021 22:23:23 +0200 Subject: [PATCH 22/63] print the destination --- lib/worker.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/worker.js b/lib/worker.js index 764d02fcf..aa1b61682 100644 --- a/lib/worker.js +++ b/lib/worker.js @@ -9,6 +9,7 @@ const { once } = require('events') module.exports = async function (transports) { transports = await Promise.all(transports.map(async (t) => { + console.log(t.destination) let stream if (t.module) { stream = await (await import(t.module)).default(t.opts) From 8e55539952f041c0202936da09465c9d6332b415 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 30 Apr 2021 22:38:19 +0200 Subject: [PATCH 23/63] file:// --- lib/worker.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/worker.js b/lib/worker.js index aa1b61682..a94e6e07d 100644 --- a/lib/worker.js +++ b/lib/worker.js @@ -4,15 +4,19 @@ const pino = require('../pino.js') const { Transform, pipeline } = require('stream') const build = require('pino-abstract-transport') const { once } = require('events') +const { isAbsolute } = require('path') // This file is not checked by the code coverage tool module.exports = async function (transports) { transports = await Promise.all(transports.map(async (t) => { - console.log(t.destination) let stream if (t.module) { - stream = await (await import(t.module)).default(t.opts) + let toLoad = t.module + if (isAbsolute(toLoad)) { + toLoad = 'file://' + toLoad + } + stream = await (await import(toLoad)).default(t.opts) } else if (t.prettyPrint) { const pretty = require('pino-pretty')(t.prettyPrint) stream = new Transform({ From 7e8f662896e37592d6b832b8697da5ea774bbe58 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 3 May 2021 14:13:24 +0200 Subject: [PATCH 24/63] Update example --- example-transports.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/example-transports.js b/example-transports.js index b58e34f3a..70c2043dc 100644 --- a/example-transports.js +++ b/example-transports.js @@ -6,15 +6,13 @@ const { join } = require('path') const file = join(tmpdir(), `pino-${process.pid}-example`) -const logger = pino(pino.multitransport({ - transports: [{ - level: 'warn', - destination: file - }, { - level: 'info', - prettyPrint: true - }] -})) +const logger = pino(pino.transport([{ + level: 'warn', + destination: file +}, { + level: 'info', + prettyPrint: true +}])) logger.info({ file From 63c4f49ba3b1c1f121f63981e9c3a5e4a8bc3b1e Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 19 May 2021 16:28:37 +0200 Subject: [PATCH 25/63] Transports --- example-transports.js | 12 +++++-- lib/multistream.js | 10 ++++++ lib/transport.js | 14 ++++++++ lib/worker.js | 46 +++++++++++++++--------- package.json | 4 +-- test/multistream.test.js | 29 +++++++++++++++ test/transport.test.js | 76 ++++++++++++++++++++++++++++++++++------ 7 files changed, 160 insertions(+), 31 deletions(-) diff --git a/example-transports.js b/example-transports.js index 70c2043dc..83b7ee1a4 100644 --- a/example-transports.js +++ b/example-transports.js @@ -6,13 +6,21 @@ const { join } = require('path') const file = join(tmpdir(), `pino-${process.pid}-example`) -const logger = pino(pino.transport([{ +const transport = pino.transport([{ level: 'warn', destination: file +}, { + level: 'info', + module: '/Users/matteo/repositories/pino-elasticsearch/lib.js', + opts: { + node: 'http://localhost:9200' + } }, { level: 'info', prettyPrint: true -}])) +}]) + +const logger = pino(transport) logger.info({ file diff --git a/lib/multistream.js b/lib/multistream.js index 3756523a6..69819b0b9 100644 --- a/lib/multistream.js +++ b/lib/multistream.js @@ -21,6 +21,7 @@ function multistream (streamsArray, opts) { write, add, flushSync, + end, minLevel: 0, streams: [], clone, @@ -103,6 +104,15 @@ function multistream (streamsArray, opts) { return res } + function end () { + for (const { stream } of this.streams) { + if (typeof stream.flushSync === 'function') { + stream.flushSync() + } + stream.end() + } + } + function clone (level) { const streams = new Array(this.streams.length) diff --git a/lib/transport.js b/lib/transport.js index c685b9630..45e33a05a 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -22,9 +22,23 @@ function transport (filename, workerData, workerOpts = {}) { stream.on('ready', function () { stream.unref() + + if (workerOpts.autoEnd !== false) { + // TODO possibly use FinalizationGroup to automatically remove + // this listener if the stream goes out scope. + process.on('exit', autoEnd) + + stream.on('close', function () { + process.removeListener('exit', autoEnd) + }) + } }) return stream + + function autoEnd () { + stream.end() + } } function multitrasport (...args) { diff --git a/lib/worker.js b/lib/worker.js index a94e6e07d..8fcdf0948 100644 --- a/lib/worker.js +++ b/lib/worker.js @@ -4,7 +4,6 @@ const pino = require('../pino.js') const { Transform, pipeline } = require('stream') const build = require('pino-abstract-transport') const { once } = require('events') -const { isAbsolute } = require('path') // This file is not checked by the code coverage tool @@ -12,10 +11,7 @@ module.exports = async function (transports) { transports = await Promise.all(transports.map(async (t) => { let stream if (t.module) { - let toLoad = t.module - if (isAbsolute(toLoad)) { - toLoad = 'file://' + toLoad - } + const toLoad = 'file://' + t.module stream = await (await import(toLoad)).default(t.opts) } else if (t.prettyPrint) { const pretty = require('pino-pretty')(t.prettyPrint) @@ -23,13 +19,20 @@ module.exports = async function (transports) { objectMode: true, transform (chunk, enc, cb) { const line = pretty(chunk.toString()) - if (line === undefined) return cb() cb(null, line) } }) // TODO figure out why sync: false does not work here - const destination = pino.destination(t.destination || 0) + // TODO remove the istanbul ignore, add a test using a + // child_process + const destination = pino.destination(t.destination || /* istanbul ignore next */ 1) + /* istanbul ignore next */ + if (destination.fd === 1) { + destination.end = function () { + this.emit('close') + } + } await once(destination, 'ready') pipeline(stream, destination, () => {}) } else { @@ -42,7 +45,7 @@ module.exports = async function (transports) { }) // TODO figure out why sync: false does not work here - const destination = pino.destination(t.destination || 0) + const destination = pino.destination(t.destination || /* istanbul ignore next */ 1) await once(destination, 'ready') pipeline(stream, destination, () => {}) } @@ -51,7 +54,24 @@ module.exports = async function (transports) { stream } })) - return build(process, { parse: 'lines', metadata: true }) + return build(process, { + parse: 'lines', + metadata: true, + close (err, cb) { + let expected = 0 + for (const transport of transports) { + expected++ + transport.stream.on('close', closeCb) + transport.stream.end() + } + + function closeCb () { + if (--expected === 0) { + cb(err) + } + } + } + }) function process (stream) { const multi = pino.multistream(transports) @@ -64,13 +84,7 @@ module.exports = async function (transports) { multi.lastObj = lastObj // TODO handle backpressure - multi.write(chunk) - }) - - stream.on('end', function () { - for (const transport of transports) { - transport.stream.end() - } + multi.write(chunk + '\n') }) } } diff --git a/package.json b/package.json index cc7898006..093132ba5 100644 --- a/package.json +++ b/package.json @@ -94,10 +94,10 @@ "fast-redact": "^3.0.0", "fast-safe-stringify": "^2.0.7", "get-caller-file": "^2.0.5", - "pino-abstract-transport": "^0.1.0", + "pino-abstract-transport": "^0.2.0", "pino-std-serializers": "^3.1.0", "quick-format-unescaped": "^4.0.3", "sonic-boom": "^1.0.2", - "thread-stream": "^0.8.0" + "thread-stream": "^0.9.0" } } diff --git a/test/multistream.test.js b/test/multistream.test.js index f5d49a0f5..621a04731 100644 --- a/test/multistream.test.js +++ b/test/multistream.test.js @@ -474,3 +474,32 @@ test('flushSync', function (t) { })() }) }) + +test('ends all streams', function (t) { + t.plan(7) + const stream = writeStream(function (data, enc, cb) { + t.pass('message') + cb() + }) + stream.flushSync = function () { + t.pass('flushSync') + } + // stream2 has no flushSync + const stream2 = writeStream(function (data, enc, cb) { + t.pass('message2') + cb() + }) + const streams = [ + { stream: stream }, + { level: 'debug', stream: stream }, + { level: 'trace', stream: stream2 }, + { level: 'fatal', stream: stream }, + { level: 'silent', stream: stream } + ] + const multi = multistream(streams) + const log = pino({ + level: 'trace' + }, multi) + log.info('info stream') + multi.end() +}) diff --git a/test/transport.test.js b/test/transport.test.js index 863e28f50..c777082f2 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -12,12 +12,14 @@ const strip = require('strip-ansi') const { pid } = process const hostname = os.hostname() -test('pino.transport with file', async ({ same }) => { +test('pino.transport with file', async ({ same, teardown }) => { const dest = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) - const instance = pino(pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js'), { dest })) + const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js'), { dest }) + teardown(transport.end.bind(transport)) + const instance = pino(transport) instance.info('hello') await watchFileCreated(dest) const result = JSON.parse(await readFile(dest)) @@ -30,12 +32,14 @@ test('pino.transport with file', async ({ same }) => { }) }) -test('pino.transport with package', async ({ same }) => { +test('pino.transport with package', async ({ same, teardown }) => { const dest = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) - const instance = pino(pino.transport('transport', { dest })) + const transport = pino.transport('transport', { dest }) + teardown(transport.end.bind(transport)) + const instance = pino(transport) instance.info('hello') await watchFileCreated(dest) const result = JSON.parse(await readFile(dest)) @@ -48,12 +52,14 @@ test('pino.transport with package', async ({ same }) => { }) }) -test('pino.transport with file URL', async ({ same }) => { +test('pino.transport with file URL', async ({ same, teardown }) => { const dest = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) - const instance = pino(pino.transport(url.pathToFileURL(join(__dirname, 'fixtures', 'to-file-transport.js')).href, { dest })) + const transport = pino.transport(url.pathToFileURL(join(__dirname, 'fixtures', 'to-file-transport.js')).href, { dest }) + teardown(transport.end.bind(transport)) + const instance = pino(transport) instance.info('hello') await watchFileCreated(dest) const result = JSON.parse(await readFile(dest)) @@ -78,12 +84,14 @@ test('pino.transport errors if file does not exists', ({ plan, pass }) => { }) }) -test('pino.transport with esm', async ({ same }) => { +test('pino.transport with esm', async ({ same, teardown }) => { const dest = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) - const instance = pino(pino.transport(join(__dirname, 'fixtures', 'to-file-transport.mjs'), { dest })) + const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.mjs'), { dest }) + const instance = pino(transport) + teardown(transport.end.bind(transport)) instance.info('hello') await watchFileCreated(dest) const result = JSON.parse(await readFile(dest)) @@ -96,7 +104,7 @@ test('pino.transport with esm', async ({ same }) => { }) }) -test('pino.transport with two files', async ({ same }) => { +test('pino.transport with two files', async ({ same, teardown }) => { const dest1 = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) @@ -114,6 +122,7 @@ test('pino.transport with two files', async ({ same }) => { module: join(__dirname, 'fixtures', 'to-file-transport.js'), opts: { dest: dest2 } }]) + teardown(transport.end.bind(transport)) const instance = pino(transport) instance.info('hello') await Promise.all([watchFileCreated(dest1), watchFileCreated(dest2)]) @@ -135,7 +144,7 @@ test('pino.transport with two files', async ({ same }) => { }) }) -test('pino.transport with two files', async ({ same }) => { +test('pino.transport with two files', async ({ same, teardown }) => { const dest1 = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) @@ -151,6 +160,7 @@ test('pino.transport with two files', async ({ same }) => { level: 'info', destination: dest2 }]) + teardown(transport.end.bind(transport)) const instance = pino(transport) instance.info('hello') await Promise.all([watchFileCreated(dest1), watchFileCreated(dest2)]) @@ -172,7 +182,7 @@ test('pino.transport with two files', async ({ same }) => { }) }) -test('pino.transport with an array including a prettyPrint destination', async ({ same, match }) => { +test('pino.transport with an array including a prettyPrint destination', async ({ same, match, teardown }) => { const dest1 = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) @@ -189,6 +199,7 @@ test('pino.transport with an array including a prettyPrint destination', async ( prettyPrint: true, destination: dest2 }]) + teardown(transport.end.bind(transport)) const instance = pino(transport) instance.info('hello') await Promise.all([watchFileCreated(dest1), watchFileCreated(dest2)]) @@ -203,3 +214,46 @@ test('pino.transport with an array including a prettyPrint destination', async ( const actual = (await readFile(dest2)).toString() match(strip(actual), /\[.*\] INFO.*hello/) }) + +test('no transport.end()', async ({ same, teardown }) => { + const dest = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js'), { dest }) + const instance = pino(transport) + instance.info('hello') + await watchFileCreated(dest) + const result = JSON.parse(await readFile(dest)) + delete result.time + same(result, { + pid, + hostname, + level: 30, + msg: 'hello' + }) +}) + +test('autoEnd = false', async ({ equal, same, teardown }) => { + const dest = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const count = process.listenerCount('exit') + const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js'), { dest }, { autoEnd: false }) + teardown(transport.end.bind(transport)) + const instance = pino(transport) + instance.info('hello') + await watchFileCreated(dest) + + equal(count, process.listenerCount('exit')) + + const result = JSON.parse(await readFile(dest)) + delete result.time + same(result, { + pid, + hostname, + level: 30, + msg: 'hello' + }) +}) From 72a9ee0d2deb3b28cb1b0d0d67c27738bfb84211 Mon Sep 17 00:00:00 2001 From: David Mark Clements Date: Mon, 31 May 2021 18:13:26 +0200 Subject: [PATCH 26/63] rm transport local dep from package.json --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 093132ba5..6053190a2 100644 --- a/package.json +++ b/package.json @@ -87,7 +87,6 @@ "tap": "^15.0.1", "tape": "^5.0.0", "through2": "^4.0.0", - "transport": "./test/fixtures/transport", "winston": "^3.3.3" }, "dependencies": { From b46171db5e7f0d93f92471b351cbd944e670bf5b Mon Sep 17 00:00:00 2001 From: David Mark Clements Date: Mon, 31 May 2021 18:15:45 +0200 Subject: [PATCH 27/63] bump sonic boom --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6053190a2..f2233a08c 100644 --- a/package.json +++ b/package.json @@ -96,7 +96,7 @@ "pino-abstract-transport": "^0.2.0", "pino-std-serializers": "^3.1.0", "quick-format-unescaped": "^4.0.3", - "sonic-boom": "^1.0.2", + "sonic-boom": "^2.0.1", "thread-stream": "^0.9.0" } } From 153969cb806a33a59db1a4beeb2a728c98840fa9 Mon Sep 17 00:00:00 2001 From: David Mark Clements Date: Mon, 31 May 2021 18:17:37 +0200 Subject: [PATCH 28/63] bump pino-std-serializers --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f2233a08c..9865116eb 100644 --- a/package.json +++ b/package.json @@ -94,7 +94,7 @@ "fast-safe-stringify": "^2.0.7", "get-caller-file": "^2.0.5", "pino-abstract-transport": "^0.2.0", - "pino-std-serializers": "^3.1.0", + "pino-std-serializers": "^4.0.0", "quick-format-unescaped": "^4.0.3", "sonic-boom": "^2.0.1", "thread-stream": "^0.9.0" From bd280249bfad3569ae888ef51397358d0c770ad1 Mon Sep 17 00:00:00 2001 From: David Mark Clements Date: Mon, 31 May 2021 18:38:50 +0200 Subject: [PATCH 29/63] transport package test working without local transport dep in package.json --- test/transport.test.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/transport.test.js b/test/transport.test.js index c777082f2..4bf58eb86 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -2,7 +2,7 @@ const os = require('os') const { join } = require('path') -const { readFile } = require('fs').promises +const { readFile, symlink, unlink } = require('fs').promises const { test } = require('tap') const { watchFileCreated } = require('./helper') const pino = require('../') @@ -37,8 +37,17 @@ test('pino.transport with package', async ({ same, teardown }) => { os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) + + await symlink( + join(__dirname, 'fixtures', 'transport'), + join(__dirname, '..', 'node_modules', 'transport') + ) + const transport = pino.transport('transport', { dest }) - teardown(transport.end.bind(transport)) + teardown(async () => { + await unlink(join(__dirname, '..', 'node_modules', 'transport')) + transport.end() + }) const instance = pino(transport) instance.info('hello') await watchFileCreated(dest) From 6ee9e90248dceac11deb201275c07e7a9a9e8645 Mon Sep 17 00:00:00 2001 From: David Mark Clements Date: Mon, 31 May 2021 19:42:32 +0200 Subject: [PATCH 30/63] typo --- lib/transport.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/transport.js b/lib/transport.js index 45e33a05a..f739c98f7 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -41,7 +41,7 @@ function transport (filename, workerData, workerOpts = {}) { } } -function multitrasport (...args) { +function multitransport (...args) { if (Array.isArray(args[0])) { return transport(join(__dirname, 'worker.js'), args[0], args[1]) } @@ -49,4 +49,4 @@ function multitrasport (...args) { return transport(...args) } -module.exports = multitrasport +module.exports = multitransport From 21abdac531a83ab64bf1d86d28f1df8a2f34f041 Mon Sep 17 00:00:00 2001 From: David Mark Clements Date: Mon, 31 May 2021 20:26:01 +0200 Subject: [PATCH 31/63] api mods, extra test --- lib/transport.js | 8 +++- test/fixtures/to-file-transport.js | 7 +++- test/fixtures/to-file-transport.mjs | 2 +- test/fixtures/transport/index.js | 2 +- test/transport.test.js | 62 ++++++++++++++++------------- 5 files changed, 49 insertions(+), 32 deletions(-) diff --git a/lib/transport.js b/lib/transport.js index f739c98f7..953d20e82 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -6,13 +6,17 @@ const { isAbsolute, join } = require('path') const ThreadStream = require('thread-stream') -function transport (filename, workerData, workerOpts = {}) { +function transport (filename, opts = {}) { if (!(isAbsolute(filename) || filename.indexOf('file://') === 0)) { const callerFile = caller() const callerRequire = createRequire(callerFile) filename = callerRequire.resolve(filename) } + const { workerData = {}, destination, ...workerOpts } = opts + + if (destination) workerData.destination = destination + const stream = new ThreadStream({ filename, workerData, @@ -43,7 +47,7 @@ function transport (filename, workerData, workerOpts = {}) { function multitransport (...args) { if (Array.isArray(args[0])) { - return transport(join(__dirname, 'worker.js'), args[0], args[1]) + return transport(join(__dirname, 'worker.js'), { workerData: args[0], ...(args[1] || {}) }) } return transport(...args) diff --git a/test/fixtures/to-file-transport.js b/test/fixtures/to-file-transport.js index 9ceb636ef..1c42af53e 100644 --- a/test/fixtures/to-file-transport.js +++ b/test/fixtures/to-file-transport.js @@ -4,7 +4,12 @@ const fs = require('fs') const { once } = require('events') async function run (opts) { - const stream = fs.createWriteStream(opts.dest) + // throw an empty error for now so as not to pollute test output + // when thread stream propagates errors to main thread, make this an error + // message that can be matched against + // eslint-disable-next-line + if (!opts.destination) throw '' + const stream = fs.createWriteStream(opts.destination) await once(stream, 'open') return stream } diff --git a/test/fixtures/to-file-transport.mjs b/test/fixtures/to-file-transport.mjs index 2fcfe2d14..e0cbeec59 100644 --- a/test/fixtures/to-file-transport.mjs +++ b/test/fixtures/to-file-transport.mjs @@ -2,7 +2,7 @@ import { createWriteStream } from 'fs' import { once } from 'events' export default async function run (opts) { - const stream = createWriteStream(opts.dest) + const stream = createWriteStream(opts.destination) await once(stream, 'open') return stream } diff --git a/test/fixtures/transport/index.js b/test/fixtures/transport/index.js index 9ceb636ef..c576a7b15 100644 --- a/test/fixtures/transport/index.js +++ b/test/fixtures/transport/index.js @@ -4,7 +4,7 @@ const fs = require('fs') const { once } = require('events') async function run (opts) { - const stream = fs.createWriteStream(opts.dest) + const stream = fs.createWriteStream(opts.destination) await once(stream, 'open') return stream } diff --git a/test/transport.test.js b/test/transport.test.js index 4bf58eb86..fcf722fb0 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -2,6 +2,7 @@ const os = require('os') const { join } = require('path') +const { once } = require('events') const { readFile, symlink, unlink } = require('fs').promises const { test } = require('tap') const { watchFileCreated } = require('./helper') @@ -13,16 +14,16 @@ const { pid } = process const hostname = os.hostname() test('pino.transport with file', async ({ same, teardown }) => { - const dest = join( + const destination = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) - const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js'), { dest }) + const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js'), { destination }) teardown(transport.end.bind(transport)) const instance = pino(transport) instance.info('hello') - await watchFileCreated(dest) - const result = JSON.parse(await readFile(dest)) + await watchFileCreated(destination) + const result = JSON.parse(await readFile(destination)) delete result.time same(result, { pid, @@ -32,8 +33,15 @@ test('pino.transport with file', async ({ same, teardown }) => { }) }) +test('pino.transport with file (no options + error handling)', async ({ equal }) => { + const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js')) + // TODO: when thread stream passess error handling to main, mop up the console.error + const [err] = await once(transport, 'error') + equal(err.message, 'The worker thread exited') +}) + test('pino.transport with package', async ({ same, teardown }) => { - const dest = join( + const destination = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) @@ -43,15 +51,15 @@ test('pino.transport with package', async ({ same, teardown }) => { join(__dirname, '..', 'node_modules', 'transport') ) - const transport = pino.transport('transport', { dest }) + const transport = pino.transport('transport', { destination }) teardown(async () => { await unlink(join(__dirname, '..', 'node_modules', 'transport')) transport.end() }) const instance = pino(transport) instance.info('hello') - await watchFileCreated(dest) - const result = JSON.parse(await readFile(dest)) + await watchFileCreated(destination) + const result = JSON.parse(await readFile(destination)) delete result.time same(result, { pid, @@ -62,16 +70,16 @@ test('pino.transport with package', async ({ same, teardown }) => { }) test('pino.transport with file URL', async ({ same, teardown }) => { - const dest = join( + const destination = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) - const transport = pino.transport(url.pathToFileURL(join(__dirname, 'fixtures', 'to-file-transport.js')).href, { dest }) + const transport = pino.transport(url.pathToFileURL(join(__dirname, 'fixtures', 'to-file-transport.js')).href, { destination }) teardown(transport.end.bind(transport)) const instance = pino(transport) instance.info('hello') - await watchFileCreated(dest) - const result = JSON.parse(await readFile(dest)) + await watchFileCreated(destination) + const result = JSON.parse(await readFile(destination)) delete result.time same(result, { pid, @@ -83,7 +91,7 @@ test('pino.transport with file URL', async ({ same, teardown }) => { test('pino.transport errors if file does not exists', ({ plan, pass }) => { plan(1) - const instance = pino.transport(join(__dirname, 'fixtures', 'non-existent-file'), {}, { + const instance = pino.transport(join(__dirname, 'fixtures', 'non-existent-file'), { stdin: true, stdout: true, stderr: true @@ -94,16 +102,16 @@ test('pino.transport errors if file does not exists', ({ plan, pass }) => { }) test('pino.transport with esm', async ({ same, teardown }) => { - const dest = join( + const destination = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) - const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.mjs'), { dest }) + const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.mjs'), { destination }) const instance = pino(transport) teardown(transport.end.bind(transport)) instance.info('hello') - await watchFileCreated(dest) - const result = JSON.parse(await readFile(dest)) + await watchFileCreated(destination) + const result = JSON.parse(await readFile(destination)) delete result.time same(result, { pid, @@ -125,11 +133,11 @@ test('pino.transport with two files', async ({ same, teardown }) => { const transport = pino.transport([{ level: 'info', module: join(__dirname, 'fixtures', 'to-file-transport.js'), - opts: { dest: dest1 } + opts: { destination: dest1 } }, { level: 'info', module: join(__dirname, 'fixtures', 'to-file-transport.js'), - opts: { dest: dest2 } + opts: { destination: dest2 } }]) teardown(transport.end.bind(transport)) const instance = pino(transport) @@ -225,15 +233,15 @@ test('pino.transport with an array including a prettyPrint destination', async ( }) test('no transport.end()', async ({ same, teardown }) => { - const dest = join( + const destination = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) - const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js'), { dest }) + const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js'), { destination }) const instance = pino(transport) instance.info('hello') - await watchFileCreated(dest) - const result = JSON.parse(await readFile(dest)) + await watchFileCreated(destination) + const result = JSON.parse(await readFile(destination)) delete result.time same(result, { pid, @@ -244,20 +252,20 @@ test('no transport.end()', async ({ same, teardown }) => { }) test('autoEnd = false', async ({ equal, same, teardown }) => { - const dest = join( + const destination = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) const count = process.listenerCount('exit') - const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js'), { dest }, { autoEnd: false }) + const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js'), { destination, autoEnd: false }) teardown(transport.end.bind(transport)) const instance = pino(transport) instance.info('hello') - await watchFileCreated(dest) + await watchFileCreated(destination) equal(count, process.listenerCount('exit')) - const result = JSON.parse(await readFile(dest)) + const result = JSON.parse(await readFile(destination)) delete result.time same(result, { pid, From 82578e9335d494cddbc8b95ae648cf8d79b3f744 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 3 Jun 2021 00:38:44 +0200 Subject: [PATCH 32/63] Updated thread-stream --- package.json | 2 +- test/fixtures/to-file-transport.js | 6 +----- test/transport.test.js | 2 +- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 9865116eb..09a973ac3 100644 --- a/package.json +++ b/package.json @@ -97,6 +97,6 @@ "pino-std-serializers": "^4.0.0", "quick-format-unescaped": "^4.0.3", "sonic-boom": "^2.0.1", - "thread-stream": "^0.9.0" + "thread-stream": "^0.10.0" } } diff --git a/test/fixtures/to-file-transport.js b/test/fixtures/to-file-transport.js index 1c42af53e..33de462f9 100644 --- a/test/fixtures/to-file-transport.js +++ b/test/fixtures/to-file-transport.js @@ -4,11 +4,7 @@ const fs = require('fs') const { once } = require('events') async function run (opts) { - // throw an empty error for now so as not to pollute test output - // when thread stream propagates errors to main thread, make this an error - // message that can be matched against - // eslint-disable-next-line - if (!opts.destination) throw '' + if (!opts.destination) throw new Error('kaboom') const stream = fs.createWriteStream(opts.destination) await once(stream, 'open') return stream diff --git a/test/transport.test.js b/test/transport.test.js index fcf722fb0..9971d3dce 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -37,7 +37,7 @@ test('pino.transport with file (no options + error handling)', async ({ equal }) const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js')) // TODO: when thread stream passess error handling to main, mop up the console.error const [err] = await once(transport, 'error') - equal(err.message, 'The worker thread exited') + equal(err.message, 'kaboom') }) test('pino.transport with package', async ({ same, teardown }) => { From 8e679ee276b9642234917b1e7eb61d0c7160f858 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 3 Jun 2021 00:50:21 +0200 Subject: [PATCH 33/63] fixed node v12 support --- lib/worker.js | 2 ++ test/transport.test.js | 38 -------------------------------------- 2 files changed, 2 insertions(+), 38 deletions(-) diff --git a/lib/worker.js b/lib/worker.js index 8fcdf0948..80932f20b 100644 --- a/lib/worker.js +++ b/lib/worker.js @@ -17,6 +17,7 @@ module.exports = async function (transports) { const pretty = require('pino-pretty')(t.prettyPrint) stream = new Transform({ objectMode: true, + autoDestroy: true, transform (chunk, enc, cb) { const line = pretty(chunk.toString()) cb(null, line) @@ -39,6 +40,7 @@ module.exports = async function (transports) { // TODO remove the transform stream = new Transform({ objectMode: true, + autoDestroy: true, transform (chunk, enc, cb) { cb(null, chunk.toString() + '\n') } diff --git a/test/transport.test.js b/test/transport.test.js index 9971d3dce..bbf4ab2f0 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -161,44 +161,6 @@ test('pino.transport with two files', async ({ same, teardown }) => { }) }) -test('pino.transport with two files', async ({ same, teardown }) => { - const dest1 = join( - os.tmpdir(), - '_' + Math.random().toString(36).substr(2, 9) - ) - const dest2 = join( - os.tmpdir(), - '_' + Math.random().toString(36).substr(2, 9) - ) - const transport = pino.transport([{ - level: 'info', - destination: dest1 - }, { - level: 'info', - destination: dest2 - }]) - teardown(transport.end.bind(transport)) - const instance = pino(transport) - instance.info('hello') - await Promise.all([watchFileCreated(dest1), watchFileCreated(dest2)]) - const result1 = JSON.parse(await readFile(dest1)) - delete result1.time - same(result1, { - pid, - hostname, - level: 30, - msg: 'hello' - }) - const result2 = JSON.parse(await readFile(dest2)) - delete result2.time - same(result2, { - pid, - hostname, - level: 30, - msg: 'hello' - }) -}) - test('pino.transport with an array including a prettyPrint destination', async ({ same, match, teardown }) => { const dest1 = join( os.tmpdir(), From 7e6fb6120fbda7d2a7b966cc57b871307d807491 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 3 Jun 2021 00:57:20 +0200 Subject: [PATCH 34/63] Skip failing test on windows --- test/transport.test.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/transport.test.js b/test/transport.test.js index bbf4ab2f0..a544307a1 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -5,7 +5,7 @@ const { join } = require('path') const { once } = require('events') const { readFile, symlink, unlink } = require('fs').promises const { test } = require('tap') -const { watchFileCreated } = require('./helper') +const { isWin, watchFileCreated } = require('./helper') const pino = require('../') const url = require('url') const strip = require('strip-ansi') @@ -40,7 +40,8 @@ test('pino.transport with file (no options + error handling)', async ({ equal }) equal(err.message, 'kaboom') }) -test('pino.transport with package', async ({ same, teardown }) => { +// TODO make this test pass on Windows +test('pino.transport with package', { skip: isWin }, async ({ same, teardown }) => { const destination = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) From 47f0df339c9ba7325c60148b90030e8843a8d436 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 3 Jun 2021 01:02:04 +0200 Subject: [PATCH 35/63] print coverage report --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 09a973ac3..9275b344b 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "browser-test": "airtap --local 8080 test/browser*test.js", "lint": "eslint .", "test": "npm run lint && tap --100 test/*test.js test/*/*test.js", - "test-ci": "npm run lint && tap test/*test.js test/*/*test.js --coverage-report=lcovonly", + "test-ci": "npm run lint && tap test/*test.js test/*/*test.js", "cov-ui": "tap --coverage-report=html test/*test.js test/*/*test.js", "bench": "node benchmarks/utils/runbench all", "bench-basic": "node benchmarks/utils/runbench basic", From 89fd700583a85507d4b5c9c839aa421614fc4529 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 3 Jun 2021 01:06:59 +0200 Subject: [PATCH 36/63] Maybe fix v12 --- lib/worker.js | 5 ++++- package.json | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/worker.js b/lib/worker.js index 80932f20b..f6f31287f 100644 --- a/lib/worker.js +++ b/lib/worker.js @@ -5,7 +5,10 @@ const { Transform, pipeline } = require('stream') const build = require('pino-abstract-transport') const { once } = require('events') -// This file is not checked by the code coverage tool +// This file is not checked by the code coverage tool, +// as it is not reliable. + +/* istanbul ignore file */ module.exports = async function (transports) { transports = await Promise.all(transports.map(async (t) => { diff --git a/package.json b/package.json index 9275b344b..09a973ac3 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "browser-test": "airtap --local 8080 test/browser*test.js", "lint": "eslint .", "test": "npm run lint && tap --100 test/*test.js test/*/*test.js", - "test-ci": "npm run lint && tap test/*test.js test/*/*test.js", + "test-ci": "npm run lint && tap test/*test.js test/*/*test.js --coverage-report=lcovonly", "cov-ui": "tap --coverage-report=html test/*test.js test/*/*test.js", "bench": "node benchmarks/utils/runbench all", "bench-basic": "node benchmarks/utils/runbench basic", From ad4ee9680f99555e7df47f82080b6dd83dc258e7 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 3 Jun 2021 01:11:08 +0200 Subject: [PATCH 37/63] print the platform for debugging sake --- test/transport.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/transport.test.js b/test/transport.test.js index a544307a1..a56329cee 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -42,6 +42,7 @@ test('pino.transport with file (no options + error handling)', async ({ equal }) // TODO make this test pass on Windows test('pino.transport with package', { skip: isWin }, async ({ same, teardown }) => { + console.log(process.platform) const destination = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) From bec0c084d143c91a4e44666aa1fc3d218f7dfb08 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 3 Jun 2021 09:46:24 +0200 Subject: [PATCH 38/63] Pass windows test --- test/helper.js | 2 +- test/transport.test.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/test/helper.js b/test/helper.js index 3803a35f5..ed8073453 100644 --- a/test/helper.js +++ b/test/helper.js @@ -72,4 +72,4 @@ function watchFileCreated (filename) { }) } -module.exports = { getPathToNull, sink, check, once, sleep, watchFileCreated } +module.exports = { getPathToNull, sink, check, once, sleep, watchFileCreated, isWin } diff --git a/test/transport.test.js b/test/transport.test.js index a56329cee..a544307a1 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -42,7 +42,6 @@ test('pino.transport with file (no options + error handling)', async ({ equal }) // TODO make this test pass on Windows test('pino.transport with package', { skip: isWin }, async ({ same, teardown }) => { - console.log(process.platform) const destination = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) From 35596e6425b90a685a035a56971e135074158f3a Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 3 Jun 2021 09:51:14 +0200 Subject: [PATCH 39/63] print coverage data --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 09a973ac3..9275b344b 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "browser-test": "airtap --local 8080 test/browser*test.js", "lint": "eslint .", "test": "npm run lint && tap --100 test/*test.js test/*/*test.js", - "test-ci": "npm run lint && tap test/*test.js test/*/*test.js --coverage-report=lcovonly", + "test-ci": "npm run lint && tap test/*test.js test/*/*test.js", "cov-ui": "tap --coverage-report=html test/*test.js test/*/*test.js", "bench": "node benchmarks/utils/runbench all", "bench-basic": "node benchmarks/utils/runbench basic", From ad07d04dbb0c41b85c05f39fb19a37c932e190f5 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 3 Jun 2021 15:19:49 +0200 Subject: [PATCH 40/63] Maybe Windows --- lib/transport.js | 2 ++ package.json | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/transport.js b/lib/transport.js index 953d20e82..a8d4ed90d 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -7,6 +7,8 @@ const { isAbsolute, join } = require('path') const ThreadStream = require('thread-stream') function transport (filename, opts = {}) { + // TODO we cannot test this on Windows.. might not work. + /* istanbul ignore if */ if (!(isAbsolute(filename) || filename.indexOf('file://') === 0)) { const callerFile = caller() const callerRequire = createRequire(callerFile) diff --git a/package.json b/package.json index 9275b344b..09a973ac3 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "browser-test": "airtap --local 8080 test/browser*test.js", "lint": "eslint .", "test": "npm run lint && tap --100 test/*test.js test/*/*test.js", - "test-ci": "npm run lint && tap test/*test.js test/*/*test.js", + "test-ci": "npm run lint && tap test/*test.js test/*/*test.js --coverage-report=lcovonly", "cov-ui": "tap --coverage-report=html test/*test.js test/*/*test.js", "bench": "node benchmarks/utils/runbench all", "bench-basic": "node benchmarks/utils/runbench basic", From 9f1c699b8746e9d23bb81cef0079918fe3b3532d Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 3 Jun 2021 15:28:35 +0200 Subject: [PATCH 41/63] caller refactor --- lib/transport.js | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/transport.js b/lib/transport.js index a8d4ed90d..3296e573b 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -7,14 +7,6 @@ const { isAbsolute, join } = require('path') const ThreadStream = require('thread-stream') function transport (filename, opts = {}) { - // TODO we cannot test this on Windows.. might not work. - /* istanbul ignore if */ - if (!(isAbsolute(filename) || filename.indexOf('file://') === 0)) { - const callerFile = caller() - const callerRequire = createRequire(callerFile) - filename = callerRequire.resolve(filename) - } - const { workerData = {}, destination, ...workerOpts } = opts if (destination) workerData.destination = destination @@ -52,7 +44,18 @@ function multitransport (...args) { return transport(join(__dirname, 'worker.js'), { workerData: args[0], ...(args[1] || {}) }) } - return transport(...args) + let filename = args.shift() + + // TODO we cannot test this on Windows.. might not work. + /* istanbul ignore if */ + if (!(isAbsolute(filename) || filename.indexOf('file://') === 0)) { + // This function call MUST stay in the top-level function of this module + const callerFile = caller() + const callerRequire = createRequire(callerFile) + filename = callerRequire.resolve(filename) + } + + return transport(filename, ...args) } module.exports = multitransport From 97e12a3a49c34443f8114df6c6efe99a0f656bf9 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 3 Jun 2021 22:48:15 +0200 Subject: [PATCH 42/63] Updated API --- lib/transport.js | 32 ++++++++------- lib/worker.js | 12 +++--- test/transport.test.js | 88 ++++++++++++++++++++++++++++-------------- 3 files changed, 83 insertions(+), 49 deletions(-) diff --git a/lib/transport.js b/lib/transport.js index 3296e573b..5aeb7c44f 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -2,15 +2,11 @@ const { createRequire } = require('module') const caller = require('get-caller-file') -const { isAbsolute, join } = require('path') +const { join } = require('path') const ThreadStream = require('thread-stream') -function transport (filename, opts = {}) { - const { workerData = {}, destination, ...workerOpts } = opts - - if (destination) workerData.destination = destination - +function buildStream (filename, workerData, workerOpts) { const stream = new ThreadStream({ filename, workerData, @@ -39,23 +35,29 @@ function transport (filename, opts = {}) { } } -function multitransport (...args) { - if (Array.isArray(args[0])) { - return transport(join(__dirname, 'worker.js'), { workerData: args[0], ...(args[1] || {}) }) - } +function transport (options) { + const { destinations, opts = {}, module, workerOpts = {} } = options + let src = options.src - let filename = args.shift() + // TODO validate only one of the options can be set + + if (destinations) { + src = join(__dirname, 'worker.js') + opts.destinations = destinations + } // TODO we cannot test this on Windows.. might not work. /* istanbul ignore if */ - if (!(isAbsolute(filename) || filename.indexOf('file://') === 0)) { + if (module) { // This function call MUST stay in the top-level function of this module const callerFile = caller() const callerRequire = createRequire(callerFile) - filename = callerRequire.resolve(filename) + src = callerRequire.resolve(module) } - return transport(filename, ...args) + // console.log(options, src) + + return buildStream(src, opts, workerOpts) } -module.exports = multitransport +module.exports = transport diff --git a/lib/worker.js b/lib/worker.js index f6f31287f..53bdee765 100644 --- a/lib/worker.js +++ b/lib/worker.js @@ -10,11 +10,11 @@ const { once } = require('events') /* istanbul ignore file */ -module.exports = async function (transports) { - transports = await Promise.all(transports.map(async (t) => { +module.exports = async function ({ destinations }) { + destinations = await Promise.all(destinations.map(async (t) => { let stream - if (t.module) { - const toLoad = 'file://' + t.module + if (t.src) { + const toLoad = 'file://' + t.src stream = await (await import(toLoad)).default(t.opts) } else if (t.prettyPrint) { const pretty = require('pino-pretty')(t.prettyPrint) @@ -64,7 +64,7 @@ module.exports = async function (transports) { metadata: true, close (err, cb) { let expected = 0 - for (const transport of transports) { + for (const transport of destinations) { expected++ transport.stream.on('close', closeCb) transport.stream.end() @@ -79,7 +79,7 @@ module.exports = async function (transports) { }) function process (stream) { - const multi = pino.multistream(transports) + const multi = pino.multistream(destinations) // TODO manage backpressure stream.on('data', function (chunk) { const { lastTime, lastMsg, lastObj, lastLevel } = this diff --git a/test/transport.test.js b/test/transport.test.js index a544307a1..994e7c958 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -18,7 +18,10 @@ test('pino.transport with file', async ({ same, teardown }) => { os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) - const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js'), { destination }) + const transport = pino.transport({ + src: join(__dirname, 'fixtures', 'to-file-transport.js'), + opts: { destination } + }) teardown(transport.end.bind(transport)) const instance = pino(transport) instance.info('hello') @@ -34,7 +37,9 @@ test('pino.transport with file', async ({ same, teardown }) => { }) test('pino.transport with file (no options + error handling)', async ({ equal }) => { - const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js')) + const transport = pino.transport({ + src: join(__dirname, 'fixtures', 'to-file-transport.js') + }) // TODO: when thread stream passess error handling to main, mop up the console.error const [err] = await once(transport, 'error') equal(err.message, 'kaboom') @@ -47,12 +52,19 @@ test('pino.transport with package', { skip: isWin }, async ({ same, teardown }) '_' + Math.random().toString(36).substr(2, 9) ) + try { + await unlink(join(__dirname, '..', 'node_modules', 'transport')) + } catch {} + await symlink( join(__dirname, 'fixtures', 'transport'), join(__dirname, '..', 'node_modules', 'transport') ) - const transport = pino.transport('transport', { destination }) + const transport = pino.transport({ + module: 'transport', + opts: { destination } + }) teardown(async () => { await unlink(join(__dirname, '..', 'node_modules', 'transport')) transport.end() @@ -75,7 +87,10 @@ test('pino.transport with file URL', async ({ same, teardown }) => { os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) - const transport = pino.transport(url.pathToFileURL(join(__dirname, 'fixtures', 'to-file-transport.js')).href, { destination }) + const transport = pino.transport({ + src: url.pathToFileURL(join(__dirname, 'fixtures', 'to-file-transport.js')).href, + opts: { destination } + }) teardown(transport.end.bind(transport)) const instance = pino(transport) instance.info('hello') @@ -92,10 +107,13 @@ test('pino.transport with file URL', async ({ same, teardown }) => { test('pino.transport errors if file does not exists', ({ plan, pass }) => { plan(1) - const instance = pino.transport(join(__dirname, 'fixtures', 'non-existent-file'), { - stdin: true, - stdout: true, - stderr: true + const instance = pino.transport({ + src: join(__dirname, 'fixtures', 'non-existent-file'), + workerOpts: { + stdin: true, + stdout: true, + stderr: true + } }) instance.on('error', function () { pass('error received') @@ -107,7 +125,10 @@ test('pino.transport with esm', async ({ same, teardown }) => { os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) - const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.mjs'), { destination }) + const transport = pino.transport({ + src: join(__dirname, 'fixtures', 'to-file-transport.mjs'), + opts: { destination } + }) const instance = pino(transport) teardown(transport.end.bind(transport)) instance.info('hello') @@ -131,15 +152,17 @@ test('pino.transport with two files', async ({ same, teardown }) => { os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) - const transport = pino.transport([{ - level: 'info', - module: join(__dirname, 'fixtures', 'to-file-transport.js'), - opts: { destination: dest1 } - }, { - level: 'info', - module: join(__dirname, 'fixtures', 'to-file-transport.js'), - opts: { destination: dest2 } - }]) + const transport = pino.transport({ + destinations: [{ + level: 'info', + src: join(__dirname, 'fixtures', 'to-file-transport.js'), + opts: { destination: dest1 } + }, { + level: 'info', + src: join(__dirname, 'fixtures', 'to-file-transport.js'), + opts: { destination: dest2 } + }] + }) teardown(transport.end.bind(transport)) const instance = pino(transport) instance.info('hello') @@ -171,14 +194,16 @@ test('pino.transport with an array including a prettyPrint destination', async ( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) - const transport = pino.transport([{ - level: 'info', - destination: dest1 - }, { - level: 'info', - prettyPrint: true, - destination: dest2 - }]) + const transport = pino.transport({ + destinations: [{ + level: 'info', + destination: dest1 + }, { + level: 'info', + prettyPrint: true, + destination: dest2 + }] + }) teardown(transport.end.bind(transport)) const instance = pino(transport) instance.info('hello') @@ -200,7 +225,10 @@ test('no transport.end()', async ({ same, teardown }) => { os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) ) - const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js'), { destination }) + const transport = pino.transport({ + src: join(__dirname, 'fixtures', 'to-file-transport.js'), + opts: { destination } + }) const instance = pino(transport) instance.info('hello') await watchFileCreated(destination) @@ -220,7 +248,11 @@ test('autoEnd = false', async ({ equal, same, teardown }) => { '_' + Math.random().toString(36).substr(2, 9) ) const count = process.listenerCount('exit') - const transport = pino.transport(join(__dirname, 'fixtures', 'to-file-transport.js'), { destination, autoEnd: false }) + const transport = pino.transport({ + src: join(__dirname, 'fixtures', 'to-file-transport.js'), + opts: { destination }, + workerOpts: { autoEnd: false } + }) teardown(transport.end.bind(transport)) const instance = pino(transport) instance.info('hello') From f0f06fc1114a921fb7b7c3402a8d6389bd55052a Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 3 Jun 2021 23:08:20 +0200 Subject: [PATCH 43/63] option validation --- lib/transport.js | 7 +++++++ test/transport.test.js | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/lib/transport.js b/lib/transport.js index 5aeb7c44f..b1875bcf7 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -39,6 +39,13 @@ function transport (options) { const { destinations, opts = {}, module, workerOpts = {} } = options let src = options.src + if ( + (src && destinations) || + (src && module) || + (module && destinations)) { + throw new Error('Only one of src, destinations or module can be specified') + } + // TODO validate only one of the options can be set if (destinations) { diff --git a/test/transport.test.js b/test/transport.test.js index 994e7c958..5e45b8b60 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -269,3 +269,43 @@ test('autoEnd = false', async ({ equal, same, teardown }) => { msg: 'hello' }) }) + +test('pino.transport with src and destinations', async ({ fail, equal }) => { + try { + pino.transport({ + src: 'a/file', + destinations: [{ + src: 'a/file' + }] + }) + fail('must throw') + } catch (err) { + equal(err.message, 'Only one of src, destinations or module can be specified') + } +}) + +test('pino.transport with src and module', async ({ fail, equal }) => { + try { + pino.transport({ + src: 'a/file', + module: 'transport' + }) + fail('must throw') + } catch (err) { + equal(err.message, 'Only one of src, destinations or module can be specified') + } +}) + +test('pino.transport with destinations and module', async ({ fail, equal }) => { + try { + pino.transport({ + destinations: [{ + src: 'a/file' + }], + module: 'transport' + }) + fail('must throw') + } catch (err) { + equal(err.message, 'Only one of src, destinations or module can be specified') + } +}) From 02a4b6841608ba65095558484ee4a063c34da3b4 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 4 Jun 2021 09:43:25 +0200 Subject: [PATCH 44/63] Support modules within destinations --- lib/transport.js | 20 ++++++++++++++++---- test/transport.test.js | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/lib/transport.js b/lib/transport.js index b1875bcf7..f9b63ba98 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -37,6 +37,10 @@ function buildStream (filename, workerData, workerOpts) { function transport (options) { const { destinations, opts = {}, module, workerOpts = {} } = options + // This function call MUST stay in the top-level function of this module + const callerFile = caller() + const callerRequire = createRequire(callerFile) + let src = options.src if ( @@ -50,15 +54,23 @@ function transport (options) { if (destinations) { src = join(__dirname, 'worker.js') - opts.destinations = destinations + opts.destinations = destinations.map((dest) => { + // TODO validate dest + if (!dest.module) { + return dest + } + + return { + ...dest, + module: undefined, + src: callerRequire.resolve(dest.module) + } + }) } // TODO we cannot test this on Windows.. might not work. /* istanbul ignore if */ if (module) { - // This function call MUST stay in the top-level function of this module - const callerFile = caller() - const callerRequire = createRequire(callerFile) src = callerRequire.resolve(module) } diff --git a/test/transport.test.js b/test/transport.test.js index 5e45b8b60..22c594f92 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -309,3 +309,42 @@ test('pino.transport with destinations and module', async ({ fail, equal }) => { equal(err.message, 'Only one of src, destinations or module can be specified') } }) + +// TODO make this test pass on Windows +test('pino.transport with package as a destination', { skip: isWin }, async ({ same, teardown }) => { + const destination = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + + try { + await unlink(join(__dirname, '..', 'node_modules', 'transport')) + } catch {} + + await symlink( + join(__dirname, 'fixtures', 'transport'), + join(__dirname, '..', 'node_modules', 'transport') + ) + + const transport = pino.transport({ + destinations: [{ + module: 'transport', + opts: { destination } + }] + }) + teardown(async () => { + await unlink(join(__dirname, '..', 'node_modules', 'transport')) + transport.end() + }) + const instance = pino(transport) + instance.info('hello') + await watchFileCreated(destination) + const result = JSON.parse(await readFile(destination)) + delete result.time + same(result, { + pid, + hostname, + level: 30, + msg: 'hello' + }) +}) From 58e2914e65b876b82a45e49ab62aea6cd6584f0c Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 4 Jun 2021 17:20:20 +0200 Subject: [PATCH 45/63] Use full names for the options --- lib/transport.js | 24 ++++++++--------- lib/worker.js | 6 ++--- test/transport.test.js | 58 +++++++++++++++++++++--------------------- 3 files changed, 43 insertions(+), 45 deletions(-) diff --git a/lib/transport.js b/lib/transport.js index f9b63ba98..3fa9632fb 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -35,26 +35,26 @@ function buildStream (filename, workerData, workerOpts) { } } -function transport (options) { - const { destinations, opts = {}, module, workerOpts = {} } = options +function transport (fullOptions) { + const { destinations, options = {}, module, worker = {} } = fullOptions // This function call MUST stay in the top-level function of this module const callerFile = caller() const callerRequire = createRequire(callerFile) - let src = options.src + let source = fullOptions.source if ( - (src && destinations) || - (src && module) || + (source && destinations) || + (source && module) || (module && destinations)) { - throw new Error('Only one of src, destinations or module can be specified') + throw new Error('Only one of source, destinations or module can be specified') } // TODO validate only one of the options can be set if (destinations) { - src = join(__dirname, 'worker.js') - opts.destinations = destinations.map((dest) => { + source = join(__dirname, 'worker.js') + options.destinations = destinations.map((dest) => { // TODO validate dest if (!dest.module) { return dest @@ -63,7 +63,7 @@ function transport (options) { return { ...dest, module: undefined, - src: callerRequire.resolve(dest.module) + source: callerRequire.resolve(dest.module) } }) } @@ -71,12 +71,10 @@ function transport (options) { // TODO we cannot test this on Windows.. might not work. /* istanbul ignore if */ if (module) { - src = callerRequire.resolve(module) + source = callerRequire.resolve(module) } - // console.log(options, src) - - return buildStream(src, opts, workerOpts) + return buildStream(source, options, worker) } module.exports = transport diff --git a/lib/worker.js b/lib/worker.js index 53bdee765..24f061b87 100644 --- a/lib/worker.js +++ b/lib/worker.js @@ -13,9 +13,9 @@ const { once } = require('events') module.exports = async function ({ destinations }) { destinations = await Promise.all(destinations.map(async (t) => { let stream - if (t.src) { - const toLoad = 'file://' + t.src - stream = await (await import(toLoad)).default(t.opts) + if (t.source) { + const toLoad = 'file://' + t.source + stream = await (await import(toLoad)).default(t.options) } else if (t.prettyPrint) { const pretty = require('pino-pretty')(t.prettyPrint) stream = new Transform({ diff --git a/test/transport.test.js b/test/transport.test.js index 22c594f92..ded32553e 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -19,8 +19,8 @@ test('pino.transport with file', async ({ same, teardown }) => { '_' + Math.random().toString(36).substr(2, 9) ) const transport = pino.transport({ - src: join(__dirname, 'fixtures', 'to-file-transport.js'), - opts: { destination } + source: join(__dirname, 'fixtures', 'to-file-transport.js'), + options: { destination } }) teardown(transport.end.bind(transport)) const instance = pino(transport) @@ -38,7 +38,7 @@ test('pino.transport with file', async ({ same, teardown }) => { test('pino.transport with file (no options + error handling)', async ({ equal }) => { const transport = pino.transport({ - src: join(__dirname, 'fixtures', 'to-file-transport.js') + source: join(__dirname, 'fixtures', 'to-file-transport.js') }) // TODO: when thread stream passess error handling to main, mop up the console.error const [err] = await once(transport, 'error') @@ -63,7 +63,7 @@ test('pino.transport with package', { skip: isWin }, async ({ same, teardown }) const transport = pino.transport({ module: 'transport', - opts: { destination } + options: { destination } }) teardown(async () => { await unlink(join(__dirname, '..', 'node_modules', 'transport')) @@ -88,8 +88,8 @@ test('pino.transport with file URL', async ({ same, teardown }) => { '_' + Math.random().toString(36).substr(2, 9) ) const transport = pino.transport({ - src: url.pathToFileURL(join(__dirname, 'fixtures', 'to-file-transport.js')).href, - opts: { destination } + source: url.pathToFileURL(join(__dirname, 'fixtures', 'to-file-transport.js')).href, + options: { destination } }) teardown(transport.end.bind(transport)) const instance = pino(transport) @@ -108,8 +108,8 @@ test('pino.transport with file URL', async ({ same, teardown }) => { test('pino.transport errors if file does not exists', ({ plan, pass }) => { plan(1) const instance = pino.transport({ - src: join(__dirname, 'fixtures', 'non-existent-file'), - workerOpts: { + source: join(__dirname, 'fixtures', 'non-existent-file'), + worker: { stdin: true, stdout: true, stderr: true @@ -126,8 +126,8 @@ test('pino.transport with esm', async ({ same, teardown }) => { '_' + Math.random().toString(36).substr(2, 9) ) const transport = pino.transport({ - src: join(__dirname, 'fixtures', 'to-file-transport.mjs'), - opts: { destination } + source: join(__dirname, 'fixtures', 'to-file-transport.mjs'), + options: { destination } }) const instance = pino(transport) teardown(transport.end.bind(transport)) @@ -155,12 +155,12 @@ test('pino.transport with two files', async ({ same, teardown }) => { const transport = pino.transport({ destinations: [{ level: 'info', - src: join(__dirname, 'fixtures', 'to-file-transport.js'), - opts: { destination: dest1 } + source: join(__dirname, 'fixtures', 'to-file-transport.js'), + options: { destination: dest1 } }, { level: 'info', - src: join(__dirname, 'fixtures', 'to-file-transport.js'), - opts: { destination: dest2 } + source: join(__dirname, 'fixtures', 'to-file-transport.js'), + options: { destination: dest2 } }] }) teardown(transport.end.bind(transport)) @@ -226,8 +226,8 @@ test('no transport.end()', async ({ same, teardown }) => { '_' + Math.random().toString(36).substr(2, 9) ) const transport = pino.transport({ - src: join(__dirname, 'fixtures', 'to-file-transport.js'), - opts: { destination } + source: join(__dirname, 'fixtures', 'to-file-transport.js'), + options: { destination } }) const instance = pino(transport) instance.info('hello') @@ -249,9 +249,9 @@ test('autoEnd = false', async ({ equal, same, teardown }) => { ) const count = process.listenerCount('exit') const transport = pino.transport({ - src: join(__dirname, 'fixtures', 'to-file-transport.js'), - opts: { destination }, - workerOpts: { autoEnd: false } + source: join(__dirname, 'fixtures', 'to-file-transport.js'), + options: { destination }, + worker: { autoEnd: false } }) teardown(transport.end.bind(transport)) const instance = pino(transport) @@ -270,29 +270,29 @@ test('autoEnd = false', async ({ equal, same, teardown }) => { }) }) -test('pino.transport with src and destinations', async ({ fail, equal }) => { +test('pino.transport with source and destinations', async ({ fail, equal }) => { try { pino.transport({ - src: 'a/file', + source: 'a/file', destinations: [{ - src: 'a/file' + source: 'a/file' }] }) fail('must throw') } catch (err) { - equal(err.message, 'Only one of src, destinations or module can be specified') + equal(err.message, 'Only one of source, destinations or module can be specified') } }) -test('pino.transport with src and module', async ({ fail, equal }) => { +test('pino.transport with source and module', async ({ fail, equal }) => { try { pino.transport({ - src: 'a/file', + source: 'a/file', module: 'transport' }) fail('must throw') } catch (err) { - equal(err.message, 'Only one of src, destinations or module can be specified') + equal(err.message, 'Only one of source, destinations or module can be specified') } }) @@ -300,13 +300,13 @@ test('pino.transport with destinations and module', async ({ fail, equal }) => { try { pino.transport({ destinations: [{ - src: 'a/file' + source: 'a/file' }], module: 'transport' }) fail('must throw') } catch (err) { - equal(err.message, 'Only one of src, destinations or module can be specified') + equal(err.message, 'Only one of source, destinations or module can be specified') } }) @@ -329,7 +329,7 @@ test('pino.transport with package as a destination', { skip: isWin }, async ({ s const transport = pino.transport({ destinations: [{ module: 'transport', - opts: { destination } + options: { destination } }] }) teardown(async () => { From ca5d4c544b36a2e43ec6b3909f17190884ea9259 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 5 Jun 2021 00:29:01 +0200 Subject: [PATCH 46/63] windows coverage --- lib/transport.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/transport.js b/lib/transport.js index 3fa9632fb..e97275ab7 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -56,6 +56,7 @@ function transport (fullOptions) { source = join(__dirname, 'worker.js') options.destinations = destinations.map((dest) => { // TODO validate dest + /* istanbul ignore else */ if (!dest.module) { return dest } From f87965329b28d6c85bf33df8a63396fc47f29330 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 5 Jun 2021 00:43:39 +0200 Subject: [PATCH 47/63] coverage report --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 09a973ac3..9275b344b 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "browser-test": "airtap --local 8080 test/browser*test.js", "lint": "eslint .", "test": "npm run lint && tap --100 test/*test.js test/*/*test.js", - "test-ci": "npm run lint && tap test/*test.js test/*/*test.js --coverage-report=lcovonly", + "test-ci": "npm run lint && tap test/*test.js test/*/*test.js", "cov-ui": "tap --coverage-report=html test/*test.js test/*/*test.js", "bench": "node benchmarks/utils/runbench all", "bench-basic": "node benchmarks/utils/runbench basic", From a791067dbee7ec66b6466c275eff4862950bb484 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 5 Jun 2021 00:54:46 +0200 Subject: [PATCH 48/63] fix coverage on windows --- lib/transport.js | 1 + package.json | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/transport.js b/lib/transport.js index e97275ab7..5ebf834c4 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -61,6 +61,7 @@ function transport (fullOptions) { return dest } + /* istanbul ignore next */ return { ...dest, module: undefined, diff --git a/package.json b/package.json index 9275b344b..09a973ac3 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "browser-test": "airtap --local 8080 test/browser*test.js", "lint": "eslint .", "test": "npm run lint && tap --100 test/*test.js test/*/*test.js", - "test-ci": "npm run lint && tap test/*test.js test/*/*test.js", + "test-ci": "npm run lint && tap test/*test.js test/*/*test.js --coverage-report=lcovonly", "cov-ui": "tap --coverage-report=html test/*test.js test/*/*test.js", "bench": "node benchmarks/utils/runbench all", "bench-basic": "node benchmarks/utils/runbench basic", From c924b92e1fbb12da7acd4df5520f1a700e3148bb Mon Sep 17 00:00:00 2001 From: David Mark Clements Date: Sun, 6 Jun 2021 19:45:54 +0200 Subject: [PATCH 49/63] docs first pass --- README.md | 7 +- docs/api.md | 29 ++++++++ docs/transports.md | 168 +++++++++++++++++++++++++++++++++------------ 3 files changed, 159 insertions(+), 45 deletions(-) diff --git a/README.md b/README.md index 95e085279..6afcbeea8 100644 --- a/README.md +++ b/README.md @@ -75,9 +75,10 @@ format logs during development: Due to Node's single-threaded event-loop, it's highly recommended that sending, alert triggering, reformatting and all forms of log processing -is conducted in a separate process. In Pino parlance we call all log processors -"transports", and recommend that the transports be run as separate -processes, piping the stdout of the application to the stdin of the transport. +be conducted in a separate process or thread. + +In Pino terminology we call all log processors "transports", and recommend that the +transports be run in a worker thread using our `pino.transport` API. For more details see our [Transports⇗](docs/transports.md) document. diff --git a/docs/api.md b/docs/api.md index e82492cc2..c2cb2cd93 100644 --- a/docs/api.md +++ b/docs/api.md @@ -23,6 +23,7 @@ * [logger.version](#version) * [Statics](#statics) * [pino.destination()](#pino-destination) + * [pino.transport()](#pino-transport) * [pino.final()](#pino-final) * [pino.multistream()](#pino-multistream) * [pino.stdSerializers](#pino-stdserializers) @@ -890,6 +891,34 @@ A `pino.destination` instance can also be used to reopen closed files * See [Reopening log files](/docs/help.md#reopening) * See [Asynchronous Logging ⇗](/docs/asynchronous.md) + +### `pino.transport(options) => ThreadStream` + +Create a a stream that routes logs to a worker thread that +wraps around a [Pino Transport](/docs/transports.md). + +```js +const pino = require('pino') +const transport = pino.transport({ + target: 'some-transport', + options: { some: 'options for', the: 'transport' } +}) +pino(transport) +``` + +* See [`Transports`](/docs/transports.md) +* See [`thread-stream` ⇗](https://github.com/mcollina/thread-stream) + +#### Options + +* `target`: The transport to pass logs through. This may be an installed module name, a path or a built-in transport (see next section) +* `options`: An options object which is serialized (see [Structured Clone Algorithm][https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm]), passed to the worker thread, parsed and then passed to the transport. +* `destinations`: -==TODO==- + +#### Built in transports + +-==TODO==- + ### `pino.final(logger, [handler]) => Function | FinalLogger` diff --git a/docs/transports.md b/docs/transports.md index 13ca52c77..ccf6440f8 100644 --- a/docs/transports.md +++ b/docs/transports.md @@ -1,79 +1,163 @@ # Transports -A "transport" for Pino is a supplementary tool which consumes Pino logs. +Pino transports can be used for both transmitting and transforming log output. -Consider the following example: +The way Pino generates logs: -```js -const split = require('split2') -const pump = require('pump') -const through = require('through2') +1. Reduces the impact of logging on an application to the absolute minimum. +2. Gives greater flexibility in how logs are processed and stored. + +It is recommended that any log transformation or transmission is performed either +in a seperate thread or a seperate process. + +Prior to Pino v7 transports would ideally operate in a seperate process - these are +now referred to as [Legacy Transports](#legacy-transports). + +From Pino v7 and upwards transports can also operate inside a [Worker Thread][worker-thread], +and can be used or configured via the options object passed to `pino` on initialization. -const myTransport = through.obj(function (chunk, enc, cb) { - // do the necessary - console.log(chunk) - cb() -}) -pump(process.stdin, split(JSON.parse), myTransport) +[worker-thread]: https://nodejs.org/dist/latest-v14.x/docs/api/worker_threads.html + +## v7+ Transports + +A transport is a module that exports a default function which returns a writable stream: + +```js +import { Writable } from 'stream' +export default (options) => { + const myTransportStream = new Writable({ + write (chunk, enc, cb) { + // apply a transform and send to stdout + console.log(chunk.toString().toUpperCase()) + cb() + } + }) + return myTransportStream +} ``` -The above defines our "transport" as the file `my-transport-process.js`. +Let's imagine the above defines our "transport" as the file `my-transport.mjs` +(ESM files are supported even if the project is written in CJS). -Logs can now be consumed using shell piping: +We would set up our transport by creating a transport stream with `pino.transport` +and passing it to the `pino` function: -```sh -node my-app-which-logs-stuff-to-stdout.js | node my-transport-process.js +```js +const pino = require('pino') +const transport = pino.transport({ + target: './my-transport.mjs' +}) +pino(transport) ``` -Ideally, a transport should consume logs in a separate process to the application, -Using transports in the same process causes unnecessary load and slows down -Node's single threaded event loop. +The transport code will be executed in a separate worker thread. The main thread +will write logs to the worker thread, which will write them to the stream returned +from function exported from the transport file/module. -## In-process transports +The exported function can also be async. Imagine the following transport: + +```js +import fs from 'fs' +import { once } from('events') +export default async (options) => { + const stream = fs.createWriteStream(opts.destination) + await once(stream, 'open') + return stream +} +``` -> **Pino *does not* natively support in-process transports.** +While initializing the stream we're able to use `await` to perform asynchronous operations. In this +case waiting for the write streams `open` event. -Pino does not support in-process transports because Node processes are -single threaded processes (ignoring some technical details). Given this -restriction, one of the methods Pino employs to achieve its speed is to -purposefully offload the handling of logs, and their ultimate destination, to -external processes so that the threading capabilities of the OS can be -used (or other CPUs). +Let's imagine the above was published to npm with the module name `some-file-transport`. -One consequence of this methodology is that "error" logs do not get written to -`stderr`. However, since Pino logs are in a parsable format, it is possible to -use tools like [pino-tee][pino-tee] or [jq][jq] to work with the logs. For -example, to view only logs marked as "error" logs: +The `options.destination` value can be set when the creating the transport stream with `pino.transport` like so: +```js +const pino = require('pino') +const transport = pino.transport({ + target: 'some-file-transport', + options: { destination: '/dev/null' } +}) +pino(transport) ``` -$ node an-app.js | jq 'select(.level == 50)' + +Note here we've specified a module by package rather than by relative path. The options object we provide +is serialized and injected into the transport worker thread, then passed to the modules exported function. +This means that the options object can only contain types that are supported by the +[Structured Clone Algorithm][sca] which is used to (de)serializing objects between threads. + +What if we wanted to use both transports, but send only error logs to `some-file-transport` while +sending all logs to `./my-transport.mjs`. We can use the `pino.transport` function's `destinations` option: + +```js +const pino = require('pino') +const transport = pino.transport({ + destinations: [ + { target: './my-transport.mjs', level: 'error' }, + { target: 'some-file-transport', options: { destination: '/dev/null' } + ] +}) +pino(transport) ``` -In short, the way Pino generates logs: +For more details on `pino.transport` see the [API docs for `pino.transport`][pino-transport]. -1. Reduces the impact of logging on an application to the absolute minimum. -2. Gives greater flexibility in how logs are processed and stored. -Given all of the above, Pino recommends out-of-process log processing. +[pino-transport]: /docs/api.md#pino-transport +[sca]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm + + + +## Legacy Transports + +A legacy Pino "transport" is a supplementary tool which consumes Pino logs. + +Consider the following example for creating a transport: + +```js +const { pipeline, Writable } = require('stream') +const split = require('split2') + +const myTransportStream = new Writable({ + write (chunk, enc, cb) { + // apply a transform and send to stdout + console.log(chunk.toString().toUpperCase()) + cb() + } +}) + +pipeline(process.stdin, split(JSON.parse), myTransportStream) +``` + +The above defines our "transport" as the file `my-transport-process.js`. + +Logs can now be consumed using shell piping: -However, it is possible to wrap Pino and perform processing in-process. -For an example of this, see [pino-multi-stream][pinoms]. +```sh +node my-app-which-logs-stuff-to-stdout.js | node my-transport-process.js +``` -[pino-tee]: https://npm.im/pino-tee -[jq]: https://stedolan.github.io/jq/ -[pinoms]: https://npm.im/pino-multi-stream +Ideally, a transport should consume logs in a separate process to the application, +Using transports in the same process causes unnecessary load and slows down +Node's single threaded event loop. ## Known Transports PR's to this document are welcome for any new transports! +### Pino v7+ Compatible + ++ [pino-elasticsearch](#pino-elasticsearch) + +### Legacy + + [pino-applicationinsights](#pino-applicationinsights) + [pino-azuretable](#pino-azuretable) + [pino-cloudwatch](#pino-cloudwatch) + [pino-couch](#pino-couch) + [pino-datadog](#pino-datadog) -+ [pino-elasticsearch](#pino-elasticsearch) + [pino-gelf](#pino-gelf) + [pino-http-send](#pino-http-send) + [pino-kafka](#pino-kafka) From 52d5e87aac60b6e6aa9a732594ad5996abec8494 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 6 Jun 2021 22:08:11 +0200 Subject: [PATCH 50/63] target and targets --- example.js => examples/basic.js | 2 +- .../transport.js | 33 ++--- lib/file-target.js | 22 ++++ lib/pretty-target.js | 32 +++++ lib/transport.js | 53 ++++---- lib/worker.js | 54 +------- test/transport.test.js | 117 ++++++++++-------- 7 files changed, 173 insertions(+), 140 deletions(-) rename example.js => examples/basic.js (96%) rename example-transports.js => examples/transport.js (76%) create mode 100644 lib/file-target.js create mode 100644 lib/pretty-target.js diff --git a/example.js b/examples/basic.js similarity index 96% rename from example.js rename to examples/basic.js index 866338e14..246ce7f86 100644 --- a/example.js +++ b/examples/basic.js @@ -1,6 +1,6 @@ 'use strict' -const pino = require('./')() +const pino = require('..')() pino.info('hello world') pino.error('this is at error level') diff --git a/example-transports.js b/examples/transport.js similarity index 76% rename from example-transports.js rename to examples/transport.js index 83b7ee1a4..25717e492 100644 --- a/example-transports.js +++ b/examples/transport.js @@ -1,24 +1,29 @@ 'use strict' -const pino = require('./') +const pino = require('..') const { tmpdir } = require('os') const { join } = require('path') const file = join(tmpdir(), `pino-${process.pid}-example`) -const transport = pino.transport([{ - level: 'warn', - destination: file -}, { - level: 'info', - module: '/Users/matteo/repositories/pino-elasticsearch/lib.js', - opts: { - node: 'http://localhost:9200' - } -}, { - level: 'info', - prettyPrint: true -}]) +const transport = pino.transport({ + targets: [{ + level: 'warn', + target: '#pino/file', + options: { + destination: file + } + }, { + level: 'info', + target: 'pino-elasticsearch', + options: { + node: 'http://localhost:9200' + } + }, { + level: 'info', + target: '#pino/pretty' + }] +}) const logger = pino(transport) diff --git a/lib/file-target.js b/lib/file-target.js new file mode 100644 index 000000000..36f3b253b --- /dev/null +++ b/lib/file-target.js @@ -0,0 +1,22 @@ +'use strict' + +const { Transform, pipeline } = require('stream') +const pino = require('../pino') +const { once } = require('events') + +module.exports = async function (opts) { + // TODO remove the transform + const stream = new Transform({ + objectMode: true, + autoDestroy: true, + transform (chunk, enc, cb) { + cb(null, chunk.toString()) + } + }) + + // TODO figure out why sync: false does not work here + const destination = pino.destination(opts.destination || /* istanbul ignore next */ 1) + await once(destination, 'ready') + pipeline(stream, destination, () => {}) + return stream +} diff --git a/lib/pretty-target.js b/lib/pretty-target.js new file mode 100644 index 000000000..795a27ea5 --- /dev/null +++ b/lib/pretty-target.js @@ -0,0 +1,32 @@ +'use strict' + +const pinoPretty = require('pino-pretty') +const { Transform, pipeline } = require('stream') +const pino = require('../pino') +const { once } = require('events') + +module.exports = async function (/* istanbul ignore next */ opts = {}) { + const pretty = pinoPretty(opts) + const stream = new Transform({ + objectMode: true, + autoDestroy: true, + transform (chunk, enc, cb) { + const line = pretty(chunk.toString()) + cb(null, line) + } + }) + + // TODO figure out why sync: false does not work here + // TODO remove the istanbul ignore, add a test using a + // child_process + const destination = pino.destination(opts.destination || /* istanbul ignore next */ 1) + /* istanbul ignore next */ + if (destination.fd === 1) { + destination.end = function () { + this.emit('close') + } + } + await once(destination, 'ready') + pipeline(stream, destination, () => {}) + return stream +} diff --git a/lib/transport.js b/lib/transport.js index 5ebf834c4..2ce8ed7fd 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -2,7 +2,7 @@ const { createRequire } = require('module') const caller = require('get-caller-file') -const { join } = require('path') +const { join, isAbsolute } = require('path') const ThreadStream = require('thread-stream') @@ -36,47 +36,44 @@ function buildStream (filename, workerData, workerOpts) { } function transport (fullOptions) { - const { destinations, options = {}, module, worker = {} } = fullOptions + const { targets, options = {}, worker = {} } = fullOptions // This function call MUST stay in the top-level function of this module const callerFile = caller() const callerRequire = createRequire(callerFile) - let source = fullOptions.source + let target = fullOptions.target - if ( - (source && destinations) || - (source && module) || - (module && destinations)) { - throw new Error('Only one of source, destinations or module can be specified') + if (target && targets) { + throw new Error('Only one of target or targets can be specified') } - // TODO validate only one of the options can be set - - if (destinations) { - source = join(__dirname, 'worker.js') - options.destinations = destinations.map((dest) => { - // TODO validate dest - /* istanbul ignore else */ - if (!dest.module) { - return dest - } - - /* istanbul ignore next */ + if (targets) { + target = join(__dirname, 'worker.js') + options.targets = targets.map((dest) => { return { ...dest, - module: undefined, - source: callerRequire.resolve(dest.module) + target: fixTarget(dest.target) } }) } - // TODO we cannot test this on Windows.. might not work. - /* istanbul ignore if */ - if (module) { - source = callerRequire.resolve(module) - } + return buildStream(fixTarget(target), options, worker) - return buildStream(source, options, worker) + function fixTarget (origin) { + if (isAbsolute(origin) || origin.indexOf('file://') === 0) { + return origin + } + + switch (origin) { + case '#pino/pretty': + return join(__dirname, 'pretty-target.js') + case '#pino/file': + return join(__dirname, 'file-target.js') + default: + // TODO we cannot test this on Windows.. might not work. + return callerRequire.resolve(origin) + } + } } module.exports = transport diff --git a/lib/worker.js b/lib/worker.js index 24f061b87..0b6c4989b 100644 --- a/lib/worker.js +++ b/lib/worker.js @@ -1,59 +1,17 @@ 'use strict' const pino = require('../pino.js') -const { Transform, pipeline } = require('stream') const build = require('pino-abstract-transport') -const { once } = require('events') // This file is not checked by the code coverage tool, // as it is not reliable. /* istanbul ignore file */ -module.exports = async function ({ destinations }) { - destinations = await Promise.all(destinations.map(async (t) => { - let stream - if (t.source) { - const toLoad = 'file://' + t.source - stream = await (await import(toLoad)).default(t.options) - } else if (t.prettyPrint) { - const pretty = require('pino-pretty')(t.prettyPrint) - stream = new Transform({ - objectMode: true, - autoDestroy: true, - transform (chunk, enc, cb) { - const line = pretty(chunk.toString()) - cb(null, line) - } - }) - - // TODO figure out why sync: false does not work here - // TODO remove the istanbul ignore, add a test using a - // child_process - const destination = pino.destination(t.destination || /* istanbul ignore next */ 1) - /* istanbul ignore next */ - if (destination.fd === 1) { - destination.end = function () { - this.emit('close') - } - } - await once(destination, 'ready') - pipeline(stream, destination, () => {}) - } else { - // TODO remove the transform - stream = new Transform({ - objectMode: true, - autoDestroy: true, - transform (chunk, enc, cb) { - cb(null, chunk.toString() + '\n') - } - }) - - // TODO figure out why sync: false does not work here - const destination = pino.destination(t.destination || /* istanbul ignore next */ 1) - await once(destination, 'ready') - pipeline(stream, destination, () => {}) - } +module.exports = async function ({ targets }) { + targets = await Promise.all(targets.map(async (t) => { + const toLoad = 'file://' + t.target + const stream = await (await import(toLoad)).default(t.options) return { level: t.level, stream @@ -64,7 +22,7 @@ module.exports = async function ({ destinations }) { metadata: true, close (err, cb) { let expected = 0 - for (const transport of destinations) { + for (const transport of targets) { expected++ transport.stream.on('close', closeCb) transport.stream.end() @@ -79,7 +37,7 @@ module.exports = async function ({ destinations }) { }) function process (stream) { - const multi = pino.multistream(destinations) + const multi = pino.multistream(targets) // TODO manage backpressure stream.on('data', function (chunk) { const { lastTime, lastMsg, lastObj, lastLevel } = this diff --git a/test/transport.test.js b/test/transport.test.js index ded32553e..ba93564fc 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -19,7 +19,7 @@ test('pino.transport with file', async ({ same, teardown }) => { '_' + Math.random().toString(36).substr(2, 9) ) const transport = pino.transport({ - source: join(__dirname, 'fixtures', 'to-file-transport.js'), + target: join(__dirname, 'fixtures', 'to-file-transport.js'), options: { destination } }) teardown(transport.end.bind(transport)) @@ -38,7 +38,7 @@ test('pino.transport with file', async ({ same, teardown }) => { test('pino.transport with file (no options + error handling)', async ({ equal }) => { const transport = pino.transport({ - source: join(__dirname, 'fixtures', 'to-file-transport.js') + target: join(__dirname, 'fixtures', 'to-file-transport.js') }) // TODO: when thread stream passess error handling to main, mop up the console.error const [err] = await once(transport, 'error') @@ -62,7 +62,7 @@ test('pino.transport with package', { skip: isWin }, async ({ same, teardown }) ) const transport = pino.transport({ - module: 'transport', + target: 'transport', options: { destination } }) teardown(async () => { @@ -88,7 +88,7 @@ test('pino.transport with file URL', async ({ same, teardown }) => { '_' + Math.random().toString(36).substr(2, 9) ) const transport = pino.transport({ - source: url.pathToFileURL(join(__dirname, 'fixtures', 'to-file-transport.js')).href, + target: url.pathToFileURL(join(__dirname, 'fixtures', 'to-file-transport.js')).href, options: { destination } }) teardown(transport.end.bind(transport)) @@ -108,7 +108,7 @@ test('pino.transport with file URL', async ({ same, teardown }) => { test('pino.transport errors if file does not exists', ({ plan, pass }) => { plan(1) const instance = pino.transport({ - source: join(__dirname, 'fixtures', 'non-existent-file'), + target: join(__dirname, 'fixtures', 'non-existent-file'), worker: { stdin: true, stdout: true, @@ -126,7 +126,7 @@ test('pino.transport with esm', async ({ same, teardown }) => { '_' + Math.random().toString(36).substr(2, 9) ) const transport = pino.transport({ - source: join(__dirname, 'fixtures', 'to-file-transport.mjs'), + target: join(__dirname, 'fixtures', 'to-file-transport.mjs'), options: { destination } }) const instance = pino(transport) @@ -153,13 +153,13 @@ test('pino.transport with two files', async ({ same, teardown }) => { '_' + Math.random().toString(36).substr(2, 9) ) const transport = pino.transport({ - destinations: [{ + targets: [{ level: 'info', - source: join(__dirname, 'fixtures', 'to-file-transport.js'), + target: join(__dirname, 'fixtures', 'to-file-transport.js'), options: { destination: dest1 } }, { level: 'info', - source: join(__dirname, 'fixtures', 'to-file-transport.js'), + target: join(__dirname, 'fixtures', 'to-file-transport.js'), options: { destination: dest2 } }] }) @@ -195,13 +195,18 @@ test('pino.transport with an array including a prettyPrint destination', async ( '_' + Math.random().toString(36).substr(2, 9) ) const transport = pino.transport({ - destinations: [{ + targets: [{ level: 'info', - destination: dest1 + target: '#pino/file', + options: { + destination: dest1 + } }, { level: 'info', - prettyPrint: true, - destination: dest2 + target: '#pino/pretty', + options: { + destination: dest2 + } }] }) teardown(transport.end.bind(transport)) @@ -226,7 +231,7 @@ test('no transport.end()', async ({ same, teardown }) => { '_' + Math.random().toString(36).substr(2, 9) ) const transport = pino.transport({ - source: join(__dirname, 'fixtures', 'to-file-transport.js'), + target: join(__dirname, 'fixtures', 'to-file-transport.js'), options: { destination } }) const instance = pino(transport) @@ -249,7 +254,7 @@ test('autoEnd = false', async ({ equal, same, teardown }) => { ) const count = process.listenerCount('exit') const transport = pino.transport({ - source: join(__dirname, 'fixtures', 'to-file-transport.js'), + target: join(__dirname, 'fixtures', 'to-file-transport.js'), options: { destination }, worker: { autoEnd: false } }) @@ -270,48 +275,22 @@ test('autoEnd = false', async ({ equal, same, teardown }) => { }) }) -test('pino.transport with source and destinations', async ({ fail, equal }) => { +test('pino.transport with target and targets', async ({ fail, equal }) => { try { pino.transport({ - source: 'a/file', - destinations: [{ - source: 'a/file' + target: 'a/file', + targets: [{ + target: 'a/file' }] }) fail('must throw') } catch (err) { - equal(err.message, 'Only one of source, destinations or module can be specified') - } -}) - -test('pino.transport with source and module', async ({ fail, equal }) => { - try { - pino.transport({ - source: 'a/file', - module: 'transport' - }) - fail('must throw') - } catch (err) { - equal(err.message, 'Only one of source, destinations or module can be specified') - } -}) - -test('pino.transport with destinations and module', async ({ fail, equal }) => { - try { - pino.transport({ - destinations: [{ - source: 'a/file' - }], - module: 'transport' - }) - fail('must throw') - } catch (err) { - equal(err.message, 'Only one of source, destinations or module can be specified') + equal(err.message, 'Only one of target or targets can be specified') } }) // TODO make this test pass on Windows -test('pino.transport with package as a destination', { skip: isWin }, async ({ same, teardown }) => { +test('pino.transport with package as a target', { skip: isWin }, async ({ same, teardown }) => { const destination = join( os.tmpdir(), '_' + Math.random().toString(36).substr(2, 9) @@ -327,8 +306,8 @@ test('pino.transport with package as a destination', { skip: isWin }, async ({ s ) const transport = pino.transport({ - destinations: [{ - module: 'transport', + targets: [{ + target: 'transport', options: { destination } }] }) @@ -348,3 +327,43 @@ test('pino.transport with package as a destination', { skip: isWin }, async ({ s msg: 'hello' }) }) + +test('pino.transport with target #pino/file', async ({ same, teardown }) => { + const destination = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const transport = pino.transport({ + target: '#pino/file', + options: { destination } + }) + teardown(transport.end.bind(transport)) + const instance = pino(transport) + instance.info('hello') + await watchFileCreated(destination) + const result = JSON.parse(await readFile(destination)) + delete result.time + same(result, { + pid, + hostname, + level: 30, + msg: 'hello' + }) +}) + +test('pino.transport with target #pino/pretty', async ({ match, teardown }) => { + const destination = join( + os.tmpdir(), + '_' + Math.random().toString(36).substr(2, 9) + ) + const transport = pino.transport({ + target: '#pino/pretty', + options: { destination } + }) + teardown(transport.end.bind(transport)) + const instance = pino(transport) + instance.info('hello') + await watchFileCreated(destination) + const actual = await readFile(destination, 'utf8') + match(strip(actual), /\[.*\] INFO.*hello/) +}) From 2bbe8c5729cc1c12ce60563b4105beab002b34af Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 7 Jun 2021 08:20:22 +0200 Subject: [PATCH 51/63] restore istanbul ignore --- lib/transport.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/transport.js b/lib/transport.js index 2ce8ed7fd..d27220139 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -69,6 +69,7 @@ function transport (fullOptions) { return join(__dirname, 'pretty-target.js') case '#pino/file': return join(__dirname, 'file-target.js') + /* istanbul ignore next */ default: // TODO we cannot test this on Windows.. might not work. return callerRequire.resolve(origin) From 2e307a8a66706a542fb9cddd709c7575e5f14e87 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 7 Jun 2021 14:17:32 +0200 Subject: [PATCH 52/63] Do not check the coverage in CI --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 09a973ac3..92aa74f10 100644 --- a/package.json +++ b/package.json @@ -19,8 +19,8 @@ "docs": "docsify serve", "browser-test": "airtap --local 8080 test/browser*test.js", "lint": "eslint .", - "test": "npm run lint && tap --100 test/*test.js test/*/*test.js", - "test-ci": "npm run lint && tap test/*test.js test/*/*test.js --coverage-report=lcovonly", + "test": "npm run lint && tap test/*test.js test/*/*test.js", + "test-ci": "npm run lint && tap --check-coverage=false test/*test.js test/*/*test.js --coverage-report=lcovonly", "cov-ui": "tap --coverage-report=html test/*test.js test/*/*test.js", "bench": "node benchmarks/utils/runbench all", "bench-basic": "node benchmarks/utils/runbench basic", From 21f0c4089ac6cfe96b6e0128dac639a13900d875 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 7 Jun 2021 15:33:03 +0200 Subject: [PATCH 53/63] --no-check-coverage --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 92aa74f10..b49ce6cca 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "browser-test": "airtap --local 8080 test/browser*test.js", "lint": "eslint .", "test": "npm run lint && tap test/*test.js test/*/*test.js", - "test-ci": "npm run lint && tap --check-coverage=false test/*test.js test/*/*test.js --coverage-report=lcovonly", + "test-ci": "npm run lint && tap --no-check-coverage test/*test.js test/*/*test.js --coverage-report=lcovonly", "cov-ui": "tap --coverage-report=html test/*test.js test/*/*test.js", "bench": "node benchmarks/utils/runbench all", "bench-basic": "node benchmarks/utils/runbench basic", From caaec82c1c02b17354abe53d24ac7b0e497ae523 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 7 Jun 2021 23:40:51 +0200 Subject: [PATCH 54/63] removed TODO --- test/transport.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/transport.test.js b/test/transport.test.js index ba93564fc..7e5f0fc9a 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -40,7 +40,6 @@ test('pino.transport with file (no options + error handling)', async ({ equal }) const transport = pino.transport({ target: join(__dirname, 'fixtures', 'to-file-transport.js') }) - // TODO: when thread stream passess error handling to main, mop up the console.error const [err] = await once(transport, 'error') equal(err.message, 'kaboom') }) From 82ebbbf011d5f1d4af4125112c95092d2421c05c Mon Sep 17 00:00:00 2001 From: David Mark Clements Date: Fri, 11 Jun 2021 18:56:21 +0200 Subject: [PATCH 55/63] built in docs, other doc tweaks --- docs/api.md | 74 ++++++++++++++++++++++++++++++++++++++++++---- docs/transports.md | 8 ++--- 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/docs/api.md b/docs/api.md index 0c8137b2a..fd79637e0 100644 --- a/docs/api.md +++ b/docs/api.md @@ -921,18 +921,82 @@ const transport = pino.transport({ pino(transport) ``` +Multiple transports may also be defined, and specific levels can be logged to each transport: + +```js +const pino = require('pino') +const transports = pino.transport([ + { + level: 'info', + target: 'some-transport', + options: { some: 'options for', the: 'transport' } + }, + { + level: 'trace', + target: '#pino/file', + options: { destination: '/path/to/store/logs' } + } +]) +pino(transports) +``` + + +For more on transports, how they work, and how to create them see the [`Transports documentation`](/docs/transports.md). + * See [`Transports`](/docs/transports.md) * See [`thread-stream` ⇗](https://github.com/mcollina/thread-stream) #### Options -* `target`: The transport to pass logs through. This may be an installed module name, a path or a built-in transport (see next section) -* `options`: An options object which is serialized (see [Structured Clone Algorithm][https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm]), passed to the worker thread, parsed and then passed to the transport. -* `destinations`: -==TODO==- +* `target`: The transport to pass logs through. This may be an installed module name, an absolute path or a built-in transport (see [Transport Builtins](#transport-builtins)) +* `options`: An options object which is serialized (see [Structured Clone Algorithm][https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm]), passed to the worker thread, parsed and then passed to the exported transport function. +* `worker`: [Worker thread](https://nodejs.org/api/worker_threads.html#worker_threads_new_worker_filename_options) configuration options. Additional, the `worker` option supports `worker.autoEnd`. If this is set to `false` logs will not be flushed on process exit, it is then up to the developer to call `transport.end()` to flush logs. +* `targets`: May be specified instead of `target`, must be an array of transport configurations. Transport configurations include the aforementioned `options` and `target` options plus a `level` option which will send only logs above a specified level to a transport. -#### Built in transports +#### Transport Builtins --==TODO==- +The `target` option may be an absolute path, a module name or builtin. + +A transport builtin takes the form `#pino/`. + +The following transport builtins are supported: + +##### `#pino/file` + +The `#pino/file` builtin routes logs to a file (or file descriptor). + +The `options.destination` property may be set to specify the desired file destination. + +```js +const pino = require('pino') +const transport = pino.transport({ + target: '#pino/file', + options: { destination: '/path/to/file' } +}) +pino(transport) +``` + +The `options.destination` property may also be a number to represent a file descriptor. Typically this would be `1` to write to STDOUT or `2` to write to STDERR. If `options.destination` is not set, it defaults to `1` which means logs will be written to STDOUT. + +The difference between using the `#pino/file` transport builtin and using `pino.destination` is that `pino.destination` runs in the main thread, whereas `#pino/file` sets up `pino.destination` in a worker thread. + +##### ``#pino/pretty` + +The `#pino/pretty` builtin prettifies logs. + + +By default the `#pino/pretty` builtin logs to STDOUT. + +The `options.destination` property may be set to log pretty logs to a file descriptor or file. The following would send the prettified logs to STDERR: + +```js +const pino = require('pino') +const transport = pino.transport({ + target: '#pino/pretty', + options: { destination: 2 } +}) +pino(transport) +``` diff --git a/docs/transports.md b/docs/transports.md index ccf6440f8..c70a2bafb 100644 --- a/docs/transports.md +++ b/docs/transports.md @@ -46,14 +46,14 @@ and passing it to the `pino` function: ```js const pino = require('pino') const transport = pino.transport({ - target: './my-transport.mjs' + target: '/absolute/path/to/my-transport.mjs' }) pino(transport) ``` The transport code will be executed in a separate worker thread. The main thread will write logs to the worker thread, which will write them to the stream returned -from function exported from the transport file/module. +from the function exported from the transport file/module. The exported function can also be async. Imagine the following transport: @@ -89,13 +89,13 @@ This means that the options object can only contain types that are supported by [Structured Clone Algorithm][sca] which is used to (de)serializing objects between threads. What if we wanted to use both transports, but send only error logs to `some-file-transport` while -sending all logs to `./my-transport.mjs`. We can use the `pino.transport` function's `destinations` option: +sending all logs to `my-transport.mjs`. We can use the `pino.transport` function's `destinations` option: ```js const pino = require('pino') const transport = pino.transport({ destinations: [ - { target: './my-transport.mjs', level: 'error' }, + { target: '/absolute/path/to/my-transport.mjs', level: 'error' }, { target: 'some-file-transport', options: { destination: '/dev/null' } ] }) From 2474e2984d8b2a7ebd4df39ed7ffeb46d6edbb7d Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 12 Jun 2021 19:21:18 +0200 Subject: [PATCH 56/63] Update test/transport.test.js Co-authored-by: David Mark Clements --- test/transport.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/transport.test.js b/test/transport.test.js index 7e5f0fc9a..6a909718e 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -279,7 +279,7 @@ test('pino.transport with target and targets', async ({ fail, equal }) => { pino.transport({ target: 'a/file', targets: [{ - target: 'a/file' + target: '/a/file' }] }) fail('must throw') From 6c49f3e75f4088eec740ffadecf71fc223057a5a Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 12 Jun 2021 19:24:00 +0200 Subject: [PATCH 57/63] Update test/transport.test.js Co-authored-by: David Mark Clements --- test/transport.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/transport.test.js b/test/transport.test.js index 6a909718e..4fdbceed2 100644 --- a/test/transport.test.js +++ b/test/transport.test.js @@ -277,7 +277,7 @@ test('autoEnd = false', async ({ equal, same, teardown }) => { test('pino.transport with target and targets', async ({ fail, equal }) => { try { pino.transport({ - target: 'a/file', + target: '/a/file', targets: [{ target: '/a/file' }] From 17dc8a90bd107c0129ee5a412655ddcd097bb415 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 3 Jul 2021 10:06:11 +0200 Subject: [PATCH 58/63] Update docs/api.md Co-authored-by: James Sumners --- docs/api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api.md b/docs/api.md index fd79637e0..49ef16742 100644 --- a/docs/api.md +++ b/docs/api.md @@ -950,7 +950,7 @@ For more on transports, how they work, and how to create them see the [`Transpor * `target`: The transport to pass logs through. This may be an installed module name, an absolute path or a built-in transport (see [Transport Builtins](#transport-builtins)) * `options`: An options object which is serialized (see [Structured Clone Algorithm][https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm]), passed to the worker thread, parsed and then passed to the exported transport function. -* `worker`: [Worker thread](https://nodejs.org/api/worker_threads.html#worker_threads_new_worker_filename_options) configuration options. Additional, the `worker` option supports `worker.autoEnd`. If this is set to `false` logs will not be flushed on process exit, it is then up to the developer to call `transport.end()` to flush logs. +* `worker`: [Worker thread](https://nodejs.org/api/worker_threads.html#worker_threads_new_worker_filename_options) configuration options. Additionally, the `worker` option supports `worker.autoEnd`. If this is set to `false` logs will not be flushed on process exit. It is then up to the developer to call `transport.end()` to flush logs. * `targets`: May be specified instead of `target`, must be an array of transport configurations. Transport configurations include the aforementioned `options` and `target` options plus a `level` option which will send only logs above a specified level to a transport. #### Transport Builtins From 8ef44090e05daf63f0a91aeed700159daaba5dbf Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 3 Jul 2021 10:06:32 +0200 Subject: [PATCH 59/63] Update docs/api.md Co-authored-by: James Sumners --- docs/api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api.md b/docs/api.md index 49ef16742..2bf8b5d2d 100644 --- a/docs/api.md +++ b/docs/api.md @@ -951,7 +951,7 @@ For more on transports, how they work, and how to create them see the [`Transpor * `target`: The transport to pass logs through. This may be an installed module name, an absolute path or a built-in transport (see [Transport Builtins](#transport-builtins)) * `options`: An options object which is serialized (see [Structured Clone Algorithm][https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm]), passed to the worker thread, parsed and then passed to the exported transport function. * `worker`: [Worker thread](https://nodejs.org/api/worker_threads.html#worker_threads_new_worker_filename_options) configuration options. Additionally, the `worker` option supports `worker.autoEnd`. If this is set to `false` logs will not be flushed on process exit. It is then up to the developer to call `transport.end()` to flush logs. -* `targets`: May be specified instead of `target`, must be an array of transport configurations. Transport configurations include the aforementioned `options` and `target` options plus a `level` option which will send only logs above a specified level to a transport. +* `targets`: May be specified instead of `target`. Must be an array of transport configurations. Transport configurations include the aforementioned `options` and `target` options plus a `level` option which will send only logs above a specified level to a transport. #### Transport Builtins From 8855a334ec4833e06f30fd8b64e90de9e919b479 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 3 Jul 2021 10:06:48 +0200 Subject: [PATCH 60/63] Update docs/api.md Co-authored-by: James Sumners --- docs/api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api.md b/docs/api.md index 2bf8b5d2d..6b5872deb 100644 --- a/docs/api.md +++ b/docs/api.md @@ -980,7 +980,7 @@ The `options.destination` property may also be a number to represent a file desc The difference between using the `#pino/file` transport builtin and using `pino.destination` is that `pino.destination` runs in the main thread, whereas `#pino/file` sets up `pino.destination` in a worker thread. -##### ``#pino/pretty` +##### `#pino/pretty` The `#pino/pretty` builtin prettifies logs. From 224474efce7a585975dcfc4959323bdff5c3cd85 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 3 Jul 2021 10:06:59 +0200 Subject: [PATCH 61/63] Update docs/transports.md Co-authored-by: James Sumners --- docs/transports.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/transports.md b/docs/transports.md index c70a2bafb..c1ca8f63d 100644 --- a/docs/transports.md +++ b/docs/transports.md @@ -84,7 +84,7 @@ pino(transport) ``` Note here we've specified a module by package rather than by relative path. The options object we provide -is serialized and injected into the transport worker thread, then passed to the modules exported function. +is serialized and injected into the transport worker thread, then passed to the module's exported function. This means that the options object can only contain types that are supported by the [Structured Clone Algorithm][sca] which is used to (de)serializing objects between threads. From 55e8b618b60ad331c0f1c1c7088c980ae32e7d9f Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 3 Jul 2021 10:07:10 +0200 Subject: [PATCH 62/63] Update docs/transports.md Co-authored-by: James Sumners --- docs/transports.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/transports.md b/docs/transports.md index c1ca8f63d..54ddf4256 100644 --- a/docs/transports.md +++ b/docs/transports.md @@ -89,7 +89,7 @@ This means that the options object can only contain types that are supported by [Structured Clone Algorithm][sca] which is used to (de)serializing objects between threads. What if we wanted to use both transports, but send only error logs to `some-file-transport` while -sending all logs to `my-transport.mjs`. We can use the `pino.transport` function's `destinations` option: +sending all logs to `my-transport.mjs`? We can use the `pino.transport` function's `destinations` option: ```js const pino = require('pino') From 45cb21cfe4cf1f4c5bc64d0be2da20eb236ece2b Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 3 Jul 2021 10:35:00 +0200 Subject: [PATCH 63/63] PR reviews --- lib/file-target.js | 6 ++-- lib/pretty-target.js | 9 ++---- test/targets.test.js | 66 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 10 deletions(-) create mode 100644 test/targets.test.js diff --git a/lib/file-target.js b/lib/file-target.js index 36f3b253b..dde0fae91 100644 --- a/lib/file-target.js +++ b/lib/file-target.js @@ -4,8 +4,7 @@ const { Transform, pipeline } = require('stream') const pino = require('../pino') const { once } = require('events') -module.exports = async function (opts) { - // TODO remove the transform +module.exports = async function (opts = {}) { const stream = new Transform({ objectMode: true, autoDestroy: true, @@ -14,8 +13,7 @@ module.exports = async function (opts) { } }) - // TODO figure out why sync: false does not work here - const destination = pino.destination(opts.destination || /* istanbul ignore next */ 1) + const destination = pino.destination({ dest: opts.destination || 1, sync: false }) await once(destination, 'ready') pipeline(stream, destination, () => {}) return stream diff --git a/lib/pretty-target.js b/lib/pretty-target.js index 795a27ea5..5a8d0821a 100644 --- a/lib/pretty-target.js +++ b/lib/pretty-target.js @@ -5,7 +5,7 @@ const { Transform, pipeline } = require('stream') const pino = require('../pino') const { once } = require('events') -module.exports = async function (/* istanbul ignore next */ opts = {}) { +module.exports = async function (opts = {}) { const pretty = pinoPretty(opts) const stream = new Transform({ objectMode: true, @@ -16,12 +16,9 @@ module.exports = async function (/* istanbul ignore next */ opts = {}) { } }) - // TODO figure out why sync: false does not work here - // TODO remove the istanbul ignore, add a test using a - // child_process - const destination = pino.destination(opts.destination || /* istanbul ignore next */ 1) - /* istanbul ignore next */ + const destination = pino.destination({ dest: opts.destination || 1, sync: false }) if (destination.fd === 1) { + // We cannot close the output destination.end = function () { this.emit('close') } diff --git a/test/targets.test.js b/test/targets.test.js new file mode 100644 index 000000000..c6bc048b5 --- /dev/null +++ b/test/targets.test.js @@ -0,0 +1,66 @@ +'use strict' + +const { test } = require('tap') +const proxyquire = require('proxyquire') +const Writable = require('stream').Writable + +test('pretty-target mocked', async function ({ equal, same, plan, pass }) { + plan(3) + let ret + const prettyTarget = proxyquire('../lib/pretty-target', { + '../pino': { + destination (opts) { + same(opts, { dest: 1, sync: false }) + + ret = new Writable() + ret.fd = opts.dest + + process.nextTick(() => { + ret.emit('ready') + }) + + Object.defineProperty(ret, 'end', { + get () { + return 'unused' + }, + set (end) { + pass('prop set') + const obj = { + emit (ev) { + equal(ev, 'close') + }, + end + } + obj.end() + } + }) + return ret + } + } + }) + + await prettyTarget() +}) + +test('file-target mocked', async function ({ equal, same, plan, pass }) { + plan(1) + let ret + const fileTarget = proxyquire('../lib/file-target', { + '../pino': { + destination (opts) { + same(opts, { dest: 1, sync: false }) + + ret = new Writable() + ret.fd = opts.dest + + process.nextTick(() => { + ret.emit('ready') + }) + + return ret + } + } + }) + + await fileTarget() +})