? ? ? Pending

User tests: Successful: Unsuccessful:

avatar okonomiyaki3000
okonomiyaki3000
23 Jan 2017

Pull Request for Issue # .

Summary of Changes

This adds a defer option to the formatted_text logger. The option is not enabled by default so the logger will continue to function in the usual way unless specifically configured.

The normal way for this logger to function is to format each received log entry as a line of text and write it to a file as it is received.

When using the defer option, received log entries will just be stored in an array when received. It will not be until the logger is destroyed (basically this happens in the shutdown process, so after the response has been sent) that the stored entries are formatted as lines of text and all written to the log file at once. This may be better for performance since multiple write operations are combined as one and (more importantly) not performed until the response has already been sent.

There is a small caveat which should be considered when deciding whether or not to use the defer option. If php encounters a fatal error, the class' __destruct function may never be called and all unwritten logs will be lost. If you are writing logs to help you determine why your fatal errors are happening, this option is not for you.

Testing Instructions

Configure a formatted_text logger and include 'defer' => true in its options array. A simple way to do this is to use the logging features of the Debug plugin. There are two formatted_text loggers already configured in that plugin (must be activated in the plugin settings) so you only need to make a minor modification to their options arrays.

Documentation Changes Required

If there is some documentation about using the formatted_text logger, it should be updated to include this option. If there isn't any, there probably should be.

Discuss?
I'm using the class __destruct function to trigger the writing of deferred logs. It's pretty convenient and it works because we can rely on the logger instance not being destroyed until the program shuts down. However, it's possible that register_shutdown_function is a better choice. I haven't tested it or even thought about it much so, if anyone has any thoughts about it, please let me know.

avatar okonomiyaki3000 okonomiyaki3000 - open - 23 Jan 2017
avatar okonomiyaki3000 okonomiyaki3000 - change - 23 Jan 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 23 Jan 2017
Category Libraries
avatar okonomiyaki3000 okonomiyaki3000 - change - 23 Jan 2017
The description was changed
avatar okonomiyaki3000 okonomiyaki3000 - edited - 23 Jan 2017
avatar zero-24 zero-24 - change - 23 Jan 2017
Labels Added: ?
avatar zero-24
zero-24 - comment - 23 Jan 2017

@okonomiyaki3000 please check: 5ace361 this should fix the most travis errors if not all. I have also marked the __destruct() nethod as public.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 23 Jan 2017

@zero-24 Thanks for that! My tabs were wrong? That's so weird...

avatar zero-24
zero-24 - comment - 23 Jan 2017

My tabs were wrong? That's so weird...

Yes it looks like your code uses 4 spaces and not tabs ?

avatar okonomiyaki3000
okonomiyaki3000 - comment - 23 Jan 2017

I better check my settings. I'm definitely not trying make a statement about that.

avatar laoneo
laoneo - comment - 30 Jan 2017

I suggest that you add some unit tests in the file https://github.com/joomla/joomla-cms/blob/staging/tests/unit/suites/libraries/joomla/log/loggers/JLogLoggerFormattedTextTest.php as well. With your text instructions it is rather hard to test this PR.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 30 Jan 2017

@laoneo Good idea.

avatar brianteeman
brianteeman - comment - 9 Sep 2017

@okonomiyaki3000
Can you look at resolving the conflicts and poviding some unit tests so that this can be tested please.

avatar joomla-cms-bot joomla-cms-bot - change - 11 Sep 2017
Category Libraries Libraries Unit Tests
avatar okonomiyaki3000
okonomiyaki3000 - comment - 11 Sep 2017

I think it should be alright now.

avatar okonomiyaki3000 okonomiyaki3000 - change - 13 Apr 2018
Labels Added: ?
avatar okonomiyaki3000
okonomiyaki3000 - comment - 13 Apr 2018

Rebased with the latest staging.

avatar priiish priiish - test_item - 23 Jul 2018 - Tested successfully
avatar priiish
priiish - comment - 23 Jul 2018

I have tested this item successfully on 26fe1b0

# Testing Steps

  • plugins/system/debug/debugger.php => modified JLog::addLogger options; 'defer' => true (for both loggers)
  • libraries/src/Log/Logger/FormattedtextLogger.php => for testing purposes, added a message to be thrown if defer=true and entries being stored (see screenshots)
  • reloaded frontend, received confirmation message for entries being stored
    @icampus

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13692.
avatar priiish
priiish - comment - 23 Jul 2018

screenshot regarding testing steps (libraries/src/Log/Logger/FormattedtextLogger.php confirmation message if defer==true)

bildschirmfoto 2018-07-23 um 15 11 20

avatar jonasgonka jonasgonka - test_item - 24 Jul 2018 - Tested successfully
avatar jonasgonka
jonasgonka - comment - 24 Jul 2018

I have tested this item successfully on 26fe1b0

Tested successfully.

Log entries were created as expected with the defer option set to true.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13692.

avatar franz-wohlkoenig franz-wohlkoenig - change - 24 Jul 2018
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 24 Jul 2018

Ready to Commit after two successful tests.

avatar mbabker mbabker - change - 2 Aug 2018
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2018-08-02 22:34:01
Closed_By mbabker
Labels Added: ? ?
Removed: ?
avatar mbabker mbabker - close - 2 Aug 2018
avatar mbabker mbabker - merge - 2 Aug 2018

Add a Comment

Login with GitHub to post a comment