How we will include pino to node core?

[H4ad]

We will do something like undici and include the entire lib as a dependency?

Or we want to push just the core and then start pushing the code of the library to the core of pino and introduce this as raw code on Node (using primordials, etc...)

I have zero idea, so I'm open to suggestions.

Update

We will basically rewrite pino to be included on Node core, bringing also the dependencies (by making our own code, or bringing to core)

[H4ad]

The dependencies of pino, pino-abstract-transport:

├─┬ pino-abstract-transport@1.2.0
│ ├─┬ readable-stream@4.5.2
│ │ ├─┬ abort-controller@3.0.0
│ │ │ └── event-target-shim@5.0.1
│ │ ├─┬ buffer@6.0.3
│ │ │ ├── base64-js@1.5.1
│ │ │ └── ieee754@1.2.1
│ │ ├── events@3.3.0
│ │ ├── process@0.11.10
│ │ └─┬ string_decoder@1.3.0
│ │   └── safe-buffer@5.2.1
│ └── split2@4.2.0
└─┬ pino@9.1.0
  ├── atomic-sleep@1.0.0
  ├── fast-redact@3.5.0
  ├── on-exit-leak-free@2.1.2
  ├── pino-abstract-transport@1.2.0 deduped
  ├── pino-std-serializers@7.0.0
  ├── process-warning@3.0.0
  ├── quick-format-unescaped@4.0.4
  ├── real-require@0.2.0
  ├── safe-stable-stringify@2.4.3
  ├─┬ sonic-boom@4.0.1
  │ └── atomic-sleep@1.0.0 deduped
  └─┬ thread-stream@3.0.1
    └── real-require@0.2.0 deduped

I removed pino-pretty, we can include it later.

There are some dependencies that we can strip/remove and just use the original code:

Pino

To be copied:

• on-exit-leak-free - Can be copied, although I think is better to keep it as a package
• pino-std-serializers - Copy to pino code

To be removed:

• fast-redact - Remove since we don't want to include redact by default (at least for now)
• process-warning - Remove since we already have ways to emit warning/deprecate on node
• quick-format-unescaped - Remove, use node:util format

To be decided:

• safe-stable-stringify - To be decided, Should we expose options.serializers? #7 and Should we support circular references by default? #3

To keep as a package:

• sonic-boom - Keep as a package, maybe we should provide this functionality on core by default via createWritableStream?
• thread-stream - Keep as a package, but should we expose this as one functionality of the worker thread?
• atomic-sleep - Just because there are other packages relying on this package.
• real-require - Just because there are other packages relying on this package.

Pino Abstract Transport

This package can be copied to core of pino.

Talking about those dependencies:

To be removed:

• readable-stream - Use the native implementation
• split2 - Copy to pino code
• string_decoder - Use native implementation (node:string_decoder)

The final result will look like:

└─┬ pino@9.1.0
  ├── atomic-sleep@1.0.0
  ├─┬ sonic-boom@4.0.1
  │ └── atomic-sleep@1.0.0 deduped
  └─┬ thread-stream@3.0.1
    └── real-require@0.2.0 deduped

[H4ad]

/cc @mcollina @jsumners @davidmarkclements @watson

Pinging you guys just to let you know what I'm trying to do, and see if makes sense to continue this research/efforts to try to include a logging lib on node core based on Pino.

Pino has a lot of good features but I think to include them on node we will need to limit the number of features that we want to expose (this is my assumption), eg: maybe do not include redact in this initial version, do not include safe-stable-stringify, expose build transport direct in the main module of node:logging.

To achieve that, or I create a fork of pino to strip all those features and just leave the bare minimum (like in my previous comment on this issue), or I try to work with you guys to push those changes on pino to shape pino to be more compatible to core (assuming that we don't want to have features of pino on core that are not used).

The first option is easier in terms of implementation but harder in terms of maintenance since we will have 2 versions of pino (in case the proposal is accepted). The second option is easier to maintain but will strip some good features that pino already exposes by default (redact/handle circular reference), and will require some breaking changes on pino.

Let me know what you guys think about these approaches, do you think they make sense or do you have other ways to approach this problem?

[jsumners]

I feel like I'm missing a lot of context. Are there conversations happening somewhere else about this?

[H4ad]

I re-opened this issue: nodejs/node#49296

It was on a conversation on Twitter, people suggested that we can introduce Pino on Node like people did with Undici, so everything started.

[H4ad]

This comment basically indicates we need to re-create pino.

About the dependencies, I will probably only leave sonic-boom and thread-stream, the other ones I will just incorporate inside the main logging lib.

[mcollina]

I think both sonic-boom and thread-stream should be re-implemented inside Node.js core as well, as the latter would likely not work when embedded into core. The former would benefit in speed by using some C++ magic.

[H4ad]

I initially thought in including sonic-boom and thread-stream as dependencies because I didn't have a clear vision of how we could expose those modules on core.

Maybe sonic boom can be something like createBufferedWriter, and not associate as a stream since we want to expose the methods of flush and flushSync.

The thread-stream I have no idea what could be the exposed API, probably we should include it on worker_threads.

Moving those libraries on core means I need to defend 3 different features instead of just one (with deps).

But if you think it's worth, I can try to work on adding those deps first, but I will need some guidance on the exposed API since you have more context than me on why those libraries are better/useful than what we have today.