Problem/Motivation

redirect_404 aggregates 404 requests in a single table with a single total counter. That's useful for users to see, but it's a problem for monitoring, as sooner or later, some URL's from bots or something will go over a static threshold, as we have no way to know which requests are from the last 24h.

Proposed resolution

Add a daily counter which resets every day by a cron job.

CommentFileSizeAuthor
#4 3100270-interdiff-2-4.txt2.84 KBarpad.rozsa
#4 3100270-add_daily_count-4.patch12.97 KBarpad.rozsa
#2 3100270-add_daily_count-2.patch12.52 KBarpad.rozsa

Comments

arpad.rozsa created an issue. See original summary.

arpad.rozsa’s picture

arpad.rozsa commented
Status: Active » Needs review
StatusFileSize
new 3100270-add_daily_count-2.patch12.52 KB
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work 
  1. +++ b/modules/redirect_404/redirect_404.module
@@ -18,6 +18,15 @@ function redirect_404_cron() {
+  if ($now->diff($last_daily_reset)->d > 0) {

    Can't we just do date('d', $last_daily_reset) != date('d')?

    We don't need a full diff, if the day doesn't match, we can reset. 

  2. +++ b/modules/redirect_404/redirect_404.module
@@ -18,6 +18,15 @@ function redirect_404_cron() {
+
+    \Drupal::state()->set('redirect_404.last_daily_reset', time());
+  }

    Lets use \Drupal::requestTime()->getRequestTime() for that. 

  3. +++ b/modules/redirect_404/tests/src/Kernel/Fix404RedirectCronJobTest.php
@@ -114,6 +114,23 @@ class Fix404RedirectCronJobTest extends KernelTestBase {
+    $result = db_query("SELECT COUNT(*) FROM {redirect_404} WHERE daily_count > 0")
+      ->fetchField();
+    $this->assertEquals(2, $result);
+
+    // Run cron to reset the daily counts in the redirect_404 test table.
+    redirect_404_cron();
+
+    $result = db_query("SELECT COUNT(*) FROM {redirect_404} WHERE daily_count > 0")

    db_query() is deprecated, should use \Drupal::database()->query()

    Lets simulate also that we don't delete it every time we call the function.

    So insert/update a row with daily counts, call cron again, make sure it's still there. 

  4. +++ b/modules/redirect_404/tests/src/Kernel/Fix404RedirectCronJobTest.php
@@ -121,21 +138,24 @@ class Fix404RedirectCronJobTest extends KernelTestBase {
    */
-  protected function insert404Row($path, $count = 1, $timestamp = 0, $langcode = 'en') {
+  protected function insert404Row($path, $count = 1, $daily_count = 0, $timestamp = 0, $langcode = 'en') {
     db_insert('redirect_404')
-    ->fields([
-      'path' => $path,

    db_insert() is deprected too. We should not update everything, but since we touch this line and the D9 patch will conflict, lets update this one too.

arpad.rozsa’s picture

arpad.rozsa commented
Status: Needs work » Needs review
StatusFileSize
new 3100270-add_daily_count-4.patch12.97 KB
new 3100270-interdiff-2-4.txt2.84 KB

Updated the patch.

  • Berdir committed 5622637 on 8.x-1.x authored by arpad.rozsa 
    Issue #3100270 by arpad.rozsa, Berdir: Add daily count to redirect 404
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed

Did some manual tests, seems to work nicely.

Status: Fixed » Closed (fixed)

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