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

Added onChild callback with tests and documentation #1541

Merged
merged 6 commits into from Sep 5, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/api.md
Expand Up @@ -508,6 +508,18 @@ when using the `transport` option. In this case an `Error` will be thrown.

* See [pino.transport()](#pino-transport)

#### `onChild` (Function)

The `onChild` function is a callback that will be called on each creation of a new child and use the child instance as it's first argument.
Diabl0269 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the callback block child creation? Do we handle any errors that occur within the callback?

Copy link
Contributor Author

@Diabl0269 Diabl0269 Aug 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The callback doesn't block the child creation, unless the it throws an error.
Should we wrap the callback in a try-catch block?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the way the callback is handled should be made clear in the documentation. Adding a try/catch during child creation will have a performance penalty. So we would want to alter the documentation to make it clear that any error in the callback will be uncaught.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. I've updated the documentation to mention the callback doesn't handle errors.

```js
const parent = require('pino')({ onChild: (instance) => {
// Exceute call back code for each newly created child.
}})
// `onChild` will now be executed with the new child.
parent.child(bindings)
```


<a id="destination"></a>
### `destination` (SonicBoom | WritableStream | String | Object)

Expand Down
2 changes: 1 addition & 1 deletion lib/proto.js
Expand Up @@ -141,7 +141,7 @@ function child (bindings, options) {
instance[chindingsSym] = asChindings(instance, bindings)
const childLevel = options.level || this.level
instance[setLevelSym](childLevel)

this.onChild(instance)
return instance
}

Expand Down
3 changes: 2 additions & 1 deletion lib/tools.js
Expand Up @@ -301,8 +301,9 @@ function createArgsNormalizer (defaultOptions) {
if ('onTerminated' in opts) {
throw Error('The onTerminated option has been removed, use pino.final instead')
}
const { enabled } = opts
const { enabled, onChild } = opts
if (enabled === false) opts.level = 'silent'
if (!onChild) opts.onChild = noop
if (!stream) {
if (!hasBeenTampered(process.stdout)) {
// If process.stdout.fd is undefined, it means that we are running
Expand Down
16 changes: 16 additions & 0 deletions pino.d.ts
Expand Up @@ -36,6 +36,12 @@ type MixinMergeStrategyFn = (mergeObject: object, mixinObject: object) => object

type CustomLevelLogger<Options> = Options extends { customLevels: Record<string, number> } ? Record<keyof Options["customLevels"], LogFn> : Record<never, LogFn>

/**
* A callback that will run on each creation of a new child.
* @param child: The newly created child logger instance.
*/
type OnChildCallback<Options = LoggerOptions> = <ChildOptions extends pino.ChildLoggerOptions>(child: pino.Logger<Options & ChildOptions>) => void

interface redactOptions {
paths: string[];
censor?: string | ((value: any, path: string[]) => any);
Expand Down Expand Up @@ -79,6 +85,11 @@ interface LoggerExtras<Options = LoggerOptions> extends EventEmitter {
*/
child<ChildOptions extends pino.ChildLoggerOptions>(bindings: pino.Bindings, options?: ChildOptions): pino.Logger<Options & ChildOptions>;

/**
* This can be used to modify the callback function on creation of a new child.
*/
onChild: OnChildCallback<Options>;

/**
* Registers a listener function that is triggered when the level is changed.
* Note: When browserified, this functionality will only be available if the `events` module has been required elsewhere
Expand Down Expand Up @@ -601,6 +612,11 @@ declare namespace pino {
* Stringification limit of properties/elements when logging a specific object/array with circular references. Default: `100`.
*/
edgeLimit?: number

/**
* Optional child creation callback.
*/
onChild?: OnChildCallback;
}

interface ChildLoggerOptions {
Expand Down
6 changes: 4 additions & 2 deletions pino.js
Expand Up @@ -102,7 +102,8 @@ function pino (...args) {
formatters,
hooks,
depthLimit,
edgeLimit
edgeLimit,
onChild
} = opts

const stringifySafe = configure({
Expand Down Expand Up @@ -175,7 +176,8 @@ function pino (...args) {
[chindingsSym]: chindings,
[formattersSym]: allFormatters,
[hooksSym]: hooks,
silent: noop
silent: noop,
onChild
})

Object.setPrototypeOf(instance, proto())
Expand Down
10 changes: 10 additions & 0 deletions test/basic.test.js
Expand Up @@ -725,3 +725,13 @@ test('throws if prettyPrint is passed in as an option', async (t) => {
})
}, new Error('prettyPrint option is no longer supported, see the pino-pretty package (https://github.com/pinojs/pino-pretty)'))
})

test('Should invoke `onChild` with the newly created child', async ({ equal }) => {
let innerChild
const child = pino({
onChild: (instance) => {
innerChild = instance
}
}).child({ foo: 'bar' })
equal(child, innerChild)
jsumners marked this conversation as resolved.
Show resolved Hide resolved
})
5 changes: 5 additions & 0 deletions test/types/pino.test-d.ts
Expand Up @@ -340,3 +340,8 @@ cclog3.myLevel('')
cclog3.childLevel('')
// child itself
cclog3.childLevel2('')

const withChildCallback = pino({
onChild: (child: Logger) => {}
})
withChildCallback.onChild = (child: Logger) => {}