Replace bunyan with pino #91

AleksandrSl · 2020-05-11T20:25:54Z

No description provided.

Errors trigger flushSync in logger but it's not supported in combination with prettifier

human-formatter/index.js

server/index.d.ts

Remove some options that have defaults.

AleksandrSl · 2020-05-11T22:37:51Z

Nice, flatMap seems to be not supported in node 10, my bad

ai · 2020-05-11T22:38:48Z

Nice, flatMap seems to be not supported in node 10, my bad

What about array.map().flat()?

AleksandrSl · 2020-05-11T22:42:58Z

From 11 version according to MDN

ai · 2020-05-11T22:46:10Z

OK, let’s fix CI and I am ready to merge

This reverts commit b6fb451. Though it looks complicated, it seems to be the best solution. flatMap and flat are supported only from node 11.

AleksandrSl · 2020-05-11T22:59:16Z

Added post-mortem about this case to commit message

ai · 2020-05-12T03:32:11Z

Do you have an idea of how we can simplify logger options? Right now we have reporter and logger.

Maybe we should keep only logger: 'human' | 'json' | PinoLogger? I think Reporter API is too low level.

AleksandrSl · 2020-05-12T08:56:47Z

Maybe we should keep only logger: 'human' | 'json' | PinoLogger?

Sounds good, considering that reporter: 'human' | 'json' are ignored, if you pass custom logger.
Not sure that Reporter is low level, cause logger is wrapped in reporter and it's a single function, however it seems more complex to setup it in the right way/

What was the main point to have both reporter and logger? To allow customization of messages depending on their type?

ai · 2020-05-12T13:13:46Z

What was the main point to have both reporter and logger?

Mostly historical.

Do you want to remove reporter from the public API? (We may need it for API between Server and BaseServer, but we can keep it as non-documented private API)

AleksandrSl · 2020-05-12T13:18:41Z

Yeah, I will try to do this

* Replace bunyan with pino with smallest interference * Replace PID in tests instead of stubbing it manually * Use pino prettifier API for HumanFormatter * Introduce hack to disable pino prettifier warning Errors trigger flushSync in logger but it's not supported in combination with prettifier * Increase test coverage * Simplify reporter creation * Remove useless TODO * Fix formatting * Clarify why I return undefined to suppress pino warning * Remove useless comments * Add eslint exception for human-formatter * Fix merge error * Add example of custom logger * Enhance custom logger example * Simplify custom logger example Remove some options that have defaults. * Fix complicated code in human-formatter * Fix old typo in in human-formatter * Revert "Fix complicated code in human-formatter" This reverts commit b6fb451. Though it looks complicated, it seems to be the best solution. flatMap and flat are supported only from node 11. Co-authored-by: Andrey Sitnik <andrey@sitnik.ru>

aslepchenkov and others added 9 commits May 11, 2020 22:47

Replace bunyan with pino with smallest interference

380f6de

Replace PID in tests instead of stubbing it manually

879d52a

Use pino prettifier API for HumanFormatter

e46c970

Introduce hack to disable pino prettifier warning

bb1f4df

Errors trigger flushSync in logger but it's not supported in combination with prettifier

Increase test coverage

f27b10f

Simplify reporter creation

607e802

Remove useless TODO

907dd37

Fix formatting

b4037d3

Clarify why I return undefined to suppress pino warning

80fcdac

ai changed the base branch from master to next May 11, 2020 20:26

ai changed the base branch from next to master May 11, 2020 20:26

ai changed the base branch from master to next May 11, 2020 20:27

Merge branch 'next' into replace-bunyan-with-pino-using-prettifier

68d2110

ai reviewed May 11, 2020

View reviewed changes