Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ WIP ] pinole integration, next gen transports #937

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/proto.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
/* eslint no-prototype-builtins: 0 */

const { EventEmitter } = require('events')
const SonicBoom = require('sonic-boom')
const pinole = require('pinole')
const flatstr = require('flatstr')
const {
lsCacheSym,
Expand Down Expand Up @@ -169,7 +169,7 @@ function write (_obj, msg, num) {
stream.lastTime = t.slice(this[timeSliceIndexSym])
stream.lastLogger = this // for child loggers
}
if (stream instanceof SonicBoom) stream.write(s)
if (stream[pinole.symbols.kPinole]) stream.write(s)
else stream.write(flatstr(s))
}

Expand Down
18 changes: 9 additions & 9 deletions lib/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

const format = require('quick-format-unescaped')
const { mapHttpRequest, mapHttpResponse } = require('pino-std-serializers')
const SonicBoom = require('sonic-boom')
const pinole = require('pinole')
const stringifySafe = require('fast-safe-stringify')
const {
lsCacheSym,
Expand Down Expand Up @@ -291,8 +291,8 @@ function hasBeenTampered (stream) {
return stream.write !== stream.constructor.prototype.write
}

function buildSafeSonicBoom (opts) {
const stream = new SonicBoom(opts)
function buildSafePinole (opts) {
const stream = pinole(opts)
stream.on('error', filterBrokenPipe)
return stream

Expand All @@ -301,7 +301,7 @@ function buildSafeSonicBoom (opts) {
if (err.code === 'EPIPE') {
// If we get EPIPE, we should stop logging here
// however we have no control to the consumer of
// SonicBoom, so we just overwrite the write method
// pinole, so we just overwrite the write method
stream.write = noop
stream.end = noop
stream.flushSync = noop
Expand All @@ -317,11 +317,11 @@ function createArgsNormalizer (defaultOptions) {
return function normalizeArgs (instance, opts = {}, stream) {
// support stream as a string
if (typeof opts === 'string') {
stream = buildSafeSonicBoom({ dest: opts, sync: true })
stream = buildSafePinole({ dest: opts, sync: true })
opts = {}
} else if (typeof stream === 'string') {
stream = buildSafeSonicBoom({ dest: stream, sync: true })
} else if (opts instanceof SonicBoom || opts.writable || opts._writableState) {
stream = buildSafePinole({ dest: stream, sync: true })
} else if (opts[pinole.symbols.kPinole] || opts.writable || opts._writableState) {
stream = opts
opts = null
}
Expand All @@ -345,7 +345,7 @@ function createArgsNormalizer (defaultOptions) {
if (enabled === false) opts.level = 'silent'
stream = stream || process.stdout
if (stream === process.stdout && stream.fd >= 0 && !hasBeenTampered(stream)) {
stream = buildSafeSonicBoom({ fd: stream.fd, sync: true })
stream = buildSafePinole({ fd: stream.fd, sync: true })
}
if (prettyPrint) {
const prettyOpts = Object.assign({ messageKey }, prettyPrint)
Expand Down Expand Up @@ -425,7 +425,7 @@ function setMetadataProps (dest, that) {

module.exports = {
noop,
buildSafeSonicBoom,
buildSafePinole,
getPrettyStream,
asChindings,
asJson,
Expand Down
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
"pump": "^3.0.0",
"semver": "^7.0.0",
"snazzy": "^8.0.0",
"sonic-boom": "^1.3.0",
"split2": "^3.1.1",
"standard": "^14.3.3",
"steed": "^1.1.3",
Expand All @@ -89,7 +90,7 @@
"fast-safe-stringify": "^2.0.7",
"flatstr": "^1.0.12",
"pino-std-serializers": "^2.4.2",
"quick-format-unescaped": "^4.0.1",
"sonic-boom": "^1.0.2"
"pinole": "0.0.0",
"quick-format-unescaped": "^4.0.1"
}
}
23 changes: 19 additions & 4 deletions pino.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const {
asChindings,
final,
stringify,
buildSafeSonicBoom,
buildSafePinole,
buildFormatters,
noop
} = require('./lib/tools')
Expand Down Expand Up @@ -202,15 +202,30 @@ module.exports.extreme = (dest = process.stdout.fd) => {
'The pino.extreme() option is deprecated and will be removed in v7. Use pino.destination({ sync: false }) instead.',
{ code: 'extreme_deprecation' }
)
return buildSafeSonicBoom({ dest, minLength: 4096, sync: false })
return buildSafePinole({ dest, minLength: 4096, sync: false })
}

module.exports.destination = (dest = process.stdout.fd) => {
if (typeof dest === 'object') {
dest.dest = dest.dest || process.stdout.fd
return buildSafeSonicBoom(dest)
return buildSafePinole(dest)
} else {
return buildSafeSonicBoom({ dest, minLength: 0, sync: true })
return buildSafePinole({ dest, minLength: 0, sync: true })
}
}

module.exports.transport = (transport, opts = {}) => {
if (typeof transport === 'object') {
opts = transport
if (typeof opts.transport !== 'string') {
throw Error('transport must be a string and must be specified')
}
return buildSafePinole(opts)
} else {
if (typeof transport !== 'string') {
throw Error('transport must be a string and must be specified')
}
return buildSafePinole({ ...opts, transport })
}
}

Expand Down
9 changes: 9 additions & 0 deletions test/fixtures/basic-transport.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import SonicBoom from 'sonic-boom'

export default (opts = { fd: 1 }) => {
if (!opts.dest && !opts.fd) opts.fd = 1
const sonic = new SonicBoom(opts)
return (data) => {
sonic.write(data, 0, true)
}
}
4 changes: 4 additions & 0 deletions test/fixtures/legacy-transport-consumers/natural-flush.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const pino = require('./../../..')
const instance = pino(pino.transport('legacy-transport'))
instance.info('hello')
setTimeout(() => {}, 100) // breathing time
7 changes: 7 additions & 0 deletions test/fixtures/legacy-transport-consumers/sync-flush.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
const pino = require('./../../..')
const tp = pino.transport('legacy-transport')
const instance = pino(tp)
instance.info('hello')
process.on('exit', () => {
tp.flushSync()
})
10 changes: 10 additions & 0 deletions test/fixtures/legacy-transport/cmd.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
'use strict'
const { Transform } = require('stream')
const transform = new Transform({
transform (chunk, enc, cb) {
const { level, msg } = JSON.parse(chunk)
cb(null, `${level}:${msg}`)
}
})
process.stdin.pipe(transform)
transform.pipe(process.stdout)
5 changes: 5 additions & 0 deletions test/fixtures/legacy-transport/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "legacy-transport",
"version": "1.0.0",
"bin": "cmd.js"
}
77 changes: 77 additions & 0 deletions test/transports.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
'use strict'
const os = require('os')
const { join } = require('path')
const { readFileSync } = require('fs')
const { spawnSync } = require('child_process')
const { test, teardown, comment } = require('tap')
const { watchFileCreated } = require('./helper')
const pino = require('../')

const { pid } = process
const hostname = os.hostname()

const basicTransport = require.resolve('./fixtures/basic-transport.mjs')

spawnSync('npm', ['link', join(__dirname, 'fixtures', 'legacy-transport')])
teardown(() => {
comment('teardown commencing, might take a little while')
spawnSync('npm', ['unlink', 'legacy-transport'])
})

test('pino.transport - transport name must be supplied and be a string', async ({ throws }) => {
throws(() => pino.transport())
throws(() => pino.transport(22))
throws(() => pino.transport({}))
throws(() => pino.transport({ transport: {} }))
})

test('pino.transport basic (transport, opts)', async ({ same }) => {
const tmp = join(
os.tmpdir(),
'_' + Math.random().toString(36).substr(2, 9)
)
const instance = pino(pino.transport(basicTransport, {
dest: tmp
}))
instance.info('hello')
await watchFileCreated(tmp)
const result = JSON.parse(readFileSync(tmp).toString())
delete result.time
same(result, {
pid,
hostname,
level: 30,
msg: 'hello'
})
})

test('pino.transport basic ({transport, ...opts})', async ({ same }) => {
const tmp = join(
os.tmpdir(),
'_' + Math.random().toString(36).substr(2, 9)
)
const instance = pino(pino.transport({
transport: basicTransport,
dest: tmp
}))
instance.info('hello')
await watchFileCreated(tmp)
const result = JSON.parse(readFileSync(tmp).toString())
delete result.time
same(result, {
pid,
hostname,
level: 30,
msg: 'hello'
})
})

test('pino.transport legacy natural flush', async ({ is }) => {
const result = spawnSync(process.execPath, [join(__dirname, 'fixtures', 'legacy-transport-consumers/natural-flush.js')])
is(result.stdout.toString(), '30:hello')
})

test('pino.transport legacy sync flush', async ({ is }) => {
const result = spawnSync(process.execPath, [join(__dirname, 'fixtures', 'legacy-transport-consumers/sync-flush.js')])
is(result.stdout.toString(), '30:hello')
})