Seeing a lot of log entries like this: "1 log entries removed for job dblog_cron". That's a lot of noise.

We should either find a way to do a single log entry or remove it completely.

Comments

Berdir created an issue. See original summary.

ModernMantra’s picture

Assigned: Unassigned » ModernMantra
Status: Active » Needs review
StatusFileSize
new1.84 KB

Made some progress, hope it is good :)

berdir’s picture

Status: Needs review » Needs work
+++ b/src/Plugin/ultimate_cron/Logger/DatabaseLogger.php
@@ -93,6 +94,12 @@ class DatabaseLogger extends LoggerBase implements PluginCleanupInterface, Conta
+    if ($counter > 0) {
+      \Drupal::logger('database_logger')->info('@count log entries removed for job @name', array(
+        '@count' => $counter,
+        '@name' => $job->id(),
+      ));
+    }

not for job @name, but "for @count jobs". you're outside of the loop here, this will simply only report it for the last job.

you need to count deleted messages together and also count the jobs for which messages were deleted but only if ther was at least one.

ModernMantra’s picture

Status: Needs work » Needs review
StatusFileSize
new1.83 KB
new1.31 KB

CHanged the logic and structure considering comment #3. Hope it looks better.

berdir’s picture

Status: Needs review » Needs work
+++ b/src/Plugin/ultimate_cron/Logger/DatabaseLogger.php
@@ -85,13 +87,20 @@ class DatabaseLogger extends LoggerBase implements PluginCleanupInterface, Conta
         // Get the plugin through the job so it has the right configuration.
-        $job->getPlugin('logger')->cleanupJob($job);
+        $job_count++;
+        $counter = $job->getPlugin('logger')->cleanupJob($job);
         $class = \Drupal::entityTypeManager()->getDefinition('ultimate_cron_job')->getClass();

counter is just the count of the last job. you need to sum them together. and as I wrote, only count jobs for which you actually delete messages:

if ($counter) {
$counters[$job->id()] = $counter;
}

For example like that, then you can use count() and array_sum()

ModernMantra’s picture

Status: Needs work » Needs review
StatusFileSize
new2.01 KB
new1.58 KB

Changed a little bit a logic considering comment #5. I apologize for my previous patch and not reading carefully comments/review.

  • Berdir committed 0a0cd67 on 8.x-2.x authored by ModernMantra
    Issue #2798589 by ModernMantra: database_logger logs a log entry for...
berdir’s picture

Status: Needs review » Fixed

Yes, I think this is correct now. Tests would be nice but seems to be pretty complex to add useful tests.

Anonymous’s picture

I agree with this, this is spamming the heck out of our logs.

Anonymous’s picture

This needs a backport to 7.

berdir’s picture

Feel free to re-open and set to 7.x. But plently of similar 7.x issues have been closed with the suggestion to use some watchdog filter module. I don't maintain that version.

Anonymous’s picture

Version: 8.x-2.x-dev » 7.x-2.x-dev
Status: Fixed » Needs review
StatusFileSize
new1.77 KB

Here's a stab at D7.

Anonymous’s picture

Assigned: ModernMantra » Unassigned

Status: Needs review » Needs work

The last submitted patch, 12: ultimate-cron_2798589_database-logger-d7.patch, failed testing.

Anonymous’s picture

Status: Needs work » Needs review
StatusFileSize
new1.77 KB
Triqueon’s picture

I would suggest moving the return statement out of the if-block in the final change block. Probably you'll be fine without it, I believe PHP imlicitly returns null from methods missing a return statement (which you would be in this case if count is 0), which also maps to false, but it's not clean, and you can eliminate the unnecessary if block that way. Apart from that, thank you for this issue and the patch :D

Jorrit’s picture

I agree with @Triqueon. I have updated the patch with this suggestion and some other improvements.

  • arnested committed fafb8fe on 7.x-2.x authored by Jorrit
    Issue #2798589 by ModernMantra, vilepickle, Jorrit, Triqueon:...
arnested’s picture

Status: Needs review » Fixed

I committed the patch from #17 and released version 7.x-2.6.

Jorrit’s picture

Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.