add convertReqRes boolean option; make req/res conversion off by default
#32
Assignees
Labels
agent-nodejs
Make available for APM Agents project planning.
Comments
trentm
added a commit
that referenced
this issue
Jan 15, 2021
BREAKING CHANGE: Require convertReqRes:true option to handle 'req' and 'res' meta keys. Also no longer support handling 'request' and 'response' meta keys. Refs: #32
trentm
added a commit
that referenced
this issue
Jan 18, 2021
BREAKING CHANGE: Require convertReqRes:true option to handle 'req' and 'res'
keys in the mergingObject, e.g. `log.info({req, res}, 'the message')`.
Also no longer support handling 'request' and 'response' keys.
Refs: #32
|
^^ Done for winston and pino, and morgan won't get the option (see "Exception" note). These will be in the next release. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The current version of the winston and pino ecs-logging formatters automatically handle logging of
req,res,request, andresponseextra fields: converting them from HTTP request and response objects to ECS HTTP, URL, and User agent fields.This ticket is proposing a BREAKING CHANGE to this while these packages are still in pre-1.x beta:
convertReqRes: trueboolean option to turn it on.reqandresas special.My understanding is the main idea for specially handling HTTP request/response objects stems from:
The argument for making this configurable is to handle the case where a user wants to log req/res but not for an HTTP request or response. The main argument for turning off req/res handling by default is that the req/res handlers in pino and Bunyan are also off by default. As well, having the option to turn on an extra feature feels a little more discoverable... but one could go either way on that.
The argument for only supporting
req / resand notrequest / responseis the prevalent usage of justreq / resin pino and Bunyan. All examples of logging in code docs for Node.js frameworks that I could find use the short "req" and "res". I would prefer to start these libraries without multiple ways to perform the same thing. If there is a strong use case brought up for them, a separate ticket could discuss re-adding them.Exception: morgan
Morgan is explicitly "HTTP request logger middleware for node.js". It doesn't provide logging methods for the app developer to even pass in extra fields. It doesn't make sense to have the default
@elastic/ecs-morgan-formatnot render request and response fields.The text was updated successfully, but these errors were encountered: