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

log4js integration #502

Merged
merged 2 commits into from
Apr 18, 2016
Merged

log4js integration #502

merged 2 commits into from
Apr 18, 2016

Conversation

hiddentao
Copy link
Contributor

This replaces console.log calls with a log4js integration and adds two command-line flags (--loglevel and --logfile) to be able to customize logging verbosity at runtime.

screen shot 2016-04-18 at 12 26 45

Benefits:

  • Console output is now timestamped and categorized - with the ability to customize the format in a single place (the logger.js file). We could also add a CLI arg to override the pattern if deemed necessary.
  • We can now add debug and trace logging in many more parts of the app without automatically clogging up the console output, yet retaining the ability to show these logs by setting the right CLI argument.
  • We can set the logging threshold using --loglevel <level>. Possible values are trace, debug, info, warn, error. Default is info.
  • We can have log4js log to a file using the --logfile <path to file> arg.
  • Log4js output formatter automatically handles things like Error objects well.

I haven't added this logging to the Meteor "interface" app yet but there's no reason we can't do that.

@frozeman
Copy link
Contributor

What does trace, debug, info (default), warn, error mean? do i either/or select? Means if i have trace, i wont see error, or info logs?

Aren't log levels like 0-6 better?

@hiddentao
Copy link
Contributor Author

They are effectively ordered as such:

  1. trace
  2. debug
  3. info
  4. warn
  5. error

So if you set the level to debug you will see everything at debug and above.

I think it's better to used labelled logging as it corresponds with console methods + it forces one to consider what type of information is being logged. In the PR I've translated most of the console.log calls into .info but I've made a few low-level items as either debug or trace for this very reason.

@frozeman
Copy link
Contributor

Sounds good, but maybe we should note in the help that trace is all. I would expect trace to contain stack traces at all times.

Apart form that it looks good!

@hiddentao
Copy link
Contributor Author

Ah, that's a good idea, I'll update the CLI help docs with that info.

@frozeman frozeman merged commit 9e571ee into ethereum:develop Apr 18, 2016
@hiddentao
Copy link
Contributor Author

hiddentao commented Apr 18, 2016

The Mist Meteor app also has a bunch of console.log calls - I was thinking that we could put that stuff through log4js too so that it's all unified. Since the GUI crashes for some people before they can get to console logs this may give us a way to capture such logs (through the --logfile option). I haven't looked into how easy it would be to make the logging work this way but I imagine it's doable.

Do you think this is a good idea?

@frozeman
Copy link
Contributor

If it adds it also to the log file, this might be useful if it works in the browser.
But i don't think the render view can write to files ;) So you would need to build some kind of internal "IPC send to backend" thing.

Not sure if thats worth the time for now.

Here is a meteor package, but it might only work for the meteor server side, which we don't use: https://atmospherejs.com/adain/log4js

@hiddentao
Copy link
Contributor Author

Looks like Meteor 1.3 has built-in support for NPM modules -> https://github.com/meteorhacks/npm

So it would be trivial to include log4js into the Meteor app (once we've upgraded Meteor of course) and then we would just need to write a custom appender which sends the log data to the IPC back-end.

I'd like to give this a go - I think it would be very useful.

@hiddentao hiddentao deleted the logging branch April 19, 2016 02:34
@hiddentao hiddentao restored the logging branch May 4, 2016 02:13
@hiddentao hiddentao deleted the logging branch May 4, 2016 02:14
@lock
Copy link

lock bot commented Apr 1, 2018

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked and limited conversation to collaborators Apr 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants