Init/ErrorHandling: absorb error log handling from Logging#11809
Open
schmitz-ilias wants to merge 13 commits into
Open
Init/ErrorHandling: absorb error log handling from Logging#11809schmitz-ilias wants to merge 13 commits into
schmitz-ilias wants to merge 13 commits into
Conversation
…application ports
…ing` in `DelegatingHandler`
… incident reporting
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With this PR I propose to move all handling of error logs from Logging (specifically
classes/error) to Init\ErrorHandling. This includes infrastructure for file handling and formatting of the error logs, a bit of configuration, and the 'Delete old or orphaned error log files' cron job.As briefly discussed with @mjansenDatabay, this PR builds on the refactoring of Init\ErrorHandling from #11658. All of my changes are on top in a5f30a6.
In the discussion under #11658 @thibsy has voiced the opinion that things should rather move in the opposite direction, and Logging should take over the whole error log infrastructure (and encapsulate it better). In my opinion, the approach here represents a better division of responsibilities between components:
classes/erroris almost fully independent from the rest of Logging. In particular, handling of the (non-error) log file is left to Monolog, while handling of the error logs is done 'by hand' inilLoggingErrorFileStorage.classes/errordoes depend on Whoops. Moving these classes to Init\ErrorHandling then allows us to keep Whoops fully encapsulated in Init\ErrorHandling (and Monolog in Logging).Besides just moving the code, I also cleaned it up and restructured it a bit (but there is of course still much room for improvement). What I have left in Logging (so far) is the GUI for 'Administration > Logging > Error logging settings'. Splitting up
ilObjLoggingSettingsGUIwould have been a bit too much work for a proposal. As a consequence, I had to instantiate a few of the newSettingsclasses by hand, see here. In the (hopefully near) future, when the ini files are accessible during bootstrapping, this can be cleaned up a bit: Init can offer theseSettingsvia the component mechanism, andilObjLoggingSettingsGUIcan grab them from the DIC (viaInit\AddModernComponents).What do you think?
Cheers, Tim