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

logger.setBindings() doesn't clear logger[Symbol(pino.parsedChindings)] #890

Closed
okofish opened this issue Aug 11, 2020 · 3 comments · Fixed by #893
Closed

logger.setBindings() doesn't clear logger[Symbol(pino.parsedChindings)] #890

okofish opened this issue Aug 11, 2020 · 3 comments · Fixed by #893

Comments

@okofish
Copy link
Contributor

okofish commented Aug 11, 2020

#754 added a very useful logger.setBindings() method. But this doesn't play well with prettifiers, which appear to make use of a cached version of the bindings:

> let child=logger.child({foo: 123});

// pino.parsedChindings is undefined at start
> child.bindings()
{ foo: 123 }
> child[Pino.symbols.parsedChindingsSym]
undefined

// when the first log call is made, pino.parsedChindings gets populated
> child.info('hello')
[2020-08-11 23:12:27.349 +0000] INFO : hello
    foo: 123
> child[Pino.symbols.parsedChindingsSym]
{ pid: 16561, hostname: 'nutrimatic', foo: 123 }

// logger.setBindings doesn't update pino.parsedChindings
> child.setBindings({bar: 456})
> child.bindings()
{ foo: 123, bar: 456 }
> child[Pino.symbols.parsedChindingsSym]
{ pid: 16561, hostname: 'nutrimatic', foo: 123 }

// so subsequent logs don't include the new bindings
> child.info('hello')
[2020-08-11 23:13:24.553 +0000] INFO : hello
    foo: 123
> child[Pino.symbols.parsedChindingsSym]
{ pid: 16561, hostname: 'nutrimatic', foo: 123 }

// manually clearing pino.parsedChindings causes it to be regenerated on the next log call
> delete child[Pino.symbols.parsedChindingsSym]
> child.info('hello')
[2020-08-11 23:14:28.240 +0000] INFO : hello
    foo: 123
    bar: 456
> child[Pino.symbols.parsedChindingsSym]
{ pid: 16561, hostname: 'nutrimatic', foo: 123, bar: 456 }

Would it be "safe" (performance-wise) to have logger.setBindings() automatically clear pino.parserChindings so that the next log call causes it to update? I think this may be OK since said updating only happens when a prettifier is enabled, so it won't impact production situations where there's no prettifier. Alternatively, could we add an additional method that manually clears pino.parserChindings?

I can submit a PR for either of these approaches; I'm just not sure which is best.

@mcollina
Copy link
Member

Good spot! The first approach (clearing the cache on setBindings()) is perfect.

@okofish
Copy link
Contributor Author

okofish commented Aug 12, 2020

Great, I've submitted #893.

@github-actions
Copy link

github-actions bot commented Feb 5, 2022

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants