Allow 'Cron run completed.' log message to be skipped

By default Drupal keeps the last 1000 log messages. While this can be increased, it comes with increased database size and processing time.

Providing the option to not log "Cron run completed." events where there are no errors would free up a lot of space and any errors won't disappear from the database log so quickly.

For example, we're running cron every minute on some sites. We use Ultimate Cron → to send any queued email every minute, while everything else is hourly or 3 hourly.

The reason for this is to improve the user experience. Users fill out a form which is submitted instantly when they click "Submit", with an email sent to them within a minute. Without queueing email it would take 3 or more seconds to submit the form.

I agree with @longwave, in this case there is no need to use null logger.

I agree with @longwave in this case, there is no need to use null logger.

+++ b/core/lib/Drupal/Core/Cron.php
@@ -311,8 +322,7 @@ protected function invokeCronHandlers() {
-    $logger = $time_logging_enabled ? $this->logger : new NullLogger();
+    $logger = $this->isDetailedLoggingEnabled() ? $this->logger : new NullLogger();
 
     // Iterate through the modules calling their cron handlers (if any):

Can the tests be expanded to capture when the setting is off and on?

Also IS still needs to be completed.

Plus #13

@imclean, @smustgrave, it would be nice if you read the full version of the code, and then write comments.
Let me clarify for you to understand.

If you check the patch from comment 2 and comment 6 from @longwave, you will see that in the proposed solution (patch 2), null logger is redundant and it is enough to use a common if condition.

Comment 7 mentions "pattern similar to Cron::invokeCronHandlers()" and if you look at the code it's appropriate to use a null logger to avoid adding a bunch of if conditions inside closure.

So my comment regarding the null logger applies exclusively to the setCronLastTime method.

@smustgrave,

Can the tests be expanded to capture when the setting is off and on?

\Drupal\Tests\dblog\Kernel\DbLogTest::testDbLogCron (starts on line 55) - this code covers on/off setting.

Updated patch included code style fixes

Reroll for D10.0.

Reroll for D10.0.

Had to update my phpcs version. Though, now I have more errors than the one used by Drupal pipeline. I hope it'll make it, otherwise I'll have to call it a day.

I agree it would be nice addition, but I don't know if we can introduce the change without a new config, the current behavior has been there for 6 years (since Drupal 8.3). At minimum it needs a change to form element descriptions for "Detailed cron logging" checkbox.

New patch to Drupal 10.1.x.

I used the same approach that in Cron::invokeCronHandlers().

Same path to Drupal 11.x

The last submitted patch, 20: D10.1-3292849-20.patch, failed testing. View results →

The last submitted patch, 21: D11-3292849-21.patch, failed testing. View results →

Require fix tests.

Reroll of #16 for 10.3.6 introducing this commit

This needs to be changed to use a branch instead of just a patch.

Looking at the implementation, I'm not sure we can add the config factory at that place in the argument list, since it will break implementations that have the other arguments already? It is a required argument, so maybe it is in the right place. I'm not sure about this.