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

add transports constructor option #1111

Merged
merged 10 commits into from Sep 8, 2021
Merged
Show file tree
Hide file tree
Changes from 7 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
27 changes: 27 additions & 0 deletions docs/api.md
Expand Up @@ -367,6 +367,33 @@ documented in the [Browser API ⇗](/docs/browser.md) documentation.

* See [Browser API ⇗](/docs/browser.md)

#### `transport` (Object)

The `transport` option is a shortcut to the [pino.transport()](#pino-transport) function.
Eomm marked this conversation as resolved.
Show resolved Hide resolved
It supports the same input options:
```js
require('pino')({
Copy link
Member

Choose a reason for hiding this comment

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

do we do this require('pino') inline thing elsewhere in the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are 18 occurrences of require('pino') in this file actually: would you like to migrate the examples to pino?

Copy link
Member

Choose a reason for hiding this comment

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

Do they assign the result of require or do they use it directly like this example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both declaration tbh

https://github.com/pinojs/pino/blob/5333d73773f59331c2d5c7962dec3ee00574fc6f/docs/api.md#destination-sonicboom--writablestream--string--object

I can update this example and the other ones if you tell me what do you prefer.
Is the cjs syntax not up to date for users?

transport: {
target: '/absolute/path/to/my-transport.mjs'
Eomm marked this conversation as resolved.
Show resolved Hide resolved
}
})

// or multiple transports
require('pino')({
transport: {
targets: [
{ target: '/absolute/path/to/my-transport.mjs', level: 'error' },
{ target: 'some-file-transport', options: { destination: '/dev/null' }
]
}
})
```

It is not possible to provide an additional [`destination`](#destination) argument
Eomm marked this conversation as resolved.
Show resolved Hide resolved
when using the `transport` option. In this case an `Error` will be thrown.

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

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

Expand Down
7 changes: 7 additions & 0 deletions lib/tools.js
Expand Up @@ -26,6 +26,7 @@ const {
nestedKeyStrSym
} = require('./symbols')
const { isMainThread } = require('worker_threads')
const transport = require('./transport')

function noop () {}

Expand Down Expand Up @@ -394,10 +395,16 @@ function createArgsNormalizer (defaultOptions) {
stream = buildSafeSonicBoom({ dest: opts, sync: true })
opts = {}
} else if (typeof stream === 'string') {
if (opts && opts.transport) {
throw Error('Only one of option.transport or stream can be specified')
Eomm marked this conversation as resolved.
Show resolved Hide resolved
}
stream = buildSafeSonicBoom({ dest: stream, sync: true })
} else if (opts instanceof SonicBoom || opts.writable || opts._writableState) {
stream = opts
opts = null
} else if (opts.transport) {
stream = transport(opts.transport)
opts = null
}
opts = Object.assign({}, defaultOptions, opts)
opts.serializers = Object.assign({}, defaultOptions.serializers, opts.serializers)
Expand Down
92 changes: 92 additions & 0 deletions test/transport.test.js
Expand Up @@ -380,3 +380,95 @@ test('stdout in worker', async ({ not }) => {
await once(child, 'close')
not(strip(actual).match(/Hello/), null)
})

test('pino transport options with target', async ({ teardown, same }) => {
const destination = join(
os.tmpdir(),
'_' + Math.random().toString(36).substr(2, 9)
)
const instance = pino({
transport: {
target: '#pino/file',
options: { destination }
}
})
const transportStream = instance[pino.symbols.streamSym]
teardown(transportStream.end.bind(transportStream))
instance.info('transport option test')
await watchFileCreated(destination)
const result = JSON.parse(await readFile(destination))
delete result.time
same(result, {
pid,
hostname,
level: 30,
msg: 'transport option test'
})
})

test('pino transport options with targets', async ({ teardown, same }) => {
const dest1 = join(
os.tmpdir(),
'_' + Math.random().toString(36).substr(2, 9)
)
const dest2 = join(
os.tmpdir(),
'_' + Math.random().toString(36).substr(2, 9)
)
const instance = pino({
transport: {
targets: [
{ target: '#pino/file', options: { destination: dest1 } },
{ target: '#pino/file', options: { destination: dest2 } }
]
}
})
const transportStream = instance[pino.symbols.streamSym]
teardown(transportStream.end.bind(transportStream))
instance.info('transport option test')

await Promise.all([watchFileCreated(dest1), watchFileCreated(dest2)])
const result1 = JSON.parse(await readFile(dest1))
delete result1.time
same(result1, {
pid,
hostname,
level: 30,
msg: 'transport option test'
})
const result2 = JSON.parse(await readFile(dest2))
delete result2.time
same(result2, {
pid,
hostname,
level: 30,
msg: 'transport option test'
})
})

test('transport options with target and targets', async ({ fail, equal }) => {
try {
pino({
transport: {
target: {},
targets: {}
}
})
fail('must throw')
} catch (err) {
equal(err.message, 'Only one of target or targets can be specified')
Eomm marked this conversation as resolved.
Show resolved Hide resolved
}
})

test('transport options with target and stream', async ({ fail, equal }) => {
try {
pino({
transport: {
target: {}
}
}, '/log/null')
fail('must throw')
} catch (err) {
equal(err.message, 'Only one of option.transport or stream can be specified')
Eomm marked this conversation as resolved.
Show resolved Hide resolved
}
})