-
Notifications
You must be signed in to change notification settings - Fork 3
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 truncation to all serializers #143
Changes from all commits
cfe5782
48d4362
193223c
453ae68
1769348
7b1f2a1
cdbae61
5cae60c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
--- | ||
'@seek/logger': major | ||
--- | ||
|
||
Apply trimming to serializers | ||
|
||
Previously, [built-in serializers](https://github.com/seek-oss/logger/tree/54f16e17a9bb94261b9d2e4b77f04f55d5a3ab4c?tab=readme-ov-file#standardised-fields) and custom ones supplied via the [`serializers` option](https://github.com/pinojs/pino/blob/8aafa88139890b97aca0d32601cb5ffdd9bda1eb/docs/api.md#serializers-object) were not subject to [trimming](https://github.com/seek-oss/logger/tree/54f16e17a9bb94261b9d2e4b77f04f55d5a3ab4c?tab=readme-ov-file#trimming). This caused some emitted error logs to be extremely large. | ||
|
||
Now, trimming is applied across all serializers by default. If you rely on deeply nested `err` properties to troubleshoot your application, tune the `maxObjectDepth` configured on your logger. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -441,10 +441,7 @@ testLog( | |
request: { | ||
_options: { | ||
method: 'get', | ||
headers: { | ||
Accept: 'application/json, text/plain, */*', | ||
authorization: '[Redacted]', | ||
}, | ||
Comment on lines
-444
to
-447
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is now truncated as a result of fixing the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Random stream of consciousness thought, do we know if the trimming impacts a potentially deep There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've excluded the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was poking around with this, do you have thoughts on 7b1f2a1? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure do that's way cleaner I like it!! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the change, refactoring is the easy part 😉 |
||
headers: '[Object]', | ||
}, | ||
domain: null, | ||
}, | ||
|
@@ -632,48 +629,36 @@ test('should log customized timestamp if timestamp logger option is supplied', a | |
expect(log.timestamp).toBe(mockTimestamp); | ||
}); | ||
|
||
class Req { | ||
get socket() { | ||
return { | ||
remoteAddress: 'localhost', | ||
remotePort: '4000', | ||
}; | ||
} | ||
} | ||
testLog( | ||
'should not truncate objects with a non error serializer', | ||
'should trim default serializers', | ||
{ | ||
req: new Req(), | ||
notSerialized: { | ||
errWithCause: { | ||
a: { | ||
b: {}, | ||
}, | ||
anyField: 'a'.repeat(555), | ||
stack: 'a'.repeat(555), | ||
}, | ||
}, | ||
{ | ||
req: { | ||
remoteAddress: 'localhost', | ||
remotePort: '4000', | ||
}, | ||
notSerialized: { | ||
a: '[Object]', | ||
}, | ||
}, | ||
'info', | ||
{ | ||
maxObjectDepth: 2, | ||
}, | ||
); | ||
|
||
testLog( | ||
'should truncate objects with error serializers', | ||
{ | ||
errWithCause: { | ||
err: { | ||
a: { | ||
b: {}, | ||
}, | ||
anyField: 'a'.repeat(555), | ||
stack: 'a'.repeat(555), | ||
}, | ||
err: { | ||
req: { | ||
method: 'GET', | ||
url: 'a'.repeat(555), | ||
headers: [], | ||
socket: { remoteAddress: 'localhost', remotePort: '4000' }, | ||
}, | ||
res: { | ||
headers: { Origin: 'a'.repeat(555) }, | ||
status: 500, | ||
foo: 'baz', | ||
}, | ||
headers: { | ||
'test-header': 'a'.repeat(555), | ||
a: { | ||
b: {}, | ||
}, | ||
|
@@ -684,44 +669,66 @@ testLog( | |
a: { | ||
b: '[Object]', | ||
}, | ||
anyField: `${'a'.repeat(512)}...`, | ||
stack: 'a'.repeat(555), | ||
}, | ||
err: { | ||
a: { | ||
b: '[Object]', | ||
}, | ||
anyField: `${'a'.repeat(512)}...`, | ||
stack: 'a'.repeat(555), | ||
}, | ||
req: { | ||
method: 'GET', | ||
url: `${'a'.repeat(512)}...`, | ||
headers: [], | ||
remoteAddress: 'localhost', | ||
remotePort: '4000', | ||
}, | ||
res: { | ||
headers: { Origin: `${'a'.repeat(512)}...` }, | ||
statusCode: 500, | ||
}, | ||
headers: { | ||
'test-header': `${'a'.repeat(512)}...`, | ||
a: { | ||
b: '[Object]', | ||
}, | ||
}, | ||
}, | ||
'info', | ||
{ | ||
maxObjectDepth: 2, | ||
maxObjectDepth: 3, | ||
}, | ||
); | ||
|
||
testLog( | ||
'should truncate strings longer than 512 characters with error serializers', | ||
'should trim custom serializer', | ||
{ | ||
err: { | ||
anyField: { | ||
anyField: 'a'.repeat(555), | ||
}, | ||
}, | ||
errWithCause: { | ||
anyField: { | ||
anyField: 'a'.repeat(555), | ||
serialize: { | ||
a: { | ||
b: { | ||
c: {}, | ||
}, | ||
}, | ||
anyField: 'a'.repeat(555), | ||
}, | ||
}, | ||
{ | ||
err: { | ||
anyField: { | ||
anyField: `${'a'.repeat(512)}...`, | ||
serialize: { | ||
a: { | ||
b: '[Object]', | ||
}, | ||
anyField: `${'a'.repeat(512)}...`, | ||
}, | ||
errWithCause: { | ||
anyField: { | ||
anyField: `${'a'.repeat(512)}...`, | ||
}, | ||
}, | ||
'info', | ||
{ | ||
serializers: { | ||
serialize: (input: unknown) => input, | ||
}, | ||
maxObjectDepth: 3, | ||
}, | ||
); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,3 @@ | ||
export type SerializerFn = (input: unknown) => unknown; | ||
export type TrimmerFn = (input: unknown) => unknown; | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
export type SerializerFn = (input: any) => unknown; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking for suggestions on this default value, given this potential usecase.