-
Notifications
You must be signed in to change notification settings - Fork 0
timestamp property is silently removed #1069
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
Comments
The issue with renaming the field is that we'd need some logic that'll be annoying to maintain. What if that field is already set? Do we then have to warn that it's been overwritten or do we not set the timestamp if that's the case? I think it's worth investigating whether there's a way we can stop Pino overwriting this property (if that's what's happening) |
This is a smaller issue than I originally thought. The timestamp is only stripped when using pino-pretty, because it has to make a decision about whether to use the The test in this PR illustrates that, in production, the timestamp is still present. Do we still want to do anything about this? It's a little confusing but possibly enough of an edge-case to ignore (anyone having the same issue in future will hopefully find this issue). @apaleslimghost, @alexmuller, thoughts? |
hahahaha okay wow. yeah let's close this |
Reported by Kara in #cp-reliability.
Node.js version: v20.10.0
Impacted package(s): logger@3.1.1 (latest)
Steps to reproduce
Expected behaviour
Dunno, maybe a warning that whatever you put in the timestamp property is gone forever?
Or (suggestion from Kara) we also rename that field to something else to keep it.
The text was updated successfully, but these errors were encountered: