feat: add support for request-received middleware (closes #66) #67
Conversation
There was a problem hiding this comment.
when we expose symbols in pino we prefix with the pino namespace, see https://github.com/pinojs/pino/blob/master/lib/symbols.js#L30
| @@ -4,6 +4,7 @@ var pino = require('pino') | |||
| var serializers = require('pino-std-serializers') | |||
|
|
|||
| var startTime = Symbol('startTime') | |||
| var requestReceivedStartTime = Symbol.for('request-received.startTime') | |||
There was a problem hiding this comment.
| var requestReceivedStartTime = Symbol.for('request-received.startTime') | |
| var startTime = Symbol.for('pino-http.startTime') |
| @@ -4,6 +4,7 @@ var pino = require('pino') | |||
| var serializers = require('pino-std-serializers') | |||
|
|
|||
| var startTime = Symbol('startTime') | |||
There was a problem hiding this comment.
| var startTime = Symbol('startTime') |
| @@ -60,7 +61,7 @@ function pinoLogger (opts, stream) { | |||
| function loggingMiddleware (req, res, next) { | |||
| req.id = genReqId(req) | |||
| req.log = res.log = logger.child({req: req}) | |||
| res[startTime] = res[startTime] || Date.now() | |||
| res[startTime] = res[startTime] || req[requestReceivedStartTime] ? req[requestReceivedStartTime].getTime() : Date.now() | |||
There was a problem hiding this comment.
| res[startTime] = res[startTime] || req[requestReceivedStartTime] ? req[requestReceivedStartTime].getTime() : Date.now() | |
| res[startTime] = res[startTime] || Date.now() |
|
@davidmarkclements these aren't symbols being exposed by pino, they're exposed by request-received, which I created as a standard for Koa and Express communities |
|
cool, since you pushed it two hours ago and don't have anyone depending on it yet it'll be super easy to update. If one module consumes another, the consuming module integrates using consumed modules API's - rarely the other way round. |
|
The point of the package is to simply expose these as Symbols for any package to consume - I didn't want to prefix it with |
|
I guess I could however update the package so that it also defines |
|
I get what you're trying to do with your package but this isn't the way to do, you'll have to think of another way. Closing - feel free to reopen if it becomes useful to you in the requested form |
|
sorry I didn't see #67 (comment) before closing - that could work. |
|
The purpose of this isn't so people use Cabin, it's so people that don't want to use Cabin's middleware and simply want to use https://github.com/cabinjs/parse-request (major update coming soon) |
|
feel free to reopen if plan is to consume the pino symbol |
|
def will do |
No description provided.