From 946f630016207decda0cdd84f29686401eaf6e7d Mon Sep 17 00:00:00 2001 From: KaKa Date: Mon, 12 Jul 2021 20:11:53 +0800 Subject: [PATCH] refactor: deprecate child bindings apply options (#1067) * refactor: deprecate child bindings apply options * chore: remove todo comment * refactor: deprecate bindings.level --- benchmarks/child-creation.bench.js | 7 ++ browser.js | 12 ++- docs/api.md | 38 ++++---- lib/proto.js | 39 +++++--- test/browser-serializers.test.js | 37 +++++++- test/custom-levels.test.js | 20 ++-- test/deprecations.test.js | 141 +++++++++++++++++++++++++++++ test/formatters.test.js | 6 +- test/serializers.test.js | 14 +-- 9 files changed, 258 insertions(+), 56 deletions(-) diff --git a/benchmarks/child-creation.bench.js b/benchmarks/child-creation.bench.js index 616e855c3..b09099f72 100644 --- a/benchmarks/child-creation.bench.js +++ b/benchmarks/child-creation.bench.js @@ -60,6 +60,13 @@ const run = bench([ child.info({ hello: 'world' }) } setImmediate(cb) + }, + function benchPinoCreationWithOption (cb) { + const child = plogDest.child({ a: 'property' }, { redact: [] }) + for (var i = 0; i < max; i++) { + child.info({ hello: 'world' }) + } + setImmediate(cb) } ], 10000) diff --git a/browser.js b/browser.js index 26a2c094d..51989ba37 100644 --- a/browser.js +++ b/browser.js @@ -113,13 +113,17 @@ function pino (opts) { set(setOpts, logger, 'trace', 'log') } - function child (bindings) { + function child (bindings, childOptions) { if (!bindings) { throw new Error('missing bindings for child Pino') } - const bindingsSerializers = bindings.serializers - if (serialize && bindingsSerializers) { - var childSerializers = Object.assign({}, serializers, bindingsSerializers) + childOptions = childOptions || {} + if (serialize && bindings.serializers) { + childOptions.serializers = bindings.serializers + } + const childOptionsSerializers = childOptions.serializers + if (serialize && childOptionsSerializers) { + var childSerializers = Object.assign({}, serializers, childOptionsSerializers) var childSerialize = opts.browser.serialize === true ? Object.keys(childSerializers) : serialize diff --git a/docs/api.md b/docs/api.md index 5437ed641..3f30471ef 100644 --- a/docs/api.md +++ b/docs/api.md @@ -680,24 +680,9 @@ const child = logger.child({foo: 'bar', level: 'debug'}) child.debug('debug!') // will log as the `level` property set the level to debug ``` -##### `bindings.serializers` (Object) +##### `bindings.serializers` (Object) - DEPRECATED -Child loggers inherit the [serializers](#opt-serializers) from the parent logger. - -Setting the `serializers` key of the `bindings` object will override -any configured parent serializers. - -```js -const logger = require('pino')() -logger.info({test: 'will appear'}) -// {"level":30,"time":1531259759482,"pid":67930,"hostname":"x","test":"will appear"} -const child = logger.child({serializers: {test: () => `child-only serializer`}}) -child.info({test: 'will be overwritten'}) -// {"level":30,"time":1531259784008,"pid":67930,"hostname":"x","test":"child-only serializer"} -``` - -* See [`serializers` option](#opt-serializers) -* See [pino.stdSerializers](#pino-stdSerializers) +Use `options.serializers` instead. #### `options` (Object) @@ -718,6 +703,25 @@ logger.info({ hello: 'world' }) * See [`redact` option](#opt-redact) +##### `options.serializers` (Object) + +Child loggers inherit the [serializers](#opt-serializers) from the parent logger. + +Setting the `serializers` key of the `options` object will override +any configured parent serializers. + +```js +const logger = require('pino')() +logger.info({test: 'will appear'}) +// {"level":30,"time":1531259759482,"pid":67930,"hostname":"x","test":"will appear"} +const child = logger.child({}, {serializers: {test: () => `child-only serializer`}}) +child.info({test: 'will be overwritten'}) +// {"level":30,"time":1531259784008,"pid":67930,"hostname":"x","test":"child-only serializer"} +``` + +* See [`serializers` option](#opt-serializers) +* See [pino.stdSerializers](#pino-stdSerializers) + ### `logger.bindings()` diff --git a/lib/proto.js b/lib/proto.js index 4fd361d8e..a38e6597d 100644 --- a/lib/proto.js +++ b/lib/proto.js @@ -77,8 +77,6 @@ module.exports = function () { } const resetChildingsFormatter = bindings => bindings -// TODO: formatters, serializers, customLevels should move to options -// and depercation warning should emit function child (bindings, options) { if (!bindings) { throw Error('missing bindings for child Pino') @@ -87,7 +85,24 @@ function child (bindings, options) { const serializers = this[serializersSym] const formatters = this[formattersSym] const instance = Object.create(this) + if (bindings.hasOwnProperty('serializers') === true) { + process.emitWarning('bindings.serializers is deprecated, use options.serializers option instead', 'Warning', 'PINODEP004') + options.serializers = bindings.serializers + } + if (bindings.hasOwnProperty('formatters') === true) { + process.emitWarning('bindings.formatters is deprecated, use options.formatters option instead', 'Warning', 'PINODEP005') + options.formatters = bindings.formatters + } + if (bindings.hasOwnProperty('customLevels') === true) { + process.emitWarning('bindings.customLevels is deprecated, use options.customLevels option instead', 'Warning', 'PINODEP006') + options.customLevels = bindings.customLevels + } + if (bindings.hasOwnProperty('level') === true) { + process.emitWarning('bindings.level is deprecated, use options.level option instead', 'Warning', 'PINODEP007') + options.level = bindings.level + } + if (options.hasOwnProperty('serializers') === true) { instance[serializersSym] = Object.create(null) for (const k in serializers) { @@ -100,17 +115,17 @@ function child (bindings, options) { instance[serializersSym][ks] = serializers[ks] } - for (const bk in bindings.serializers) { - instance[serializersSym][bk] = bindings.serializers[bk] + for (const bk in options.serializers) { + instance[serializersSym][bk] = options.serializers[bk] } - const bindingsSymbols = Object.getOwnPropertySymbols(bindings.serializers) + const bindingsSymbols = Object.getOwnPropertySymbols(options.serializers) for (var bi = 0; bi < bindingsSymbols.length; bi++) { const bks = bindingsSymbols[bi] - instance[serializersSym][bks] = bindings.serializers[bks] + instance[serializersSym][bks] = options.serializers[bks] } } else instance[serializersSym] = serializers - if (bindings.hasOwnProperty('formatters')) { - const { level, bindings: chindings, log } = bindings.formatters + if (options.hasOwnProperty('formatters')) { + const { level, bindings: chindings, log } = options.formatters instance[formattersSym] = buildFormatters( level || formatters.level, chindings || resetChildingsFormatter, @@ -123,9 +138,9 @@ function child (bindings, options) { formatters.log ) } - if (bindings.hasOwnProperty('customLevels') === true) { - assertNoLevelCollisions(this.levels, bindings.customLevels) - instance.levels = mappings(bindings.customLevels, instance[useOnlyCustomLevelsSym]) + if (options.hasOwnProperty('customLevels') === true) { + assertNoLevelCollisions(this.levels, options.customLevels) + instance.levels = mappings(options.customLevels, instance[useOnlyCustomLevelsSym]) genLsCache(instance) } @@ -140,7 +155,7 @@ function child (bindings, options) { } instance[chindingsSym] = asChindings(instance, bindings) - const childLevel = bindings.level || this.level + const childLevel = options.level || this.level instance[setLevelSym](childLevel) return instance diff --git a/test/browser-serializers.test.js b/test/browser-serializers.test.js index 11d07ce01..77d6dee56 100644 --- a/test/browser-serializers.test.js +++ b/test/browser-serializers.test.js @@ -165,6 +165,32 @@ if (process.title !== 'browser') { console.error = consoleError + logger.child({ + aBinding: 'test' + }, { + serializers: { + key: () => 'serialized', + key2: () => 'serialized2', + key3: () => 'serialized3' + } + }).fatal({ key: 'test' }, { key2: 'test' }, 'str should skip', [{ foo: 'array should skip' }], { key3: 'test' }) + end() + }) + + test('serialize filter applies to child loggers through bindings', ({ end, is }) => { + const consoleError = console.error + console.error = function (binding, test, test2, test3, test4, test5) { + is(test.key, 'test') + is(test2.key2, 'serialized2') + is(test5.key3, 'test') + } + + const logger = fresh('../browser', require)({ + browser: { serialize: ['key2'] } + }) + + console.error = consoleError + logger.child({ aBinding: 'test', serializers: { @@ -208,7 +234,8 @@ if (process.title !== 'browser') { console.error = consoleError logger.child({ - key: 'test', + key: 'test' + }, { serializers: { key: () => 'serialized' } @@ -233,7 +260,7 @@ test('child does not overwrite parent serializers', ({ end, is }) => { } } }) - const child = parent.child({ serializers: childSerializers }) + const child = parent.child({}, { serializers: childSerializers }) parent.fatal({ test: 'test' }) child.fatal({ test: 'test' }) @@ -266,7 +293,7 @@ test('children serializers get called', ({ end, is }) => { } }) - const child = parent.child({ a: 'property', serializers: childSerializers }) + const child = parent.child({ a: 'property' }, { serializers: childSerializers }) child.fatal({ test: 'test' }) end() @@ -284,7 +311,7 @@ test('children serializers get called when inherited from parent', ({ end, is }) } }) - const child = parent.child({ serializers: { test: () => 'pass' } }) + const child = parent.child({}, { serializers: { test: () => 'pass' } }) child.fatal({ test: 'fail' }) end() @@ -317,7 +344,7 @@ test('non overridden serializers are available in the children', ({ end, is }) = } }) - const child = parent.child({ serializers: cSerializers }) + const child = parent.child({}, { serializers: cSerializers }) child.fatal({ shared: 'test' }) child.fatal({ onlyParent: 'test' }) diff --git a/test/custom-levels.test.js b/test/custom-levels.test.js index 194f857fc..ecae49af4 100644 --- a/test/custom-levels.test.js +++ b/test/custom-levels.test.js @@ -98,10 +98,11 @@ test('custom levels are inherited by children', async ({ equal }) => { test('custom levels can be specified on child bindings', async ({ equal }) => { const stream = sink() const logger = pino(stream).child({ + childMsg: 'ok' + }, { customLevels: { foo: 35 - }, - childMsg: 'ok' + } }) logger.foo('test') @@ -114,10 +115,11 @@ test('custom levels can be specified on child bindings', async ({ equal }) => { test('customLevels property child bindings does not get logged', async ({ equal }) => { const stream = sink() const logger = pino(stream).child({ + childMsg: 'ok' + }, { customLevels: { foo: 35 - }, - childMsg: 'ok' + } }) logger.foo('test') @@ -131,7 +133,7 @@ test('throws when specifying pre-existing parent labels via child bindings', asy customLevels: { foo: 35 } - }, stream).child({ + }, stream).child({}, { customLevels: { foo: 45 } @@ -144,7 +146,7 @@ test('throws when specifying pre-existing parent values via child bindings', asy customLevels: { foo: 35 } - }, stream).child({ + }, stream).child({}, { customLevels: { bar: 35 } @@ -153,7 +155,7 @@ test('throws when specifying pre-existing parent values via child bindings', asy test('throws when specifying core values via child bindings', async ({ throws }) => { const stream = sink() - throws(() => pino(stream).child({ + throws(() => pino(stream).child({}, { customLevels: { foo: 30 } @@ -199,11 +201,11 @@ test('custom level below level threshold will not log', async ({ equal }) => { test('does not share custom level state across siblings', async ({ doesNotThrow }) => { const stream = sink() const logger = pino(stream) - logger.child({ + logger.child({}, { customLevels: { foo: 35 } }) doesNotThrow(() => { - logger.child({ + logger.child({}, { customLevels: { foo: 35 } }) }) diff --git a/test/deprecations.test.js b/test/deprecations.test.js index 5b8cec135..ce0fb46a9 100644 --- a/test/deprecations.test.js +++ b/test/deprecations.test.js @@ -1,6 +1,7 @@ 'use strict' /* eslint no-prototype-builtins: 0 */ +const { hostname } = require('os') const { test } = require('tap') const { sink, once } = require('./helper') const pino = require('../') @@ -99,3 +100,143 @@ test('pino.* serializer', async ({ match, equal, pass }) => { match(await o, { level: 30 }) process.removeListener('warning', onWarning) }) + +test('child(bindings.serializers)', async ({ match, equal, pass }) => { + process.on('warning', onWarning) + function onWarning (warn) { + equal(warn.code, 'PINODEP004') + } + + const stream = sink() + const parent = pino({ serializers: { test: () => 'parent' } }, stream) + const child = parent.child({ + serializers: { + test () { + pass('called') + return 'child' + } + } + }) + + const o = once(stream, 'data') + child.fatal({ test: 'test' }) + match(await o, { test: 'child' }) + process.removeListener('warning', onWarning) +}) + +test('child(bindings.formatters)', async ({ match, equal, pass }) => { + process.on('warning', onWarning) + function onWarning (warn) { + equal(warn.code, 'PINODEP005') + } + + const stream = sink() + const logger = pino({ + formatters: { + level (label, number) { + return { + log: { + level: label + } + } + }, + bindings (bindings) { + return { + process: { + pid: bindings.pid + }, + host: { + name: bindings.hostname + } + } + }, + log (obj) { + return { hello: 'world', ...obj } + } + } + }, stream) + + const child = logger.child({ + foo: 'bar', + nested: { object: true }, + formatters: { + bindings (bindings) { + pass('called') + return { ...bindings, faz: 'baz' } + } + } + }) + + const o = once(stream, 'data') + child.info('hello world') + match(await o, { + log: { + level: 'info' + }, + process: { + pid: process.pid + }, + host: { + name: hostname() + }, + hello: 'world', + foo: 'bar', + nested: { object: true }, + faz: 'baz' + }) + process.removeListener('warning', onWarning) +}) + +test('child(bindings.customLevels)', async ({ match, equal, pass }) => { + process.on('warning', onWarning) + function onWarning (warn) { + equal(warn.code, 'PINODEP006') + } + + const stream = sink() + const logger = pino(stream).child({ + childMsg: 'ok', + customLevels: { + foo: 35 + } + }, { + formatters: { + level (label, number) { + if (label === 'foo' && number === 35) { + pass('using customLevels') + } + return { level: number } + } + } + }) + + const o = once(stream, 'data') + logger.foo('test') + match(await o, { + level: 35, + childMsg: 'ok', + msg: 'test' + }) + process.removeListener('warning', onWarning) +}) + +test('child(bindings.level)', async ({ equal, pass }) => { + process.on('warning', onWarning) + function onWarning (warn) { + equal(warn.code, 'PINODEP007') + } + + const stream = sink() + const logger = pino({ + level: 'info' + }, stream).child({ + level: 'trace' + }) + + const o = once(stream, 'data') + logger.info('test') + if (await o === null) { + pass('child can overrid parent level') + } + process.removeListener('warning', onWarning) +}) diff --git a/test/formatters.test.js b/test/formatters.test.js index e5afb4d11..7385999db 100644 --- a/test/formatters.test.js +++ b/test/formatters.test.js @@ -170,7 +170,8 @@ test('Formatters in child logger', async ({ match }) => { const child = logger.child({ foo: 'bar', - nested: { object: true }, + nested: { object: true } + }, { formatters: { bindings (bindings) { return { ...bindings, faz: 'baz' } @@ -226,7 +227,8 @@ test('Formatters without bindings in child logger', async ({ match }) => { const child = logger.child({ foo: 'bar', - nested: { object: true }, + nested: { object: true } + }, { formatters: { log (obj) { return { other: 'stuff', ...obj } diff --git a/test/serializers.test.js b/test/serializers.test.js index af12d3a37..5655be743 100644 --- a/test/serializers.test.js +++ b/test/serializers.test.js @@ -70,7 +70,7 @@ test('undefined overrides default err namespace error serializer', async ({ equa test('serializers override values', async ({ equal }) => { const stream = sink() const parent = pino({ serializers: parentSerializers }, stream) - parent.child({ serializers: childSerializers }) + parent.child({}, { serializers: childSerializers }) parent.fatal({ test: 'test' }) const o = await once(stream, 'data') @@ -80,7 +80,7 @@ test('serializers override values', async ({ equal }) => { test('child does not overwrite parent serializers', async ({ equal }) => { const stream = sink() const parent = pino({ serializers: parentSerializers }, stream) - const child = parent.child({ serializers: childSerializers }) + const child = parent.child({}, { serializers: childSerializers }) parent.fatal({ test: 'test' }) @@ -99,7 +99,7 @@ test('Symbol.for(\'pino.serializers\')', async ({ equal, not }) => { equal(parent[Symbol.for('pino.serializers')], parentSerializers) equal(child[Symbol.for('pino.serializers')], parentSerializers) - const child2 = parent.child({ + const child2 = parent.child({}, { serializers: { a } @@ -133,7 +133,7 @@ test('children inherit parent Symbol serializers', async ({ equal, not }) => { equal(parent[Symbol.for('pino.serializers')], symbolSerializers) - const child = parent.child({ + const child = parent.child({}, { serializers: { [Symbol.for('a')]: a, a @@ -156,7 +156,7 @@ test('children serializers get called', async ({ equal }) => { test: 'this' }, stream) - const child = parent.child({ a: 'property', serializers: childSerializers }) + const child = parent.child({ a: 'property' }, { serializers: childSerializers }) child.fatal({ test: 'test' }) const o = await once(stream, 'data') @@ -170,7 +170,7 @@ test('children serializers get called when inherited from parent', async ({ equa serializers: parentSerializers }, stream) - const child = parent.child({ serializers: { test: function () { return 'pass' } } }) + const child = parent.child({}, { serializers: { test: function () { return 'pass' } } }) child.fatal({ test: 'fail' }) const o = await once(stream, 'data') @@ -191,7 +191,7 @@ test('non-overridden serializers are available in the children', async ({ equal const parent = pino({ serializers: pSerializers }, stream) - const child = parent.child({ serializers: cSerializers }) + const child = parent.child({}, { serializers: cSerializers }) const o = once(stream, 'data') child.fatal({ shared: 'test' })