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 support to hide metadata in log line #445

Closed
wants to merge 10 commits into from
2 changes: 2 additions & 0 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ node app.js | pino-pretty
- `--hideObject` (`-H`): Hide objects from output (but not error object)
- `--singleLine` (`-S`): Print each log message on a single line (errors will still be multi-line)
- `--config`: Specify a path to a config file containing the pino-pretty options. pino-pretty will attempt to read from a `.pino-prettyrc` in your current directory (`process.cwd`) if not specified
- `--hideMetadata` (`-M`): Hide Metadata (e.g. `timestamp` and `level`) in the beginning of each log line

<a id="integration"></a>
## Programmatic Integration
Expand Down Expand Up @@ -258,6 +259,7 @@ The options accepted have keys corresponding to the options described in [CLI Ar
levelLabel: 'levelLabel', // --levelLabel
minimumLevel: 'info', // --minimumLevel
useOnlyCustomProps: true, // --useOnlyCustomProps
hideMetadata: true, // --hideMetadata
// The file or file descriptor (1 is stdout) to write to
destination: 1,

Expand Down
3 changes: 2 additions & 1 deletion bin.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ let opts = minimist(process.argv, {
ignore: 'i',
include: 'I',
hideObject: 'H',
singleLine: 'S'
singleLine: 'S',
hideMetadata: 'M'
},
default: {
messageKey: DEFAULT_VALUE,
Expand Down
76 changes: 44 additions & 32 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ const defaultOptions = {
hideObject: false,
ignore: 'hostname',
include: undefined,
singleLine: false
singleLine: false,
hideMetadata: false
}

function prettyFactory (options) {
Expand Down Expand Up @@ -127,39 +128,10 @@ function prettyFactory (options) {
log = filterLog({ log, ignoreKeys, includeKeys })
}

const prettifiedLevel = prettifyLevel({ log, colorizer, levelKey, prettifier: customPrettifiers.level, ...customProps })
const prettifiedMetadata = prettifyMetadata({ log, prettifiers: customPrettifiers })
const prettifiedTime = prettifyTime({ log, translateFormat: opts.translateTime, timestampKey, prettifier: customPrettifiers.time })

let line = ''
if (opts.levelFirst && prettifiedLevel) {
line = `${prettifiedLevel}`
}

if (prettifiedTime && line === '') {
line = `${prettifiedTime}`
} else if (prettifiedTime) {
line = `${line} ${prettifiedTime}`
}

if (!opts.levelFirst && prettifiedLevel) {
if (line.length > 0) {
line = `${line} ${prettifiedLevel}`
} else {
line = prettifiedLevel
}
}

if (prettifiedMetadata) {
if (line.length > 0) {
line = `${line} ${prettifiedMetadata}:`
} else {
line = prettifiedMetadata
}
}

if (line.endsWith(':') === false && line !== '') {
line += ':'
if (!opts.hideMetadata) {
Copy link
Member

Choose a reason for hiding this comment

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

Can this be done in some other way than continually nesting the core code of this function further and further to the right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have something in mind. I could see some options. One with higher refactoring effort like using strategy pattern (https://refactoring.guru/design-patterns/strategy) and some with lower effort like using switch statements/ lookup tables.

Copy link
Member

@jsumners jsumners Jul 30, 2023

Choose a reason for hiding this comment

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

I think a simple approach would be to move the now optional code path into a function. Note, such a function should return a string, not mutate one through a side effect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an approach

line += createLineMetadata()
}

if (prettifiedMessage !== undefined) {
Expand Down Expand Up @@ -206,6 +178,46 @@ function prettyFactory (options) {
}

return line

function createLineMetadata () {
Copy link
Member

Choose a reason for hiding this comment

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

Does this function need to be defined as an inner function of prettyFactory? Or can we put it in lib/utils?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could put it in lib/utils but would have to pass many parameters like log, colorizer, levelKey etc. Think i would prefer to leave it in index.js. What do you think about the naming?

Copy link
Member

Choose a reason for hiding this comment

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

My concern is legibility and maintainability. The inlined function is easier to read than the nested conditional version, but it is still a lot of code deeply nested within another function.

I think the function name is fine.

Copy link
Member

Choose a reason for hiding this comment

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

@timlohse1104 I think you'll find it much easier to accommodate the ask now that #453 has been merged. Unfortunately, it'll be easier to start with a fresh PR. Rebasing this one may be too difficult.

If you have any questions, don't hesitate to ask.

let lineMetadata = ''

const prettifiedLevel = prettifyLevel({ log, colorizer, levelKey, prettifier: customPrettifiers.level, ...customProps })
const prettifiedMetadata = prettifyMetadata({ log, prettifiers: customPrettifiers })
const prettifiedTime = prettifyTime({ log, translateFormat: opts.translateTime, timestampKey, prettifier: customPrettifiers.time })

if (opts.levelFirst && prettifiedLevel) {
lineMetadata = `${prettifiedLevel}`
}

if (prettifiedTime && lineMetadata === '') {
lineMetadata = `${prettifiedTime}`
} else if (prettifiedTime) {
lineMetadata = `${lineMetadata} ${prettifiedTime}`
}

if (!opts.levelFirst && prettifiedLevel) {
if (lineMetadata.length > 0) {
lineMetadata = `${lineMetadata} ${prettifiedLevel}`
} else {
lineMetadata = prettifiedLevel
}
}

if (prettifiedMetadata) {
if (lineMetadata.length > 0) {
lineMetadata = `${lineMetadata} ${prettifiedMetadata}:`
} else {
lineMetadata = prettifiedMetadata
}
}

if (lineMetadata.endsWith(':') === false && lineMetadata !== '') {
lineMetadata += ':'
}

return lineMetadata
}
}
}

Expand Down
20 changes: 20 additions & 0 deletions test/basic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,26 @@ test('basic prettifier tests', (t) => {
log.info('foo')
})

t.test('can hide date and level', (t) => {
t.plan(1)
const destination = new Writable({
write (formatted, enc, cb) {
t.equal(
formatted.toString(),
'foo\n'
)
cb()
}
})
const pretty = pinoPretty({
destination,
hideMetadata: true,
colorize: false
})
const log = pino({}, pretty)
log.info('foo')
})

t.test('can print message key value when its a string', (t) => {
t.plan(1)
const pretty = prettyFactory()
Expand Down
13 changes: 13 additions & 0 deletions test/cli.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,19 @@ test('cli', (t) => {
})
})

;['--hideMetadata', '-M'].forEach((optionName) => {
t.test(`hides epoch, level and metadata via ${optionName}`, (t) => {
t.plan(1)
const child = spawn(process.argv[0], [bin, optionName], { env })
child.on('error', t.threw)
child.stdout.on('data', (data) => {
t.equal(data.toString(), 'hello world\n')
})
child.stdin.write(logLine)
t.teardown(() => child.kill())
})
})

;['--translateTime', '-t'].forEach((optionName) => {
t.test(`translates time to default format via ${optionName}`, (t) => {
t.plan(1)
Expand Down