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

Ensure '(node)' prefix for stdout logging #3789

Closed
silverwind opened this issue Nov 12, 2015 · 14 comments
Closed

Ensure '(node)' prefix for stdout logging #3789

silverwind opened this issue Nov 12, 2015 · 14 comments
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. good first issue Issues that are suitable for first-time contributors.

Comments

@silverwind
Copy link
Contributor

To discern messages logged by node vs. messages logged by the application, we agreed to prefix messages printed to stdout with (node) like here or here.

There are however cases in the code base where this prefix is missing like here which gets logged here. I'm sure there are more cases in src, possibly even some in lib. One could search for console or the printf variants to identify these cases.

@silverwind silverwind added c++ Issues and PRs that require attention from people who are familiar with C++. good first issue Issues that are suitable for first-time contributors. labels Nov 12, 2015
@silverwind silverwind changed the title Ensure '(node)' prefix to everything logged to stdout Ensure '(node)' prefix for stdout logging Nov 12, 2015
@bnoordhuis
Copy link
Member

PID would be useful too. Think clustered applications.

@JungMinu
Copy link
Member

@silverwind @bnoordhuis I will handle this 😄

@silverwind
Copy link
Contributor Author

@JungMinu thanks!

I don't see PID as a requirement, but we could do (node 12345) message. Maybe through a shared function in lib/internal/util.js, if all our prints come from JS.

@JungMinu
Copy link
Member

@silverwind Thanks 😃

@silverwind
Copy link
Contributor Author

Let's wait for others to chime in on the PID topic, I feel it's kind of unnecessary. Maybe only include it when in a cluster child?

@JungMinu
Copy link
Member

@silverwind Yes, 👍

@JungMinu
Copy link
Member

Let's wait for others to chime in on the PID topic, I feel it's kind of unnecessary. Maybe only include it when in a cluster child?

@Fishrock123 Whaddya reckon?

@cjihrig
Copy link
Contributor

cjihrig commented Nov 13, 2015

Might want to always include it for consistency with debug(). Also, not all child processes are cluster workers.

@silverwind
Copy link
Contributor Author

Maybe (node) [pid] message would be a better format? I'm still not conviced we need to include the PID, and would love some more feedback about that from collaborators.

@bnoordhuis
Copy link
Member

Think of it like this: is there ever a time when the PID is distracting or harmful? If not, then include it - because there will be times when it's invaluable.

@jwueller
Copy link
Contributor

Absolutely, the PID can be vital. In terms of format, (node) [pid] is
easier to grep for than (node pid).

Ben Noordhuis [email protected] schrieb am Fr., 13. Nov. 2015,
09:22:

Think of it like this: is there ever a time when the PID is distracting or
harmful? If not, then include it - because there will be times when it's
invaluable.


Reply to this email directly or view it on GitHub
#3789 (comment).

@JungMinu
Copy link
Member

I will submit a PR for this issue ASAP, Thanks 😄

@JungMinu
Copy link
Member

a PR for this issue:#3833

@silverwind
Copy link
Contributor Author

Resolved in 94b9948

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

No branches or pull requests

5 participants