Closed simoneb closed 2 years ago
That was the original design, but pino mixins do not allow you to alter the formatting of the final output that is written to stdout. So although we could mixin the data from the request, we could not change the log format to the one required by Cloudwatch
Interesting, then it's definitely something we need to look into on the pino side. I'll get back to you about this, thanks!
I think we could find a good balance to do it using the default Pino apis.
Here is a quick&dirty example using:
The pino transport would be the best fit (the JSON.parse
can be skipped), but I don't know if it works as expected on the Lambda environment
'use strict'
const pino = require('pino')
const { Transform, Writable } = require('stream')
const cloudwatchWritable = new Writable({
defaultEncoding: 'utf8',
write(chunk, encoding, callback) {
const obj = JSON.parse(chunk)
const time = new Date().toISOString();
console.log(`${time}\t${obj.awsRequestId}\t${obj.msg}`)
callback()
}
})
let n = 0
const logger = pino({
mixin() {
return { awsRequestId: ++n }
}
}, cloudwatchWritable)
logger.info('hello')
// 2021-10-21T16:46:49.985Z 1 hello
@carbonrobot what do you think of @Eomm's proposed approach? Would you be interested in turning this library into something that could be plugged into pino instead of replacing it? We can help out on this. As an alternative, we may create a different library, but we'd prefer to avoid duplicated efforts.
@simoneb I will begin testing this today for a new version
@simoneb I will begin testing this today for a new version
If you need any support here at NearForm we're happy to contribute.
Any progress here guys? Would love to help on this if i can!
Beta package release https://www.npmjs.com/package/pino-lambda/v/4.0.0-rc1
Beta packages release https://www.npmjs.com/package/pino-lambda/v/4.0.0-rc2
Beta package release (final test) https://www.npmjs.com/package/pino-lambda/v/4.0.0-rc3
@carbonrobot where is this development happening?
@simoneb The branch is https://github.com/FormidableLabs/pino-lambda/tree/release/v4
Cool, I can see it now (it wasn't when I commented). Amazing 🚀
When this is out can you do a PR to include it in the pino ecosystem docs page? https://github.com/pinojs/pino/blob/master/docs/ecosystem.md
Will do! I am testing it today against two very large ecommerce sites and hopefully can release later today if all goes well.
:tada: This issue has been resolved in version 4.0.0 :tada:
The release is available on:
Your semantic-release bot :package::rocket:
Thanks for this package. In reference to https://github.com/pinojs/pino/issues/648, I was thinking that a more robust way to expose this functionality might be to take a composition over inheritance^1 approach.
Instead of wrapping pino and use this library as a drop-in replacement, you could simply expose the mixin instead, so a user could compose it with pino.
The API would look something like:
In this way this package would become part of the pino ecosystem instead of being a replacement for pino.
What do you think?