fix(keystone): INFRA-296 move gql related logging to pino auto-loggin… #4727
Conversation
|
| @@ -1,6 +1,7 @@ | |||
| const serializers = require('pino-std-serializers') | |||
There was a problem hiding this comment.
This should it in package.json of keystone package
There was a problem hiding this comment.
Good point, I'll do it right here
| @@ -14,6 +15,12 @@ function getKeystonePinoOptions () { | |||
| customProps: (req, res) => { | |||
| return getReqLoggerContext(req) | |||
| }, | |||
| serializers: { | |||
| res: (res) => { | |||
| const errors = res.raw.req.errors | |||
There was a problem hiding this comment.
Should it be wrapped with get or optional chain in case of no errors or smth like this?
| serializers: { | ||
| res: (res) => { | ||
| const errors = res.raw.req.errors | ||
| return { gqlErrors: errors, ...serializers.req(res) } |
There was a problem hiding this comment.
Hm, is it correct behaviour? (You're overriding serializers.res, but using .req here)
There was a problem hiding this comment.
Yes, that's correct. It was done for security reasons (hide body of the logged response)
| @@ -35,7 +49,7 @@ function getReqLoggerContext (req) { | |||
| } | |||
| } | |||
|
|
|||
| return { reqId, sessionId, user, ip, fingerprint, complexity } | |||
| return omitBy({ reqId, sessionId, user, ip, fingerprint, complexity, operationName, variables, query, operationId }, isNil) | |||
There was a problem hiding this comment.
Is omitting previously existing properties on object safe?
There was a problem hiding this comment.
What did you mean? That op is just filtering out empty values of the resulting log output
…g context