Add truncation to all serializers #143
Conversation
🦋 Changeset detectedLatest commit: 5cae60c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
src/serializers/types.ts
Outdated
| @@ -1 +1,2 @@ | |||
| export type SerializerFn = (input: unknown) => unknown; | |||
| export type TrimmerFn = (input: unknown) => unknown; | |||
| export type SerializerFn<T> = (input: T) => unknown; | |||
There was a problem hiding this comment.
Not convinced with this one because of the different placed I had to change 😬
| @@ -1,6 +1,8 @@ | |||
| import { trimmer } from 'dtrim'; | |||
| import type { LoggerOptions } from 'pino'; | |||
|
|
|||
| export const DEFAULT_MAX_OBJECT_DEPTH = 4; | |||
There was a problem hiding this comment.
Looking for suggestions on this default value, given this potential usecase.
| headers: { | ||
| Accept: 'application/json, text/plain, */*', | ||
| authorization: '[Redacted]', | ||
| }, |
There was a problem hiding this comment.
This is now truncated as a result of fixing the max object depth.
There was a problem hiding this comment.
Random stream of consciousness thought, do we know if the trimming impacts a potentially deep err.stack trace? Perhaps we could pull some (non-sensitive) ones out of Splunk and have a test case around it? The stack trace tends to be the highest value field in an error log, so retaining a long stack may be a safer default.
There was a problem hiding this comment.
I've excluded the stack field from being trimmed, though the sharing of a trimmer between serializers means that its not just the top level fields of err and errWithCause that are affected by this exclusion.
There was a problem hiding this comment.
I was poking around with this, do you have thoughts on 7b1f2a1?
There was a problem hiding this comment.
Sure do that's way cleaner I like it!!
Cleaned up and condensed all the bits of spread out logic that I implemented and didn't like 😆 🙏
There was a problem hiding this comment.
Thanks for the change, refactoring is the easy part 😉
src/index.ts
Outdated
| const trim = trimmer({ | ||
| depth: (opts.maxObjectDepth ?? DEFAULT_MAX_OBJECT_DEPTH) - 1, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Am torn between sharing a trimmer like this or just re-declaring for both default and custom serializers. 🤔
| headers: { | ||
| Accept: 'application/json, text/plain, */*', | ||
| authorization: '[Redacted]', | ||
| }, |
There was a problem hiding this comment.
Thanks for the change, refactoring is the easy part 😉
Co-authored-by: Ryan Ling <[email protected]>
Purpose
Adds changes and refactors based on suggestions from #141
Approach
stackfield from trimming inerranderrWithCauseserializersmaxObjectDepthonce