Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Code changes like these (API change from #1289536: Switch Watchdog to a PSR-3 logging framework) make it impossible to extract log categories/channels, so attempting to translate them is pointless:
BEFORE:
watchdog('blabla', 'Message');
AFTER:
$logger = $container->get('logger.factory')->get('blabla');
$logger->notice('Message');
Extraction of the string 'blabla' here is getting ridiculously hit and miss. There are bazillions of ways you may get that logger and the get() method is impossible to target with a code parser. With complex code structures like that, it seems totally pointless to even try to translate the message category at this point. That's what you get for nice and dynamic code structures.
Proposed resolution
There are 2 choices:
- keep translating log category/channel names like we did for 9 years (although time is not necessarily an indication of good practice :), then we need to rename get() on the logger channel interface to something more concrete like getChannel(), see #21
- stop translating log category/channel names, see #3
Remaining tasks
Discuss. Review. Commit.
User interface changes
If option 1 is chosen, type like 'dblog', 'comment', 'node', etc. will never be translated on the UI (both shown as a column in dblog's view and in a select box to filter).
API changes
If option 2 is chosen the log channel API is changed to use getChannel() instead
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | getchannel-21.patch | 4.27 KB | Gábor Hojtsy |
| #3 | dblog-type-translate-2.patch | 1.2 KB | Gábor Hojtsy |
Comments
Comment #1
Gábor HojtsyComment #2
webchickCould we handle this with a specific wrapper function like Logger::get() and use that everywhere instead of magic, undocumentable methods?
Comment #3
Gábor HojtsyShould change the place where we build the select box options.
Comment #4
Gábor Hojtsy@webchick: because of injection: the way eg. #2228763: Create a comment-type config entity and use that as comment bundles, require selection in field settings form uses the log API it injects it via the container. With the API set so loose, there is really no way of limiting them not to inject the logger in some way, store it on a property like it is done there, etc.
One way to make it extractable regardless of injection is to rename get() to getLoggerChannel() or something very specific so we can target it in an extractor. Maybe I jumped the gun too soon here and should have suggested a nice rename like that :D I did assume the channel concept is part of the PSR-3 logger, but it does not seem to be reading https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-3-logg... -- it looks like we added that on top. If we are to rename it, we'll need to document the hell out of it, or people will want to rename it to get() because of DX.
Comment #5
sunTranslating these internal log message channel/type identifiers, or even treating them as human-readable string labels, never made sense to me in the first place.
They are not properly capitalized, they are as short as technically possible/sensible, and they make no sense without context.
The channel names should be internal identifiers like "routing", "authentication", etc.pp. — They may be mapped to human-readable labels for UI purposes, but that would and should be a separate process, so that e.g. "auth" or "authentication" turns into "Authentication".
Comment #6
andypostMakes sense! Also let's remove broken examples
Comment #7
Gábor Hojtsy@sun: if that is a universally accepted position (no problem, I agree these strings are at a minimum at the border of pointless being translated either way), we should probably not have the comment I added. The comment makes sense if we think they should ideally be translated but we don't want to make the DX of loggers worse just so translated sites get translated log categories (although they got it in several prior major core versions).
@andypost: this issue is not about translating (or not) of the messages themselves, but the 'channel' string that comes before that on the lines you modified.
Comment #8
ParisLiakos CreditAttribution: ParisLiakos commentedi am fine with renaming get() to getLogger().
But i also agree with sun. i dont really see a point translating channel names
Comment #9
andypostSo I filed separate issue #2287063: Remove remains of t() in watchdog messages
Comment #10
Gábor HojtsyBTW as for the messages themselves, the new logger API introduced 9 new ways to define translatable strings, see #2285063: Support for Drupal 8 logger API. (Tenth would be the category/channel).
Comment #11
sunI'm not sure whether we ever really laid out a proper plan for log channels.
Given that watchdog/logging was and still is our only construct for global/application-wide notifications for site administrators, and given that the UI implementation of dblog is enabled by default, we collectively developed a habit of (ab)using log message channel/type names as a way to categorize "notifications" on an additional axis that is normally not supposed to exist in a well-formed logging architecture.
The prime example for that are the "not found" and "access denied" log channel/type names. They belong to respective "routing" and "access" channels under any normal circumstances. Each of these channels ought to contain other messages other than 404 or 403 messages. Each message has a severity, but that's it. Using a custom/different channel name for each possible log message defeats the whole purpose of log channels. E.g., an "access" channel should focus on and reveal access internals only, possibly even giving hints on why exactly access was granted or denied.
The lack of a clear and proper design causes contrib to invent really stupid own channel names (or to not perform any logging at all).
In short, I think there's much more for us to figure out and fix in this architectural space. A registry of available channels may or may not help. Based on that, it would be easier to evaluate whether mapping/translating channel names makes any sense or not.
Comment #12
Gábor HojtsyThe Drupal 8 API still promotes per module / component names for channels, so you may get access denied messages from channels like 'comment' and 'og'.
As for translating the type name, we started translating watchdog() type names in 4.4.0 for menu items and in the filter form and the table display in 4.6.0. (So this is a practice we did for the last 9 years at least). Its fine to say this is not something we want to continue doing IMHO, if we decide with the consequences in mind.
Comment #13
larowlanI'll ping grom358 to see if pharborist can help here
Comment #14
Gábor Hojtsy@larowlan: I don't know what kind of high level help would be needed here. I think there are 2 choices:
- keep translating log category/channel names like we did for 9 years (although time is not necessarily an indication of good practice :), then we need to rename get() on the logger channel interface to something more concrete like getChannel(), patch for that attached, although will probably fail at places
- stop translating log category/channel names (see prior patches on this issue)
Retitled for more options and updated issue summary.
Comment #15
Gábor HojtsyComment #17
Gábor HojtsyComment #19
Gábor HojtsyThings that not even phpstorm's magic refactoring can resolve :P
Comment #21
Gábor HojtsyAnd the remaining ones I found & the tests found.
Comment #22
Gábor HojtsyComment #23
Gábor HojtsyNow two complete patches for the two approaches #21 and #3. One of them is the solution to this issue. Feedback please.
Comment #24
andypostViews does not translate type, suppose it's good to combine patches
Comment #25
Gábor Hojtsy@andypost: solution to that depends on what we decide here. Any feedback on the patches / which way to go?
Comment #26
andypost@Gabor, I see #3 as solution - no reason to translate channel name!
I can't get how renaming of the method affects "translatability" of channel? separate issue?
Comment #27
Gábor Hojtsy@andypost: maybe read the issue summary? Let me rephrase it for you. The channel getter method is the only place which has the channel name. $logger_factory->get('comment');. This is impossible to target with string extraction therefore the source string will never be able to appear as a source translatable string (at least until the code is run). IF we want to keep it translated, the only way to make the extraction work is to give the method a more specific name, so extraction can target it. The issue summary has both options laid out.
Comment #28
jhedstromPatch above no longer applies and will need a reroll if this is still being considered.
Comment #37
catchDiscussed this with Gábor and agreed leaving these untranslatable is fine.