Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swallowing all stdout/stderr terminal output #60

Open
devinrhode2 opened this issue Mar 28, 2022 · 2 comments
Open

Swallowing all stdout/stderr terminal output #60

devinrhode2 opened this issue Mar 28, 2022 · 2 comments
Labels
wontfix This will not be worked on

Comments

@devinrhode2
Copy link

I've noticed that pino-applicationinsights swallows everything into it's writeStream.

If I try to simulate prod locally, and already have a server running on port 3000, server will start, and then immediately crash, next.js will normally print an error message that port 3000 is already in use, the application insights writeStream was not flushing that to stdout.

Maybe it's desirable from a performance perspective to simple capture all stdout/stderr and not let it print to the users terminal.

On the other hand, if server fails to start, or logs are not being sent correctly, then we should send all the logs we've collected to stdout/stderr.

It seems that pinoms is generally a legacy/compatibility package.

pino-applicationinsights Seems to be the weakest link. I think it was built with pino v6 in mind:
CleanShot 2022-03-25 at 03 56 44@2x
And the docs there led me to use pino-multi-stream in the first place: https://github.com/ovhemert/pino-applicationinsights/blob/master/docs/API.md

But it turns out, pinoms DOES support pino v7..

If I comment out my whole next-logger.config.js file, then the default logger will print logs to the terminal. So there's something wrong with my Azure ApplicationInsights logger.

This was the fix:
CleanShot 2022-03-28 at 15 44 04@2x

I actually stole the idea from https://github.com/pinojs/pino-elasticsearch docs

We could probably steal the same snippet for docs for this project

@devinrhode2
Copy link
Author

For extra context see: sainsburys-tech/next-logger#12

@stale
Copy link

stale bot commented Aug 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Aug 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

1 participant