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

Add logging system #1399

Closed
ryanrolds opened this issue Jan 27, 2017 · 10 comments · Fixed by #1425
Closed

Add logging system #1399

ryanrolds opened this issue Jan 27, 2017 · 10 comments · Fixed by #1425
Assignees

Comments

@ryanrolds
Copy link
Contributor

It's popular to use console.log() in web development. The problem is that unless developers are careful to remove to remove all their console.log() calls we can quickly end up with console spam. In many cases simply removing the console.log calls is desired, but a consistent way for the framework and plugins to log a fatal problems, errors, warnings, and debug information would be helpful to course authors, developers, and operations.

A logging model could have functions for reporting fatal, error, warn, info, and debug events and details about the event. If these events are then emitted as a traditional event (log:fatal, log:warn, etc...) plugins that wish to receive these events and relay them to a central logging system can. Developers can use debug and info level events to add debugging and publish their plugins without having to worry about log spam. Course developers can disable or adjust the log level so that only high severity issues are written to their consoles and/or reported to log forwarder plugins. In the event that a course author experiences a problem they can lower the log level and see debug or info logs which will help improve bug reports.

I've done a little work on this and when I have the code in working state I will post a link.

@moloko
Copy link
Contributor

moloko commented Jan 27, 2017

funnily enough @chris-steele was talking to me just the other day about whether we should have something like this. we certainly would be interested to see this.

@ryanrolds
Copy link
Contributor Author

I'm running in to issues with the load order. It's desirable to get logging up as early as possible (minimize the parts of the system that can't use it). One problem I'm hitting is that plugins are starting to load before ConfigModel has finished loading the configs. So, if we want the main config to control what the logging module relays to forwarders or is written to console log, it really needs to load before the plugins. I have a working logging module and it uses reasonable defaults so that it can be available before the config loading completes and while the plugins are doing their thing.

A lot of this could be avoided if app.js (or some other init/bootstrap code) loaded the configs followed by setup of the logging module and Adapt. Once Adapt is in a sold state it fires off plugin loading. I've been able to delay the plugin loading (and still have them included in adapt.min.js), but I'm seeing a lot of plugins trying to hook in to earlier events and missing them because of the delayed loading. Ensuring the configs are loaded and Adapt is ready before the plugins are let loose would probably simplify plugin bootstrapping among other things.

@oliverfoster
Copy link
Member

Put it in the core alongside offlineStorage. We've got to change the fundamental architecture of adapt + bower to get loader orders installation etc.

@ryanrolds
Copy link
Contributor Author

@oliverfoster Done. It would be nice to have the logging module not depend on Adapt in it's factory. That way it could be loaded prior to Adapt's setup and could be used by Adapt during it's setup/factory.

@oliverfoster
Copy link
Member

Need to think about hooking into this from the AT

@ryanrolds
Copy link
Contributor Author

ryanrolds commented Feb 6, 2017

AT? Authoring tool? What's the purpose of hooking in? Console?

@oliverfoster
Copy link
Member

oliverfoster commented Feb 6, 2017

Yea, Authoring tool. @taylortom has been wanting a better logging system from the Preview into the AT. So I've assigned you both. He'll be able to give input with whatever comes out and be able to get his head around this to use over there # points to the AT ppl #

@oliverfoster
Copy link
Member

oliverfoster commented Feb 17, 2017

@taylortom it should be pretty easy to hook into this https://github.com/adaptlearning/adapt_framework/blob/master/src/core/js/logging.js#L87

require("core/js/adapt").on("log", function(typeString, data) {

});

we might want to turn the loglevels into a properly enumerated object https://github.com/adaptlearning/adapt_framework/blob/master/src/core/js/logging.js#L10
and have them return on the event trigger.
but otherwise. 👍

@taylortom
Copy link
Member

Kewl, looks good to me

@moloko
Copy link
Contributor

moloko commented Mar 27, 2017

done in v2.0.17

@moloko moloko closed this as completed Mar 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants