Skip to content

Add an option to defer writing logs.#13692

Merged
mbabker merged 4 commits intojoomla:3.9-devfrom
okonomiyaki3000:improve-log
Aug 2, 2018
Merged

Add an option to defer writing logs.#13692
mbabker merged 4 commits intojoomla:3.9-devfrom
okonomiyaki3000:improve-log

Conversation

@okonomiyaki3000
Copy link
Copy Markdown
Contributor

@okonomiyaki3000 okonomiyaki3000 commented Jan 23, 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.

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Jan 23, 2017

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

@okonomiyaki3000
Copy link
Copy Markdown
Contributor Author

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

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Jan 23, 2017

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

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

@okonomiyaki3000
Copy link
Copy Markdown
Contributor Author

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

@laoneo
Copy link
Copy Markdown
Member

laoneo commented Jan 30, 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.

@okonomiyaki3000
Copy link
Copy Markdown
Contributor Author

@laoneo Good idea.

@brianteeman
Copy link
Copy Markdown
Contributor

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

@okonomiyaki3000
Copy link
Copy Markdown
Contributor Author

I think it should be alright now.

@okonomiyaki3000
Copy link
Copy Markdown
Contributor Author

Rebased with the latest staging.

@priiish
Copy link
Copy Markdown

priiish commented Jul 23, 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.

@priiish
Copy link
Copy Markdown

priiish commented Jul 23, 2018

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

bildschirmfoto 2018-07-23 um 15 11 20

@jonasgonka
Copy link
Copy Markdown
Contributor

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.

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.10.0 milestone Jul 24, 2018
@ghost
Copy link
Copy Markdown

ghost commented Jul 24, 2018

Ready to Commit after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 24, 2018
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 2, 2018
@okonomiyaki3000 okonomiyaki3000 deleted the improve-log branch August 3, 2018 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants