refactor: deprecate child bindings apply options (#1067)

* refactor: deprecate child bindings apply options * chore: remove todo comment * refactor: deprecate bindings.level
pinojs · Jul 12, 2021 · 946f630 · 946f630
1 parent dbb828a
commit 946f630
Show file tree

Hide file tree

Showing 9 changed files with 258 additions and 56 deletions.
diff --git 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
@@ -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
@@ -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)
+
 <a id="bindings"></a>
 ### `logger.bindings()`
 

diff --git 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
@@ -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
@@ -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 }
     })
   })