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

Implement a logging framework like winston #1268

Closed
KilianB opened this issue Apr 19, 2018 · 8 comments
Closed

Implement a logging framework like winston #1268

KilianB opened this issue Apr 19, 2018 · 8 comments
Labels

Comments

@KilianB
Copy link

KilianB commented Apr 19, 2018

The module development readme states:

MagicMirror Logger

The Magic Mirror contains a convenience wrapper for logging. Currently, this logger is a simple proxy to the original console.log methods. But it might get additional features in the future. The Loggers is currently only available in the core module file (not in the node_helper).

Do we have any updates on weather this idea is going to be worked on in the future? I distributed my own modules via npm to import them in magic mirror and found winston to be of great help (take a look at named loggers). Sadly this currently only allows me to use my loggers in node_helper.js.

Using a logging framework would allow to turn off/on logging for specific modules while formatting the logger in a ways to exactly know which module send the message.


e.g. The rss feed of nytimes currently has a broken xml entry

<item>
  <title/>
  <link>https://www.nytimes.com//?partner=rss&emc=rss</link>
  <guid isPermaLink="true">https://www.nytimes.com//</guid>
  <atom:link rel="standout" href="https://www.nytimes.com//?partner=rss&emc=rss"/>
  <description/>
  <dc:creator/>
  <pubDate/>
</item>

resulting in https://github.com/MichMich/MagicMirror/blob/60b9a5b9dad36a7748a5d1bdb3b32c0c6384e4c0/modules/default/newsfeed/fetcher.js#L63-L68
being printed to console every time the newsfeed module tries to update it's feed. It would be really convenient to simply to turn of logging for this module.

@KilianB KilianB changed the title Implement a logging framework like winston to Implement a logging framework like winston Apr 19, 2018
@nhubbard
Copy link
Contributor

@KillianB This is really a good idea. If you have the ability to, we would love for you to contribute!

@E3V3A
Copy link
Contributor

E3V3A commented Apr 28, 2018

@KilianB So what's the correct xml code for NY times? (I was wondering for a while why that was not working.) Also, please summarize:

  1. Why Winston is more useful or better for MM?
  2. What it would take to get it imlemented?

@justjim1220
Copy link

@MichMich
Copy link
Collaborator

MichMich commented May 1, 2018

Feel free to send a PR. As long as it's not a breaking change and doesn't require a massive load of dependencies.

@KilianB
Copy link
Author

KilianB commented May 11, 2018

@E3V3A the news module requires a title and a publish date. The corresponding lines of code are here

var pubdate = item.pubdate || item.published || item.updated || item["dc:date"];
   if (title && pubdate) {
   }else { error; }

Therefore, a valid xml entry has to have content for the title and publish date which the supplied example does not provide. It points towards the default nytimes homepage.

  1. Winston is more useful than no logging framework due to the reasons that developers gain more access to the ways logging is handled. Currently all log messages are simply forwarded to the console which don't allow for monitoring or error tracking throughout restarts of nodejs.

For example

  • Logging severity can be easily controlled (which level of logging do we want? Warnings, errors, debug statements ..)
  • How are the log statements formatted? Timestamps, class origin, module names?
  • Log endpoints. Frameworks usually allow to define some kind of appenders that specific how logs are send. Are they saved to disk, emailed to users on a certain interval, rolled over after reaching x entries displayed as html ...
  • Mute logs from certain modules

Overall I think it's really important to allow developers to log messages in either modules and node_helper with the same syntax, which currently is not possible. I am not saying that winston is the go to framework but it's simply what I am currently using.

  1. @nhubbard currently my time is severely constrained by other projects and I fear this will have to wait quite a bit before I can devote a fair share of it towards mm. NodeJS also isn't really a technology I have much experience with therefore, my code might not live up to the standards required here.
    Without putting much thought into it one possible approach would be to create a named logger during the module loading in the Loader.js. To preserve backward compatibility logger.js has to be bound to use the logger for the corresponding module. I am not sure how you would go about this. maybe instead of exporting Log through module-types.ts the logger object itself can be passed to the module instead. But again ECMAScript isn't really my strong suit.

@nhubbard it's great to see a fellow first robotics enthusiast participating here. Due to me moving away from the US I sadly can not participate actively anymore but I have been present at last years kickoff together with the teams in grand rapids and if my memory serves correctly participated against your team a few times ;).

Nevertheless if by any chance I get some spare time I am more than happy to look into this issue.

@MichMich
Copy link
Collaborator

FYI: #1331

@shbatm
Copy link
Contributor

shbatm commented Jan 2, 2019

Saw this issue and wanted to share: I created a module to implement tracer on the back-end server (Node.JS) side. If set as one of the first modules in the config, it will overwrite all module's existing console printing functions and format them.

https://github.com/shbatm/MMM-Logging

@stale
Copy link

stale bot commented Jul 2, 2019

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 label Jul 2, 2019
@stale stale bot closed this as completed Jul 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants