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

Timestamp in debug message? #5

Open
moritzraho opened this issue Sep 26, 2019 · 12 comments
Open

Timestamp in debug message? #5

moritzraho opened this issue Sep 26, 2019 · 12 comments
Assignees
Labels
enhancement New feature or request

Comments

@moritzraho
Copy link
Member

Wdyt?

@shazron
Copy link
Member

shazron commented Sep 27, 2019

Definitely. I thought it was already in there.

new Date().toISOString()

@Himavanth
Copy link
Contributor

My original plan was to remove DebugLogger because it really doesn't fit here as a complete logging provider. It doesn't have a bunch of stuff like log levels, filtering based on levels and built-in libraries for basic transports. It's use is best for just debug statements as the name suggests. I have some more digging to do on the custom redirection of its logs and then take the decision of either removing it or enhancing it.

@moritzraho
Copy link
Member Author

@Himavanth I think debug logs are useful for libraries, in libraries you don't want to log things unless you are debugging.
Maybe we can get rid of the debuglogger if winston supports a debug mode that works in a similar manner?
Also (other topic) we should do some more investigation on how to enable debug logs in an action as you cannot set env vars from outside of the action. Currently the only solution is to change the action code (i.e. by adding process.env['DEBUG'] = 'mymodule') which is not practical

@Himavanth
Copy link
Contributor

@moritzraho Winston does have debug mode as one of several log levels.

@moritzraho
Copy link
Member Author

moritzraho commented Sep 30, 2019

ok but is there a way for the user of a library (that logs using winston.debug) to turn debug logs on/off like in the debug module ?

@shazron
Copy link
Member

shazron commented Oct 3, 2019

Right now you would do it like this, to only show <applabel>:info:

DEBUG=<applabel>:info

To turn other debug levels on, you add them as comma separated values:

DEBUG=<applabel>:info,<applabel>:error

However, this is not how debug levels should work. The levels should accumulate. For example, the silly level should show the logs for the other levels as well. Something like: https://www.npmjs.com/package/debug-levels

@Himavanth
Copy link
Contributor

@shazron Correct. Debug package is very rudimentary and does not have this feature (and others like custom transports) which means we would have to build it ourselves (it's not a lot of effort though). I spent the first part of this week trying to figure out if it makes sense for us to support it. I have built a custom transport for sending logs to a runtime action here https://github.com/adobe/aio-lib-core-logging/tree/logcollector. Looks like it's ok to give it a try building some of the features in debug package.

@meryllblanchet meryllblanchet added the enhancement New feature or request label Nov 25, 2019
@purplecabbage
Copy link
Member

Is this still an issue?

@meryllblanchet
Copy link

@moritzraho @Himavanth is this still an issue?
Please close otherwise.

@moritzraho
Copy link
Member Author

moritzraho commented Apr 9, 2020

Not sure, I would say yes, but then as @Himavanth mentioned we might end up having to add many other features to the debug provider.
Otherwise instead of enriching the debug provider, we can deprecate it and implement the DEBUG= env var into the winston provider.
One way or the other, I think that continuing to support the ability to set DEBUG= is important as this env var is supported by many libraries in the node world.
@shazron @Himavanth wdyt?

@meryllblanchet
Copy link

@moritzraho @shazron @Himavanth any conclusion on this one?

@Himavanth
Copy link
Contributor

debug npm does not log timestamps when stdout is tty but it does if stdout is say a file. Looks like it is pretty much hardcoded in that aspect. A workaround is to use DEBUG_COLORS=no. This seems to enable the timestamps.

It will be good if winston can be controlled using the DEBUG env var. But we will have to implement a tiny parser here because DEBUG var supports wildcard and "excluding" as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants