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

[Feature Request]: Add option to Console transport for logging through console.log #2175

Open
coreyjpieper opened this issue Aug 8, 2022 · 6 comments
Labels
Feature Request Request for new functionality to support use cases not already covered Needs Investigation

Comments

@coreyjpieper
Copy link

🔎 Search Terms

console.log, jest, lambda

The vision

The Console transport should have a configuration option so that users can choose whether logging is done through process.stdout.write (the default currently) or console.log. I believe pull request #982 would resolve my issue and related issues from others.

Use case

The fact that Winston uses process.stdout.write instead of console.log has been the root cause of several problems for me:

  1. When using the json format to log messages in AWS Lambda, each line is split up into a different log event.
  2. When logging messages in AWS Lambda, the request ID and timestamp that is usually automatically added is lost.
  3. When running tests, logging through stdout ignores the Jest --silent option, which means I can no longer control the output from tests from the command line.

None of these things were problems before when using console.log. I spent several hours researching and creating workarounds for each of these situations just to get Winston to the same level of functionality that console.log provides. Winston does not document anywhere that it defaults to stdout for the Console transport, which means that many people have to spend time debugging problems related to the usage of stdout and digging through the Winston source code.

I believe that all of these problems (in addition to VSCode and Chrome debugging issues mentioned in #981, #1305, #1544) could be solved by just adding a configuration option to the Console transport to override use of stdout. There have been pull requests to fix this like #982 but they have been closed. I do not see why giving users more customization options is a bad thing, after all shouldn't the user be able to decide what's best for them?

This issue will continue to bite people in unforeseen and different ways as long as it remains unaddressed because many applications simply do not expect you to use stdout and do not support using it. There are already examples where this has affected popular tools and services like VS Code, Chrome, Jest, and AWS Lambda.

Additional information

No response

@szolnokit
Copy link
Contributor

Added a "forceConsole" option, to force use "console.log"

0757afe

@wbt
Copy link
Contributor

wbt commented Feb 6, 2023

Referenced commit is in PR #2276 for better cross-ref.

@wbt
Copy link
Contributor

wbt commented Feb 6, 2023

I'm inclined to close for reasoning described here - do you think that applies?

@dancrumb
Copy link

dancrumb commented Mar 7, 2023

I opened this issue with the VSCode team for this very reason.

As I explored the issue, it became clear what was happening.

If I understand correctly when you use console.* in node while --inspect is in play, a consoleAPICalled event is fired, which allows VSCode to capture the message properly.

The reasoning here doesn't apply for the reasons in the bug I opened. In short, using the stdout and stderr streams means that the VSCode debug console can't tell the difference between multiple single-line entries and a single multi-line entry.

While it's true that one could create one's own Console transport, that's a lot of replication of a core component for what amounts to 3 lines of code.

Are you open to reconsidering this? The PR seems pretty innocuous to me and would be fully backward-compatible.

@dancrumb
Copy link

dancrumb commented Mar 7, 2023

FWIW, an alternative approach would be to move the

if (stream) {
stream.write(`${info[MESSAGE]}${this.eol}`);
} else {
  console[logMethod](info[MESSAGE]);
}

Into a public method. That way, people could extend the transport and just override that method.

@coreyjpieper
Copy link
Author

coreyjpieper commented Mar 12, 2023

I'm inclined to close for reasoning described here - do you think that applies?

The reasoning that is cited:

For the record Pino also suffers from this problem. I don't think filing issues on the logging libraries and convincing them to change to console.log is the way to go here, especially since console.log is slower.

is misleading. I'm not asking for Winston to switch to console.log, I'm simply asking for an option to choose between stdout and console.log. This can be done in a backwards compatible manner so that stdout remains the default.

I don't see why you wouldn't want to do this, given that multiple people are asking for this and it can be backwards compatible. People will continue opening feature requests for this as long as the option isn't there. Consider the fact that if #982 was merged 6 years ago, you would have avoided many issues related to this and I would've never opened this issue. You would be saving time for users and yourself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Request for new functionality to support use cases not already covered Needs Investigation
Projects
None yet
Development

No branches or pull requests

4 participants