Surfacing this here for planning purposes.
Over on Forge, we’ve been working on figuring out how to finish the transition to using PSR-3 for all logging. cf: Epic #94356: Embrace PSR-3 - TYPO3 Core - TYPO3 Forge
That runs into some interesting challenges, as a number of systems read from
sys_log directly for various tasks, violating encapsulation. That means we cannot simply stop writing to
sys_log, because the logging framework can be configured to write everything or nothing to the log table. Net result: For this to work, not only do we need to remove all writes to the
sys_log table from anywhere but the logging system, we also need to refactor anything else that relied on
sys_log to do something else. That’s a not-small task.
My current thinking on this front is captured below, and will be updated based on feedback. Naturally, this is complicated by how late in the v11 cycle we are right now so some changes may simply not be possible, or we/I won’t have time to do so. TBD. Feedback and/or context where I don’t know things is welcome and requested.
This appears to be getting metadata about objects for the grid display. However… if I trace back the function calls, it’s only called once, from a method that’s only called once, from a method that’s only called once, from
RemoteServer::getWorkspaceInfos(), which my IDE says is never called. I’m not sure what’s going on here, or if this code path even needs to still exist.
If it does, then I suspect the same data should be derivable from another source, although I’m not sure what that source is yet.
This is getting a summary of how many errors were logged in a given timeframe, for dashboard purposes. Assuming
sys_log remains as the target for
DatabaseWriter, that’s probably a valid use case to keep. However, it should ideally be refactored to use the same data model as belog does, I think. But this isn’t a blocker, aside from the caveat that the data there is not reliable unless you are logging all errors to the DB (which is certainly an option, but not the only way to configure it).
Tracking the number of failed logins specifically seems like a task that should be its own small subsystem. That could handle tracking of failed logins specifically, pruning of old data, etc. Oddly this routine doesn’t appear to track the user that generated the failed login, which… limits its usefulness as now it cannot be used for flood control. This is also a dashboard-only widget, so I’m not sure how critical it is.
This is writing exceptions to the log. The new PSR-3 code is already there, so I have a simple patch to just kill the old code path: https://review.typo3.org/c/Packages/TYPO3.CMS/+/69524
This is a ginormous class, and I don’t quite follow what all it does. (From the comments, it appears the answer is “everything”, which is a separate issue to address.) It… looks like it generates up to 256 Flash messages off of log entries. It’s called from 5 places: 4 in
sysext/backend/Classes/Controller and 1 in
sysext/recordlist/Classes/Controller. I don’t quite understand the use case, or why you’d want to dump that much data into flash messages, so I’m not quite sure what to do here. I think it needs to be replaced outright with something that displays data properly and we can determine from that what it should actually be doing, but that’s going to be a larger lift.
This is the biggie. It’s a write location, not a read location, and it’s called from a zillion places. Fortunately, I think this is probably one of the easier places to fix as it’s already centralized; it should, in theory, be a “simple” matter of replacing the existing code there with the equivalent PSR-3 write.
Curiously, its sister-class,
FrontendUserAuthentication, inherits the parent’s noop implementation of
writelog(). I do not understand that at all, but it suggests to me that the frontend/backend user logic is unnecessarily intertwined. That’s a topic for another time, though.
This is part of belog’s display code, to show
sys_log records in the UI. It may need to get modified as more data is transitioned to context, rather than dedicated fields (or possibly the context can be denormalized on write? TBD), but if anything it’s the most legitimate
sys_log read case in the whole system. Basically, stet for now.
This is, I think, the code that shows the “hey, you’ve got bad stuff in your logs!” message icon in the admin. While not ideal, I think like belog itself it’s reasonably fine as-is. It’s telling you there is something in the DB log that you could view through belog’s viewer, which is… accurate. Stet, although I think it would benefit from unification with belog’s data model, for simplicity.
Another case that should be straightforward to just swap to PSR-3 and call it a day.
NumberOfFailedLoginsDataProvider, I think this would benefit from using a proper flood control system instead. The
sys_log table is an unreliable data source, and will become moreso in time.
I think, then, there’s a few tasks coming out of this. Please correct my misconceptions as appropriate:
PasswordResetare write locations, and those can be directly converted to PSR-3. That should be the easy part.
SystemInformationControllerare probably safe to leave as is for now. They are all read contexts on the
sys_logtable, which is a legit use case, with the caveat that certain log configurations would result in the data being unreliable. But that is essentially by design, so stet.
- Add a proper flood control subsystem, and modify
NumberOfFailedLoginsDataProviderto use that instead of
- I have no idea what to do with
DataHandling. It seems wrong to me on several levels, but adjusting that is a not-small lift. Please advise.
RemoteServerI could also use feedback on, as I don’t know what it’s trying to do, or if it should be doing it at all.
I will start on the tasks for point 1 as those are straightforward, but input on the rest is welcome. Thanks.