Skip to content

Enable custom Logger(s).#5022

Merged
weitzman merged 6 commits intodrush-ops:11.xfrom
weitzman:any-logger
Jan 17, 2022
Merged

Enable custom Logger(s).#5022
weitzman merged 6 commits intodrush-ops:11.xfrom
weitzman:any-logger

Conversation

@weitzman
Copy link
Member

@weitzman weitzman commented Jan 15, 2022

Refs #4422 and implements the suggestion at consolidation/log#20 (comment). DrushLoggerManager is a slightly customized version of the Consolidation/Log/LoggerManager.

The changes in Runtime.php illustrate my guess as to when and how a site would swap a custom Logger for Drush's. I think that we could refactor our front controller (i.e. $runtime->run()) such that a site could replace that controller with one that swaps in own Logger. Is that a promising way to go?

@greg-1-anderson
Copy link
Member

There are a number of options, depending on how much flexibility / complexity we want to bring in.

Robo provides a RelativeNamespaceDiscovery mechanism for finding well-known class names (matching a pattern) via the autoloader tables. An example of this is the Robo plugin class discovery method. One option would be to find specific class names using this discovery method; a site could then hook into Drush behavior by declaring a class that matched the pattern. If we did this, rather than having a plurality of classes discovered this way (over time), I'd recommend we discover a single class which then implements specific interfaces to override behavior. e.g. if we discovered a DrushController class it might override an interface that declared a method for configuring the logger. If the controller implements that interface, then Drush could call the logger configuration method, passing in the logger manager.

An alternate plan wold be to allow sites to simply declare a command file or a hook file via the existing annotated command mechanism, and use a @hook init to ask for the logger manager directly from the Drush logger manager. Doing that has the advantage of not requiring a new hook / extension mechanism, but has the disadvantage that the logger would not be swapped in until very late in the bootstrap. Finding the DrushController could happen pretty early.

I didn't review the Drush initialization code to figure out where this could happen. The place you have selected already has a reference to all of the command files; this is late enough that we could check each command file for a specific interface, e.g. to configure the logger, and not wait for @hook init. I think that finding the DrushController via discovery could happen much earlier. If we want to try to allow this to happen as early as possible, we should probably make this part of Drush 12, and get rid of the secondary autoloader ($loader = $this->preflight->loadSiteAutoloader();) and prohibit Drush from operating on any Drupal site other than the one it shares an autoloader / vendor with.

There is also the question of what to do with log messages that happen before the logger is fully configured. We could ignore this problem, and just let early log messages go to the "wrong" logger, or formalize the wrongness by having a simple "early log" instance that just prints log messages. Alternately, we could have the logger manager cache all of the early logs, and dump them all to the custom logger after the configuration step is done. If we did this, we'd also want the shutdown handler to dump all of the early log messages via print should Drush exit before the logger is configured.

@weitzman
Copy link
Member Author

weitzman commented Jan 16, 2022

I went with @hook init and its working well except the ConsoleLogger that gets swapped in is logging to stdout instead of stderr (see the new test). If someone really does that with Drush, our redispatch commands like updatedb will break. Maybe our recommendation should be to add a logger instead of replacing Drush's?

Lets not worry about the log messages that happen before the swap.

@greg-1-anderson
Copy link
Member

Note that Drush by default does not use the Symfony ConsoleLogger directly, but instead uses the logger from consolidation/log, which replaces it. The Symfony logger only uses stderr for log levels that are "errors"; this is a common misunderstanding vis-a-vis the purpose of the stderr stream.

See https://github.com/symfony/console/blob/5.4/Logger/ConsoleLogger.php#L75

@weitzman weitzman merged commit 6ab4edf into drush-ops:11.x Jan 17, 2022
@weitzman weitzman deleted the any-logger branch January 17, 2022 03:40
attiks added a commit to UN-OCHA/rwint9-site that referenced this pull request Aug 16, 2025
adam-vessey added a commit to discoverygarden/islandora_drush_utils that referenced this pull request Oct 22, 2025
Drush 13 introduced assertions, expecting the use of a "logger manager".

See: drush-ops/drush#5022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants