@mattab opened this issue on November 7th 2014

Currently WARN level logs are logged to the screen: https://github.com/piwik/piwik/blob/master/config/global.ini.php#L54-L61

This is causing problems, for example core:archive command errors with:

ERROR CoreConsole[2014-10-26 23:59:53] [7ece2] Got invalid
response from API request:
Response was 'WARN ScheduledReports[2014-10-26 23:59:45] [59c46]
Preventing the same scheduled report from being sent again (report #25
for period "Week 13 October - 19 October 2014") WARN
ScheduledReports[2014-10-26 23:59:46] [59c46] Preventing the same
scheduled report from being sent again (report #3 for period "Week 13
October - 19 October 2014") 

Which then fails the archiving.

To prevent WARNINGS message from failing the archiving, I propose to change default logger level to ERROR so only ERRORS are logged to screen. (helps us detect bugs faster as users will report them or learn from them and fix it themselves)

I had a look at other Log::warning and could see those to change from warning to error: - https://github.com/piwik/piwik/blob/9e6329baf67cb1e59c55ba05c35761b77d37b1c1/plugins/UserCountry/GeoIPAutoUpdater.php#L541 - https://github.com/piwik/piwik/blob/master/core/Session.php#L118-118 - https://github.com/piwik/piwik/blob/master/core/Plugin/Visualization.php#L180-180

(Other Log::warning look like they are real warning and not errors.)

@mnapoli commented on November 7th 2014

Here the warning messages are logged to the HTML output of the http://demo.piwik.org/index.php?module=API&method=CoreAdminHome.runScheduledTasks&format=csv&convertToUnicode=0&token_auth=trigger=archivephp HTTP request right?

Maybe there should be different logger backends depending on the environment? I.e. it doesn't make sense to log to the HTML output in an HTTP request context, however it makes totally sense to log to stdout when we are in a CLI context.

@diosmosis commented on November 7th 2014

FYI, errors are always logged to the screen. The reason warnings are logged to the screen is because 'screen' is in log_writers setting in global.ini.php.

@mnapoli commented on November 7th 2014

Yeah a fatal error/uncatched exception is a special case from logging really.

Warnings should be logged. And maybe we shouldn't take about "screen" because it's confusing. So what about setting different logging adapters/backends depending on the context? - web application -> log to the HTML output somewhere in the page - HTTP API -> don't log to the JSON it doesn't make sense - CLI -> log to stdout - logging to common backends like file of course

@mattab commented on November 7th 2014

I've changed default logger level to ERROR mostly because I needed to do something for 2.9.0 where currently the archiving breaks if any warning is emitted. I'll close issue but @mnapoli feel free to create another issue with your suggestions if you want

@diosmosis commented on November 7th 2014

@mnapoli My comment wasn't an argument against your idea, just a note about the issue ;)

@mnapoli commented on November 9th 2014

@diosmosis yes sorry if my wording wasn't the best ;)

I've opened a separate issue about the logger: #6622

This issue was closed on November 7th 2014
Powered by GitHub Issue Mirror