Repository: pinojs/pino
Pull request #1336 (2022-02-15 12:54:19)

add option `mixinMergeStrategy` (to v7)
Forward port of https://github.com/pinojs/pino/pull/1328 to v7

mcollina: PTAL for the changes that just landed, this conflicts now.
There was a prior bug that this PR is also solving.

Patch:
diff --git a/.github/workflows/target-main.yml b/.github/workflows/target-main.yml
new file mode 100644
index 0000000..3ca6f11
--- /dev/null
+++ b/.github/workflows/target-main.yml
@@ -0,0 +1,18 @@
+on:
+  pull_request_target:
+    types: [opened]
+
+jobs:
+  comment:
+    if: ${{ github.base_ref }} != "master"
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/github-script@v6
+        with:
+          script: |
+            github.rest.issues.createComment({
+              issue_number: context.issue.number,
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              body: '⚠️ This pull request does not target the master branch.'
+            })
diff --git a/docs/api.md b/docs/api.md
index ac6c451..3efe637 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -107,6 +107,7 @@ Default: `100`
 
 Option to limit stringification of properties/elements when logging a specific object/array with circular references.
 
+<a id="opt-mixin"></a>
 #### `mixin` (Function):
 
 Default: `undefined`
@@ -130,7 +131,8 @@ logger.info('world')
 ```
 
 The result of `mixin()` is supposed to be a _new_ object. For performance reason, the object returned by `mixin()` will be mutated by pino.
-In the following example, passing `mergingObject` argument to the first `info` call will mutate the global `mixin` object:
+In the following example, passing `mergingObject` argument to the first `info` call will mutate the global `mixin` object by default:
+(* See [`mixinMergeStrategy` option](#opt-mixin-merge-strategy)):
 ```js
 const mixin = {
     appName: 'My app'
@@ -154,6 +156,67 @@ logger.info('Message 2')
 If the `mixin` feature is being used merely to add static metadata to each log message,
 then a [child logger ⇗](/docs/child-loggers.md) should be used instead.
 
+<a id="opt-mixin-merge-strategy"></a>
+#### `mixinMergeStrategy` (Function):
+
+Default: `undefined`
+
+If provided, the `mixinMergeStrategy` function is called each time one of the active
+logging methods is called. The first parameter is the value `mergeObject` or an empty object,
+the second parameter is the value resulting from `mixin()` (* See [`mixin` option](#opt-mixin) or an empty object.
+The function must synchronously return an object.
+
+```js
+// Default strategy, `mergeObject` has priority
+const logger = pino({
+    mixin() {
+        return { tag: 'docker' }
+    },
+    // mixinMergeStrategy(mergeObject, mixinObject) {
+    //     return Object.assign(mixinMeta, mergeObject)
+    // }
+})
+
+logger.info({
+  tag: 'local'
+}, 'Message')
+// {"level":30,"time":1591195061437,"pid":16012,"hostname":"x","tag":"local","msg":"Message"}
+```
+
+```js
+// Custom mutable strategy, `mixin` has priority
+const logger = pino({
+    mixin() {
+        return { tag: 'k8s' }
+    },
+    mixinMergeStrategy(mergeObject, mixinObject) {
+        return Object.assign(mergeObject, mixinObject)
+    }
+})
+
+logger.info({
+    tag: 'local'
+}, 'Message')
+// {"level":30,"time":1591195061437,"pid":16012,"hostname":"x","tag":"k8s","msg":"Message"}
+```
+
+```js
+// Custom immutable strategy, `mixin` has priority
+const logger = pino({
+    mixin() {
+        return { tag: 'k8s' }
+    },
+    mixinMergeStrategy(mergeObject, mixinObject) {
+        return Object.assign({}, mergeObject, mixinObject)
+    }
+})
+
+logger.info({
+    tag: 'local'
+}, 'Message')
+// {"level":30,"time":1591195061437,"pid":16012,"hostname":"x","tag":"k8s","msg":"Message"}
+```
+
 <a id="opt-redact"></a>
 #### `redact` (Array | Object):
 
diff --git a/lib/proto.js b/lib/proto.js
index 1eca123..f51e5d7 100644
--- a/lib/proto.js
+++ b/lib/proto.js
@@ -13,6 +13,7 @@ const {
   mixinSym,
   asJsonSym,
   writeSym,
+  mixinMergeStrategySym,
   timeSym,
   timeSliceIndexSym,
   streamSym,
@@ -158,9 +159,22 @@ function setBindings (newBindings) {
   delete this[parsedChindingsSym]
 }
 
+/**
+ * Default strategy for creating `mergeObject` from arguments and the result from `mixin()`.
+ * Fields from `mergeObject` have higher priority in this strategy.
+ *
+ * @param {Object} mergeObject The object a user has supplied to the logging function.
+ * @param {Object} mixinObject The result of the `mixin` method.
+ * @return {Object}
+ */
+function defaultMixinMergeStrategy (mergeObject, mixinObject) {
+  return Object.assign(mixinObject, mergeObject)
+}
+
 function write (_obj, msg, num) {
   const t = this[timeSym]()
   const mixin = this[mixinSym]
+  const mixinMergeStrategy = this[mixinMergeStrategySym] || defaultMixinMergeStrategy
   let obj
 
   if (_obj === undefined || _obj === null) {
@@ -178,7 +192,7 @@ function write (_obj, msg, num) {
   }
 
   if (mixin) {
-    obj = Object.assign(mixin(obj), obj)
+    obj = mixinMergeStrategy(obj, mixin(obj))
   }
 
   const s = this[asJsonSym](obj, msg, num, t)
diff --git a/lib/symbols.js b/lib/symbols.js
index 2845cba..75195ff 100644
--- a/lib/symbols.js
+++ b/lib/symbols.js
@@ -26,6 +26,7 @@ const formatOptsSym = Symbol('pino.formatOpts')
 const messageKeySym = Symbol('pino.messageKey')
 const nestedKeySym = Symbol('pino.nestedKey')
 const nestedKeyStrSym = Symbol('pino.nestedKeyStr')
+const mixinMergeStrategySym = Symbol('pino.mixinMergeStrategy')
 
 const wildcardFirstSym = Symbol('pino.wildcardFirst')
 
@@ -64,5 +65,6 @@ module.exports = {
   useOnlyCustomLevelsSym,
   formattersSym,
   hooksSym,
-  nestedKeyStrSym
+  nestedKeyStrSym,
+  mixinMergeStrategySym
 }
diff --git a/pino.d.ts b/pino.d.ts
index 52d7bd4..31efc4f 100644
--- a/pino.d.ts
+++ b/pino.d.ts
@@ -31,7 +31,8 @@ import type { WorkerOptions } from "worker_threads";
 type ThreadStream = any
 
 type TimeFn = () => string;
-type MixinFn = () => object;
+type MixinFn = (mergeObject: object) => object;
+type MixinMergeStrategyFn = (mergeObject: object, mixinObject: object) => object;
 
 type CustomLevelLogger<Options> = Options extends { customLevels: Record<string, number> } ? Record<keyof Options["customLevels"], LogFn> : Record<never, LogFn>
 
@@ -112,7 +113,7 @@ interface LoggerExtras<Options = LoggerOptions> extends EventEmitter {
 
 declare namespace pino {
     //// Exported types and interfaces
-    
+
     interface BaseLogger {
         /**
          * Set this property to the desired logging level. In order of priority, available levels are:
@@ -130,7 +131,7 @@ declare namespace pino {
          * You can pass `'silent'` to disable logging.
          */
         level: pino.LevelWithSilent | string;
-    
+
         /**
          * Log at `'fatal'` level the given msg. If the first argument is an object, all its properties will be included in the JSON line.
          * If more args follows `msg`, these will be used to format `msg` using `util.format`.
@@ -204,14 +205,14 @@ declare namespace pino {
 
     type SerializerFn = (value: any) => any;
     type WriteFn = (o: object) => void;
-    
+
     type LevelChangeEventListener = (
         lvl: LevelWithSilent | string,
         val: number,
         prevLvl: LevelWithSilent | string,
         prevVal: number,
     ) => void;
-    
+
     type LogDescriptor = Record<string, any>;
 
     type Logger<Options = LoggerOptions> = BaseLogger & LoggerExtras<Options> & CustomLevelLogger<Options>;
@@ -336,6 +337,14 @@ declare namespace pino {
          */
         mixin?: MixinFn;
 
+        /**
+         * If provided, the `mixinMergeStrategy` function is called each time one of the active
+         * logging methods is called. The first parameter is the value `mergeObject` or an empty object,
+         * the second parameter is the value resulting from `mixin()` or an empty object.
+         * The function must synchronously return an object.
+         */
+        mixinMergeStrategy?: MixinMergeStrategyFn
+
         /**
          * As an array, the redact option specifies paths that should have their values redacted from any log output.
          *
@@ -671,12 +680,12 @@ declare namespace pino {
         readonly formattersSym: unique symbol;
         readonly hooksSym: unique symbol;
     };
- 
+
     /**
      * Exposes the Pino package version. Also available on the logger instance.
      */
     export const version: string;
- 
+
     /**
      * Provides functions for generating the timestamp property in the log output. You can set the `timestamp` option during
      * initialization to one of these functions to adjust the output format. Alternatively, you can specify your own time function.
@@ -701,7 +710,7 @@ declare namespace pino {
             */
         isoTime: TimeFn;
     };
- 
+
     //// Exported functions
 
     /**
@@ -755,7 +764,7 @@ declare function pino<Options extends LoggerOptions | DestinationStream>(options
  * @returns a new logger instance.
  */
 declare function pino<Options extends LoggerOptions>(options: Options, stream: DestinationStream): Logger<Options>;
- 
+
 
 // Pass through all the top-level exports, allows `import {version} from "pino"`
 // Constants and functions
diff --git a/pino.js b/pino.js
index 315e8d4..2afe6d2 100644
--- a/pino.js
+++ b/pino.js
@@ -39,7 +39,8 @@ const {
   useOnlyCustomLevelsSym,
   formattersSym,
   hooksSym,
-  nestedKeyStrSym
+  nestedKeyStrSym,
+  mixinMergeStrategySym
 } = symbols
 const { epochTime, nullTime } = time
 const { pid } = process
@@ -94,6 +95,7 @@ function pino (...args) {
     level,
     customLevels,
     mixin,
+    mixinMergeStrategy,
     useOnlyCustomLevels,
     formatters,
     hooks,
@@ -166,6 +168,7 @@ function pino (...args) {
     [nestedKeyStrSym]: nestedKey ? `,${JSON.stringify(nestedKey)}:{` : '',
     [serializersSym]: serializers,
     [mixinSym]: mixin,
+    [mixinMergeStrategySym]: mixinMergeStrategy,
     [chindingsSym]: chindings,
     [formattersSym]: allFormatters,
     [hooksSym]: hooks,


Test patch:
diff --git a/test/mixin-merge-strategy.test.js b/test/mixin-merge-strategy.test.js
new file mode 100644
index 0000000..3c6dfe4
--- /dev/null
+++ b/test/mixin-merge-strategy.test.js
@@ -0,0 +1,55 @@
+'use strict'
+
+const { test } = require('tap')
+const { sink, once } = require('./helper')
+const pino = require('../')
+
+const level = 50
+const name = 'error'
+
+test('default merge strategy', async ({ ok, same }) => {
+  const stream = sink()
+  const instance = pino({
+    base: {},
+    mixin () {
+      return { tag: 'k8s' }
+    }
+  }, stream)
+  instance.level = name
+  instance[name]({
+    tag: 'local'
+  }, 'test')
+  const result = await once(stream, 'data')
+  ok(new Date(result.time) <= new Date(), 'time is greater than Date.now()')
+  delete result.time
+  same(result, {
+    level,
+    msg: 'test',
+    tag: 'local'
+  })
+})
+
+test('custom merge strategy with mixin priority', async ({ ok, same }) => {
+  const stream = sink()
+  const instance = pino({
+    base: {},
+    mixin () {
+      return { tag: 'k8s' }
+    },
+    mixinMergeStrategy (mergeObject, mixinObject) {
+      return Object.assign(mergeObject, mixinObject)
+    }
+  }, stream)
+  instance.level = name
+  instance[name]({
+    tag: 'local'
+  }, 'test')
+  const result = await once(stream, 'data')
+  ok(new Date(result.time) <= new Date(), 'time is greater than Date.now()')
+  delete result.time
+  same(result, {
+    level,
+    msg: 'test',
+    tag: 'k8s'
+  })
+})