Repository: pinojs/pino
Pull request #489 (2018-08-16 09:35:17)

Do not use SonicBoom for stdout on Lambda
Fixes https://github.com/pinojs/pino/issues/488

@thomasmichaelwallace can you please verify that this PR fixes the problem?

davidmarkclements: I'm not sure about this, perhaps we should instead have a pino-lamda module - that calls pino with process.stdout , and uses process.nextTick to flushSync at the end of the function 
mcollina: You can’t pass in a serverless-specific config, because we overwrite the stream if it’s stdout. That’s part of what this PR fixes as well.
davidmarkclements: ah yes.. what about an option: `useNodeSTDIO`
mcollina: I would prefer if it worked by default on Lambda.
Il giorno gio 16 ago 2018 alle 16:31 David Mark Clements <
notifications@github.com> ha scritto:

> ah yes.. what about an option: useNodeSTDIO
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/pinojs/pino/pull/489#issuecomment-413565575>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AADL4737wHvT_C2nDYaoBua3MAvrZlqfks5uRYIogaJpZM4V_hMN>
> .
>

jsumners: I agree with @mcollina. Now that I do everything in AWS (Lambda, ECS, CloudWatch, all of that) there are things here that I think we need to be more lenient about to make the library useful in an increasingly popular environment / usage pattern (more on it later). The short of it is that someone using Pino in Lambda shouldn't have to think about it; it should just work.

Also, regarding the pipes, that's definitely an issue. It's definitely possible to freeze a process in a regular *nix environment -- http://web.mit.edu/gnu/doc/html/features_5.html
davidmarkclements: ok, fair enough - let's go with it
davidmarkclements: we still should think of a more graceful way around the flushSync situation 

Patch:
diff --git a/docs/api.md b/docs/api.md
index a69ec82..6570fe4 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -203,6 +203,8 @@ const stderrLogger = require('pino')(opts, pino.destination(2))
 const fileLogger = require('pino')('/log/path')
 ```
 
+On AWS Lambda the default destination is `process.stdout` instead.
+
 * See [`pino.destination`](#pino-destination) 
 * See [`pino.extreme`](#pino-extreme) 
 
@@ -609,6 +611,9 @@ By default, `pino.destination` will use `process.stdout.fd` (1) as the file desc
 A `pino.destination` instance can also be used to reopen closed files 
 (for example, for some log rotation scenarios), see [Reopening log files](/docs/help.md#reopening).
 
+On AWS Lambda we recommend to call `destination.flushSync()` at the end
+of each function execution to avoid losing data.
+
 * See [`destination` parameter](#destination)
 * See [`sonic-boom` ⇗](https://github.com/mcollina/sonic-boom)
 * See [Reopening log files](/docs/help.md#reopening)
@@ -634,6 +639,9 @@ module.
 A `pino.extreme` instance can also be used to reopen closed files 
 (for example, for some log rotation scenarios), see [Reopening log files](/docs/help.md#reopening).
 
+On AWS Lambda we recommend to call `extreme.flushSync()` at the end
+of each function execution to avoid losing data.
+
 * See [`destination` parameter](#destination)
 * See [`sonic-boom` ⇗](https://github.com/mcollina/sonic-boom)
 * See [Extreme mode ⇗](/docs/extreme.md)
diff --git a/lib/tools.js b/lib/tools.js
index 01e0839..774e328 100644
--- a/lib/tools.js
+++ b/lib/tools.js
@@ -21,6 +21,11 @@ const {
   streamSym
 } = require('./symbols')
 
+// FIXME we need to check if we are using AWS Lambda
+// because we cannot use pino.destination/Sonic Boom
+// there, as data might get lost during freeze
+const isAWSLambda = !!process.env.LAMBDA_TASK_ROOT
+
 function noop () {}
 
 function genLog (z) {
@@ -251,7 +256,7 @@ function createArgsNormalizer (defaultOptions) {
     const { enabled, prettyPrint, prettifier, messageKey } = opts
     if (enabled === false) opts.level = 'silent'
     stream = stream || process.stdout
-    if (stream === process.stdout && stream.fd >= 0) {
+    if (stream === process.stdout && stream.fd >= 0 && !isAWSLambda) {
       stream = new SonicBoom(stream.fd)
     }
     if (prettyPrint) {


Test patch:
diff --git a/test/lambda.test.js b/test/lambda.test.js
new file mode 100644
index 0000000..b0daace
--- /dev/null
+++ b/test/lambda.test.js
@@ -0,0 +1,15 @@
+'use strict'
+
+// hack, we are on lambda now
+process.env.LAMBDA_TASK_ROOT = __dirname
+
+const t = require('tap')
+const pino = require('..')
+const { symbols } = pino
+
+const logger = pino()
+
+// this test verifies that the default constructor of pino
+// does not use SonicBoom on lambda by default
+
+t.equal(logger[symbols.streamSym], process.stdout)