Skip to content
This repository has been archived by the owner on Sep 8, 2023. It is now read-only.

chore: updates docs and example to use transports #49

Merged
merged 31 commits into from Feb 3, 2022
Merged

Conversation

aaestrada
Copy link
Contributor

@aaestrada aaestrada commented Dec 16, 2021

Uses thread-stream to create a single worker to forward data to all target destinations

Note we will need to resolve the issue at hapi-pino.

Uses thread-stream to create a single worker to forward data to all target destinations
@aaestrada
Copy link
Contributor Author

Node js profile test no1

Screen Shot 2021-12-16 at 10 49 12

Node js profile test n2

Screen Shot 2021-12-16 at 11 11 06

@mannyluvstacos
Copy link

Looks like updating the unit tests for index.test.js should have the tests running 💯 again.

🤝

Long Live Catalyst 🚀✨🌕

@aaestrada
Copy link
Contributor Author

Looks like updating the unit tests for index.test.js should have the tests running 💯 again.

🤝

Long Live Catalyst 🚀✨🌕

Thanks @mannyluvstacos! I will review the unit tests once my code is ready! I wanna use the manifest.json to register this code instead the index.js file.

@aaestrada
Copy link
Contributor Author

aaestrada commented Jan 3, 2022

Node js profile test no1
Screen Shot 2022-01-03 at 15 37 53

Node js profile test no2
Screen Shot 2022-01-03 at 15 44 32

Copy link
Contributor

@mcjfunk mcjfunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is not doing what the title says. It is only adding an example for how to use transports with pino-pretty in local development.

lib/config.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@aaestrada
Copy link
Contributor Author

node js profile for pino-http-print no 1

Screen Shot 2022-01-11 at 6 45 15

node js profile for pino-http-print no 2

Screen Shot 2022-01-11 at 6 48 27

@aaestrada
Copy link
Contributor Author

node js profile for pino-pretty no 1
Screen Shot 2022-01-12 at 12 22 43

node js profile for pino-pretty no 2

Screen Shot 2022-01-12 at 12 24 11

Copy link
Contributor

@mcjfunk mcjfunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment. Also, when squashing and merging, we should update the commit message to be something like: chore: updates docs and example to use transports for pino-pretty

examples/simple/readme.md Outdated Show resolved Hide resolved
examples/simple/readme.md Outdated Show resolved Hide resolved
examples/simple/readme.md Outdated Show resolved Hide resolved
examples/simple/readme.md Outdated Show resolved Hide resolved
examples/simple/readme.md Outdated Show resolved Hide resolved
aaestrada and others added 2 commits January 25, 2022 15:17
Co-authored-by: Bryan Shell <shellbj@users.noreply.github.com>
Co-authored-by: Bryan Shell <shellbj@users.noreply.github.com>
Copy link
Contributor

@mcjfunk mcjfunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to advertise this approach? What is the advantage here? pino-rotating-file is similarly in its own process plus it supports log rotation. Can/should we just show that in the Readme rather than create examples that we need to keep updated?

@shellbj
Copy link
Contributor

shellbj commented Jan 26, 2022

Do we really want to advertise this approach?

Not really.

What is the advantage here?

It cleaned up some of the configuration in the example and docs to show how to move pretty into a conditional transport.

@vrbo/pino-rotating-file is similarly in its own process plus it supports log rotation. Can/should we just show that in the Readme rather than create examples that we need to keep updated?

This and update the example/default to use a pino/file transport for stdout.

@aaestrada
Copy link
Contributor Author

aaestrada commented Jan 26, 2022

Yes! this pr will help our developers a quick and easy way to transport and transform logs for production...

if necessary, the examples serve as a guide/introduction to make a more advanced configuration.

examples/simple/readme.md Outdated Show resolved Hide resolved
lib/config.json Outdated Show resolved Hide resolved
Copy link
Contributor

@mcjfunk mcjfunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just a reminder... when squashing and merging, please update the commit message to be something like: chore: updates docs and example to use transports

@aaestrada aaestrada changed the title Move PINO logging out of the main execution thread chore: updates docs and example to use transports Feb 3, 2022
@aaestrada aaestrada merged commit 3aaebb5 into master Feb 3, 2022
@github-actions
Copy link

🎉 This PR is included in version 5.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

None yet

4 participants